From 71b45a9a042ceab80b3ce21ca1a78919a3675ac0 Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Thu, 23 Nov 2023 16:38:03 -0800
Subject: [PATCH] FIX: prevent holiday duplicates when username is changed
 (#488)

When username was changed, system was creating duplicated holidays.

In addition to fix, migration was added to remove duplicated and
incorrect records.
---
 ...231123233308_delete_duplicated_holidays.rb | 19 +++++++++++++++++++
 jobs/scheduled/create_holiday_events.rb       |  2 +-
 .../scheduled/create_holiday_events_spec.rb   | 13 +++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20231123233308_delete_duplicated_holidays.rb

diff --git a/db/migrate/20231123233308_delete_duplicated_holidays.rb b/db/migrate/20231123233308_delete_duplicated_holidays.rb
new file mode 100644
index 00000000..99ea8a20
--- /dev/null
+++ b/db/migrate/20231123233308_delete_duplicated_holidays.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class DeleteDuplicatedHolidays < ActiveRecord::Migration[7.0]
+  def up
+    execute <<~SQL
+      DELETE
+        FROM calendar_events ce
+      WHERE
+        ce.id IN (SELECT ce2.id FROM calendar_events ce2
+                  INNER JOIN users ON users.id = ce2.user_id
+                  WHERE ce2.post_id IS NULL
+                    AND ce2.username != users.username)
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/jobs/scheduled/create_holiday_events.rb b/jobs/scheduled/create_holiday_events.rb
index dfbe40ab..1fd6391c 100644
--- a/jobs/scheduled/create_holiday_events.rb
+++ b/jobs/scheduled/create_holiday_events.rb
@@ -79,12 +79,12 @@ module Jobs
                 CalendarEvent.find_or_initialize_by(
                   topic_id: topic_id,
                   user_id: user_id,
-                  username: usernames[user_id],
                   description: holiday[:name],
                   start_date: date,
                   region: region,
                 )
 
+              event.username = usernames[user_id]
               event.timezone = tz.name if tz
               event.save!
             end
diff --git a/spec/jobs/scheduled/create_holiday_events_spec.rb b/spec/jobs/scheduled/create_holiday_events_spec.rb
index 7b791a05..327066e7 100644
--- a/spec/jobs/scheduled/create_holiday_events_spec.rb
+++ b/spec/jobs/scheduled/create_holiday_events_spec.rb
@@ -111,6 +111,19 @@ describe DiscourseCalendar::CreateHolidayEvents do
     expect(CalendarEvent.pluck(:region, :description, :start_date, :username)).to eq([])
   end
 
+  it "does not create duplicates when username is changed" do
+    frenchy
+    DiscourseCalendar::CreateHolidayEvents.new.execute(nil)
+    created_event = CalendarEvent.last
+    expect(created_event.username).to eq(frenchy.username)
+    frenchy.update!(username: "new_username")
+
+    expect { DiscourseCalendar::CreateHolidayEvents.new.execute(nil) }.not_to change {
+      CalendarEvent.count
+    }
+    expect(created_event.reload.username).to eq("new_username")
+  end
+
   it "cleans up holidays from deactivated/silenced/suspended users" do
     frenchy
     freeze_time Time.zone.local(2019, 8, 1)