Improve translations import

- Make sure duplicates are not importer from the import data
- Make sure existing translation keys are not overridden, only new
  keys will be imported
- Make sure the translations for the imported keys are created for
  all available organization locales

app/commands/decidim/term_customizer/admin/import_set_translations.rb

@@ -64,13 +64,14 @@ def import_translations
         # Returns Array or nil. The returned value is an array of the imported
         # translations when the import is successful, otherwise nil.
         def import_file(filepath, mime_type)
-          importer_for(filepath, mime_type).import do |collection|
-            collection.each do |record|
-              record.translation_set = translation_set
-              record.save!
-            end
-
-            return collection
+          importer_for(filepath, mime_type).import do |records|
+            import = TranslationImportCollection.new(
+              translation_set,
+              records,
+              form.current_organization.available_locales
+            )
+
+            return translation_set.translations.create(import.import_attributes)
           end
 
           nil

lib/decidim/term_customizer.rb

@@ -12,6 +12,7 @@ module TermCustomizer
     autoload :Loader, "decidim/term_customizer/loader"
     autoload :Resolver, "decidim/term_customizer/resolver"
     autoload :TranslationDirectory, "decidim/term_customizer/translation_directory"
+    autoload :TranslationImportCollection, "decidim/term_customizer/translation_import_collection"
     autoload :TranslationParser, "decidim/term_customizer/translation_parser"
     autoload :TranslationSerializer, "decidim/term_customizer/translation_serializer"
     autoload :TranslationStore, "decidim/term_customizer/translation_store"

lib/decidim/term_customizer/translation_import_collection.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+module Decidim
+  module TermCustomizer
+    class TranslationImportCollection
+      def initialize(translation_set, records, locales)
+        @translation_set = translation_set
+        @records = records
+        @locales = locales
+      end
+
+      def import_attributes
+        attributes_for_all_locales
+      end
+
+      private
+
+      attr_reader :translation_set, :records, :locales
+
+      def collection_attributes
+        @collection_attributes ||= records.map do |translation|
+          # Skip all translation keys that already exists
+          next if translation_set.translations.where(
+            key: translation.key
+          ).present?
+
+          {
+            locale: translation.locale,
+            key: translation.key,
+            value: translation.value
+          }
+        end.compact
+      end
+
+      def unique_attributes
+        @unique_attributes ||= collection_attributes.uniq do |attr|
+          "#{attr[:locale]}.#{attr[:key]}"
+        end
+      end
+
+      def attribute_keys
+        @attribute_keys ||= unique_attributes.map { |attr| attr[:key] }.uniq
+      end
+
+      def attributes_for_all_locales
+        @attributes_for_all_locales ||= attribute_keys.map do |key|
+          locales.map do |locale|
+            # Find if the item with the locale already exists in the
+            # unique collection.
+            item = unique_attributes.find do |attr|
+              attr[:key] == key && attr[:locale] == locale.to_s
+            end
+
+            # In case the item does not exist for the key and locale, create
+            # a new item with the default I18n translation. Otherwise, return
+            # the found item to the final array.
+            if item.nil?
+              {
+                key: key,
+                locale: locale.to_s,
+                value: I18n.t(key, locale: locale, default: "")
+              }
+            else
+              item
+            end
+          end
+        end.flatten
+      end
+    end
+  end
+end

spec/commands/decidim/term_customizer/admin/import_set_translations_spec.rb

@@ -5,7 +5,8 @@
 describe Decidim::TermCustomizer::Admin::ImportSetTranslations do
   let(:form_klass) { Decidim::TermCustomizer::Admin::TranslationsImportForm }
 
-  let(:organization) { create(:organization) }
+  let(:locales) { [:en, :fi] }
+  let(:organization) { create(:organization, available_locales: locales) }
   let(:translation_set) { create(:translation_set, organization: organization) }
   let(:file) { nil }
   let(:form_params) { { file: file } }
@@ -17,6 +18,10 @@
     )
   end
 
+  before do
+    I18n.available_locales = locales
+  end
+
   describe "call" do
     let(:command) do
       described_class.new(form, translation_set)

spec/lib/decidim/term_customizer/translation_import_collection_spec.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe Decidim::TermCustomizer::TranslationImportCollection do
+  subject { described_class.new(translation_set, records, locales) }
+
+  let(:translation_set) { create(:translation_set) }
+  let(:records) do
+    build_list(:translation, 10, translation_set: translation_set)
+  end
+  let(:locales) { [:en] }
+
+  describe "#import_attributes" do
+    let(:expected_attributes) do
+      records.map do |tr|
+        { locale: tr.locale, key: tr.key, value: tr.value }
+      end
+    end
+
+    it "returns the correct attributes" do
+      expect(subject.import_attributes).to eq(expected_attributes)
+    end
+
+    context "with multiple locales" do
+      let(:locales) { [:en, :fi, :sv] }
+
+      let(:expected_attributes) do
+        records.map do |tr|
+          locales.map do |locale|
+            if locale.to_s == tr.locale
+              { locale: tr.locale, key: tr.key, value: tr.value }
+            else
+              { locale: locale.to_s, key: tr.key, value: "" }
+            end
+          end
+        end.flatten
+      end
+
+      before do
+        I18n.available_locales = locales
+      end
+
+      it "returns the correct attributes" do
+        expect(subject.import_attributes).to eq(expected_attributes)
+      end
+    end
+
+    context "with duplicate keys" do
+      let(:records) do
+        duplicate_attributes.map do |attr|
+          build(
+            :translation,
+            locale: attr[:locale],
+            key: attr[:key],
+            value: attr[:value],
+            translation_set: translation_set
+          )
+        end
+      end
+
+      let(:duplicate_attributes) do
+        [
+          { locale: "en", key: "key1", value: "Value1" },
+          { locale: "en", key: "key1", value: "Value1-1" },
+          { locale: "en", key: "key2", value: "Value2" },
+          { locale: "en", key: "key2", value: "Value2-2" }
+        ]
+      end
+
+      let(:expected_attributes) do
+        [
+          { locale: "en", key: "key1", value: "Value1" },
+          { locale: "en", key: "key2", value: "Value2" }
+        ]
+      end
+
+      it "returns the correct attributes with duplicates removed" do
+        expect(subject.import_attributes).to eq(expected_attributes)
+      end
+    end
+  end
+end

spec/spec_helper.rb

@@ -18,6 +18,10 @@
 
 RSpec.configure do |config|
   config.before do
+    # Reset the locales to Decidim defaults before each test.
+    # Some tests may change this which is why this is important.
+    I18n.available_locales = [:en, :ca, :es]
+
     # Revert back to the simple backend before every test because otherwise the
     # tests may be interfered by the backend set by the specific tests. You
     # could otherwise see the following errors in case the tests are not run in