fix js/hbs i18n key inference, fixes CNVS-18093

ensure inferred keys are never scoped to the file

this fixes the current broken behavior:

  {{#t}}...{{/t}}
    scoped at extraction time
    not scoped at runtime

  {{t ...}} and I18n.t
    not scoped at extraction time
    scoped at runtime

ruby key inference already works correctly (never scoped to the file)

notes:

running i18n:generate will now put all inferred keys at the root level.
since no js/hbs inferred keys were actually resolving to their
translations, this won't break any currently working translations and
will fix a lot of stuff...

the inline hbs t and I18n.t calls will start working immediately (since it
was just a runtime scoping issue), the #t calls will automatically sort
themselves out with a transifex round trip (since they were scoped
incorrectly at extraction time). there are over 300 over the former and
about a dozen of the latter.

test plan:
1. confirm missing strings listed on ticket now show up. a good example of
   this is the new course wizard; whereas before nothing was translated,
   now everything is (apart from a few new strings that haven't made the
   transifex round trip)
2. see all the new specs
3. also, try it out yourself. essentially:
   1. run `rake i18n:generate`
   2. find the string you want to verify, and put a corresponding value
      in the right place in es.yml (or wherever) if there isn't already
      one
   3. run `rake canvas:compile_assets`
   4. run canvas with `RAILS_LOAD_ALL_LOCALES=true USE_OPTIMIZED_JS=true`
   5. switch to es (or whatever) and verify the translation is displayed

Change-Id: I0574b90acbdc4f6fda9c814a3607a9002c9e6a00
Reviewed-on: https://gerrit.instructure.com/47626
Tested-by: Jenkins
Reviewed-by: Jennifer Stern <[email protected]>
QA-Review: Clare Strong <[email protected]>
Product-Review: Jon Jensen <[email protected]>

gems/canvas_i18nliner/js/scoped_hbs_pre_processor.js

@@ -26,11 +26,9 @@ PreProcessor.injectScope = function(node) {
   if (!node.hash)
     node.hash = node.sexpr.hash = new HashNode([]);
   pairs = node.hash.pairs;
-  // to match our .rb scoping behavior, don't scope inferred keys
-  if (pairs.length && pairs[pairs.length - 1][0] === "i18n_inferred_key") {
-    node.hash.pairs = pairs.slice(0, pairs.length - 1);
-  }
-  else {
+  // to match our .rb scoping behavior, don't scope inferred keys...
+  // if inferred, it's always the last option
+  if (!pairs.length || pairs[pairs.length - 1][0] !== "i18n_inferred_key") {
     node.hash.pairs = pairs.concat([["scope", new StringNode(this.scope)]]);
   }
   return node;

gems/canvas_i18nliner/js/scoped_translate_call.js

@@ -17,7 +17,10 @@ module.exports = function(TranslateCall) {
   };
 
   ScopedTranslateCall.prototype.normalize = function() {
-    if (!this.inferredKey) this.key = this.normalizeKey(this.key);
+    // TODO: make i18nliner-js use the latter, just like i18nliner(.rb) ...
+    // i18nliner-handlebars can't use the former
+    if (!this.inferredKey && !this.options.i18n_inferred_key)
+      this.key = this.normalizeKey(this.key);
     TranslateCall.prototype.normalize.call(this);
   };
 

gems/canvas_i18nliner/package.json

@@ -10,5 +10,11 @@
     "i18nliner": "0.0.15",
     "i18nliner-handlebars": "0.0.11",
     "minimist": "^1.1.0"
+  },
+  "devDependencies": {
+    "jasmine-node": "^1.14.5"
+  },
+  "scripts": {
+    "test": "./node_modules/.bin/jasmine-node ./test"
   }
 }

gems/canvas_i18nliner/test.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+result=0
+
+echo "################ canvas_i18nliner ################"
+npm install
+npm test
+let result=$result+$?
+
+if [ $result -eq 0 ]; then
+  echo "SUCCESS"
+else
+  echo "FAILURE"
+fi
+
+exit $result

gems/canvas_i18nliner/test/fixtures/hbs/app/views/jst/foo/_barBaz.hbs

@@ -0,0 +1,7 @@
+<p>{{#t "#absolute_key"}}Absolute key{{/t}}</p>
+<p>{{#t "relative_key"}}Relative key{{/t}}</p>
+<p>{{#t}}Inferred key{{/t}}</p>
+
+<p>{{t "#inline_with_absolute_key" "Inline with absolute key"}}</p>
+<p>{{t "inline_with_relative_key" "Inline with relative key"}}</p>
+<p>{{t "Inline with inferred key"}}</p>

gems/canvas_i18nliner/test/fixtures/js/public/javascripts/foo.js

@@ -0,0 +1,12 @@
+define(["i18n!foo"], function(I18n) {
+  I18n.t("#absolute_key", "Absolute key");
+  I18n.t("Inferred key");
+  define(["i18n!nested"], function(I18n) {
+    I18n.t("relative_key", "Relative key in nested scope");
+  });
+  I18n.t("relative_key", "Relative key");
+});
+
+define(["i18n!bar"], function(I18n) {
+  I18n.t("relative_key", "Another relative key");
+});

gems/canvas_i18nliner/test/i18nliner_spec.js

@@ -0,0 +1,51 @@
+var I18nliner = require("../js/main").I18nliner;
+
+var subject = function(path) {
+  var command = new I18nliner.Commands.Check({});
+  var origDir = process.cwd();
+  try {
+    process.chdir(path);
+    command.run();
+  }
+  finally {
+    process.chdir(origDir);
+  }
+  return command.translations.masterHash.translations;
+}
+
+describe("I18nliner", function() {
+  describe("handlebars", function() {
+    it("extracts default translations", function() {
+      expect(subject("test/fixtures/hbs")).toEqual({
+        absolute_key: "Absolute key",
+        inferred_key_c49e3743: "Inferred key",
+        inline_with_absolute_key: "Inline with absolute key",
+        inline_with_inferred_key_88e68761: "Inline with inferred key",
+        foo: {
+          bar_baz: {
+            inline_with_relative_key: "Inline with relative key",
+            relative_key: "Relative key"
+          }
+        }
+      });
+    });
+  });
+
+  describe("javascript", function() {
+    it("extracts default translations", function() {
+      expect(subject("test/fixtures/js")).toEqual({
+        absolute_key: "Absolute key",
+        inferred_key_c49e3743: "Inferred key",
+        foo: {
+          relative_key: "Relative key"
+        },
+        bar: {
+          relative_key: "Another relative key"
+        },
+        nested: {
+          relative_key: "Relative key in nested scope"
+        }
+      });
+    });
+  });
+});

public/javascripts/i18nObj.js

@@ -174,13 +174,23 @@ I18n.strftime = function(date, format) {
 
 I18n.Utils.HtmlSafeString = htmlEscape.SafeString; // this is what we use elsewhere in canvas, so make i18nliner use it too
 I18n.CallHelpers.keyPattern = /^\#?\w+(\.\w+)+$/ // handle our absolute keys
+
+// when inferring the key at runtime (i.e. js/coffee or inline hbs `t`
+// call), signal to normalizeKey that it shouldn't be scoped.
+// TODO: make i18nliner-js set i18n_inferred_key, which will DRY things up
+// slightly
+var origInferKey = I18n.CallHelpers.inferKey;
+I18n.CallHelpers.inferKey = function() {
+  return "#" + origInferKey.apply(this, arguments);
+};
+
 I18n.CallHelpers.normalizeKey = function(key, options) {
   if (key[0] === '#') {
     key = key.slice(1);
     delete options.scope;
   }
   return key;
-}
+};
 
 if (window.ENV && window.ENV.lolcalize) {
   I18n.CallHelpers.normalizeDefault = i18nLolcalize;

spec/selenium/common.rb

@@ -427,6 +427,22 @@ def require_exec(*args)
     driver.execute_async_script(js)
   end
 
+  # add some JS translations to the current page; they'll be merged in at
+  # the root level, so the top-most key should be the locale, e.g.
+  #
+  #   set_translations fr: {key: "Bonjour"}
+  def set_translations(translations)
+    add_translations = "$.extend(true, I18n, {translations: #{translations.to_json}});"
+    if ENV['USE_OPTIMIZED_JS']
+      driver.execute_script <<-JS
+        define('translations/test', ['i18nObj', 'jquery'], function(I18n, $) {
+          #{add_translations}
+        });
+      JS
+    else
+      driver.execute_script add_translations
+    end
+  end
 end
 
 shared_examples_for "all selenium tests" do

spec/selenium/handlebars_spec.rb

@@ -8,14 +8,6 @@
     get "/"
   end
 
-  def set_translations(translations)
-    driver.execute_script <<-JS
-      define('translations/test', ['i18nObj', 'jquery'], function(I18n, $) {
-        $.extend(true, I18n, {translations: #{translations.to_json}});
-      });
-    JS
-  end
-
   def run_template(template, context, locale = 'en')
     compiled = HandlebarsTasks::Handlebars.compile_template(template, 'test')
     driver.execute_script compiled
@@ -82,25 +74,41 @@ def run_template(template, context, locale = 'en')
 
   it "should translate the content" do
     translations = {
-      :pigLatin => {
-        :sup => 'upsay',
-        :test => {
-          :it_should_work => 'isthay ouldshay ebay anslatedtray frday'
+      pigLatin: {
+        absolute_key: "Absoluteay eykay",
+        inferred_key_c49e3743: "Inferreday eykay",
+        inline_with_absolute_key: "Inlineay ithway absoluteay eykay",
+        inline_with_inferred_key_88e68761: "Inlineay ithway inferreday eykay",
+        test: {
+          inline_with_relative_key: "Inlineay ithway elativeray eykay",
+          relative_key: "Elativeray eykay"
         }
       }
     }
     set_translations(translations)
 
     template = <<-HTML
-      <p>{{#t "#sup"}}sup{{/t}}</p>
-      <p>{{#t 'it_should_work'}}this should be translated frd{{/t}}</p>
-      <p>{{#t "not_yet_translated"}}but this shouldn't be{{/t}}</p>
+      <p>{{#t "#absolute_key"}}Absolute key{{/t}}</p>
+      <p>{{#t "relative_key"}}Relative key{{/t}}</p>
+      <p>{{#t}}Inferred key{{/t}}</p>
+
+      <p>{{t "#inline_with_absolute_key" "Inline with absolute key"}}</p>
+      <p>{{t "inline_with_relative_key" "Inline with relative key"}}</p>
+      <p>{{t "Inline with inferred key"}}</p>
+
+      <p>{{#t "not_yet_translated"}}No translation yet{{/t}}</p>
     HTML
 
     expect(run_template(template, {}, 'pigLatin')).to eq <<-HTML
-      <p>#{translations[:pigLatin][:sup]}</p>
-      <p>#{translations[:pigLatin][:test][:it_should_work]}</p>
-      <p>but this shouldn't be</p>
+      <p>Absoluteay eykay</p>
+      <p>Elativeray eykay</p>
+      <p>Inferreday eykay</p>
+
+      <p>Inlineay ithway absoluteay eykay</p>
+      <p>Inlineay ithway elativeray eykay</p>
+      <p>Inlineay ithway inferreday eykay</p>
+
+      <p>No translation yet</p>
     HTML
   end
 

spec/selenium/i18n_js_spec.rb

@@ -50,5 +50,16 @@
         )
       end
     end
+
+    it "should not scope inferred keys" do
+      set_translations({
+        pigLatin: {
+          inferred_key_c49e3743: "Inferreday eykay",
+          test: {inferred_key_c49e3743: "Otnay isthay!"}
+        }
+      })
+      expect(require_exec('i18n!test', "I18n.locale = 'pigLatin'; i18n.t('Inferred key')"))
+        .to eq("Inferreday eykay")
+    end
   end
 end