fix import for unbalanced pluralizations

fixes CNVS-4533

current i18n gem doesn't like pluralization sets
where you specify "other" but not "one".  This
pulls the i18n import into it's own class and
specifically specs out the copying over of the
"other" key into the "one" key in a pluralization
set unless a "one" key already exists.  Importing
any translation set will now solve the pluralization
view errors.

TEST PLAN:
1) confirm that the errors specified in CNVS-4533
are still occurring for the japanese language
set with the current translations.

2) run the i18n import process for the japanese
translations

3) confirm that the view errors are no longer
cropping up under the japanese translation set.

Change-Id: Idb856bd72f4d8e526a645f70a8fcc5556c4a4f98
Reviewed-on: https://gerrit.instructure.com/18526
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Ethan Vizitei <ethan@12spokes.com>
QA-Review: Clare Hetherington <clare@instructure.com>

lib/i18n/hash_extensions.rb

@@ -0,0 +1,33 @@
+module I18n
+  module HashExtensions
+
+    def flatten_keys(result={}, prefix='')
+      each_pair do |k, v|
+        if v.is_a?(Hash)
+          v.flatten_keys(result, "#{prefix}#{k}.")
+        else
+          result["#{prefix}#{k}"] = v
+        end
+      end
+      result
+    end
+
+    def expand_keys(result = {})
+      each_pair do |k, v|
+        parts = k.split('.')
+        last = parts.pop
+        parts.inject(result){ |h, k2| h[k2] ||= {}}[last] = v
+      end
+      result
+    end
+
+    def to_ordered
+      keys.sort_by(&:to_s).inject ActiveSupport::OrderedHash.new do |h, k|
+        v = fetch(k)
+        h[k] = v.is_a?(Hash) ? v.to_ordered : v
+        h
+      end
+    end
+
+  end
+end

lib/i18n_import.rb

@@ -0,0 +1,115 @@
+Hash.send(:include, I18n::HashExtensions) unless Hash.new.kind_of?(I18n::HashExtensions)
+
+class I18nImport
+  attr_reader :source_translations, :new_translations, :language
+
+  def initialize(source_translations, new_translations)
+    @source_translations = init_source(source_translations)
+    @language = init_language(new_translations)
+    @new_translations = new_translations[language].flatten_keys
+  end
+
+  def compile_complete_translations(warning)
+    return nil unless warning.call(missing_keys.sort, "missing translations") if missing_keys.present?
+    return nil unless warning.call(unexpected_keys.sort, "unexpected translations") if unexpected_keys.present?
+
+    find_mismatches
+
+    if @placeholder_mismatches.size > 0
+      return nil unless warning.call(mismatch_diff(@placeholder_mismatches), "placeholder mismatches")
+    end
+
+    if @markdown_mismatches.size > 0
+      return nil unless warning.call(mismatch_diff(@markdown_mismatches), "markdown/wrapper mismatches")
+    end
+
+    complete_translations
+  end
+
+  def complete_translations
+    I18n.available_locales
+    base = (I18n.backend.send(:translations)[language.to_sym] || {})
+    translations = base.flatten_keys.merge(new_translations)
+    fix_plural_keys(translations)
+    translations.expand_keys
+  end
+
+  def fix_plural_keys(flat_hash)
+    other_keys = flat_hash.keys.grep(/\.other$/)
+    other_keys.each do |other_key|
+      one_key = other_key.gsub(/other$/, 'one')
+      if flat_hash[one_key].nil?
+        flat_hash[one_key] = flat_hash[other_key]
+      end
+    end
+  end
+
+  def missing_keys
+    source_translations.keys - new_translations.keys
+  end
+
+  def unexpected_keys
+    new_translations.keys - source_translations.keys
+  end
+
+  def find_mismatches
+    @placeholder_mismatches = {}
+    @markdown_mismatches = {}
+    new_translations.keys.each do |key|
+      p1 = placeholders(source_translations[key].to_s)
+      p2 = placeholders(new_translations[key].to_s)
+      @placeholder_mismatches[key] = [p1, p2] if p1 != p2
+
+      m1 = markdown_and_wrappers(source_translations[key].to_s)
+      m2 = markdown_and_wrappers(new_translations[key].to_s)
+      @markdown_mismatches[key] = [m1, m2] if m1 != m2
+    end
+  end
+
+  def markdown_and_wrappers(str)
+    # some stuff this doesn't check (though we don't use):
+    #   blockquotes, e.g. "> some text"
+    #   reference links, e.g. "[an example][id]"
+    #   indented code
+    (
+      scan_and_report(str, /\\[\\`\*_\{\}\[\]\(\)#\+\-\.!]/) +
+      scan_and_report(str, /(\*+|_+|`+)[^\s].*?[^\s]?\1/).map{|m|"#{m}-wrap"} +
+      scan_and_report(str, /(!?\[)[^\]]+\]\(([^\)"']+).*?\)/).map{|m|"link:#{m.last}"} +
+      scan_and_report(str, /^((\s*\*\s*){3,}|(\s*-\s*){3,}|(\s*_\s*){3,})$/).map{"hr"} +
+      scan_and_report(str, /^[^=\-\n]+\n^(=+|-+)$/).map{|m|m.first[0]=='=' ? 'h1' : 'h2'} +
+      scan_and_report(str, /^(\#{1,6})\s+[^#]*#*$/).map{|m|"h#{m.first.size}"} +
+      scan_and_report(str, /^ {0,3}(\d+\.|\*|\+|\-)\s/).map{|m|m.first =~ /\d/ ? "1." : "*"}
+    ).sort
+  end
+
+  def placeholders(str)
+    str.scan(/%h?\{[^\}]+\}/).sort
+  rescue ArgumentError => e
+    puts "Unable to scan string: #{str.inspect}"
+    raise e
+  end
+
+  def scan_and_report(str, re)
+    str.scan(re)
+  rescue ArgumentError => e
+    puts "Unable to scan string: #{str.inspect}"
+    raise e
+  end
+
+  private
+  def init_source(translations)
+    raise "Source does not have any English strings" unless translations.keys.include?('en')
+    translations['en'].flatten_keys
+  end
+
+  def init_language(translations)
+    raise "Translation file contains multiple languages" if translations.size > 1
+    language = translations.keys.first
+    raise "Translation file appears to have only English strings" if language == 'en'
+    language
+  end
+
+  def mismatch_diff(mismatches)
+    mismatches.map{|k,(p1,p2)| "#{k}: expected #{p1.inspect}, got #{p2.inspect}"}.sort
+  end
+end

lib/tasks/i18n.rake

@@ -1,34 +1,6 @@
-namespace :i18n do
-  module HashExtensions
-    def flatten_keys(result={}, prefix='')
-      each_pair do |k, v|
-        if v.is_a?(Hash)
-          v.flatten_keys(result, "#{prefix}#{k}.")
-        else
-          result["#{prefix}#{k}"] = v
-        end
-      end
-      result
-    end
-
-    def expand_keys(result = {})
-      each_pair do |k, v|
-        parts = k.split('.')
-        last = parts.pop
-        parts.inject(result){ |h, k2| h[k2] ||= {}}[last] = v
-      end
-      result
-    end
-
-    def to_ordered
-      keys.sort_by(&:to_s).inject ActiveSupport::OrderedHash.new do |h, k|
-        v = fetch(k)
-        h[k] = v.is_a?(Hash) ? v.to_ordered : v
-        h
-      end
-    end
-  end
+require 'i18n/hash_extensions'
 
+namespace :i18n do
   def infer_scope(filename)
     case filename
       when /app\/views\/.*\.handlebars\z/
@@ -181,7 +153,7 @@ namespace :i18n do
     I18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.{rb,yml}')] +
                       Dir[Rails.root.join('vendor', 'plugins', '*', 'config', 'locales', '**', '*.{rb,yml}')]
 
-    Hash.send :include, HashExtensions
+    Hash.send :include, I18n::HashExtensions
 
     file_translations = {}
 
@@ -260,7 +232,7 @@ define(['i18nObj', 'jquery'], function(I18n, $) {
 
   desc "Exports new/changed English strings to be translated"
   task :export => :environment do
-    Hash.send :include, HashExtensions
+    Hash.send :include, I18n::HashExtensions
 
     begin
       base_filename = "config/locales/generated/en.yml"
@@ -364,55 +336,21 @@ define(['i18nObj', 'jquery'], function(I18n, $) {
   desc "Validates and imports new translations"
   task :import => :environment do
     require 'ya2yaml'
-    Hash.send :include, HashExtensions
-
-    def placeholders(str)
-      str.scan(/%h?\{[^\}]+\}/).sort
-    rescue ArgumentError => e
-      puts "Unable to scan string: #{str.inspect}"
-      raise e
-    end
-
-    def scan_and_report(str, re)
-      str.scan(re)
-    rescue ArgumentError => e
-      puts "Unable to scan string: #{str.inspect}"
-      raise e
-    end
-
-    def markdown_and_wrappers(str)
-      # some stuff this doesn't check (though we don't use):
-      #   blockquotes, e.g. "> some text"
-      #   reference links, e.g. "[an example][id]"
-      #   indented code
-      (
-        scan_and_report(str, /\\[\\`\*_\{\}\[\]\(\)#\+\-\.!]/) +
-        scan_and_report(str, /(\*+|_+|`+)[^\s].*?[^\s]?\1/).map{|m|"#{m}-wrap"} +
-        scan_and_report(str, /(!?\[)[^\]]+\]\(([^\)"']+).*?\)/).map{|m|"link:#{m.last}"} +
-        scan_and_report(str, /^((\s*\*\s*){3,}|(\s*-\s*){3,}|(\s*_\s*){3,})$/).map{"hr"} +
-        scan_and_report(str, /^[^=\-\n]+\n^(=+|-+)$/).map{|m|m.first[0]=='=' ? 'h1' : 'h2'} +
-        scan_and_report(str, /^(\#{1,6})\s+[^#]*#*$/).map{|m|"h#{m.first.size}"} +
-        scan_and_report(str, /^ {0,3}(\d+\.|\*|\+|\-)\s/).map{|m|m.first =~ /\d/ ? "1." : "*"}
-      ).sort
-    end
+    Hash.send(:include, I18n::HashExtensions) unless Hash.new.kind_of?(I18n::HashExtensions)
 
     begin
       puts "Enter path to original en.yml file:"
       arg = $stdin.gets.strip
       source_translations = File.exist?(arg) && YAML.safe_load(File.read(arg)) rescue nil
     end until source_translations
-    raise "Source does not have any English strings" unless source_translations.keys.include?('en')
-    source_translations = source_translations['en'].flatten_keys
 
     begin
       puts "Enter path to translated file:"
       arg = $stdin.gets.strip
       new_translations = File.exist?(arg) && YAML.safe_load(File.read(arg)) rescue nil
     end until new_translations
-    raise "Translation file contains multiple languages" if new_translations.size > 1
-    language = new_translations.keys.first
-    raise "Translation file appears to have only English strings" if language == 'en'
-    new_translations = new_translations[language].flatten_keys
+
+    import = I18nImport.new(source_translations, new_translations)
 
     item_warning = lambda { |error_items, description|
       begin
@@ -429,37 +367,11 @@ define(['i18nObj', 'jquery'], function(I18n, $) {
       true
     }
 
-    missing_keys = source_translations.keys - new_translations.keys
-    next unless item_warning.call(missing_keys.sort, "missing translations") if missing_keys.present?
-
-    unexpected_keys = new_translations.keys - source_translations.keys
-    next unless item_warning.call(unexpected_keys.sort, "unexpected translations") if unexpected_keys.present?
-
-    placeholder_mismatches = {}
-    markdown_mismatches = {}
-    new_translations.keys.each do |key|
-      p1 = placeholders(source_translations[key].to_s)
-      p2 = placeholders(new_translations[key].to_s)
-      placeholder_mismatches[key] = [p1, p2] if p1 != p2
-
-      m1 = markdown_and_wrappers(source_translations[key].to_s)
-      m2 = markdown_and_wrappers(new_translations[key].to_s)
-      markdown_mismatches[key] = [m1, m2] if m1 != m2
-    end
-
-    if placeholder_mismatches.size > 0
-      next unless item_warning.call(placeholder_mismatches.map{|k,(p1,p2)| "#{k}: expected #{p1.inspect}, got #{p2.inspect}"}.sort, "placeholder mismatches")
-    end
-
-    if markdown_mismatches.size > 0
-      next unless item_warning.call(markdown_mismatches.map{|k,(p1,p2)| "#{k}: expected #{p1.inspect}, got #{p2.inspect}"}.sort, "markdown/wrapper mismatches")
-    end
-
-    I18n.available_locales
+    complete_translations = import.compile_complete_translations(item_warning)
+    next if complete_translations.nil?
 
-    new_translations = (I18n.backend.send(:translations)[language.to_sym] || {}).flatten_keys.merge(new_translations)
-    File.open("config/locales/#{language}.yml", "w") { |f|
-      f.write({language => new_translations.expand_keys}.ya2yaml(:syck_compatible => true))
+    File.open("config/locales/#{import.language}.yml", "w") { |f|
+      f.write({import.language => complete_translations}.ya2yaml(:syck_compatible => true))
     }
   end
 end

spec/lib/i18n_import_spec.rb

@@ -0,0 +1,23 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+require File.expand_path('../../lib/i18n_import', File.dirname(__FILE__))
+
+describe I18nImport do
+  describe '#fix_plural_keys' do
+    it 'copies over the other key if there is no one key' do
+      import = I18nImport.new({'en'=>{}}, {'ja'=>{}})
+      hash = { 'some.key.other' => 'value' }
+      import.fix_plural_keys(hash)
+      hash.should == { 'some.key.other' => 'value', 'some.key.one' => 'value' }
+    end
+
+    it 'leaves the one key alone if it already exists' do
+      import = I18nImport.new({'en'=>{}}, {'ja'=>{}})
+      hash = {
+        'some.key.other' => 'value',
+        'some.key.one' => 'other value'
+      }
+      import.fix_plural_keys(hash)
+      hash.should == { 'some.key.other' => 'value', 'some.key.one' => 'other value' }
+    end
+  end
+end