fix[Op#59433]: simplify reminder model to directly linked to notifica…

…tion

Introduce simple status enums to track various reminder states

app/models/reminder.rb

@@ -29,9 +29,7 @@
 class Reminder < ApplicationRecord
   belongs_to :remindable, polymorphic: true
   belongs_to :creator, class_name: "User"
+  belongs_to :notification, optional: true
 
-  has_many :reminder_notifications, dependent: :destroy
-
-  def notified?  = notified_at.present?
-  def scheduled? = job_id.present?
+  enum :status, { pending: 0, scheduled: 1, notified: 2, done: 9 }
 end

app/services/reminders/create_service.rb

@@ -31,7 +31,7 @@ class CreateService < ::BaseServices::Create
     def after_perform(service_call)
       reminder = service_call.result
       job = Reminders::ScheduleReminderJob.schedule(reminder)
-      reminder.update_column(:job_id, job.job_id)
+      reminder.update_columns(status: :scheduled, job_id: job.job_id)
 
       service_call
     end

app/workers/reminders/schedule_reminder_job.rb

@@ -40,8 +40,7 @@ def perform(reminder)
       create_notification_service = create_notification_from_reminder(reminder)
 
       create_notification_service.on_success do |service_result|
-        ReminderNotification.create!(reminder:, notification: service_result.result)
-        reminder.update_column(:notified_at, Time.current)
+        reminder.update_columns(status: :notified, notification_id: service_result.result.id)
       end
 
       create_notification_service.on_failure do |service_result|

db/migrate/20241119131205_create_reminders.rb

@@ -3,10 +3,11 @@ def change
     create_table :reminders do |t|
       t.references :remindable, polymorphic: true, null: false
       t.references :creator, null: false, foreign_key: { to_table: :users }
+      t.references :notification, foreign_key: true
       t.datetime :remind_at, null: false
-      t.datetime :notified_at
+      t.integer :status, default: 0, null: false
       t.string :job_id
-      t.text :note
+      t.string :note
 
       t.timestamps
     end

spec/factories/reminders_factory.rb

@@ -38,7 +38,9 @@
     end
 
     trait :notified do
-      notified_at { remind_at + 1.second }
+      job_id { SecureRandom.uuid }
+      status { :notified }
+      notification factory: :notification
     end
   end
 end

spec/models/reminder_spec.rb

@@ -32,34 +32,14 @@
   describe "Associations" do
     it { is_expected.to belong_to(:remindable) }
     it { is_expected.to belong_to(:creator).class_name("User") }
-    it { is_expected.to have_many(:reminder_notifications).dependent(:destroy) }
+    it { is_expected.to belong_to(:notification).optional }
   end
 
-  describe "#notified?" do
-    context "when notified_at is present" do
-      subject { build(:reminder, :notified) }
-
-      it { is_expected.to be_notified }
-    end
-
-    context "when notified_at is not present" do
-      subject { build(:reminder) }
-
-      it { is_expected.not_to be_notified }
-    end
-  end
-
-  describe "#scheduled?" do
-    context "when job_id is present" do
-      subject { build(:reminder, :scheduled) }
-
-      it { is_expected.to be_scheduled }
-    end
-
-    context "when job_id is not present" do
-      subject { build(:reminder) }
-
-      it { is_expected.not_to be_scheduled }
+  describe "Enumns" do
+    it do
+      expect(subject).to define_enum_for(:status)
+        .with_values({ pending: 0, scheduled: 1, notified: 2, done: 9 })
+        .backed_by_column_of_type(:integer)
     end
   end
 end

spec/services/reminders/create_service_spec.rb

@@ -36,7 +36,7 @@
     let(:factory) { :reminder }
 
     before do
-      allow(model_instance).to receive(:update_column).and_return(true)
+      allow(model_instance).to receive(:update_columns)
       allow(Reminders::ScheduleReminderJob).to receive(:schedule)
         .with(model_instance)
         .and_return(instance_double(Reminders::ScheduleReminderJob, job_id: 1))
@@ -46,7 +46,7 @@
       subject
 
       expect(Reminders::ScheduleReminderJob).to have_received(:schedule).with(model_instance)
-      expect(model_instance).to have_received(:update_column).with(:job_id, 1)
+      expect(model_instance).to have_received(:update_columns).with(status: :scheduled, job_id: 1)
     end
   end
 end

spec/workers/reminders/schedule_reminder_job_spec.rb

@@ -49,28 +49,32 @@
 
     it "creates a notification from the reminder" do
       notification_svc = nil
-      expect { notification_svc = subject }.to change(Notification, :count).by(1) & change(ReminderNotification, :count).by(1)
+      expect { notification_svc = subject }.to change(Notification, :count).by(1) & change(reminder, :status).to("notified")
 
-      aggregate_failures "notification attributes" do
-        notification = notification_svc.result
+      notification = notification_svc.result
 
+      aggregate_failures "notification attributes" do
         expect(notification.recipient_id).to eq(reminder.creator_id)
         expect(notification.resource).to eq(reminder.remindable)
         expect(notification.reason).to eq("reminder")
       end
 
       aggregate_failures "marks the reminder as notified" do
         expect(reminder.reload).to be_notified
+        expect(reminder.notification_id).to eq(notification.id)
       end
     end
 
     context "when the reminder is already notified" do
+      let(:reminder) { build_stubbed(:reminder, :notified) }
+
       before do
-        reminder.update_column(:notified_at, Time.current)
+        allow(Notifications::CreateService).to receive(:new).and_call_original
       end
 
       it "does not create a notification from the reminder" do
         expect { subject }.not_to change(Notification, :count)
+        expect(Notifications::CreateService).not_to have_received(:new)
       end
     end
   end