From 9d0de04f7c2ec4516039b7a6a74cfe7d7d426bff Mon Sep 17 00:00:00 2001 From: Tejaswini Chile Date: Mon, 19 Jun 2023 16:10:03 +0530 Subject: [PATCH] fix: set custom filter count in redis (#7164) --- .../v1/accounts/custom_filters_controller.rb | 1 + ...custom_filters_records_count_update_job.rb | 9 +++ app/jobs/sync_custom_filter_count_job.rb | 9 +++ app/models/custom_filter.rb | 23 ++++++ app/policies/custom_filter_policy.rb | 21 +++++ app/services/conversations/filter_service.rb | 8 +- .../v1/models/_custom_filter.json.jbuilder | 1 + lib/redis/redis_keys.rb | 1 + .../custom_filters_controller_spec.rb | 78 +++++++++++++------ 9 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 app/jobs/custom_filters_records_count_update_job.rb create mode 100644 app/jobs/sync_custom_filter_count_job.rb create mode 100644 app/policies/custom_filter_policy.rb diff --git a/app/controllers/api/v1/accounts/custom_filters_controller.rb b/app/controllers/api/v1/accounts/custom_filters_controller.rb index 6cd70b07d..f458c018f 100644 --- a/app/controllers/api/v1/accounts/custom_filters_controller.rb +++ b/app/controllers/api/v1/accounts/custom_filters_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Accounts::CustomFiltersController < Api::V1::Accounts::BaseController + before_action :check_authorization before_action :fetch_custom_filters, except: [:create] before_action :fetch_custom_filter, only: [:show, :update, :destroy] DEFAULT_FILTER_TYPE = 'conversation'.freeze diff --git a/app/jobs/custom_filters_records_count_update_job.rb b/app/jobs/custom_filters_records_count_update_job.rb new file mode 100644 index 000000000..49b26a396 --- /dev/null +++ b/app/jobs/custom_filters_records_count_update_job.rb @@ -0,0 +1,9 @@ +class CustomFiltersRecordsCountUpdateJob < ApplicationJob + queue_as :low + + def perform + CustomFilter.find_each(batch_size: 25) do |filter| + SyncCustomFilterCountJob.perform_later(filter) + end + end +end diff --git a/app/jobs/sync_custom_filter_count_job.rb b/app/jobs/sync_custom_filter_count_job.rb new file mode 100644 index 000000000..f89c196d6 --- /dev/null +++ b/app/jobs/sync_custom_filter_count_job.rb @@ -0,0 +1,9 @@ +class SyncCustomFilterCountJob < ApplicationJob + queue_as :low + + def perform(filter) + Redis::Alfred.set(filter.filter_count_key, 0) if filter.filter_records.nil? + + filter.set_record_count_in_redis + end +end diff --git a/app/models/custom_filter.rb b/app/models/custom_filter.rb index b3d58fd17..2089d42c3 100644 --- a/app/models/custom_filter.rb +++ b/app/models/custom_filter.rb @@ -24,6 +24,29 @@ class CustomFilter < ApplicationRecord enum filter_type: { conversation: 0, contact: 1, report: 2 } validate :validate_number_of_filters + def records_count + fetch_record_count_from_redis + end + + def filter_records + Conversations::FilterService.new(query.with_indifferent_access, user, account).perform + end + + def set_record_count_in_redis + records = filter_records + Redis::Alfred.set(filter_count_key, records[:count][:all_count]) + end + + def fetch_record_count_from_redis + number_of_records = Redis::Alfred.get(filter_count_key) + SyncCustomFilterCountJob.perform_later(self) if number_of_records.nil? + number_of_records.to_i + end + + def filter_count_key + format(::Redis::Alfred::CUSTOM_FILTER_RECORDS_COUNT_KEY, account_id: account_id, filter_id: id, user_id: user_id) + end + def validate_number_of_filters return true if account.custom_filters.where(user_id: user_id).size < MAX_FILTER_PER_USER diff --git a/app/policies/custom_filter_policy.rb b/app/policies/custom_filter_policy.rb new file mode 100644 index 000000000..88cfa251f --- /dev/null +++ b/app/policies/custom_filter_policy.rb @@ -0,0 +1,21 @@ +class CustomFilterPolicy < ApplicationPolicy + def create? + @account_user.administrator? || @account_user.agent? + end + + def show? + @account_user.administrator? || @account_user.agent? + end + + def index? + @account_user.administrator? || @account_user.agent? + end + + def update? + @account_user.administrator? || @account_user.agent? + end + + def destroy? + @account_user.administrator? || @account_user.agent? + end +end diff --git a/app/services/conversations/filter_service.rb b/app/services/conversations/filter_service.rb index 012dd662f..11c2e0ef3 100644 --- a/app/services/conversations/filter_service.rb +++ b/app/services/conversations/filter_service.rb @@ -1,6 +1,11 @@ class Conversations::FilterService < FilterService ATTRIBUTE_MODEL = 'conversation_attribute'.freeze + def initialize(params, user, filter_account = nil) + @filter_account = filter_account + super(params, user) + end + def perform @conversations = conversation_query_builder mine_count, unassigned_count, all_count, = set_count_for_all_conversations @@ -49,7 +54,8 @@ class Conversations::FilterService < FilterService end def base_relation - Current.account.conversations.left_outer_joins(:labels) + account = @filter_account || Current.account + account.conversations.left_outer_joins(:labels) end def current_page diff --git a/app/views/api/v1/models/_custom_filter.json.jbuilder b/app/views/api/v1/models/_custom_filter.json.jbuilder index 473f2d2c2..c1c032e85 100644 --- a/app/views/api/v1/models/_custom_filter.json.jbuilder +++ b/app/views/api/v1/models/_custom_filter.json.jbuilder @@ -4,3 +4,4 @@ json.filter_type resource.filter_type json.query resource.query json.created_at resource.created_at json.updated_at resource.updated_at +json.count resource.records_count diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index 67e709e52..0de5ff9ca 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -31,4 +31,5 @@ module Redis::RedisKeys LATEST_CHATWOOT_VERSION = 'LATEST_CHATWOOT_VERSION'.freeze # Check if a message create with same source-id is in progress? MESSAGE_SOURCE_KEY = 'MESSAGE_SOURCE_KEY::%s'.freeze + CUSTOM_FILTER_RECORDS_COUNT_KEY = 'CUSTOM_FILTER::%d::%d::%d'.freeze end diff --git a/spec/controllers/api/v1/accounts/custom_filters_controller_spec.rb b/spec/controllers/api/v1/accounts/custom_filters_controller_spec.rb index 2b0815add..0f40cd789 100644 --- a/spec/controllers/api/v1/accounts/custom_filters_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/custom_filters_controller_spec.rb @@ -2,6 +2,24 @@ require 'rails_helper' RSpec.describe 'Custom Filters API', type: :request do let(:account) { create(:account) } + let(:user) { create(:user, account: account, role: :agent) } + let!(:custom_filter) { create(:custom_filter, user: user, account: account) } + + before do + create(:conversation, account: account, assignee: user, status: 'open') + create(:conversation, account: account, assignee: user, status: 'resolved') + custom_filter.query = { payload: [ + { + values: ['open'], + attribute_key: 'status', + query_operator: nil, + attribute_model: 'standard', + filter_operator: 'equal_to', + custom_attribute_type: '' + } + ] } + custom_filter.save + end describe 'GET /api/v1/accounts/{account.id}/custom_filters' do context 'when it is an unauthenticated user' do @@ -13,9 +31,6 @@ RSpec.describe 'Custom Filters API', type: :request do end context 'when it is an authenticated user' do - let(:user) { create(:user, account: account) } - let!(:custom_filter) { create(:custom_filter, user: user, account: account) } - it 'returns all custom_filter related to the user' do get "/api/v1/accounts/#{account.id}/custom_filters", headers: user.create_new_auth_token, @@ -26,13 +41,21 @@ RSpec.describe 'Custom Filters API', type: :request do expect(response_body.first['name']).to eq(custom_filter.name) expect(response_body.first['query']).to eq(custom_filter.query) end + + it 'returns custom_filter conversations count when set in redis' do + get "/api/v1/accounts/#{account.id}/custom_filters", + headers: user.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_body = response.parsed_body + expect(response_body.first['name']).to eq(custom_filter.name) + expect(response_body.first['count']).to eq(custom_filter.fetch_record_count_from_redis) + end end end describe 'GET /api/v1/accounts/{account.id}/custom_filters/:id' do - let(:user) { create(:user, account: account) } - let!(:custom_filter) { create(:custom_filter, user: user, account: account) } - context 'when it is an unauthenticated user' do it 'returns unauthorized' do get "/api/v1/accounts/#{account.id}/custom_filters/#{custom_filter.id}" @@ -43,18 +66,29 @@ RSpec.describe 'Custom Filters API', type: :request do context 'when it is an authenticated user' do it 'shows the custom filter' do + custom_filter.set_record_count_in_redis + get "/api/v1/accounts/#{account.id}/custom_filters/#{custom_filter.id}", headers: user.create_new_auth_token, as: :json expect(response).to have_http_status(:success) expect(response.body).to include(custom_filter.name) + json_response = response.parsed_body + expect(json_response['count']).to eq 1 end end end describe 'POST /api/v1/accounts/{account.id}/custom_filters' do - let(:payload) { { custom_filter: { name: 'vip-customers', filter_type: 'conversation', query: { labels: ['vip-customers'] } } } } + let(:payload) do + { custom_filter: { + name: 'vip-customers', filter_type: 'conversation', + query: { payload: [{ + values: ['open'], attribute_key: 'status', attribute_model: 'standard', filter_operator: 'equal_to' + }] } + } } + end context 'when it is an unauthenticated user' do it 'returns unauthorized' do @@ -65,22 +99,18 @@ RSpec.describe 'Custom Filters API', type: :request do end context 'when it is an authenticated user' do - let(:user) { create(:user, account: account) } - let(:new_account) { create(:account) } - let(:new_user) { create(:user, account: new_account) } - it 'creates the filter' do - expect do - post "/api/v1/accounts/#{account.id}/custom_filters", headers: user.create_new_auth_token, - params: payload - end.to change(CustomFilter, :count).by(1) + post "/api/v1/accounts/#{account.id}/custom_filters", headers: user.create_new_auth_token, + params: payload expect(response).to have_http_status(:success) json_response = response.parsed_body expect(json_response['name']).to eq 'vip-customers' + expect(json_response['count']).to be_zero end it 'gives the error for 51st record' do + CustomFilter.delete_all CustomFilter::MAX_FILTER_PER_USER.times do create(:custom_filter, user: user, account: account) end @@ -100,9 +130,14 @@ RSpec.describe 'Custom Filters API', type: :request do end describe 'PATCH /api/v1/accounts/{account.id}/custom_filters/:id' do - let(:payload) { { custom_filter: { name: 'vip-customers', filter_type: 'contact', query: { labels: ['vip-customers'] } } } } - let(:user) { create(:user, account: account) } - let!(:custom_filter) { create(:custom_filter, user: user, account: account) } + let(:payload) do + { custom_filter: { + name: 'vip-customers', filter_type: 'conversation', + query: { payload: [{ + values: ['resolved'], attribute_key: 'status', attribute_model: 'standard', filter_operator: 'equal_to' + }] } + } } + end context 'when it is an unauthenticated user' do it 'returns unauthorized' do @@ -122,8 +157,8 @@ RSpec.describe 'Custom Filters API', type: :request do expect(response).to have_http_status(:success) expect(custom_filter.reload.name).to eq('vip-customers') - expect(custom_filter.reload.filter_type).to eq('contact') - expect(custom_filter.reload.query['labels']).to eq(['vip-customers']) + expect(custom_filter.reload.filter_type).to eq('conversation') + expect(custom_filter.reload.query['payload'][0]['values']).to eq(['resolved']) end it 'prevents the update of custom filter of another user/account' do @@ -142,9 +177,6 @@ RSpec.describe 'Custom Filters API', type: :request do end describe 'DELETE /api/v1/accounts/{account.id}/custom_filters/:id' do - let(:user) { create(:user, account: account) } - let!(:custom_filter) { create(:custom_filter, user: user, account: account) } - context 'when it is an unauthenticated user' do it 'returns unauthorized' do delete "/api/v1/accounts/#{account.id}/custom_filters/#{custom_filter.id}"