fix(aws-amplify-react-native): Update Hub listener for sign out events #5587

amhinson · 2020-04-28T00:12:13Z

Issue #, if available:
Closes #4057

Description of changes:

Update Hub "auth" listener to handle calling Auth.signOut() directly instead of using the sign out button in Greeting. The onHubCapsule method was refactored based on the web UI components.
change handleStateChange to only block setting state if this._isMounted === false instead of the entire method. This also makes some changes to more closely resemble the web UI logic.
Also, this makes sure checkUser is only called once instead of every time an auth event occurs. That behavior is also to reflect the web UI.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

lgtm-com · 2020-04-28T00:20:18Z

This pull request fixes 2 alerts when merging b8fed61 into 14f1ef9 - view on LGTM.com

fixed alerts:

2 for Unused variable, import, function or class

codecov · 2020-04-28T00:29:13Z

Codecov Report

Merging #5587 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5587   +/-   ##
=======================================
  Coverage   72.91%   72.91%           
=======================================
  Files         197      197           
  Lines       11536    11536           
  Branches     2178     2178           
=======================================
  Hits         8412     8412           
  Misses       2971     2971           
  Partials      153      153

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f03e22...06b408f. Read the comment docs.

Amplifiyer · 2020-04-28T01:08:34Z

Can we accompany this with unit tests and/or integration tests?

amhinson · 2020-04-28T01:42:53Z

Just added a test to the existing PR for React Native Auth integration tests: https://github.com/aws-amplify/amplify-js-samples-staging/pull/76

packages/aws-amplify-react-native/src/Auth/Authenticator.js

Co-Authored-By: Eric Clemmons <eric@smarterspam.com>

lgtm-com · 2020-04-28T17:24:57Z

This pull request fixes 2 alerts when merging ea588fe into ebe0b69 - view on LGTM.com

fixed alerts:

2 for Unused variable, import, function or class

ashika01

Couple of nits. Looks good. Agree with Praveen, could we have a unit test added for this here?

packages/aws-amplify-react-native/src/Auth/Authenticator.js

amhinson · 2020-04-28T23:14:42Z

Couple of nits. Looks good. Agree with Praveen, could we have a unit test added for this here?

There are no existing unit tests for aws-amplify-react-native ... 😱 so that might need to be a separate PR to get all that setup. However, the integ test linked above covers this case now (and making sure this doesn't break anything else).

Co-Authored-By: Ashika <35131273+ashika01@users.noreply.github.com>

lgtm-com · 2020-04-28T23:22:49Z

This pull request fixes 2 alerts when merging d437641 into e89ac00 - view on LGTM.com

fixed alerts:

2 for Unused variable, import, function or class

ashika01 · 2020-04-30T05:11:01Z

packages/aws-amplify-react-native/src/Auth/Authenticator.js

+		let nextAuthState;
 		if (state === 'signedOut') {
-			state = 'signIn';
+			nextAuthState = this._initialAuthState;
+		} else {
+			nextAuthState = state;
+		}
+
+		let nextAuthData = this.state.authData;
+		if (data !== undefined) {
+			nextAuthData = data;
 		}


I know the important bit here is the hub listener for signout and looks good.

We can clean nextAuthData and nextAuthState with ternary operator so this is readable and become easier later. :)

Great call 👍 I just updated the PR

lgtm-com · 2020-04-30T16:24:10Z

This pull request fixes 2 alerts when merging f090258 into 41fb52c - view on LGTM.com

fixed alerts:

2 for Unused variable, import, function or class

ashika01

Looks good 🎉 👍

ericclemmons

Thanks for the updates @amhinson! Looking good!

lgtm-com · 2020-04-30T17:27:16Z

This pull request fixes 2 alerts when merging 06b408f into 8f03e22 - view on LGTM.com

fixed alerts:

2 for Unused variable, import, function or class

nihp · 2020-07-08T08:08:34Z

Can you able to look this?
#6257

This case is if the user is already in the app and the admin has disabled the user. So I called Auth.signOut. After this it changing the authState to signIn and then immediately changed to signedIn. So it not properly navigate to login screen.

Again reloaded the app will show the SignIn screen.

Steps:

Login the app
Admin disable the user
Then it not shows the user is disabled state while i am in (i.e) it not show anything in UI (expected behaviour is not to allow this user to do anything in the app)
So reload the app it will change the authState to signIn (It shows login screen for a second)
Again it immediately change the authState to signedIn (So it shows a white screen)
Because of this I need to reload again the app (Then it changes the authstate to signIn and shows the login screen as expected)

Here: I don't want to reload the app in the 6th step

Also, can you give any solution for the 3rd point

Your reply will be helpful

github-actions · 2021-07-08T16:05:43Z

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

update Hub listener for sign out

edf7fcd

amhinson requested review from ericclemmons, jordanranz and Ashish-Nanda April 28, 2020 00:12

amhinson added the React Native React Native related issue label Apr 28, 2020

amhinson requested a review from ashika01 April 28, 2020 00:12

Merge branch 'master' into rn-listen-for-signout

b8fed61

ericclemmons reviewed Apr 28, 2020

View reviewed changes

packages/aws-amplify-react-native/src/Auth/Authenticator.js Outdated Show resolved Hide resolved

Update packages/aws-amplify-react-native/src/Auth/Authenticator.js

ea588fe

Co-Authored-By: Eric Clemmons <eric@smarterspam.com>

ashika01 reviewed Apr 28, 2020

View reviewed changes

packages/aws-amplify-react-native/src/Auth/Authenticator.js Outdated Show resolved Hide resolved

packages/aws-amplify-react-native/src/Auth/Authenticator.js Outdated Show resolved Hide resolved

Merge branch 'master' into rn-listen-for-signout

19f5f24

Apply suggestions from code review

d437641

Co-Authored-By: Ashika <35131273+ashika01@users.noreply.github.com>

Merge branch 'master' into rn-listen-for-signout

f402ee5

ashika01 reviewed Apr 30, 2020

View reviewed changes

refactor handleStateChange variables

f090258

Merge branch 'master' into rn-listen-for-signout

06b408f

ashika01 approved these changes Apr 30, 2020

View reviewed changes

ericclemmons approved these changes Apr 30, 2020

View reviewed changes

amhinson merged commit b8d2c4d into aws-amplify:master Apr 30, 2020

amhinson deleted the rn-listen-for-signout branch April 30, 2020 17:28

github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(aws-amplify-react-native): Update Hub listener for sign out events #5587

fix(aws-amplify-react-native): Update Hub listener for sign out events #5587

amhinson commented Apr 28, 2020 •

edited

lgtm-com bot commented Apr 28, 2020

codecov bot commented Apr 28, 2020 •

edited

Amplifiyer commented Apr 28, 2020

amhinson commented Apr 28, 2020

lgtm-com bot commented Apr 28, 2020

ashika01 left a comment

amhinson commented Apr 28, 2020 •

edited

lgtm-com bot commented Apr 28, 2020

ashika01 Apr 30, 2020

amhinson Apr 30, 2020

lgtm-com bot commented Apr 30, 2020

ashika01 left a comment

ericclemmons left a comment

lgtm-com bot commented Apr 30, 2020

nihp commented Jul 8, 2020 •

edited

github-actions bot commented Jul 8, 2021

fix(aws-amplify-react-native): Update Hub listener for sign out events #5587

fix(aws-amplify-react-native): Update Hub listener for sign out events #5587

Conversation

amhinson commented Apr 28, 2020 • edited

lgtm-com bot commented Apr 28, 2020

codecov bot commented Apr 28, 2020 • edited

Codecov Report

Amplifiyer commented Apr 28, 2020

amhinson commented Apr 28, 2020

lgtm-com bot commented Apr 28, 2020

ashika01 left a comment

Choose a reason for hiding this comment

amhinson commented Apr 28, 2020 • edited

lgtm-com bot commented Apr 28, 2020

ashika01 Apr 30, 2020

Choose a reason for hiding this comment

amhinson Apr 30, 2020

Choose a reason for hiding this comment

lgtm-com bot commented Apr 30, 2020

ashika01 left a comment

Choose a reason for hiding this comment

ericclemmons left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Apr 30, 2020

nihp commented Jul 8, 2020 • edited

github-actions bot commented Jul 8, 2021

amhinson commented Apr 28, 2020 •

edited

codecov bot commented Apr 28, 2020 •

edited

amhinson commented Apr 28, 2020 •

edited

nihp commented Jul 8, 2020 •

edited