feat(auth): adds hub events to updateUserAttributes api #10731
Conversation
Codecov Report
@@ Coverage Diff @@
## main #10731 +/- ##
==========================================
+ Coverage 86.14% 86.16% +0.01%
==========================================
Files 196 196
Lines 18337 18351 +14
Branches 3902 3905 +3
==========================================
+ Hits 15797 15812 +15
+ Misses 2465 2464 -1
Partials 75 75
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…k, adds third parameter to callback of updateAttributes api
There was a problem hiding this comment.
Can you add the change on the d.ts file because is a public facing API, I also have a nit comment.
Overall looks good.
Thanks @helgabalashova
There was a problem hiding this comment.
Minor issue with Cognito unit test, with that fix I will approve it
Thanks @helgabalashova
| .mockImplementationOnce((attrs, callback) => { | ||
| callback(null, 'SUCCESS', codeDeliverDetailsResult); |
There was a problem hiding this comment.
This mock is not testing anything in particular, is just testing the mock. What you should do here is mock the cognito client instead. Like this https://github.com/aws-amplify/amplify-js/blob/main/packages/amazon-cognito-identity-js/__tests__/CognitoUser.test.js#L384
There was a problem hiding this comment.
For testing there are no regressions for the version that has two arguments we need to have a TypeScript test that validates is not introducing a regression. I will approve the PR but before launching to latest we need the integ test in place
| const spyon = jest.spyOn(CognitoUser.prototype, 'updateAttributes') | ||
| .mockImplementationOnce((attrs, callback) => { | ||
| callback(null, 'SUCCESS', codeDeliverDetailsResult); |
There was a problem hiding this comment.
Same as the other, this is testing the mock not the actual code look on this one
https://github.com/aws-amplify/amplify-js/blob/main/packages/amazon-cognito-identity-js/__tests__/CognitoUser.test.js#L384
| @@ -2,6 +2,7 @@ declare module 'amazon-cognito-identity-js' { | |||
| //import * as AWS from "aws-sdk"; | |||
|
|
|||
| export type NodeCallback<E, T> = (err?: E, result?: T) => void; | |||
| export type UpdateAttributesNodeCallback<E, T, K> = (err?: E, result?: T, details?: K) => void; | |||
helgabalashova
dismissed stale reviews from elorzafe, israx, haverchuck, and jimblanc
via
b0ea117
|
The commit message should be feat(@aws-amplify/auth,amazon-cognito-identity-js): return code delivery details for updateUserAttributes on Hub event |
Description of changes
adds hub event to
Auth.updateUserAttributesto determine if verification of the attribute is required. Feature requested by UI team. Decision to implement it using Hub events was made by Auth and UI team to avoid breaking change.Issue #, if available
#9134
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.