From c1c359f03063cec4d10d6c21052f6fa5b1e2a68d Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@37signals.com>
Date: Thu, 28 Aug 2025 12:29:40 +0200
Subject: [PATCH] Review existing bundles when changing the pending bundles

---
 app/models/notification/bundle.rb       |  4 +--
 app/models/user/settings.rb             | 36 +++++++++++++++++++++++++
 test/models/notification/bundle_test.rb |  6 ++---
 test/models/user/settings_test.rb       | 36 +++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100644 test/models/user/settings_test.rb

diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb
index 78d4b5dd8..77f8d73ec 100644
--- a/app/models/notification/bundle.rb
+++ b/app/models/notification/bundle.rb
@@ -48,11 +48,9 @@ class Notification::Bundle < ApplicationRecord
   end
 
   private
-    AGGREGATION_PERIOD = 4.hours
-
     def set_default_window
       self.starts_at ||= Time.current
-      self.ends_at ||= AGGREGATION_PERIOD.from_now
+      self.ends_at ||= user.settings.bundle_aggregation_period.from_now
     end
 
     def window
diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb
index eda952eee..3de7d8a79 100644
--- a/app/models/user/settings.rb
+++ b/app/models/user/settings.rb
@@ -3,4 +3,40 @@ class User::Settings < ApplicationRecord
 
   enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ],
     default: :never, prefix: :bundle_email
+
+  after_update :review_pending_bundles, if: :saved_change_to_bundle_email_frequency?
+
+  def bundle_aggregation_period
+    case bundle_email_frequency
+    when "every_few_hours"
+      4.hours
+    when "daily"
+      1.day
+    when "weekly"
+      1.week
+    else
+      1.day
+    end
+  end
+
+  private
+    def review_pending_bundles
+      if bundle_email_never?
+        cancel_pending_bundles
+      else
+        reschedule_pending_bundles
+      end
+    end
+
+    def cancel_pending_bundles
+      user.notification_bundles.pending.find_each do |bundle|
+        bundle.destroy
+      end
+    end
+
+    def reschedule_pending_bundles
+      user.notification_bundles.pending.find_each do |bundle|
+        bundle.deliver_later
+      end
+    end
 end
diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb
index 5da167f64..a84627b3e 100644
--- a/test/models/notification/bundle_test.rb
+++ b/test/models/notification/bundle_test.rb
@@ -23,7 +23,7 @@ class Notification::BundleTest < ActiveSupport::TestCase
     notification_2 = assert_no_difference -> { @user.notification_bundles.count } do
       @user.notifications.create!(source: events(:logo_published), creator: @user)
     end
-    travel_to 3.hours.from_now
+    travel_to 3.days.from_now
 
     notification_3 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do
       @user.notifications.create!(source: events(:logo_published), creator: @user)
@@ -84,7 +84,7 @@ class Notification::BundleTest < ActiveSupport::TestCase
 
     bundle.update!(ends_at: 1.minute.ago)
 
-    perform_enqueued_jobs do
+    perform_enqueued_jobs only: Notification::Bundle::DeliverJob do
       Notification::Bundle.deliver_all
     end
 
@@ -98,7 +98,7 @@ class Notification::BundleTest < ActiveSupport::TestCase
 
     bundle.update!(ends_at: 1.minute.from_now)
 
-    perform_enqueued_jobs do
+    perform_enqueued_jobs only: Notification::Bundle::DeliverJob do
       Notification::Bundle.deliver_all
     end
 
diff --git a/test/models/user/settings_test.rb b/test/models/user/settings_test.rb
new file mode 100644
index 000000000..a508325df
--- /dev/null
+++ b/test/models/user/settings_test.rb
@@ -0,0 +1,36 @@
+require "test_helper"
+
+class User::SettingsTest < ActiveSupport::TestCase
+  setup do
+    @user = users(:david)
+    @settings = @user.settings
+  end
+
+  test "changing the bundle email frequency to never will cancel pending bundles" do
+    @settings.update!(bundle_email_frequency: :every_few_hours)
+    bundle = @user.notification_bundles.create!
+    @settings.update!(bundle_email_frequency: :never)
+    assert_nil Notification::Bundle.find_by(id: bundle.id)
+  end
+
+  test "changing the bundle email frequency will deliver pending bundles" do
+    bundle = @user.notification_bundles.create!
+    assert bundle.pending?
+
+    perform_enqueued_jobs only: Notification::Bundle::DeliverJob do
+      @settings.update!(bundle_email_frequency: :daily)
+    end
+
+    assert bundle.reload.delivered?
+  end
+
+  test "changing other settings will not affect pending bundles" do
+    bundle = @user.notification_bundles.create!
+
+    perform_enqueued_jobs only: Notification::Bundle::DeliverJob do
+      @settings.update!(updated_at: 1.hour.from_now)
+    end
+
+    assert bundle.reload.pending?
+  end
+end