From 81d1f10acf807f7cd907bba231ecd44c48c849a7 Mon Sep 17 00:00:00 2001
From: arunthampi <arun.thampi@gmail.com>
Date: Thu, 12 Jan 2017 07:20:07 -0800
Subject: [PATCH] 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       | 12 ++--
 spec/services/notification_service_spec.rb | 78 ++++++++++++++++++++++
 2 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index e5737a14..fa9d5199 100644
--- a/app/services/notification_service.rb
+++ b/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}"
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 6c3987ab..6eb7f1d8 100644
--- a/spec/services/notification_service_spec.rb
+++ b/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