Avoid sending duplicate notifications

app/models/member.rb

@@ -51,6 +51,8 @@ class Member < ApplicationRecord
   def regenerate_notifications
     notifications.where(status: 'scheduled').delete_all
 
+    dates = notifications.pluck(:to_be_sent_on)
+
     return if expires_on.nil?
 
     [
@@ -60,7 +62,9 @@ class Member < ApplicationRecord
       { to_be_sent_on: expires_on + 30.days, template: "expired_30d_ago" },
       { to_be_sent_on: expires_on + 60.days, template: "expired_60d_ago" },
       { to_be_sent_on: expires_on + 90.days, template: "cancelled" },
-    ].reject { |n| n[:to_be_sent_on].past? }.each do |n|
+    ].reject do |n|
+      n[:to_be_sent_on].past? || dates.include?(n[:to_be_sent_on])
+    end.each do |n|
       notifications.create(n.merge(status: "scheduled"))
     end
   end

app/models/notification.rb

@@ -14,7 +14,6 @@ class Notification < ApplicationRecord
     NotificationMailer.with(notification: self).send(template).deliver_now!
 
     update(status: 'sent', sent_at: Time.current)
-  rescue
     # TODO: do something about failures
   end
 end

test/fixtures/notifications.yml

@@ -1,11 +0,0 @@
-# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
-
-# This model initially had no columns defined. If you add columns to the
-# model remove the "{}" from the fixture names and add the columns immediately
-# below each fixture, per the syntax in the comments below
-#
-one: {}
-# column: value
-#
-two: {}
-# column: value

test/mailers/notification_mailer_test.rb

@@ -1,52 +0,0 @@
-require "test_helper"
-
-class NotificationMailerTest < ActionMailer::TestCase
-  test "expiration_in_60d" do
-    mail = NotificationMailer.expiration_in_60d
-    assert_equal "Expiration in 60d", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-  test "expiration_in_30d" do
-    mail = NotificationMailer.expiration_in_30d
-    assert_equal "Expiration in 30d", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-  test "expired" do
-    mail = NotificationMailer.expired
-    assert_equal "Expired", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-  test "expired_30d_ago" do
-    mail = NotificationMailer.expired_30d_ago
-    assert_equal "Expired 30d ago", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-  test "expired_60d_ago" do
-    mail = NotificationMailer.expired_60d_ago
-    assert_equal "Expired 60d ago", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-  test "cancelled" do
-    mail = NotificationMailer.cancelled
-    assert_equal "Cancelled", mail.subject
-    assert_equal ["to@example.org"], mail.to
-    assert_equal ["from@example.com"], mail.from
-    assert_match "Hi", mail.body.encoded
-  end
-
-end

test/models/member_test.rb

@@ -23,4 +23,33 @@ class MemberTest < ActiveSupport::TestCase
         assert_equal @member.expected_status, :cancelled
      end
    end
+
+   test "regenerate notifications creates all 6 notifications" do
+     @member.regenerate_notifications
+
+     assert_equal @member.notifications.count, 6
+  end
+
+  test "regenerate_notifications does not create notifications in the past" do
+    Timecop.freeze(Date.today + 1.year) do
+      @member.regenerate_notifications
+      assert_equal @member.notifications.count, 4
+    end
+
+    Timecop.freeze(Date.today + 2.year) do
+      @member.regenerate_notifications
+      assert_equal @member.notifications.count, 0
+    end
+  end
+
+  test "regenerate_notifications does not create manually sent notifications" do
+    @member.regenerate_notifications
+
+    @member.notifications.first.deliver!
+    assert_equal 5, @member.notifications.where(status: 'scheduled').count
+
+    @member.regenerate_notifications
+    assert_equal 1, @member.notifications.where(status: 'sent').count
+    assert_equal 5, @member.notifications.where(status: 'scheduled').count
+  end
 end