Bugfix

lib/ad_localize.rb

@@ -33,6 +33,7 @@
 require 'ad_localize/mappers/translation_group_mapper'
 require 'ad_localize/mappers/android_translation_mapper'
 require 'ad_localize/mappers/ios_translation_mapper'
+require 'ad_localize/mappers/key_validator'
 
 require 'ad_localize/entities/key'
 require 'ad_localize/entities/translation'

lib/ad_localize/entities/key.rb

@@ -46,6 +46,11 @@ def same_as?(key:)
         raw_label == key.raw_label
       end
 
+      def ==(o)
+        o.class == self.class &&
+        o.raw_label == raw_label
+      end
+
       private
 
       def compute_label

lib/ad_localize/entities/translation.rb

@@ -15,6 +15,14 @@ def initialize(locale:, key:, value:, comment: nil)
         @value = value
         @comment = comment
       end
+
+      def ==(o)
+        o.class == self.class &&
+        o.locale == locale &&
+        o.key == key &&
+        o.value == value &&
+        o.comment == comment
+      end
     end
   end
 end

lib/ad_localize/interactors/export_g_spreadsheet.rb

@@ -33,8 +33,12 @@ def export_without_service_account(export_request:)
         export_request.csv_paths = downloaded_files.map(&:path)
         if export_request.has_csv_files?
           ExportCSVFiles.new.call(export_request: export_request)
+        elsif export_request.has_empty_files?
+          # When downloading an empty spreadsheet, the content type of the downloaded file is "inode/x-empty"
+          LOGGER.warn("Your spreadsheet is empty. Add content and retry.")
         else
-          LOGGER.error("invalid export request. check the spreadsheet share configuration")
+          # When shared configuration is misconfigured, the content type of the downloaded file is "text/html"
+          LOGGER.error("Invalid export request. Check the spreadsheet share configuration")
         end
         downloaded_files.select { |downloaded_file| File.exist?(downloaded_file.path) }.each do |downloaded_file|
           downloaded_file.close

lib/ad_localize/mappers/csv_path_to_wording.rb

@@ -5,19 +5,16 @@ def map(csv_path:)
         @headers = CSV.foreach(csv_path).first
         return unless valid?(csv_path: csv_path)
         translations = []
+        validator = KeyValidator.new
 
         CSV.foreach(csv_path, headers: true, skip_blanks: true) do |row|
           row_translations = map_row(row: row, locales: locales)
           next if row_translations.blank?
 
-          existing_keys = translations.map(&:key)
-          new_translations = row_translations.reject do |translation|
-            existing_keys.any? do |key|
-              existing_plural_key = key.label == translation.key.label && key.plural? && translation.key.singular?
-              key.same_as?(key: translation.key) || existing_plural_key
-            end
-          end
-          translations.concat(new_translations)
+          current_key = row_translations.first.key
+          next if validator.has_warnings?(current_key)
+
+          translations.concat(row_translations)
         end
 
         locale_wordings = translations.group_by(&:locale).map do |locale, group|

lib/ad_localize/mappers/key_validator.rb

@@ -0,0 +1,31 @@
+module AdLocalize
+  module Mappers
+    class KeyValidator
+
+      def initialize
+        @existing_key_for_label = {}
+      end
+
+      def has_warnings?(current_key)
+        current_label = current_key.label
+        existing_key = @existing_key_for_label[current_label]
+
+        has_warnings = false
+
+        unless existing_key.nil?
+          existing_plural_key = existing_key.label == current_key.label && existing_key.plural? && current_key.singular?
+          existing_singular_key = existing_key.label == current_key.label && existing_key.singular? && current_key.plural?
+          is_same_key = existing_key.same_as?(key: current_key)
+          LOGGER.warn "A plural value already exist for key '#{current_label}'. Remove duplicates." if existing_plural_key
+          LOGGER.warn "A singular value already exist for key '#{current_label}'. Remove duplicates." if existing_singular_key
+          LOGGER.warn "Some values already exist for key '#{current_label}'. Remove duplicates." if is_same_key
+          has_warnings = is_same_key || existing_plural_key || existing_singular_key
+        end
+
+        @existing_key_for_label[current_label] = current_key
+
+        has_warnings
+      end
+    end
+  end
+end

lib/ad_localize/mappers/locale_wording_to_hash.rb

@@ -10,6 +10,14 @@ def map(locale_wording:)
                 hash[translation.key.label] = {} unless hash.key? translation.key.label
                 hash[translation.key.label][translation.key.plural_key] = translation.value
               else
+                unless hash.is_a?(Hash)
+                  LOGGER.warn "Corrupted input. Trying to insert a value for key '#{translation.key.label}' but a value already exists for '#{inner_keys[0..index-1].join(".")}'. Skipping."
+                  break # skip this corrupted key
+                end
+                previous_value = hash[inner_key.to_s]
+                if !previous_value.blank? && previous_value.is_a?(Hash)
+                  LOGGER.warn "Corrupted input. Trying to insert a value for key '#{translation.key.label}' but values already exist for keys '#{translation.key.label}.*'. Previous values will be lost."
+                end
                 hash[inner_key.to_s] = translation.value
               end
             else

lib/ad_localize/mappers/value_range_to_wording.rb

@@ -17,18 +17,16 @@ def map(value_range:)
 
       def map_rows(values:)
         translations = []
+        validator = KeyValidator.new
+
         values[1..-1].each do |row|
           row_translations = map_row(row: row)
           next if row_translations.blank?
 
-          existing_keys = translations.map(&:key)
-          new_translations = row_translations.reject do |translation|
-            existing_keys.any? do |key|
-              existing_plural_key = key.label == translation.key.label && key.plural? && translation.key.singular?
-              key.same_as?(key: translation.key) || existing_plural_key
-            end
-          end
-          translations.concat(new_translations)
+          current_key = row_translations.first.key
+          next if validator.has_warnings?(current_key)
+
+          translations.concat(row_translations)
         end
         translations
       end

lib/ad_localize/requests/export_request.rb

@@ -4,6 +4,7 @@ class ExportRequest
       SUPPORTED_PLATFORMS = %w(ios android yml json properties csv).freeze
       DEFAULT_EXPORT_FOLDER = 'exports'.freeze
       CSV_CONTENT_TYPES = %w(text/csv text/plain application/csv).freeze
+      EMPTY_CONTENT_TYPE = 'inode/x-empty'.freeze
 
       def initialize(**args)
         @locales = Array(args[:locales].presence)
@@ -34,6 +35,10 @@ def has_csv_files?
         !@csv_paths.blank? && @csv_paths.all? { |csv_path| File.exist?(csv_path) && is_csv?(path: csv_path) }
       end
 
+      def has_empty_files?
+        !@csv_paths.blank? && @csv_paths.all? { |csv_path| File.exist?(csv_path) && is_empty?(path: csv_path) }
+      end
+
       def has_g_spreadsheet_options?
         @g_spreadsheet_options.present?
       end
@@ -61,7 +66,15 @@ def valid_platforms?
       end
 
       def is_csv?(path:)
-        CSV_CONTENT_TYPES.include?(`file --brief --mime-type "#{path}"`.strip)
+        CSV_CONTENT_TYPES.include? content_type(path: path)
+      end
+
+      def is_empty?(path:)
+        content_type(path: path) == EMPTY_CONTENT_TYPE
+      end
+
+      def content_type(path:)
+        `file --brief --mime-type "#{path}"`.strip
       end
 
       def valid_g_spreadsheet_options?

test/fixtures/exports_reference_duplicates/en.lproj/Localizable.strings

@@ -0,0 +1,2 @@
+"duplicate" = "Duplicate";
+"singular_before_plural" = "Singular before Plural";

test/fixtures/exports_reference_duplicates/en.lproj/Localizable.stringsdict

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<plist>
+    <dict>
+        <key>plural_before_singular</key>
+        <dict>
+            <key>NSStringLocalizedFormatKey</key>
+            <string>%#@key@</string>
+            <key>key</key>
+            <dict>
+                <key>NSStringFormatSpecTypeKey</key>
+                <string>NSStringPluralRuleType</string>
+                <key>NSStringFormatValueTypeKey</key>
+                <string>d</string>
+                <key>one</key>
+                <string>Plural before Singular</string>
+                <key>other</key>
+                <string>Plural before Singular</string>
+            </dict>
+        </dict>
+    </dict>
+</plist>

test/fixtures/exports_reference_duplicates/fr.lproj/Localizable.strings

@@ -0,0 +1,2 @@
+"duplicate" = "Duplicate";
+"singular_before_plural" = "Singular before Plural";

test/fixtures/exports_reference_duplicates/fr.lproj/Localizable.stringsdict

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<plist>
+    <dict>
+        <key>plural_before_singular</key>
+        <dict>
+            <key>NSStringLocalizedFormatKey</key>
+            <string>%#@key@</string>
+            <key>key</key>
+            <dict>
+                <key>NSStringFormatSpecTypeKey</key>
+                <string>NSStringPluralRuleType</string>
+                <key>NSStringFormatValueTypeKey</key>
+                <string>d</string>
+                <key>one</key>
+                <string>Plural before Singular</string>
+                <key>other</key>
+                <string>Plural before Singular</string>
+            </dict>
+        </dict>
+    </dict>
+</plist>

test/fixtures/exports_reference_json_bad_input/en.json

@@ -0,0 +1 @@
+{"en":{"aaa":{"bbb":"first"},"bbb":{"ccc":"second"},"zzz":"first"}}

test/fixtures/reference.csv

@@ -8,7 +8,6 @@
 ,save,Enregistrer,,Save,
 #Plurals,,,,,
 ,assess_rate_trip_voiceover##{one},Attribuer %1$@ étoile,,Rate %1$@ star,
-,assess_rate_trip_voiceover##{one},Attribuer %1$@ étoile,,Rate %1$@ star,
 ,assess_rate_trip_voiceover##{other},Attribuer %1$@ étoiles,Si %1$@<=3 : prononcer également assess_comment_placeholder. Si %1$@>3 : prononcer également assess_optional_comment_placeholder,Rate %1$@ stars,
 #InfoPlist,,,,,
 ,NSCameraUsageDescription,Camera utilisé pour blabla,,Camera used for blabla,

test/fixtures/reference1.csv

@@ -8,7 +8,6 @@
 ,save,Sauvegarder,,Save,
 #Plurals,,,,,
 ,assess_rate_trip_voiceover##{one},Attribuer %1$@ étoile,,Rate %1$@ star,
-,assess_rate_trip_voiceover##{one},Attribuer %1$@ étoile,,Rate %1$@ star,
 ,assess_rate_trip_voiceover##{other},Attribuer %1$@ étoiles,Si %1$@<=3 : prononcer également assess_comment_placeholder. Si %1$@>3 : prononcer également assess_optional_comment_placeholder,Rate %1$@ stars,
 #InfoPlist,,,,,
 ,NSCameraUsageDescription,Camera utilisé pour blabla,,Camera used for blabla,

test/fixtures/reference_duplicates.csv

@@ -0,0 +1,11 @@
+,key,fr,comment fr,en,comment en
+#Generic actions,,,,,
+,duplicate,Duplicate,,Duplicate,
+,duplicate,Erreur,,Error,
+#Plurals,,,,,
+,plural_before_singular##{one},Plural before Singular,,Plural before Singular,
+,plural_before_singular##{one},Plural before Singular,,Plural before Singular,
+,plural_before_singular##{other},Plural before Singular,,Plural before Singular,
+,plural_before_singular,Erreur,,Error,,
+,singular_before_plural,Singular before Plural,,Singular before Plural,,
+,singular_before_plural##{one},Erreur,,Error,,

test/fixtures/reference_json_bad_input.csv

@@ -0,0 +1,6 @@
+,key,en,
+,aaa.bbb,first,
+,aaa.bbb.ccc,second,
+,bbb.ccc.ddd,first,
+,bbb.ccc,second,
+,zzz,first

test/interactors/execute_export_request_test.rb

@@ -13,6 +13,50 @@ def teardown
         FileUtils.rm_rf('exports')
       end
 
+      test 'it should export json file with bad inputs' do
+        # Given
+        csv_file = "test/fixtures/reference_json_bad_input.csv"
+        reference_dir = "test/fixtures/exports_reference_json_bad_input"
+        assert(File.exist?(csv_file), "File does not exists #{csv_file}")
+        assert(File.exist?(reference_dir), "File does not exists #{reference_dir}")
+
+        # When
+        export_request = Requests::ExportRequest.new(csv_paths: [csv_file], platforms: 'json')
+        ExecuteExportRequest.new.call(export_request: export_request)
+
+        # Then
+        reference_file = "#{reference_dir}/en.json"
+        generated_file = "exports/en.json"
+        assert(File.exist?(reference_file), "File does not exists #{reference_file}")
+        assert(File.exist?(generated_file), "File does not exists #{generated_file}")
+        diff = Diffy::Diff.new(reference_file, generated_file, :source => 'files')
+        assert_empty(diff.to_s, "File #{generated_file} do not match reference. Diff: \n\n#{diff}\n")
+      end
+
+      test 'it should export ios file with duplicated keys' do
+        # Given
+        csv_file = "test/fixtures/reference_duplicates.csv"
+        reference_dir = "test/fixtures/exports_reference_duplicates"
+        assert(File.exist?(csv_file), "File does not exists #{csv_file}")
+        assert(File.exist?(reference_dir), "File does not exists #{reference_dir}")
+
+        # When
+        export_request = Requests::ExportRequest.new(csv_paths: [csv_file], platforms: 'ios')
+        ExecuteExportRequest.new.call(export_request: export_request)
+
+        # Then
+        ios_files(with_platform_directory: false)
+          .reject { |file| file.include? "InfoPlist.strings" }
+          .each do |file|
+            reference_file = "#{reference_dir}/#{file}"
+            generated_file = "exports/#{file}"
+            assert(File.exist?(reference_file), "File does not exists #{reference_file}")
+            assert(File.exist?(generated_file), "File does not exists #{generated_file}")
+            diff = Diffy::Diff.new(reference_file, generated_file, :source => 'files')
+            assert_empty(diff.to_s, "File #{generated_file} do not match reference. Diff: \n\n#{diff}\n")
+          end
+      end
+
       test 'it should export correct platforms files' do
         # Given
         csv_file = "test/fixtures/reference.csv"