CV2- 4715 no method error undefined method user agent for nil class (#…

…1919) * CV2-4441: fix sentry issue * CV2-4715: fix test coverage
meedan · Jun 10, 2024 · 897ab2e · 897ab2e
1 parent e75b7e2
commit 897ab2e
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 15 deletions.
diff --git a/app/mailers/security_mailer.rb b/app/mailers/security_mailer.rb
@@ -4,23 +4,25 @@ class SecurityMailer < ApplicationMailer
   def notify(user_id, type, activity_id)
     user = User.find_by_id(user_id)
     activity = LoginActivity.find_by_id(activity_id)
+    return if user.nil? || activity.nil?
     address = []
     Geocoder.configure(language: I18n.locale)
-    ip_result = Geocoder.search(activity&.ip)&.first
+    ip_result = Geocoder.search(activity.ip)&.first
     unless ip_result.blank? || ip_result.data["loc"].blank?
       loc_result = Geocoder.search(ip_result.data["loc"]).first
       address = [loc_result.city, loc_result.country] unless loc_result.nil?
     end
     @user = user
     @type = type
     email = user.email
-    @user_agent = UserAgent.parse(activity.user_agent)
+    user_agent = UserAgent.parse(activity.user_agent)
     @location = address.compact.join(', ')
     @timestamp = activity.created_at
     @ip = activity.ip
-    @platform = begin @user_agent.os.split.first rescue 'Unknown' end
+    @platform = begin user_agent.os.split.first rescue 'Unknown' end
+    @browser = begin user_agent.browser rescue 'Unknown' end
     subject = I18n.t("mail_security.#{type}_subject",
-      app_name: CheckConfig.get('app_name'), browser: @user_agent.browser, platform: @platform)
+      app_name: CheckConfig.get('app_name'), browser: @browser, platform: @platform)
     mail(to: email, subject: subject)
   end
 
@@ -29,5 +31,4 @@ def custom_notification(user_id, subject)
     attachments.inline['signup.png'] = File.read('public/images/signup.png')
     mail(to: @user.email, subject: subject)
   end
-
 end
diff --git a/app/views/security_mailer/notify.html.erb b/app/views/security_mailer/notify.html.erb
@@ -35,7 +35,7 @@
                         </tr>
                       </table>
                       <div class="h1 notify-title__header" style="font-size: 40px; font-weight: bold; letter-spacing: -0.8px; line-height: 40px;">
-                        <%= I18n.t("mail_security.#{@type}", browser: @user_agent.browser, platform: @platform,
+                        <%= I18n.t("mail_security.#{@type}", browser: @browser, platform: @platform,
                           location: @location)
                           %>
                         </div>
@@ -131,7 +131,7 @@
                                                 </table>
                                               </div>
                                               <div class="text-gray" style="color: #9a9a9a !important;"><span class="span" style="font-size: 15px; line-height: 15px;">
-                                                <%= I18n.t("mail_security.devise_name", browser: @user_agent.browser, platform: @platform) %>
+                                                <%= I18n.t("mail_security.devise_name", browser: @browser, platform: @platform) %>
                                                 </span>
                                               </div>
                                             </div>

diff --git a/app/views/security_mailer/notify.text.erb b/app/views/security_mailer/notify.text.erb
@@ -1,13 +1,13 @@
 <%= I18n.t("mails_notifications.greeting", username: @user.name) %>
 =======================================================
-<%= I18n.t("mail_security.#{@type}", browser: @user_agent.browser, platform: @platform, location: @location) %>
+<%= I18n.t("mail_security.#{@type}", browser: @browser, platform: @platform, location: @location) %>
 =======================================================
 
 <%= I18n.t("mail_security.#{@type}_text", app_name: CheckConfig.get('app_name'), change_password: t("mail_security.password_text")) %>
 <%= "#{CheckConfig.get('checkdesk_client')}/check/user/password-reset" %>
 
 <%= I18n.t("mail_security.time_h").upcase %> (<%= I18n.l @timestamp, format: :email %>)
-<%= I18n.t("mail_security.device_h").upcase %> (<%= I18n.t("mail_security.devise_name", browser: @user_agent.browser, platform: @platform) %>)
+<%= I18n.t("mail_security.device_h").upcase %> (<%= I18n.t("mail_security.devise_name", browser: @browser, platform: @platform) %>)
 <%= I18n.t("mail_security.location_h").upcase %> (<%= @location %>)
 <%= I18n.t("mail_security.location_disclaimer") %>
 <%= I18n.t("mail_security.ip_h").upcase %> (<%= @ip %>)

diff --git a/app/workers/smooch_worker.rb b/app/workers/smooch_worker.rb
@@ -8,12 +8,6 @@ class SmoochWorker
 
   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(message_json, app_id, User.current, request_type, associated_id, associated_class)
     end

diff --git a/test/models/bot/smooch_3_test.rb b/test/models/bot/smooch_3_test.rb
@@ -223,6 +223,9 @@ def teardown
       end
       message['mediaUrl'] = @audio_url_2
       Bot::Smooch.save_message(message.to_json, @app_id)
+      # should rescue invalid URL
+      message['mediaUrl'] = random_url
+      Bot::Smooch.detect_media_type(message)
     end
   end