🐛 Corrects sort order of factors #194
Conversation
|
@mauriciocastillosilva-okta Please review this PR |
gowthamidommety-okta
force-pushed
the
gd-OKTA-99123-signInWidgetFactorSortOrder
branch
from
0124d01
to
2b82201
Compare
| @@ -327,7 +336,7 @@ function (_, $, OktaAuth, Util, EnrollChoicesForm, Beacon, Expect, Router, | |||
| itp('has a setup button for each unenrolled optional factor which navigates to the correct page (On-Prem)', | |||
| function () { | |||
| return setupWithAllOptionalSomeEnrolled(true).then(function (test) { | |||
| expect(test.form.factorButton('OKTA_VERIFY').length).toBe(1); | |||
| expect(test.form.factorButton('OKTA_VERIFY_PUSH').length).toBe(0); | |||
There was a problem hiding this comment.
this should be 1 right? Since that factor is there now instead of OKTA_VERIFY
There was a problem hiding this comment.
setupWithAllOptionalSomeEnrolled sets the first factor in the xhr to enrolled. In MFA_ENROLLED_allFactors_OnPrem.js, the first factor was changed from QUESTION to OKTA_VERIFY_PUSH. Because OKTA_VERIFY_PUSH is now enrolled, it will not have a factor button, so length will be 0. I decided to write it this way, so the test would ensure the first factor does not have a button, and the rest do. Previously the test only ensured the un-enrolled factors had a button.
There was a problem hiding this comment.
Ah makes sense. Would it be a good idea then to add QUESTION and check that the Setup button is there for it?
There was a problem hiding this comment.
Yes! Good point. I think some of the others are missing too. Ill add them in as well.
test/unit/spec/EnrollChoices_spec.js
Outdated
| 'YUBIKEY','DUO','GOOGLE_AUTH','SYMANTEC_VIP','RSA_SECURID','QUESTION']); | ||
| }); | ||
| }); | ||
| itp(' with push and onPrem is in correct order', function () { |
There was a problem hiding this comment.
nit: no need for the space at the beginning of the test description
There was a problem hiding this comment.
👍 I will remove the space.
| 'QUESTION': 10 | ||
| }; | ||
|
|
||
| function clickFactorInDropdown(test, factorName) { |
gowthamidommety-okta
force-pushed
the
gd-OKTA-99123-signInWidgetFactorSortOrder
branch
from
2b82201
to
b7c6dc2
Compare
|
@mauriciocastillosilva-okta Implemented suggestions, Please review again. |
| @@ -1,4 +1,4 @@ | |||
| define(['jquery', './Form'], function ($, Form) { | |||
| define(['jquery', './Form', 'underscore'], function ($, Form, _) { | |||
There was a problem hiding this comment.
nit: missed this, but generally we have jquery and underscore before other files
define(['jquery', 'underscore', './Form'], function ($, _, Form) {
There was a problem hiding this comment.
👍 agreed!
| @@ -68,6 +68,13 @@ define(['jquery', './Form'], function ($, Form) { | |||
|
|
|||
| optionalFactorListTitle: function () { | |||
| return this.optionalFactorList().find('.list-title').trimmedText(); | |||
| }, | |||
|
|
|||
| getFactorSequence: function() { | |||
There was a problem hiding this comment.
nit: getFactorList or getAllFactorsList
There was a problem hiding this comment.
Changing this
test/unit/spec/EnrollChoices_spec.js
Outdated
| }); | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: no need for this extra line
There was a problem hiding this comment.
Deleting extra line
Factors are now in the correct order when signing in. The correct order can be viewed in the ticket. Also, added and updated tests to honor this new order. Resolves: OKTA-99123
gowthamidommety-okta
force-pushed
the
gd-OKTA-99123-signInWidgetFactorSortOrder
branch
from
a445dc6
to
ca31c1f
Compare
gowthamidommety-okta
deleted the
gd-OKTA-99123-signInWidgetFactorSortOrder
branch
Factors are now in the correct order when signing in. The correct order can be viewed in the ticket. Also, added and updated tests to honor this new order.
Resolves: OKTA-99123