Correctly handle timezones that are invalid

Facebook sometimes sends timezones such as 14

Signed-off-by: arunthampi <arun.thampi@gmail.com>

app/services/notification_service.rb

@@ -83,14 +83,12 @@ def scheduled_at(bot_user)
     return nil if notification.scheduled_at.blank?
 
     if time_zone = bot_user.user_attributes['timezone']
-      begin
+      if ActiveSupport::TimeZone[time_zone]
         notification.scheduled_at.in_time_zone(time_zone)
-      # If timezone is bad one (Facebook has known to set timezones as -7.5 for e.g.)
-      # round it up to the next one and try again
-      rescue ArgumentError => e
-        if e.message =~ /\AInvalid Timezone:/
-          return notification.scheduled_at.in_time_zone(time_zone.to_f.round)
-        end
+      elsif ActiveSupport::TimeZone[time_zone = time_zone.to_f.round]
+        notification.scheduled_at.in_time_zone(time_zone)
+      else
+        notification.scheduled_at.in_time_zone('GMT')
       end
     else
       Rails.logger.warn "[FAILED NOTIFICATION::TimeZone] Failed to schedule Notification #{notification.id} for BotUser #{bot_user.inspect}"

spec/services/notification_service_spec.rb

@@ -285,6 +285,32 @@
           expect(SendMessageJob).to_not have_received(:perform_async)
         end
       end
+
+      context 'user has an invalid timezone' do
+        before do
+          bot_user.user_attributes['timezone'] = 14
+          bot_user.save
+        end
+
+        it 'creates and enqueues messages (after rounding up timezone to the nearest timezone)' do
+          expect {
+            service.enqueue_messages
+            notification.reload
+          }.to change(notification.messages, :count).by(1)
+
+          expect(FilterBotUsersService).to have_received(:new).with(query_set)
+
+          message = notification.messages.last
+          expect(message.team_id).to eq bot_user.bot_instance.team_id
+          expect(message.user).to eq bot_user.uid
+          expect(message.text).to eq notification.content
+
+          # with time zone information from bot_user
+          expect(message.scheduled_at).to eq notification.scheduled_at.in_time_zone('GMT').utc
+
+          expect(SendMessageJob).to_not have_received(:perform_async)
+        end
+      end
     end
 
     context 'facebook' do
@@ -422,6 +448,32 @@
           expect(SendMessageJob).to_not have_received(:perform_async)
         end
       end
+
+      context 'user has an invalid timezone' do
+        before do
+          bot_user.user_attributes['timezone'] = 14
+          bot_user.save
+        end
+
+        it 'creates and enqueues messages (after rounding up timezone to the nearest timezone)' do
+          expect {
+            service.enqueue_messages
+            notification.reload
+          }.to change(notification.messages, :count).by(1)
+
+          expect(FilterBotUsersService).to have_received(:new).with(query_set)
+
+          message = notification.messages.last
+          expect(message.team_id).to eq bot_user.bot_instance.team_id
+          expect(message.user).to eq bot_user.uid
+          expect(message.text).to eq notification.content
+
+          # with time zone information from bot_user
+          expect(message.scheduled_at).to eq notification.scheduled_at.in_time_zone('GMT').utc
+
+          expect(SendMessageJob).to_not have_received(:perform_async)
+        end
+      end
     end
 
     context 'kik' do
@@ -559,6 +611,32 @@
           expect(SendMessageJob).to_not have_received(:perform_async)
         end
       end
+
+      context 'user has an invalid timezone' do
+        before do
+          bot_user.user_attributes['timezone'] = 14
+          bot_user.save
+        end
+
+        it 'creates and enqueues messages (after rounding up timezone to the nearest timezone)' do
+          expect {
+            service.enqueue_messages
+            notification.reload
+          }.to change(notification.messages, :count).by(1)
+
+          expect(FilterBotUsersService).to have_received(:new).with(query_set)
+
+          message = notification.messages.last
+          expect(message.team_id).to eq bot_user.bot_instance.team_id
+          expect(message.user).to eq bot_user.uid
+          expect(message.text).to eq notification.content
+
+          # with time zone information from bot_user
+          expect(message.scheduled_at).to eq notification.scheduled_at.in_time_zone('GMT').utc
+
+          expect(SendMessageJob).to_not have_received(:perform_async)
+        end
+      end
     end
   end
 end