From 7fad4398708c7051084a717762c94d3bd82690ea Mon Sep 17 00:00:00 2001
From: Martin Gruner <mg@zammad.com>
Date: Tue, 29 Mar 2022 19:44:58 +0200
Subject: [PATCH] Fixes #4020 - Problems with duplicate records in Translations
 table.

---
 app/models/translation.rb                     |  2 +
 ...329075919_remove_duplicate_translations.rb | 36 +++++++++++
 .../remove_duplicate_translations_spec.rb     | 60 +++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 db/migrate/20220329075919_remove_duplicate_translations.rb
 create mode 100644 spec/db/migrate/remove_duplicate_translations_spec.rb

diff --git a/app/models/translation.rb b/app/models/translation.rb
index b28f7a136..dcf5c32c8 100644
--- a/app/models/translation.rb
+++ b/app/models/translation.rb
@@ -5,6 +5,8 @@ class Translation < ApplicationModel
 
   before_create :set_initial
 
+  validates :locale, presence: true
+
 =begin
 
 reset translations to origin
diff --git a/db/migrate/20220329075919_remove_duplicate_translations.rb b/db/migrate/20220329075919_remove_duplicate_translations.rb
new file mode 100644
index 000000000..3899c79f2
--- /dev/null
+++ b/db/migrate/20220329075919_remove_duplicate_translations.rb
@@ -0,0 +1,36 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class RemoveDuplicateTranslations < ActiveRecord::Migration[6.0]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    # For some obsure reason, in legacy systems there was a chance that Translation records with the same locale and source
+    #   appeared multiple times.
+
+    Locale.all.each do |locale|
+
+      # Remove duplicates of all synchronized strings first.
+      cleanup_duplicates_of_synchronized(locale)
+
+      # Remove other duplicates of unsynchronized strings as well.
+      cleanup_duplicates_of_unsynchronized(locale)
+    end
+  end
+
+  def cleanup_duplicates_of_synchronized(locale)
+    unsync_translations = Translation.where(locale: locale.locale, is_synchronized_from_codebase: false).all
+    Translation.where(locale: locale.locale, is_synchronized_from_codebase: true).all.each do |t|
+      unsync_translations.select { |unsync_t| unsync_t.source == t.source }.each(&:destroy)
+    end
+  end
+
+  def cleanup_duplicates_of_unsynchronized(locale)
+    unsync_translations = Translation.where(locale: locale.locale, is_synchronized_from_codebase: false).order(:id).all
+    unsync_translations.each do |t|
+      next if t.destroyed?
+
+      unsync_translations.select { |check_t| check_t.id > t.id && check_t.source == t.source && !check_t.destroyed? }.each(&:destroy)
+    end
+  end
+end
diff --git a/spec/db/migrate/remove_duplicate_translations_spec.rb b/spec/db/migrate/remove_duplicate_translations_spec.rb
new file mode 100644
index 000000000..1ee25f744
--- /dev/null
+++ b/spec/db/migrate/remove_duplicate_translations_spec.rb
@@ -0,0 +1,60 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe RemoveDuplicateTranslations, type: :db_migration do
+
+  def create_translation(attributes)
+    Translation.new(attributes.merge({ locale: 'de-de', created_by_id: 1, updated_by_id: 1 })).tap(&:save!)
+  end
+
+  context 'when having unsynchronized entries with duplicates' do
+    let!(:unrelated_entry_one) { create_translation({ source: 'unknown entry', target: 'unknown translation', is_synchronized_from_codebase: false }) }
+    let!(:unrelated_entry_two) { create_translation({ source: 'unknown entry', target: 'unknown translation', is_synchronized_from_codebase: false }) }
+    let!(:unrelated_entry_three) { create_translation({ source: 'unknown entry', target: 'unknown translation', is_synchronized_from_codebase: false }) }
+
+    before do
+      migrate
+    end
+
+    it 'does not delete the first' do
+      expect(unrelated_entry_one.reload).to be_present
+    end
+
+    it 'deletes the second' do
+      expect { unrelated_entry_two.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'deletes the third' do
+      expect { unrelated_entry_three.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+
+  context 'when having multiple duplicate records of existing synchronized records' do
+    # 'yes': 'ja' already exists as synchronized entry in the translations for 'de-de'.
+    let!(:duplicate_one) { create_translation({ source: 'yes', target: 'ja', is_synchronized_from_codebase: false }) }
+    let!(:duplicate_two) { create_translation({ source: 'yes', target: 'ja', is_synchronized_from_codebase: false }) }
+
+    before do
+      migrate
+    end
+
+    it 'deletes the first' do
+      expect { duplicate_one.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'deletes the second' do
+      expect { duplicate_two.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'keeps the original' do
+      expect(Translation.where(locale: 'de-de', source: 'yes').count { |e| e.source == 'yes' }).to eq(1)
+    end
+  end
+
+  context 'when no duplicates are present' do
+    it 'does nothing' do
+      expect { migrate }.not_to change(Translation, :count)
+    end
+  end
+end