From fe6a0f903a723596933f7557361a9f74a9e42f77 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Tue, 4 Jun 2024 18:49:55 +0300 Subject: [PATCH] CV2-4666: Sentry issue (#1907) * CV2-4666: check existing smooch_message_id before creating new one and pass id to SmoochWorker instead of object * CV2-4666: fix tests and apply PR comments --- app/models/concerns/smooch_messages.rb | 9 +++++++-- app/workers/smooch_worker.rb | 11 ++++++++--- test/models/bot/smooch_3_test.rb | 10 +++------- test/models/bot/smooch_4_test.rb | 4 +--- test/models/bot/smooch_6_test.rb | 8 ++++---- test/models/bot/smooch_7_test.rb | 18 +++++++++--------- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/app/models/concerns/smooch_messages.rb b/app/models/concerns/smooch_messages.rb index 3322ce3a50..0e8d90b28a 100644 --- a/app/models/concerns/smooch_messages.rb +++ b/app/models/concerns/smooch_messages.rb @@ -322,7 +322,9 @@ def save_message_later(message, app_id, request_type = 'default_requests', annot queue = RequestStore.store[:smooch_bot_queue].to_s queue = queue.blank? ? 'smooch_priority' : (mapping[queue] || 'smooch_priority') type = (message['type'] == 'text' && !message['text'][/https?:\/\/[^\s]+/, 0].blank?) ? 'link' : message['type'] - SmoochWorker.set(queue: queue).perform_in(1.second + interval.seconds, message.to_json, type, app_id, request_type, YAML.dump(annotated)) + associated_id = annotated&.id + associated_class = annotated.class.name + SmoochWorker.set(queue: queue).perform_in(1.second + interval.seconds, message.to_json, type, app_id, request_type, associated_id, associated_class) end def default_archived_flag @@ -330,8 +332,11 @@ def default_archived_flag Bot::Alegre.team_has_alegre_bot_installed?(team_id) ? CheckArchivedFlags::FlagCodes::PENDING_SIMILARITY_ANALYSIS : CheckArchivedFlags::FlagCodes::NONE end - def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_obj = nil) + def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_id = nil, associated_class = nil) message = JSON.parse(message_json) + return if TiplineRequest.where(smooch_message_id: message['_id']).exists? + associated_obj = nil + associated_obj = associated_class.constantize.where(id: associated_id).last unless associated_id.nil? self.get_installation(self.installation_setting_id_keys, app_id) Team.current = Team.find self.config['team_id'].to_i ApplicationRecord.transaction do diff --git a/app/workers/smooch_worker.rb b/app/workers/smooch_worker.rb index f307669605..89ea760b50 100644 --- a/app/workers/smooch_worker.rb +++ b/app/workers/smooch_worker.rb @@ -6,11 +6,16 @@ class SmoochWorker sidekiq_retry_in { |_count, _exception| 20 } - def perform(json_message, type, app_id, request_type, annotated) - annotated = YAML.load(annotated) + def perform(message_json, type, app_id, request_type, associated_id = nil, associated_class = nil) User.current = BotUser.smooch_user + # This is a temp code for existing jobs and should remove it in next deployment + annotated = begin YAML.load(associated_id) rescue nil end + unless annotated.nil? + associated_id = annotated.id + associated_class = annotated.class.name + end benchmark.send("smooch_save_#{type}_message") do - Bot::Smooch.save_message(json_message, app_id, User.current, request_type, annotated) + Bot::Smooch.save_message(message_json, app_id, User.current, request_type, associated_id, associated_class) end benchmark.finish User.current = nil diff --git a/test/models/bot/smooch_3_test.rb b/test/models/bot/smooch_3_test.rb index 046a4f501a..84a56a7e6d 100644 --- a/test/models/bot/smooch_3_test.rb +++ b/test/models/bot/smooch_3_test.rb @@ -35,7 +35,7 @@ def teardown language: 'en' }.to_json assert_difference 'ProjectMedia.count' do - SmoochWorker.perform_async(json_message, 'image', @app_id, 'default_requests', YAML.dump({})) + SmoochWorker.perform_async(json_message, 'image', @app_id, 'default_requests') end end end @@ -203,9 +203,7 @@ def teardown Bot::Smooch.save_message(message.to_json, @app_id) end message['mediaUrl'] = @video_url_2 - assert_raises 'ActiveRecord::RecordInvalid' do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) # audio message = { type: 'file', @@ -224,9 +222,7 @@ def teardown Bot::Smooch.save_message(message.to_json, @app_id) end message['mediaUrl'] = @audio_url_2 - assert_raises 'ActiveRecord::RecordInvalid' do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) end end diff --git a/test/models/bot/smooch_4_test.rb b/test/models/bot/smooch_4_test.rb index 0a1201b503..5ed99aa1e7 100644 --- a/test/models/bot/smooch_4_test.rb +++ b/test/models/bot/smooch_4_test.rb @@ -596,9 +596,7 @@ def teardown medias_count = Media.count assert_no_difference 'ProjectMedia.count' do - assert_raises ActiveRecord::StatementInvalid do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) end assert_equal medias_count, Media.count end diff --git a/test/models/bot/smooch_6_test.rb b/test/models/bot/smooch_6_test.rb index bcbd691027..baf44e760c 100644 --- a/test/models/bot/smooch_6_test.rb +++ b/test/models/bot/smooch_6_test.rb @@ -396,7 +396,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm.id, pm.class.name) message = { type: 'text', text: random_string, @@ -413,7 +413,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm.id, pm.class.name) # verifiy new channel value data = {"main" => CheckChannels::ChannelCodes::MANUAL, "others" => [CheckChannels::ChannelCodes::WHATSAPP, CheckChannels::ChannelCodes::MESSENGER]} assert_equal data, pm.reload.channel @@ -433,7 +433,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm2) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm2.id, pm2.class.name) # verifiy new channel value data = {"main" => CheckChannels::ChannelCodes::WHATSAPP, "others" => [CheckChannels::ChannelCodes::MESSENGER]} assert_equal data, pm2.reload.channel @@ -466,7 +466,7 @@ def send_message_outside_24_hours_window(template, pm = nil) assert_raises ActiveRecord::StatementInvalid do 3.times do |i| threads << Thread.new { - Bot::Smooch.save_message(message.to_json, @app_id, author, 'timeout_requests', nil) + Bot::Smooch.save_message(message.to_json, @app_id, author, 'timeout_requests', nil, nil) } end threads.map(&:join) diff --git a/test/models/bot/smooch_7_test.rb b/test/models/bot/smooch_7_test.rb index 6e170ecc11..ad852ffb92 100644 --- a/test/models/bot/smooch_7_test.rb +++ b/test/models/bot/smooch_7_test.rb @@ -502,12 +502,12 @@ def teardown language: 'en', } end - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'timeout_search_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'timeout_search_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) message = lambda do { type: 'text', @@ -526,9 +526,9 @@ def teardown language: 'en', } end - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm2) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm2.id, pm2.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2.id, pm2.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2.id, pm2.class.name) # Verify cached field assert_equal 6, pm.tipline_search_results_count assert_equal 2, pm.positive_tipline_search_results_count