:Bug: Fix bundle in sign in widget. #313

nikhilvenkatraman-okta · 2017-10-02T23:00:40Z

In the sign in widget, we don't use the courage bundle that is referenced inside courage components. To fix this there was a change to point all courage bundle references to login. We made this change in most places, but a few places were missed.

@mauriciocastillosilva-okta @hor-kanchan-okta @rchild-okta

@upteam-okta

Test by changing browser language

mauriciocastillosilva-okta

looks good to me.
Can you test this with an override? Meaning a key that is passed in the config for other language

nikhilvenkatraman-okta · 2017-10-03T00:14:28Z

@mauriciocastillosilva-okta

Tried this with the following config.

                  baseUrl: 'http://rain.okta1.com:1802',
                  logo: '/img/logo_widgico.png',
                  logoText: 'Windico',
                  i18n: {
                  fr: {
                      'primaryauth.title': 'French Sign in'
                    }
                  },
                  features: {
                    router: true,
                    rememberMe: true,
                    multiOptionalFactorEnroll: true
                  },
                  // Host the assets (i.e. jsonp files) locally
                  assets: {
                    baseUrl: '/'
                  }

mauriciocastillosilva-okta · 2017-10-03T01:16:29Z

Please try it as well with an override of a string that is defined in courage bundle

nikhilvenkatraman-okta · 2017-10-03T04:46:57Z

@mauriciocastillosilva-okta @hor-kanchan-okta added a few unit tests

mauriciocastillosilva-okta

Thanks for adding the tests. Just minor comments see if we can reuse functions we already have.

mauriciocastillosilva-okta · 2017-10-03T13:44:25Z

test/unit/helpers/dom/Form.js

@@ -6,6 +6,10 @@ define(['okta/jquery', 'okta/underscore', './Dom'], function ($, _, Dom) {
      return this.el('o-form-head').trimmedText();
    },

+    errorBannerText: function () {


can you use any of the error functions we already have? I think errorMessage should work, but we also have hasErrors, errorBox and error

Makes sense. I'll update the test

mauriciocastillosilva-okta · 2017-10-03T13:52:29Z

test/unit/helpers/xhr/labels_login_ja.js

@@ -3,6 +3,7 @@ define({
  'responseType': 'json',
  'response': {
    'enroll.call.setup': 'JA: enroll.call.setup',
-    'security.disliked_food': 'JA: What is the food you least liked as a child?'
+    'security.disliked_food': 'JA: What is the food you least liked as a child?',
+    'oform.errorbanner.title': 'Japanese error banner title'


Doesn't this need the JA: prefix?

We don't need the JA prefix. My goal for this test is just to make sure that I am validating the text value for the key oform.errorbanner.title that is coming from login_ja file. Essentially mocking the behavior we get when we have bundles.login loaded with japanese translations.

right, I think it's more for consistency with the other strings we have... JA: oform.errorbanner.title, but I guess it is more a nit than anything else.

nikhilvenkatraman-okta · 2017-10-03T17:21:04Z

@mauriciocastillosilva-okta updated tests

mauriciocastillosilva-okta

Minor nits. Fix those and ⛵️

mauriciocastillosilva-okta · 2017-10-03T17:57:00Z

test/unit/helpers/xhr/labels_login_ja.js

@@ -3,6 +3,7 @@ define({
  'responseType': 'json',
  'response': {
    'enroll.call.setup': 'JA: enroll.call.setup',
-    'security.disliked_food': 'JA: What is the food you least liked as a child?'
+    'security.disliked_food': 'JA: What is the food you least liked as a child?',
+    'oform.errorbanner.title': 'Japanese error banner title'


right, I think it's more for consistency with the other strings we have... JA: oform.errorbanner.title, but I guess it is more a nit than anything else.

mauriciocastillosilva-okta · 2017-10-03T18:10:12Z

test/unit/spec/LoginRouter_spec.js

@@ -975,6 +975,48 @@ function (Okta, Q, Backbone, SharedUtil, CryptoUtil, CookieUtil, Logger, OktaAut
          expect(test.form.selectedCountry()).toBe('Nihon');
        });
      });
+
+      itp('overrides text for keys in courage bundle for non english languages', function () {


nit: overrides text in the courage bundle for non English language

mauriciocastillosilva-okta · 2017-10-03T18:13:12Z

test/unit/spec/LoginRouter_spec.js

+        });
+      });
+
+      itp('Text for keys in courage bundle are in jp as set in settings.language', function () {


nit: Strings in courage bundle are in jp as set in settings.language

mauriciocastillosilva-okta · 2017-10-03T18:14:28Z

test/unit/spec/LoginRouter_spec.js

+        });
+      });
+
+      itp('Text for keys in courage bundle are in en as set in settings.language', function () {


This might be make more sense as Strings in courage bundle are in English by default and remove the settings object. Thinking because we already tested that a specific language works in the test above (jp)

nikhilvenkatraman-okta · 2017-10-03T18:36:30Z

@mauriciocastillosilva-okta address nits with the last commit.

mauriciocastillosilva-okta · 2017-10-03T18:40:54Z

🏎

mauriciocastillosilva-okta approved these changes Oct 3, 2017

View reviewed changes

nikhilvenkatraman-okta changed the title ~~🐛 Fix bundle in sign in widget.~~ :Bug: Fix bundle in sign in widget. Oct 3, 2017

hor-kanchan-okta approved these changes Oct 3, 2017

View reviewed changes

mauriciocastillosilva-okta requested changes Oct 3, 2017

View reviewed changes

nikhilvenkatraman-okta force-pushed the nv-OKTA-143497 branch from 2184443 to c685b6e Compare October 3, 2017 17:50

mauriciocastillosilva-okta approved these changes Oct 3, 2017

View reviewed changes

nikhilvenkatraman-okta added 4 commits October 3, 2017 18:35

Fix bundle in sign in widget.

d2fa68c

Add tests

2c08bc0

Update test to use existing method.

c70aed2

Fix nits

b089217

nikhilvenkatraman-okta force-pushed the nv-OKTA-143497 branch from c685b6e to b089217 Compare October 3, 2017 18:35

nikhilvenkatraman-okta merged commit 69bcd94 into okta:master Oct 3, 2017

hor-kanchan-okta mentioned this pull request Feb 16, 2018

Partial error message placeholder appears after clicking "Sign In" with no credentials #390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

:Bug: Fix bundle in sign in widget. #313

:Bug: Fix bundle in sign in widget. #313

nikhilvenkatraman-okta commented Oct 2, 2017 •

edited

Loading

mauriciocastillosilva-okta left a comment

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta commented Oct 3, 2017

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta left a comment

mauriciocastillosilva-okta Oct 3, 2017

nikhilvenkatraman-okta Oct 3, 2017

mauriciocastillosilva-okta Oct 3, 2017

nikhilvenkatraman-okta Oct 3, 2017

mauriciocastillosilva-okta Oct 3, 2017

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta left a comment

mauriciocastillosilva-okta Oct 3, 2017

mauriciocastillosilva-okta Oct 3, 2017

mauriciocastillosilva-okta Oct 3, 2017

mauriciocastillosilva-okta Oct 3, 2017

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta commented Oct 3, 2017

:Bug: Fix bundle in sign in widget. #313

:Bug: Fix bundle in sign in widget. #313

Conversation

nikhilvenkatraman-okta commented Oct 2, 2017 • edited Loading

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta commented Oct 3, 2017

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikhilvenkatraman-okta commented Oct 3, 2017

mauriciocastillosilva-okta commented Oct 3, 2017

nikhilvenkatraman-okta commented Oct 2, 2017 •

edited

Loading