GPII-3496: Auto-login with user folder #839
Conversation
* upstream/master: (647 commits) GPII-4303: Further adjusted timing per feedback in chat. GPII-4303: Adjusted timing based on observed performance in CI and commented out detailed timing logging. GPII-4303: Adjust test timing and log the observed times. GPII-3828: Address the new review comment. GPII-3828: Distribute defaultSettingsDataPromise rather than resolved values to avoid the race condition. GPII-3828: Fix the issue that StartupAPITests.js fails in Fedora VM. GPII-3828: Filter out preferences in the pspChannel request that have "undefined" values. GPII-3828: Address the review comment to fix the risk of a race at the system startup. GPII-4258: Adjusted expected test values and (ugh) canned SR data in tests to match updates. GPII-4258: Fixed linting error. GPII-4258: Improved validation error output for payload tests. Fixed invalid payloads. GPII-4256: Fixed contrast settings. GPII-4258: Standardised language codes with available language packs to fix validation errors when saving language settings. Revert "Merge branch 'GPII-4231'" GPII-4250: Addressed code review comments. GPII-3828: Adjusted PSPChannel.js to accommodate the change to the workflow for loading default settings. GPII-4250: Address review comments. GPII-4250: Remove options modifications after the component construction from flowManager. GPII-4118: Changed old name for setting with new convention GPII-3828: Improve docs. ...
|
I'm having test failures - need to figure out why |
|
CI job failed: https://ci.gpii.net/job/universal-tests/2088/ |
* upstream/master: (38 commits) GPII-4225: Fix the issue with keying in "jaws". Revert "Merge branch 'GPII-3119-part2'" GPII-3119 Adding ticket number to comment GPII-4323: Added explicit test of 'intra-application' transforms. GPII-4323: Update SR validation to transform before validation (adds support for intra-application aliases). GPII-4300: Remove the feature that calculates inferred common terms from application terms and generate matchmaking results based on both. This ensures the matchmaker only selects the requested application. GPII-3958: Address review comments. GPII-4306: Modified to write out the full git revision into a file GPII-3119 Optionally including supportedSettings in payload if they exist. GPII-3119 Minor stubs for supportedSettings to browser tests GPII-3119 Minor linting, and passing supportedSettings through lifecycle manager. GPII-3119 Fixing remaining tests. GPII-3119 Removing another missed path setting GPII-3119 Temporarily commenting out integration tests. GPII-3119 Removing more uncessary paths from prefssets and putting more supportedSettings in test specs. GPII-3119 Adding supported settings metadata to invoke settings handler payload. GPII-3119 Testing addition of supported settings to acceptance tests. GPII-3119 Removing now redundant path from capabilities transforms and fixing tests. GPII-3119 Adding optional SPI settings path to solutions registry json schema GPII-3119 Adding SPI to supportedSettings sections. Waiting to remove it from capabilitiesTransforms until the acceptanceTests are reworked for it. ...
|
CI job failed: https://ci.gpii.net/job/universal-tests/2131/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/2132/ |
| * Get an instance of the gpii.userListeners.pcsc component. | ||
| */ | ||
| gpii.tests.userListener.getPcscListener = function () { | ||
| var userListeners = gpii.userListeners({ |
There was a problem hiding this comment.
Why did these new global functions need to get added for each test function?
Note that all of this component tree will leak between tests. I'm not sure I understand what is going on here.
There was a problem hiding this comment.
Also.... nothing calls this function?
| @@ -208,3 +229,78 @@ gpii.userListeners.failed = function (that, err) { | |||
| }); | |||
| }); | |||
| }; | |||
|
|
|||
| /** | |||
| * Reads a token from the file. | |||
There was a problem hiding this comment.
Please replace the use of "token" with "GPII key". Thanks.
| promise.reject({ | ||
| isError: true, | ||
| message: message, | ||
| error: err |
There was a problem hiding this comment.
According to the convention, the path for the original error object is named as "originalError" rather than "error".
| gpii.userListeners.userFolder.addLoginListener = function (that, firer) { | ||
| if (that.options.saveLastLogin) { | ||
| firer.addListener(function (gradeName, gpiiKey) { | ||
| if (gpiiKey !== "noUser") { |
There was a problem hiding this comment.
You might like to check the gpiiKey is not any of reserved keys. A list of reserved keys can be found [here|https://github.com/GPII/universal/blob/master/gpii/node_modules/flowManager/src/SystemUtils.js#L19|.
| /* | ||
| * userFolder User listener tests | ||
| * | ||
| * Copyright 2017 Raising the Floor - International |
There was a problem hiding this comment.
2017 -> 2020.
This comment applies to all other files.
| }); | ||
|
|
||
| fluid.promise.sequence([ | ||
| function () { |
There was a problem hiding this comment.
The functions for three test cases in this sequence are pretty much identical. It would be good if you can consolidate them into one. Thanks.
| message: "userFolder.attemptLogin had already been called." | ||
| }); | ||
| } else { | ||
| that.done = true; |
There was a problem hiding this comment.
Shall this line be inside the promise.then() block down below so that in the case that.getToken() fails, that.done will not be set to true.
| var eventRaised = false; | ||
|
|
||
| userFolder.events.onTokenArrive.addListener(function (that, token) { | ||
| jqUnit.assertFalse("Token event should only fire once", eventRaised); |
There was a problem hiding this comment.
Take the chance to test the status of that.done too.
It would be nice to have a test case for when that.getToken() cannot read a GPII key.
|
UPDATE: This PR has been mothballed. Right now, the Windows login component supports this functionality (https://github.com/GPII/windows/blob/83af4e86ee618d9796d95ef187bd7a222db553d1/gpii/node_modules/userListeners/src/windowsLogin.js#L176-L181) and can be specified in the gpii-app's siteconfig file. At this moment we don't need this functionality to be cross-platform and we can resume the work on it by the time mac implementation is more mature (or even the gnu/linux one). |
Replaces #725