use local time table to find reminder recipients

app/models/users/scopes/having_reminder_mail_to_send_now.rb

@@ -37,71 +37,42 @@ module Users::Scopes
       # * That user has an unread notification
       # * The user hasn't been informed about the unread notification before
       # * The user has configured reminder mails to be sent now.
+      # This assumes that users only have full hours specified for the time they desire
+      # to receive a reminder mail.
       def having_reminder_mail_to_send_now
         # Left outer join as not all user instances have preferences associated
-        # but we still want to select them
-        recipient_candidates = User.active
-                                   .left_joins(:preference)
-                                   .where(where_statement)
+        # but we still want to select them.
+        recipient_candidates = User
+                                 .active
+                                 .left_joins(:preference)
+                                 .joins(local_time_join)
 
         subscriber_ids = Notification
                            .unsent_reminders_before(recipient: recipient_candidates, time: Time.current)
                            .group(:recipient_id)
                            .select(:recipient_id)
 
-        User.where(id: subscriber_ids)
+        where(id: subscriber_ids)
       end
 
-      def where_statement
-        current_timestamp_utc = Time.current.getutc
-
-        age = age_statement(current_timestamp_utc)
-        <<-SQL.squish
-        #{age} < make_interval(mins=>30)
-        AND
-        #{age} >= make_interval(mins=>0)
-        SQL
-      end
-
-      # Creates a SQL snippet for calculating the time between now and
-      # the reminder time slot with the each user's time zone
-      # @param [Time] current_timestamp_utc
-      def age_statement(current_timestamp_utc)
-        year = current_timestamp_utc.year
-        month = current_timestamp_utc.month
-        day = current_timestamp_utc.day
-
-        case_statement = case_statement_zone_name_to_offset
-
-        slot_time = Time.zone.parse(Setting.notification_email_digest_time)
-
-        <<-SQL.squish
-        age(
-          now(),
-          make_timestamptz(#{year}, #{month}, #{day}, #{slot_time.hour}, #{slot_time.min}, 0, (#{case_statement}))
-        )
+      def local_time_join
+        <<~SQL.squish
+          JOIN (#{local_time_table}) AS local_times
+          ON COALESCE(user_preferences.settings->>'time_zone', 'UTC') = local_times.zone
+          AND local_times.time = '#{Setting.notification_email_digest_time}:00+00:00'
         SQL
       end
 
-      def case_statement_zone_name_to_offset
-        return @case_statement_zone_name_to_offset if @case_statement_zone_name_to_offset
-
+      def local_time_table
         current_time = Time.current
-        statement = ActiveSupport::TimeZone.all.map do |zone|
-          offset = current_offset(zone, current_time)
-          "WHEN user_preferences.settings->>'time_zone' = '#{zone.name.gsub("'") { "''" }}' THEN '#{offset}'"
-        end
-        @case_statement_zone_name_to_offset = "CASE\n#{statement.join("\n")}\nELSE '+00:00'\nEND"
-      end
 
-      # The real offset of a time zone depends of the moment we ask. Winter and summer time
-      # have different offsets of UTC. For instance, time zone "Berlin" in winter has +01:00
-      # and in summer +02:00
-      # @param [Time] time
-      # @param [TimeZone] zone
-      # @return [String]
-      def current_offset(zone, time = Time.current)
-        time.in_time_zone(zone).formatted_offset
+        times_with_zones = ActiveSupport::TimeZone
+                           .all
+                           .map do |z|
+                             [current_time.in_time_zone(z).strftime('%H:00:00+00:00'), z.name.gsub("'", "''")]
+                           end
+
+        "SELECT * FROM #{arel_table.grouping(Arel::Nodes::ValuesList.new(times_with_zones)).as('t(time, zone)').to_sql}"
       end
     end
   end

app/workers/notifications/schedule_reminder_mails_job.rb

@@ -30,8 +30,8 @@
 
 module Notifications
   class ScheduleReminderMailsJob < Cron::CronJob
-    # runs every 30min step, so 00:00, 00:30, 01:00, 01:30...
-    self.cron_expression = '/30 * * * *'
+    # runs every hour, so 00:00, 01:00...
+    self.cron_expression = '0 * * * *'
 
     def perform
       User.having_reminder_mail_to_send_now.pluck(:id).each do |subscriber_ids|

spec/features/notifications/reminder_mail_spec.rb

@@ -41,7 +41,7 @@ describe "Reminder email", type: :feature, js: true do
     end
   end
 
-  context 'configuring via the my page' do
+  context 'when configuring via the my page' do
     let(:reminders_settings_page) { Pages::My::Reminders.new(current_user) }
 
     current_user do
@@ -51,7 +51,7 @@ describe "Reminder email", type: :feature, js: true do
     it_behaves_like 'reminder settings'
   end
 
-  context 'configuring via the user administration page' do
+  context 'when configuring via the user administration page' do
     let(:reminders_settings_page) { Pages::Reminders::Settings.new(other_user) }
 
     let(:other_user) { FactoryBot.create :user }
@@ -63,7 +63,7 @@ describe "Reminder email", type: :feature, js: true do
     it_behaves_like 'reminder settings'
   end
 
-  context 'sending' do
+  describe 'sending' do
     let!(:project) { FactoryBot.create :project, members: { current_user => role } }
     let!(:mute_project) { FactoryBot.create :project, members: { current_user => role } }
     let(:role) { FactoryBot.create(:role, permissions: %i[view_work_packages]) }
@@ -134,6 +134,5 @@ describe "Reminder email", type: :feature, js: true do
       expect(ActionMailer::Base.deliveries.first.subject)
         .to eql "OpenProject - 1 unread notification including a mention"
     end
-
   end
 end

spec/support/schedule_reminder_mails.rb

@@ -29,6 +29,5 @@
 ##
 # Calculates a slot in the user's local time that hits for scheduling reminder mail jobs
 def hitting_reminder_slot_for(user, current_utc_time = Time.current.getutc)
-  local_time = current_utc_time.in_time_zone(user.time_zone)
-  "#{'%02d' % local_time.hour}:#{local_time.min <= 30 ? '00' : '30'}"
+  current_utc_time.in_time_zone(user.time_zone).strftime('%H:00')
 end