Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-27861 signal successful login to other tabs #6319

Merged
merged 6 commits into from
Sep 8, 2020
Merged

MM-27861 signal successful login to other tabs #6319

merged 6 commits into from
Sep 8, 2020

Conversation

bhargav50
Copy link
Contributor

Summary

Signals login event to other tabs on successful login

Ticket Link

Fixes mattermost/mattermost#15302

Changes introduced

  • calls signalLogin in componentDidUpdate of logged_in component
  • reloads another tab on focus after it receives signalLogin event

Unit tests added

  • logged_in component should call signalLogin once loggedIn state is received from redux store
  • root component should auto login on focus after it receives signalLogin

@bhargav50 bhargav50 changed the title signal successful login to other tabs [MM-27861] signal successful login to other tabs Aug 29, 2020
@bhargav50 bhargav50 changed the title [MM-27861] signal successful login to other tabs MM-27861 signal successful login to other tabs Aug 29, 2020
@jasonblais jasonblais added 1: PM Review Requires review by a product manager Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Aug 29, 2020
@jasonblais
Copy link
Contributor

Thank you @bhargav50!

@esethna esethna added 3: QA Review Requires review by a QA tester 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager labels Aug 31, 2020
@esethna esethna added this to the v5.28 milestone Aug 31, 2020
hmhealey
hmhealey reviewed Sep 1, 2020
View reviewed changes
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @bhargav50! I have a few questions, but they're mostly since it's been a long time since I last looked at this code.

components/root/root.jsx Outdated Show resolved Hide resolved
components/root/root.jsx Show resolved Hide resolved
components/root/root.test.jsx Show resolved Hide resolved
components/root/root.test.jsx Outdated Show resolved Hide resolved
hmhealey
hmhealey approved these changes Sep 3, 2020
View reviewed changes
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Thanks for answering my questions. This looks good to me then!

@nevyangelova
Copy link
Contributor

LGTM can you resolve the conflict and sync with master? Thanks for the contribution 👍

jgilliam17
jgilliam17 approved these changes Sep 7, 2020
View reviewed changes
Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thank you @bhargav50
Tested, looks good to merge.

  • Verified login is signaled to other tabs (Windows, macOS and Linux browsers).
    After user has been logged in, login on other tabs completes when focus is placed on those tabs and not immediately at the time of initial login.
    @nevyangelova Can you please help merge? Thanks.

@jgilliam17 jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Sep 7, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@bhargav50
Copy link
Contributor Author

bhargav50 commented Sep 7, 2020 •
edited
Loading

I have resolved conflicts with master and pushed but one check is still failing. @nevyangelova can you help me figuring out what is this actually suggesting?

@jasonblais
Copy link
Contributor

/update-branch

@jasonblais
Copy link
Contributor

@nevyangelova @bhargav50 The failed build check was for an outdated branch, so I ran /update-check to rebase against master. It looks like one of the tests are failing now (though there weren't any code changes due to the rebase: 3d7e5b0).

@hmhealey
Copy link
Member

hmhealey commented Sep 8, 2020

I've updated again to fix a merge conflict. Hopefully the build failure was just due to a random failure. I'll keep an eye on it to ensure it passes.

@hmhealey hmhealey merged commit 879768c into mattermost:master Sep 8, 2020
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Sep 15, 2020
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
@bhargav50 @mattermod
@hmhealey @jfrerich
MM-27861 signal successful login to other tabs (#6319)
6042921 
* signal successful login to other tabs

* signal login to other tabs

Co-authored-by: Mattermod <[email protected]>
Co-authored-by: Harrison Healey <[email protected]>
@hmhealey hmhealey mentioned this pull request Apr 28, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hmhealey hmhealey hmhealey approved these changes

@esethna esethna esethna approved these changes

@nevyangelova nevyangelova nevyangelova approved these changes

@jgilliam17 jgilliam17 jgilliam17 approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation
Projects
None yet
Milestone
v5.28.0
9 participants
@bhargav50 @jasonblais @nevyangelova @mm-cloud-bot @hmhealey @esethna @jgilliam17 @amyblais @mattermod