Merge pull request #3817 from AlchemyCMS/fix-duplicate-legacy-urls-deletion

Fix duplicate legacy URL removal for all DB engines

Author: tvdeyen

lib/alchemy/tasks/tidy.rb

@@ -63,13 +63,12 @@ def remove_trashed_elements
 
       def remove_duplicate_legacy_urls
         puts "\n## Removing duplicate legacy URLs"
-        sql = <<~SQL
-          DELETE FROM alchemy_legacy_page_urls A USING alchemy_legacy_page_urls B
-          WHERE A.page_id = B.page_id
-            AND A.urlname = B.urlname
-            AND A.id < B.id
-        SQL
-        count = ActiveRecord::Base.connection.exec_delete(sql)
+        keep_ids = Alchemy::LegacyPageUrl
+          .select("MAX(id)")
+          .group(:page_id, :urlname)
+        count = Alchemy::LegacyPageUrl
+          .where.not(id: keep_ids)
+          .delete_all
         log "Deleted #{count} duplicate legacy URLs"
       end
 

spec/libraries/tasks/tidy_spec.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+require "alchemy/tasks/tidy"
+
+RSpec.describe Alchemy::Tidy do
+  describe ".remove_duplicate_legacy_urls" do
+    let(:page) { create(:alchemy_page) }
+
+    it "removes duplicate legacy URLs keeping the latest" do
+      duplicate = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+      original = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+
+      described_class.remove_duplicate_legacy_urls
+
+      expect(Alchemy::LegacyPageUrl.where(id: original.id)).to exist
+      expect(Alchemy::LegacyPageUrl.where(id: duplicate.id)).not_to exist
+    end
+
+    it "does not remove unique legacy URLs" do
+      original = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+      other_original = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-other-page")
+
+      described_class.remove_duplicate_legacy_urls
+
+      expect(Alchemy::LegacyPageUrl.where(id: [original.id, other_original.id]).count).to eq(2)
+    end
+
+    it "removes all duplicates when more than two exist" do
+      duplicate_one = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+      duplicate_two = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+      original = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+
+      described_class.remove_duplicate_legacy_urls
+
+      expect(Alchemy::LegacyPageUrl.where(id: original.id)).to exist
+      expect(Alchemy::LegacyPageUrl.where(id: [duplicate_one.id, duplicate_two.id]).count).to eq(0)
+    end
+
+    it "handles duplicates across different pages" do
+      other_page = create(:alchemy_page)
+      original = Alchemy::LegacyPageUrl.create!(page: page, urlname: "my-page")
+      other_original = Alchemy::LegacyPageUrl.create!(page: other_page, urlname: "my-page")
+
+      described_class.remove_duplicate_legacy_urls
+
+      expect(Alchemy::LegacyPageUrl.where(id: [original.id, other_original.id]).count).to eq(2)
+    end
+  end
+end