Skip to content

Commit

Permalink
CV2-4666: Sentry issue (#1907)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
melsawy authored Jun 4, 2024
1 parent 410ef56 commit fe6a0f9
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 28 deletions.
9 changes: 7 additions & 2 deletions app/models/concerns/smooch_messages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,16 +322,21 @@ 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
team_id = self.config['team_id'].to_i
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
Expand Down
11 changes: 8 additions & 3 deletions app/workers/smooch_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 3 additions & 7 deletions test/models/bot/smooch_3_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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

Expand Down
4 changes: 1 addition & 3 deletions test/models/bot/smooch_4_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test/models/bot/smooch_6_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 9 additions & 9 deletions test/models/bot/smooch_7_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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
Expand Down

0 comments on commit fe6a0f9

Please sign in to comment.