feat: Refactor event reminder job and notifier to use params for event and recipient, add tests for job functionality

Author: rsmithlal

app/jobs/better_together/event_reminder_job.rb

@@ -45,7 +45,7 @@ def send_reminders_to_attendees(event, attendees, reminder_type)
 
     def send_reminder_to_attendee(event, attendee, reminder_type)
       BetterTogether::EventReminderNotifier.with(
-        event: event,
+        record: event,
         reminder_type: reminder_type
       ).deliver(attendee)
     rescue StandardError => e

app/mailers/better_together/event_mailer.rb

@@ -4,28 +4,28 @@ module BetterTogether
   # Mailer for event-related notifications
   class EventMailer < ApplicationMailer
     # Sends event reminder emails
-    def event_reminder(recipient, event, reminder_type = '24_hours')
-      @event = event
-      @reminder_type = reminder_type
-      @recipient = recipient
+    def event_reminder
+      @event = params[:event]
+      @reminder_type = params[:reminder_type] || '24_hours'
+      @recipient = params[:person]
       @platform = BetterTogether::Platform.find_by(host: true)
 
       mail(
-        to: recipient.email,
-        subject: reminder_subject(event)
+        to: @recipient.email,
+        subject: reminder_subject(@event)
       )
     end
 
     # Sends event update emails
-    def event_update(recipient, event, changed_attributes)
-      @event = event
-      @changed_attributes = changed_attributes
-      @recipient = recipient
+    def event_update
+      @event = params[:event]
+      @changed_attributes = params[:changed_attributes]
+      @recipient = params[:person]
       @platform = BetterTogether::Platform.find_by(host: true)
 
       mail(
-        to: recipient.email,
-        subject: update_subject(event)
+        to: @recipient.email,
+        subject: update_subject(@event)
       )
     end
 

app/notifiers/better_together/event_reminder_notifier.rb

@@ -7,17 +7,28 @@ class EventReminderNotifier < ApplicationNotifier
       config.if = -> { should_notify? }
     end
     deliver_by :email, mailer: 'BetterTogether::EventMailer', method: :event_reminder, params: :email_params do |config|
-      config.if = -> { recipient_has_email? && should_notify? }
+      config.wait = 15.minutes
+      config.if = -> { send_email_notification? }
     end
 
-    param :event, :reminder_type
+    validates :record, presence: true
+    required_param :reminder_type
 
-    notification_methods do
-      delegate :event, :reminder_type, to: :event
+    def event
+      record
+    end
+
+    def reminder_type
+      params[:reminder_type] || '24_hours'
     end
 
-    def event = params[:event]
-    def reminder_type = params[:reminder_type] || '24_hours'
+    def identifier
+      event.id
+    end
+
+    def url
+      ::BetterTogether::Engine.routes.url_helpers.event_url(event, locale: I18n.locale)
+    end
 
     def title
       I18n.t('better_together.notifications.event_reminder.title',
@@ -36,12 +47,22 @@ def body
       end
     end
 
-    def build_message(_notification)
-      { title:, body: }
+    def build_message(notification)
+      {
+        title:,
+        body:,
+        identifier:,
+        url:,
+        unread_count: notification.recipient.notifications.unread.count
+      }
     end
 
     def email_params(_notification)
-      { event:, reminder_type: }
+      {
+        event: event,
+        person: recipient,
+        reminder_type: reminder_type
+      }
     end
 
     private
@@ -72,17 +93,35 @@ def formatted_start_time
     end
 
     notification_methods do
-      def recipient_has_email?
-        recipient.respond_to?(:email) && recipient.email.present? &&
-          (!recipient.respond_to?(:notification_preferences) ||
-           recipient.notification_preferences.fetch('notify_by_email', true))
+      delegate :event, to: :event
+      delegate :url, to: :event
+      delegate :identifier, to: :event
+      delegate :reminder_type, to: :event
+
+      def send_email_notification?
+        recipient.email.present? && recipient.notify_by_email && should_send_email?
       end
 
       def should_notify?
         event.present? && event.starts_at.present? &&
           (!recipient.respond_to?(:notification_preferences) ||
            recipient.notification_preferences.fetch('event_reminders', true))
       end
+
+      def should_send_email?
+        # Check for unread notifications for the recipient for the record's event
+        unread_notifications = recipient.notifications.where(
+          event_id: BetterTogether::EventReminderNotifier.where(params: { event_id: event.id }).select(:id),
+          read_at: nil
+        ).order(created_at: :desc)
+
+        if unread_notifications.none?
+          false
+        else
+          # Only send one email per unread notifications per event
+          event.id == unread_notifications.last.event.record_id
+        end
+      end
     end
   end
 end

spec/jobs/better_together/event_reminder_job_spec.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+module BetterTogether
+  RSpec.describe EventReminderJob do
+    include ActiveJob::TestHelper
+
+    let(:person) { create(:person) }
+    let(:event) { create(:event, :upcoming, :with_attendees) }
+
+    describe '#perform' do
+      context 'with valid event' do
+        it 'sends reminders to all going attendees' do
+          expect do
+            described_class.perform_now(event)
+          end.to change(Noticed::Notification, :count).by_at_least(1)
+        end
+
+        it 'accepts reminder type parameter' do
+          expect do
+            described_class.perform_now(event, '1_hour')
+          end.not_to raise_error
+        end
+      end
+
+      context 'with invalid event' do
+        it 'handles missing event gracefully' do
+          expect do
+            described_class.perform_now(nil)
+          end.not_to raise_error
+        end
+
+        it 'handles event without start time' do
+          draft_event = create(:event, :draft)
+          expect do
+            described_class.perform_now(draft_event)
+          end.not_to raise_error
+        end
+      end
+
+      context 'when event has no attendees' do
+        let(:event_without_attendees) { create(:event, :upcoming) }
+
+        it 'completes without sending notifications' do
+          expect do
+            described_class.perform_now(event_without_attendees)
+          end.not_to change(Noticed::Notification, :count)
+        end
+      end
+    end
+
+    describe 'queue configuration' do
+      it 'uses the notifications queue' do
+        expect(described_class.queue_name).to eq('notifications')
+      end
+    end
+
+    describe 'retry and error handling' do
+      it 'handles errors gracefully' do
+        allow_any_instance_of(described_class).to receive(:find_event).and_raise(StandardError, 'Test error')
+        expect do
+          described_class.perform_now(event)
+        end.not_to raise_error
+      end
+
+      it 'handles missing events gracefully' do
+        expect do
+          described_class.perform_now(999_999)
+        end.not_to raise_error
+      end
+    end
+  end
+end