From 7365a56e51a0482485abaf332de1f04d5a9815ae Mon Sep 17 00:00:00 2001 From: Nicolas Florentin Date: Thu, 5 Oct 2023 13:49:29 +0200 Subject: [PATCH] Fix a bug: fix ReservationReminderWorker, was sending reservation reminder to users with a event reservation not validated by admin + adds tests for all scenarios --- CHANGELOG.md | 4 + app/workers/reservation_reminder_worker.rb | 2 + .../reservation_reminder_worker_test.rb | 109 ++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 test/workers/reservation_reminder_worker_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 2147156cb..5a75c3f8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog Fab-manager +## next release + +- Fix a bug: fix ReservationReminderWorker, was sending reservation reminder to users with a event reservation not validated by admin + adds tests for all scenarios + ## v6.1.2 2023 October 2 - Fix a bug: minor pb (exception raised) when bot hit api/projects#search without beeing authenticated diff --git a/app/workers/reservation_reminder_worker.rb b/app/workers/reservation_reminder_worker.rb index ca68ebbbc..7c8dacc47 100644 --- a/app/workers/reservation_reminder_worker.rb +++ b/app/workers/reservation_reminder_worker.rb @@ -17,6 +17,7 @@ class ReservationReminderWorker Reservation.joins(slots_reservations: :slot) .where('slots.start_at >= ? AND slots.start_at <= ? AND slots_reservations.canceled_at IS NULL', starting, ending) + .includes(:reservable, :slots_reservations) .each do |r| already_sent = Notification.where( attached_object_type: Reservation.name, @@ -24,6 +25,7 @@ class ReservationReminderWorker notification_type_id: NotificationType.find_by(name: 'notify_member_reservation_reminder') ).count next if already_sent.positive? + next if r.reservable.is_a?(Event) && r.reservable.pre_registration? && !r.slots_reservations.all?(&:is_valid?) NotificationCenter.call type: 'notify_member_reservation_reminder', receiver: r.user, diff --git a/test/workers/reservation_reminder_worker_test.rb b/test/workers/reservation_reminder_worker_test.rb new file mode 100644 index 000000000..f718559b1 --- /dev/null +++ b/test/workers/reservation_reminder_worker_test.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require 'test_helper' +require 'minitest/autorun' + +class ReservationReminderWorkerTest < ActiveSupport::TestCase + include ActionMailer::TestHelper + + setup do + @worker = ReservationReminderWorker.new + + @training_slot = slots(:slot_1) + + @event = events(:event_1) + @event_slot = slots(:slot_129) + @event_reservation = Reservation.create!( + reservable: @event, + nb_reserve_places: 1, + statistic_profile_id: statistic_profiles(:pdurand).id, + slots_reservations_attributes: [slot_id: @event_slot.id] + ) + end + + test 'send a reminder 24 hours before by default and is idempotent' do + travel_to @training_slot.start_at - 24.hours + + assert_enqueued_emails 1 do + @worker.perform + end + + assert_enqueued_emails 0 do + @worker.perform + end + end + + test 'reminder_delay can be changed and is respected' do + Setting.set('reminder_delay', 15) + + travel_to @training_slot.start_at - 17.hours + + assert_enqueued_emails 0 do + @worker.perform + end + + travel_to @training_slot.start_at - 13.hours + + assert_enqueued_emails 0 do + @worker.perform + end + + travel_to @training_slot.start_at - 15.hours + + assert_enqueued_emails 1 do + @worker.perform + end + + assert_enqueued_emails 0 do + @worker.perform + end + end + + test 'do nothing if setting reminder_enable is false' do + Setting.set('reminder_enable', false) + + assert_enqueued_emails 0 do + assert_nil @worker.perform + end + end + + test 'do nothing if slots_reservations is canceled' do + travel_to @training_slot.start_at - 24.hours + + @training_slot.slots_reservations[0].update!(canceled_at: 1.day.ago) + + assert_enqueued_emails 0 do + @worker.perform + end + end + + test '[event] do nothing if event.pre_registration is true and slots_reservation is not valid' do + @event.update!(pre_registration: true) + @event_reservation.slots_reservations.update_all(is_valid: false) + + travel_to @event_slot.start_at - 24.hours + + assert_enqueued_emails 0 do + @worker.perform + end + end + + test '[event] do send the notification if event.pre_registration is true and slots_reservation is valid' do + @event.update!(pre_registration: true) + @event_reservation.slots_reservations.update_all(is_valid: true) + + travel_to @event_slot.start_at - 24.hours + + assert_enqueued_emails 1 do + @worker.perform + end + end + + test '[event] do send the notification if event.pre_registration is false' do + travel_to @event_slot.start_at - 24.hours + + assert_enqueued_emails 1 do + @worker.perform + end + end +end