From 7e62000e1b90ffa7f91a4db9a67655f3c6ba8056 Mon Sep 17 00:00:00 2001 From: Subin T P Date: Sat, 2 May 2020 16:04:52 +0530 Subject: [PATCH] Chore: Remove redis backed reporting (#669) --- Gemfile | 3 - Gemfile.lock | 4 +- app/builders/report_builder.rb | 77 --------------- .../api/v1/accounts/reports_controller.rb | 99 ------------------- app/dispatchers/async_dispatcher.rb | 2 +- app/identities/account_identity.rb | 10 -- app/identities/agent_identity.rb | 8 -- config/initializers/redis.rb | 1 - config/reports_redis.yml | 19 ---- config/routes.rb | 11 --- lib/constants/report.rb | 18 ---- lib/custom_exceptions/report.rb | 33 ------- spec/listeners/reporting_listener_spec.rb | 39 -------- 13 files changed, 2 insertions(+), 322 deletions(-) delete mode 100644 app/builders/report_builder.rb delete mode 100644 app/controllers/api/v1/accounts/reports_controller.rb delete mode 100644 app/identities/account_identity.rb delete mode 100644 app/identities/agent_identity.rb delete mode 100644 config/reports_redis.yml delete mode 100644 lib/constants/report.rb delete mode 100644 lib/custom_exceptions/report.rb delete mode 100644 spec/listeners/reporting_listener_spec.rb diff --git a/Gemfile b/Gemfile index 9f4d3cf40..997e76563 100644 --- a/Gemfile +++ b/Gemfile @@ -54,9 +54,6 @@ gem 'pundit' # https://karolgalanciak.com/blog/2019/11/30/from-activerecord-callbacks-to-publish-slash-subscribe-pattern-and-event-driven-design/ gem 'wisper', '2.0.0' -##--- gems for reporting ---## -gem 'nightfury' - ##--- gems for billing ---## gem 'chargebee' diff --git a/Gemfile.lock b/Gemfile.lock index 577676cf5..75127f6b4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,7 +282,6 @@ GEM multi_xml (0.6.0) multipart-post (2.1.1) netrc (0.11.0) - nightfury (1.0.1) nio4r (2.5.2) nokogiri (1.10.9) mini_portile2 (~> 2.4.0) @@ -533,8 +532,7 @@ DEPENDENCIES letter_opener listen mini_magick - mock_redis! - nightfury + mock_redis pg pry-rails puma diff --git a/app/builders/report_builder.rb b/app/builders/report_builder.rb deleted file mode 100644 index 87e4909e6..000000000 --- a/app/builders/report_builder.rb +++ /dev/null @@ -1,77 +0,0 @@ -class ReportBuilder - include CustomExceptions::Report - - # Usage - # rb = ReportBuilder.new a, { metric: 'conversations_count', type: :account, id: 1} - # rb = ReportBuilder.new a, { metric: 'avg_first_response_time', type: :agent, id: 1} - - IDENTITY_MAPPING = { - account: AccountIdentity, - agent: AgentIdentity - }.freeze - - def initialize(account, params) - @account = account - @params = params - @identity = get_identity - @start_time, @end_time = validate_times - end - - def build - metric = @identity.send(@params[:metric]) - if metric.get.nil? - metric.delete - result = {} - else - result = metric.get_padded_range(@start_time, @end_time) || {} - end - formatted_hash(result) - end - - private - - def get_identity - identity_class = IDENTITY_MAPPING[@params[:type]] - raise InvalidIdentity if identity_class.nil? - - @params[:id] = @account.id if identity_class == AccountIdentity - identity_id = @params[:id] - raise IdentityNotFound if identity_id.nil? - - tags = identity_class == AccountIdentity ? nil : { account_id: @account.id } - identity = identity_class.new(identity_id, tags: tags) - raise MetricNotFound if @params[:metric].blank? - raise MetricNotFound unless identity.respond_to?(@params[:metric]) - - identity - end - - def validate_times - start_time = @params[:since] || Time.now.end_of_day - 30.days - end_time = @params[:until] || Time.now.end_of_day - start_time = begin - parse_date_time(start_time) - rescue StandardError - raise(InvalidStartTime) - end - end_time = begin - parse_date_time(end_time) - rescue StandardError - raise(InvalidEndTime) - end - [start_time, end_time] - end - - def parse_date_time(datetime) - return datetime if datetime.is_a?(DateTime) - return datetime.to_datetime if datetime.is_a?(Time) || datetime.is_a?(Date) - - DateTime.strptime(datetime, '%s') - end - - def formatted_hash(hash) - hash.each_with_object([]) do |p, arr| - arr << { value: p[1], timestamp: p[0] } - end - end -end diff --git a/app/controllers/api/v1/accounts/reports_controller.rb b/app/controllers/api/v1/accounts/reports_controller.rb deleted file mode 100644 index c93574b6c..000000000 --- a/app/controllers/api/v1/accounts/reports_controller.rb +++ /dev/null @@ -1,99 +0,0 @@ -class Api::V1::Accounts::ReportsController < Api::BaseController - include CustomExceptions::Report - include Constants::Report - - around_action :report_exception - - def account - builder = ReportBuilder.new(current_account, account_report_params) - data = builder.build - render json: data - end - - def agent - builder = ReportBuilder.new(current_account, agent_report_params) - data = builder.build - render json: data - end - - def account_summary - render json: account_summary_metrics - end - - def agent_summary - render json: agent_summary_metrics - end - - private - - def report_exception - yield - rescue InvalidIdentity, IdentityNotFound, MetricNotFound, InvalidStartTime, InvalidEndTime => e - render_error_response(e) - end - - def current_account - current_user.account - end - - def account_summary_metrics - summary_metrics(ACCOUNT_METRICS, :account_summary_params, AVG_ACCOUNT_METRICS) - end - - def agent_summary_metrics - summary_metrics(AGENT_METRICS, :agent_summary_params, AVG_AGENT_METRICS) - end - - def summary_metrics(metrics, calc_function, avg_metrics) - metrics.each_with_object({}) do |metric, result| - data = ReportBuilder.new(current_account, send(calc_function, metric)).build - result[metric] = calculate_metric(data, metric, avg_metrics) - end - end - - def calculate_metric(data, metric, avg_metrics) - sum = data.inject(0) { |val, hash| val + hash[:value].to_i } - if avg_metrics.include?(metric) - sum /= data.length unless sum.zero? - end - sum - end - - def account_summary_params(metric) - { - metric: metric.to_s, - type: :account, - since: params[:since], - until: params[:until] - } - end - - def agent_summary_params(metric) - { - metric: metric.to_s, - type: :agent, - since: params[:since], - until: params[:until], - id: params[:id] - } - end - - def account_report_params - { - metric: params[:metric], - type: :account, - since: params[:since], - until: params[:until] - } - end - - def agent_report_params - { - metric: params[:metric], - type: :agent, - id: params[:id], - since: params[:since], - until: params[:until] - } - end -end diff --git a/app/dispatchers/async_dispatcher.rb b/app/dispatchers/async_dispatcher.rb index a5ff03937..64ea55be3 100644 --- a/app/dispatchers/async_dispatcher.rb +++ b/app/dispatchers/async_dispatcher.rb @@ -9,7 +9,7 @@ class AsyncDispatcher < BaseDispatcher end def listeners - listeners = [EventListener.instance, ReportingListener.instance, WebhookListener.instance] + listeners = [EventListener.instance, WebhookListener.instance] listeners << SubscriptionListener.instance if ENV['BILLING_ENABLED'] listeners end diff --git a/app/identities/account_identity.rb b/app/identities/account_identity.rb deleted file mode 100644 index be3072b1a..000000000 --- a/app/identities/account_identity.rb +++ /dev/null @@ -1,10 +0,0 @@ -class AccountIdentity < Nightfury::Identity::Base - metric :conversations_count, :count_time_series, store_as: :b, step: :day - metric :incoming_messages_count, :count_time_series, step: :day - metric :outgoing_messages_count, :count_time_series, step: :day - metric :avg_first_response_time, :avg_time_series, store_as: :d, step: :day - metric :avg_resolution_time, :avg_time_series, store_as: :f, step: :day - metric :resolutions_count, :count_time_series, store_as: :g, step: :day -end - -AccountIdentity.store_as = :ci diff --git a/app/identities/agent_identity.rb b/app/identities/agent_identity.rb deleted file mode 100644 index 838ca48bb..000000000 --- a/app/identities/agent_identity.rb +++ /dev/null @@ -1,8 +0,0 @@ -class AgentIdentity < Nightfury::Identity::Base - metric :avg_first_response_time, :avg_time_series, store_as: :d, step: :day - metric :avg_resolution_time, :avg_time_series, store_as: :f, step: :day - metric :resolutions_count, :count_time_series, store_as: :g, step: :day - tag :account_id, store_as: :co -end - -AgentIdentity.store_as = :ai diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb index 6b70bd6c6..d893b281c 100644 --- a/config/initializers/redis.rb +++ b/config/initializers/redis.rb @@ -3,7 +3,6 @@ app_redis_config = { password: ENV.fetch('REDIS_PASSWORD', nil).presence } redis = Rails.env.test? ? MockRedis.new : Redis.new(app_redis_config) -Nightfury.redis = Redis::Namespace.new('reports', redis: redis) # Alfred - Used currently for round robin and conversation emails. # Add here as you use it for more features diff --git a/config/reports_redis.yml b/config/reports_redis.yml deleted file mode 100644 index 6c1c01f44..000000000 --- a/config/reports_redis.yml +++ /dev/null @@ -1,19 +0,0 @@ -defaults: &defaults - host: 0.0.0.0 - port: 6379 - namespace: "local" - -development: - <<: *defaults - -slave: - <<: *defaults - -test: - <<: *defaults - -staging: - <<: *defaults - -production: - <<: *defaults diff --git a/config/routes.rb b/config/routes.rb index 3ecb582d4..abf31fbad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,17 +79,6 @@ Rails.application.routes.draw do resources :notifications, only: [:index, :update] resource :notification_settings, only: [:show, :update] - resources :reports, only: [] do - collection do - get :account - get :agent - end - member do - get :account_summary - get :agent_summary - end - end - # this block is only required if subscription via chargebee is enabled resources :subscriptions, only: [:index] do collection do diff --git a/lib/constants/report.rb b/lib/constants/report.rb deleted file mode 100644 index 43276d1a3..000000000 --- a/lib/constants/report.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module Constants::Report - ACCOUNT_METRICS = [:conversations_count, - :incoming_messages_count, - :outgoing_messages_count, - :avg_first_response_time, - :avg_resolution_time, - :resolutions_count].freeze - - AVG_ACCOUNT_METRICS = [:avg_first_response_time, :avg_resolution_time].freeze - - AGENT_METRICS = [:avg_first_response_time, - :avg_resolution_time, - :resolutions_count].freeze - - AVG_AGENT_METRICS = [:avg_first_response_time, :avg_resolution_time].freeze -end diff --git a/lib/custom_exceptions/report.rb b/lib/custom_exceptions/report.rb deleted file mode 100644 index e1a30cee9..000000000 --- a/lib/custom_exceptions/report.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module CustomExceptions::Report - class InvalidIdentity < CustomExceptions::Base - def message - 'Invalid type' - end - end - - class IdentityNotFound < CustomExceptions::Base - def message - 'Type with the specified id not found' - end - end - - class MetricNotFound < CustomExceptions::Base - def message - 'Metric for the specified type not found' - end - end - - class InvalidStartTime < CustomExceptions::Base - def message - 'Invalid start_time' - end - end - - class InvalidEndTime < CustomExceptions::Base - def message - 'Invalid end_time' - end - end -end diff --git a/spec/listeners/reporting_listener_spec.rb b/spec/listeners/reporting_listener_spec.rb deleted file mode 100644 index 856a730c7..000000000 --- a/spec/listeners/reporting_listener_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'rails_helper' -describe ReportingListener do - let(:listener) { described_class.instance } - let!(:account) { create(:account) } - let(:report_identity) { Reports::UpdateAccountIdentity.new(account, Time.zone.now) } - let!(:user) { create(:user, account: account) } - let!(:inbox) { create(:inbox, account: account) } - let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) } - - describe '#message_created' do - let(:event_name) { :'conversation.created' } - - context 'when user activity message' do - it 'does not increment messages count' do - activity_message = create(:message, message_type: 'activity', account: account, inbox: inbox, conversation: conversation) - event = Events::Base.new(event_name, Time.zone.now, message: activity_message) - - allow(Reports::UpdateAccountIdentity).to receive(:new).and_return(report_identity) - allow(report_identity).to receive(:incr_outgoing_messages_count).exactly(0).times - allow(report_identity).to receive(:incr_incoming_messages_count).exactly(0).times - - listener.message_created(event) - end - end - - context 'when user conversation message' do - it 'increments messages count' do - conversation_message = create(:message, message_type: 'outgoing', account: account, inbox: inbox, conversation: conversation) - event = Events::Base.new(event_name, Time.zone.now, message: conversation_message) - - allow(Reports::UpdateAccountIdentity).to receive(:new).and_return(report_identity) - allow(report_identity).to receive(:incr_outgoing_messages_count).once - allow(report_identity).to receive(:incr_incoming_messages_count).exactly(0).times - - listener.message_created(event) - end - end - end -end