Chore: clean up Reporting Events (#4044)
Tech debt clean up Fixes #4057 Co-authored-by: Aswin Dev P S <aswin@chatwoot.com>
This commit is contained in:
parent
12c0be002e
commit
4260441f8c
21 changed files with 54 additions and 186 deletions
|
@ -95,15 +95,15 @@ class V2::ReportBuilder
|
||||||
end
|
end
|
||||||
|
|
||||||
def avg_first_response_time
|
def avg_first_response_time
|
||||||
(get_grouped_values scope.events.where(name: 'first_response')).average(:value)
|
(get_grouped_values scope.reporting_events.where(name: 'first_response')).average(:value)
|
||||||
end
|
end
|
||||||
|
|
||||||
def avg_resolution_time
|
def avg_resolution_time
|
||||||
(get_grouped_values scope.events.where(name: 'conversation_resolved')).average(:value)
|
(get_grouped_values scope.reporting_events.where(name: 'conversation_resolved')).average(:value)
|
||||||
end
|
end
|
||||||
|
|
||||||
def avg_resolution_time_summary
|
def avg_resolution_time_summary
|
||||||
avg_rt = scope.events
|
avg_rt = scope.reporting_events
|
||||||
.where(name: 'conversation_resolved', created_at: range)
|
.where(name: 'conversation_resolved', created_at: range)
|
||||||
.average(:value)
|
.average(:value)
|
||||||
|
|
||||||
|
@ -113,7 +113,7 @@ class V2::ReportBuilder
|
||||||
end
|
end
|
||||||
|
|
||||||
def avg_first_response_time_summary
|
def avg_first_response_time_summary
|
||||||
avg_frt = scope.events
|
avg_frt = scope.reporting_events
|
||||||
.where(name: 'first_response', created_at: range)
|
.where(name: 'first_response', created_at: range)
|
||||||
.average(:value)
|
.average(:value)
|
||||||
|
|
||||||
|
|
|
@ -12,10 +12,10 @@ class AsyncDispatcher < BaseDispatcher
|
||||||
[
|
[
|
||||||
CampaignListener.instance,
|
CampaignListener.instance,
|
||||||
CsatSurveyListener.instance,
|
CsatSurveyListener.instance,
|
||||||
EventListener.instance,
|
|
||||||
HookListener.instance,
|
HookListener.instance,
|
||||||
InstallationWebhookListener.instance,
|
InstallationWebhookListener.instance,
|
||||||
NotificationListener.instance,
|
NotificationListener.instance,
|
||||||
|
ReportingEventListener.instance,
|
||||||
WebhookListener.instance,
|
WebhookListener.instance,
|
||||||
AutomationRuleListener.instance
|
AutomationRuleListener.instance
|
||||||
]
|
]
|
||||||
|
|
|
@ -1,2 +0,0 @@
|
||||||
module Api::V1::ReportsHelper
|
|
||||||
end
|
|
|
@ -1,9 +1,9 @@
|
||||||
class EventListener < BaseListener
|
class ReportingEventListener < BaseListener
|
||||||
def conversation_resolved(event)
|
def conversation_resolved(event)
|
||||||
conversation = extract_conversation_and_account(event)[0]
|
conversation = extract_conversation_and_account(event)[0]
|
||||||
time_to_resolve = conversation.updated_at.to_i - conversation.created_at.to_i
|
time_to_resolve = conversation.updated_at.to_i - conversation.created_at.to_i
|
||||||
|
|
||||||
event = Event.new(
|
reporting_event = ReportingEvent.new(
|
||||||
name: 'conversation_resolved',
|
name: 'conversation_resolved',
|
||||||
value: time_to_resolve,
|
value: time_to_resolve,
|
||||||
account_id: conversation.account_id,
|
account_id: conversation.account_id,
|
||||||
|
@ -11,7 +11,7 @@ class EventListener < BaseListener
|
||||||
user_id: conversation.assignee_id,
|
user_id: conversation.assignee_id,
|
||||||
conversation_id: conversation.id
|
conversation_id: conversation.id
|
||||||
)
|
)
|
||||||
event.save
|
reporting_event.save
|
||||||
end
|
end
|
||||||
|
|
||||||
def first_reply_created(event)
|
def first_reply_created(event)
|
||||||
|
@ -19,7 +19,7 @@ class EventListener < BaseListener
|
||||||
conversation = message.conversation
|
conversation = message.conversation
|
||||||
first_response_time = message.created_at.to_i - conversation.created_at.to_i
|
first_response_time = message.created_at.to_i - conversation.created_at.to_i
|
||||||
|
|
||||||
event = Event.new(
|
reporting_event = ReportingEvent.new(
|
||||||
name: 'first_response',
|
name: 'first_response',
|
||||||
value: first_response_time,
|
value: first_response_time,
|
||||||
account_id: conversation.account_id,
|
account_id: conversation.account_id,
|
||||||
|
@ -27,6 +27,6 @@ class EventListener < BaseListener
|
||||||
user_id: conversation.assignee_id,
|
user_id: conversation.assignee_id,
|
||||||
conversation_id: conversation.id
|
conversation_id: conversation.id
|
||||||
)
|
)
|
||||||
event.save
|
reporting_event.save
|
||||||
end
|
end
|
||||||
end
|
end
|
|
@ -1,45 +0,0 @@
|
||||||
class ReportingListener < BaseListener
|
|
||||||
def conversation_created(event)
|
|
||||||
conversation, account = extract_conversation_and_account(event)
|
|
||||||
timestamp = event.timestamp
|
|
||||||
|
|
||||||
::Reports::UpdateAccountIdentity.new(account, timestamp).incr_conversations_count
|
|
||||||
end
|
|
||||||
|
|
||||||
def conversation_resolved(event)
|
|
||||||
conversation, account = extract_conversation_and_account(event)
|
|
||||||
timestamp = event.timestamp
|
|
||||||
|
|
||||||
time_to_resolve = conversation.updated_at.to_i - conversation.created_at.to_i
|
|
||||||
|
|
||||||
if conversation.assignee.present?
|
|
||||||
agent = conversation.assignee
|
|
||||||
::Reports::UpdateAgentIdentity.new(account, agent, timestamp).update_avg_resolution_time(time_to_resolve)
|
|
||||||
::Reports::UpdateAgentIdentity.new(account, agent, timestamp).incr_resolutions_count
|
|
||||||
end
|
|
||||||
|
|
||||||
::Reports::UpdateAccountIdentity.new(account, timestamp).update_avg_resolution_time(time_to_resolve)
|
|
||||||
::Reports::UpdateAccountIdentity.new(account, timestamp).incr_resolutions_count
|
|
||||||
end
|
|
||||||
|
|
||||||
def first_reply_created(event)
|
|
||||||
message, account = extract_message_and_account(event)
|
|
||||||
timestamp = event.timestamp
|
|
||||||
|
|
||||||
conversation = message.conversation
|
|
||||||
agent = conversation.assignee
|
|
||||||
first_response_time = message.created_at.to_i - conversation.created_at.to_i
|
|
||||||
::Reports::UpdateAgentIdentity.new(account, agent, timestamp).update_avg_first_response_time(first_response_time) if agent.present?
|
|
||||||
::Reports::UpdateAccountIdentity.new(account, timestamp).update_avg_first_response_time(first_response_time)
|
|
||||||
end
|
|
||||||
|
|
||||||
def message_created(event)
|
|
||||||
message, account = extract_message_and_account(event)
|
|
||||||
timestamp = event.timestamp
|
|
||||||
|
|
||||||
return unless message.reportable?
|
|
||||||
|
|
||||||
identity = ::Reports::UpdateAccountIdentity.new(account, timestamp)
|
|
||||||
message.outgoing? ? identity.incr_outgoing_messages_count : identity.incr_incoming_messages_count
|
|
||||||
end
|
|
||||||
end
|
|
|
@ -4,6 +4,6 @@ module Reportable
|
||||||
extend ActiveSupport::Concern
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
included do
|
included do
|
||||||
has_many :events, dependent: :destroy_async
|
has_many :reporting_events, dependent: :destroy
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -39,8 +39,8 @@ class Label < ApplicationRecord
|
||||||
account.messages.where(conversation_id: conversations.pluck(:id))
|
account.messages.where(conversation_id: conversations.pluck(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
def events
|
def reporting_events
|
||||||
account.events.where(conversation_id: conversations.pluck(:id))
|
account.reporting_events.where(conversation_id: conversations.pluck(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
# == Schema Information
|
# == Schema Information
|
||||||
#
|
#
|
||||||
# Table name: events
|
# Table name: reporting_events
|
||||||
#
|
#
|
||||||
# id :bigint not null, primary key
|
# id :bigint not null, primary key
|
||||||
# name :string
|
# name :string
|
||||||
|
@ -14,14 +14,14 @@
|
||||||
#
|
#
|
||||||
# Indexes
|
# Indexes
|
||||||
#
|
#
|
||||||
# index_events_on_account_id (account_id)
|
# index_reporting_events_on_account_id (account_id)
|
||||||
# index_events_on_created_at (created_at)
|
# index_reporting_events_on_created_at (created_at)
|
||||||
# index_events_on_inbox_id (inbox_id)
|
# index_reporting_events_on_inbox_id (inbox_id)
|
||||||
# index_events_on_name (name)
|
# index_reporting_events_on_name (name)
|
||||||
# index_events_on_user_id (user_id)
|
# index_reporting_events_on_user_id (user_id)
|
||||||
#
|
#
|
||||||
|
|
||||||
class Event < ApplicationRecord
|
class ReportingEvent < ApplicationRecord
|
||||||
validates :account_id, presence: true
|
validates :account_id, presence: true
|
||||||
validates :name, presence: true
|
validates :name, presence: true
|
||||||
validates :value, presence: true
|
validates :value, presence: true
|
|
@ -45,7 +45,7 @@ class Team < ApplicationRecord
|
||||||
account.messages.where(conversation_id: conversations.pluck(:id))
|
account.messages.where(conversation_id: conversations.pluck(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
def events
|
def reporting_events
|
||||||
account.events.where(conversation_id: conversations.pluck(:id))
|
account.reporting_events.where(conversation_id: conversations.pluck(:id))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
class RenameEventsToReportEvents < ActiveRecord::Migration[6.0]
|
||||||
|
def change
|
||||||
|
rename_table :events, :reporting_events
|
||||||
|
end
|
||||||
|
end
|
32
db/schema.rb
32
db/schema.rb
|
@ -434,22 +434,6 @@ ActiveRecord::Schema.define(version: 2022_02_18_120357) do
|
||||||
t.index ["name", "account_id"], name: "index_email_templates_on_name_and_account_id", unique: true
|
t.index ["name", "account_id"], name: "index_email_templates_on_name_and_account_id", unique: true
|
||||||
end
|
end
|
||||||
|
|
||||||
create_table "events", force: :cascade do |t|
|
|
||||||
t.string "name"
|
|
||||||
t.float "value"
|
|
||||||
t.integer "account_id"
|
|
||||||
t.integer "inbox_id"
|
|
||||||
t.integer "user_id"
|
|
||||||
t.integer "conversation_id"
|
|
||||||
t.datetime "created_at", precision: 6, null: false
|
|
||||||
t.datetime "updated_at", precision: 6, null: false
|
|
||||||
t.index ["account_id"], name: "index_events_on_account_id"
|
|
||||||
t.index ["created_at"], name: "index_events_on_created_at"
|
|
||||||
t.index ["inbox_id"], name: "index_events_on_inbox_id"
|
|
||||||
t.index ["name"], name: "index_events_on_name"
|
|
||||||
t.index ["user_id"], name: "index_events_on_user_id"
|
|
||||||
end
|
|
||||||
|
|
||||||
create_table "inbox_members", id: :serial, force: :cascade do |t|
|
create_table "inbox_members", id: :serial, force: :cascade do |t|
|
||||||
t.integer "user_id", null: false
|
t.integer "user_id", null: false
|
||||||
t.integer "inbox_id", null: false
|
t.integer "inbox_id", null: false
|
||||||
|
@ -664,6 +648,22 @@ ActiveRecord::Schema.define(version: 2022_02_18_120357) do
|
||||||
t.datetime "updated_at", precision: 6, null: false
|
t.datetime "updated_at", precision: 6, null: false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
create_table "reporting_events", force: :cascade do |t|
|
||||||
|
t.string "name"
|
||||||
|
t.float "value"
|
||||||
|
t.integer "account_id"
|
||||||
|
t.integer "inbox_id"
|
||||||
|
t.integer "user_id"
|
||||||
|
t.integer "conversation_id"
|
||||||
|
t.datetime "created_at", precision: 6, null: false
|
||||||
|
t.datetime "updated_at", precision: 6, null: false
|
||||||
|
t.index ["account_id"], name: "index_reporting_events_on_account_id"
|
||||||
|
t.index ["created_at"], name: "index_reporting_events_on_created_at"
|
||||||
|
t.index ["inbox_id"], name: "index_reporting_events_on_inbox_id"
|
||||||
|
t.index ["name"], name: "index_reporting_events_on_name"
|
||||||
|
t.index ["user_id"], name: "index_reporting_events_on_user_id"
|
||||||
|
end
|
||||||
|
|
||||||
create_table "taggings", id: :serial, force: :cascade do |t|
|
create_table "taggings", id: :serial, force: :cascade do |t|
|
||||||
t.integer "tag_id"
|
t.integer "tag_id"
|
||||||
t.string "taggable_type"
|
t.string "taggable_type"
|
||||||
|
|
|
@ -1,10 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Reports::UpdateAccountIdentity < Reports::UpdateIdentity
|
|
||||||
attr_reader :account
|
|
||||||
|
|
||||||
def initialize(account, timestamp = Time.now)
|
|
||||||
super(account, timestamp)
|
|
||||||
@identity = ::AccountIdentity.new(account.id)
|
|
||||||
end
|
|
||||||
end
|
|
|
@ -1,11 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Reports::UpdateAgentIdentity < Reports::UpdateIdentity
|
|
||||||
attr_reader :agent
|
|
||||||
|
|
||||||
def initialize(account, agent, timestamp = Time.now)
|
|
||||||
super(account, timestamp)
|
|
||||||
@agent = agent
|
|
||||||
@identity = ::AgentIdentity.new(agent.id, tags: { account_id: account.id })
|
|
||||||
end
|
|
||||||
end
|
|
|
@ -1,69 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Reports::UpdateIdentity
|
|
||||||
attr_reader :account, :identity
|
|
||||||
attr_accessor :timestamp
|
|
||||||
|
|
||||||
def initialize(account, timestamp = Time.now)
|
|
||||||
@account = account
|
|
||||||
@timestamp = timestamp
|
|
||||||
end
|
|
||||||
|
|
||||||
def incr_conversations_count(step = 1)
|
|
||||||
update_conversations_count(:incr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def decr_conversations_count(step = 1)
|
|
||||||
update_conversations_count(:decr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def incr_incoming_messages_count(step = 1)
|
|
||||||
update_incoming_messages_count(:incr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def decr_incoming_messages_count(step = 1)
|
|
||||||
update_incoming_messages_count(:decr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def incr_outgoing_messages_count(step = 1)
|
|
||||||
update_outgoing_messages_count(:incr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def decr_outgoing_messages_count(step = 1)
|
|
||||||
update_outgoing_messages_count(:decr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def incr_resolutions_count(step = 1)
|
|
||||||
update_resolutions_count(:incr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def decr_resolutions_count(step = 1)
|
|
||||||
update_resolutions_count(:decr, step)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_avg_first_response_time(response_time)
|
|
||||||
identity.avg_first_response_time.set(response_time, timestamp)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_avg_resolution_time(response_time)
|
|
||||||
identity.avg_resolution_time.set(response_time, timestamp)
|
|
||||||
end
|
|
||||||
|
|
||||||
private
|
|
||||||
|
|
||||||
def update_conversations_count(method, step)
|
|
||||||
identity.conversations_count.send(method, step, timestamp)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_incoming_messages_count(method, step)
|
|
||||||
identity.incoming_messages_count.send(method, step, timestamp)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_outgoing_messages_count(method, step)
|
|
||||||
identity.outgoing_messages_count.send(method, step, timestamp)
|
|
||||||
end
|
|
||||||
|
|
||||||
def update_resolutions_count(method, step)
|
|
||||||
identity.resolutions_count.send(method, step, timestamp)
|
|
||||||
end
|
|
||||||
end
|
|
|
@ -227,7 +227,7 @@ describe ::V2::ReportBuilder do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns average first response time' do
|
it 'returns average first response time' do
|
||||||
label_2.events.update(value: 1.5)
|
label_2.reporting_events.update(value: 1.5)
|
||||||
|
|
||||||
params = {
|
params = {
|
||||||
metric: 'avg_first_response_time',
|
metric: 'avg_first_response_time',
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
FactoryBot.define do
|
FactoryBot.define do
|
||||||
factory :event do
|
factory :reporting_event do
|
||||||
name { 'MyString' }
|
name { 'MyString' }
|
||||||
value { 1.5 }
|
value { 1.5 }
|
||||||
account_id { 1 }
|
account_id { 1 }
|
|
@ -1,5 +1,5 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
describe EventListener do
|
describe ReportingEventListener do
|
||||||
let(:listener) { described_class.instance }
|
let(:listener) { described_class.instance }
|
||||||
let!(:account) { create(:account) }
|
let!(:account) { create(:account) }
|
||||||
let!(:user) { create(:user, account: account) }
|
let!(:user) { create(:user, account: account) }
|
||||||
|
@ -12,19 +12,19 @@ describe EventListener do
|
||||||
|
|
||||||
describe '#conversation_resolved' do
|
describe '#conversation_resolved' do
|
||||||
it 'creates conversation_resolved event' do
|
it 'creates conversation_resolved event' do
|
||||||
expect(account.events.where(name: 'conversation_resolved').count).to be 0
|
expect(account.reporting_events.where(name: 'conversation_resolved').count).to be 0
|
||||||
event = Events::Base.new('conversation.resolved', Time.zone.now, conversation: conversation)
|
event = Events::Base.new('conversation.resolved', Time.zone.now, conversation: conversation)
|
||||||
listener.conversation_resolved(event)
|
listener.conversation_resolved(event)
|
||||||
expect(account.events.where(name: 'conversation_resolved').count).to be 1
|
expect(account.reporting_events.where(name: 'conversation_resolved').count).to be 1
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#first_reply_created' do
|
describe '#first_reply_created' do
|
||||||
it 'creates first_response event' do
|
it 'creates first_response event' do
|
||||||
previous_count = account.events.where(name: 'first_response').count
|
previous_count = account.reporting_events.where(name: 'first_response').count
|
||||||
event = Events::Base.new('first.reply.created', Time.zone.now, message: message)
|
event = Events::Base.new('first.reply.created', Time.zone.now, message: message)
|
||||||
listener.first_reply_created(event)
|
listener.first_reply_created(event)
|
||||||
expect(account.events.where(name: 'first_response').count).to eql previous_count + 1
|
expect(account.reporting_events.where(name: 'first_response').count).to eql previous_count + 1
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
|
@ -18,7 +18,7 @@ RSpec.describe Account do
|
||||||
it { is_expected.to have_many(:web_widgets).class_name('::Channel::WebWidget').dependent(:destroy_async) }
|
it { is_expected.to have_many(:web_widgets).class_name('::Channel::WebWidget').dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:webhooks).dependent(:destroy_async) }
|
it { is_expected.to have_many(:webhooks).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:notification_settings).dependent(:destroy_async) }
|
it { is_expected.to have_many(:notification_settings).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:events) }
|
it { is_expected.to have_many(:reporting_events) }
|
||||||
it { is_expected.to have_many(:kbase_portals).dependent(:destroy_async) }
|
it { is_expected.to have_many(:kbase_portals).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:kbase_categories).dependent(:destroy_async) }
|
it { is_expected.to have_many(:kbase_categories).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:teams).dependent(:destroy_async) }
|
it { is_expected.to have_many(:teams).dependent(:destroy_async) }
|
||||||
|
|
|
@ -29,7 +29,7 @@ RSpec.describe Inbox do
|
||||||
|
|
||||||
it { is_expected.to have_many(:webhooks).dependent(:destroy_async) }
|
it { is_expected.to have_many(:webhooks).dependent(:destroy_async) }
|
||||||
|
|
||||||
it { is_expected.to have_many(:events) }
|
it { is_expected.to have_many(:reporting_events) }
|
||||||
|
|
||||||
it { is_expected.to have_many(:hooks) }
|
it { is_expected.to have_many(:hooks) }
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
RSpec.describe Event, type: :model do
|
RSpec.describe ReportingEvent, type: :model do
|
||||||
describe 'validations' do
|
describe 'validations' do
|
||||||
it { is_expected.to validate_presence_of(:account_id) }
|
it { is_expected.to validate_presence_of(:account_id) }
|
||||||
it { is_expected.to validate_presence_of(:name) }
|
it { is_expected.to validate_presence_of(:name) }
|
|
@ -19,7 +19,7 @@ RSpec.describe User do
|
||||||
it { is_expected.to have_many(:inbox_members).dependent(:destroy_async) }
|
it { is_expected.to have_many(:inbox_members).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:notification_settings).dependent(:destroy_async) }
|
it { is_expected.to have_many(:notification_settings).dependent(:destroy_async) }
|
||||||
it { is_expected.to have_many(:messages) }
|
it { is_expected.to have_many(:messages) }
|
||||||
it { is_expected.to have_many(:events) }
|
it { is_expected.to have_many(:reporting_events) }
|
||||||
it { is_expected.to have_many(:teams) }
|
it { is_expected.to have_many(:teams) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue