Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Added setUserData to called after logout #343 #344

Merged
merged 2 commits into from
Aug 24, 2021

Conversation

codesankalp
Copy link
Member

@codesankalp codesankalp commented Aug 11, 2021

If the session is active then setUserData is not called after captive
portal logout. Added setUserData with initialData.userData after
performing captive portal logout.

Fixes #343
Closes #306

@codesankalp codesankalp self-assigned this Aug 11, 2021
@codesankalp codesankalp force-pushed the issue-343-prev-user-detail-bug branch from afda845 to e398a00 Compare August 13, 2021 04:24
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase on the latest master and fix conflicts please?

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codesankalp can you please indicate if there's any tests which replicates the issue and fails without the fix? If not, could you add at least one?

If the session is active then setUserData is not called after captive
portal logout. Added setUserData with initialData.userData after
performing captive portal logout.

Fixes #343
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @codesankalp, I have a few doubts, see below.

client/utils/handle-logout.js Show resolved Hide resolved
client/utils/handle-logout.js Show resolved Hide resolved
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better now, @pandafy could you please double check if the error you were having is fixed?

@pandafy
Copy link
Member

pandafy commented Aug 23, 2021

I confirm this solves the issue I was facing. 👍🏼

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codesankalp could you squash the commits so we have one per issue? Thanks.

@nemesifier
Copy link
Member

I confirm this solves the issue I was facing. 👍🏼

Thank you @pandafy 👏.

Added validateToken where user can logout.
Made handleLogout function reusable.
Used `mustLogout=true` in handleLogout for captive portal logout.

Closes #306
@nemesifier nemesifier merged commit a7bf82f into master Aug 24, 2021
@nemesifier nemesifier deleted the issue-343-prev-user-detail-bug branch August 24, 2021 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants