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

Fixing redirect on removed from channel #4068

Merged
merged 5 commits into from
Oct 31, 2019
Merged

Fixing redirect on removed from channel #4068

merged 5 commits into from
Oct 31, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Oct 27, 2019

Summary

When a user was removed from a channel the redirection was made by the modal
component, now is made by the handler in the websocket function. And is using
the correct approach to redirect the user.

Ticket Link

MM-19040

@jespino jespino added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 27, 2019
@jespino jespino added this to the v5.17.0 milestone Oct 27, 2019
@jespino jespino requested review from srkgupta and a team and removed request for srkgupta October 27, 2019 15:08
@ghost ghost requested review from lieut-data and marianunez and removed request for a team October 27, 2019 15:08
@jespino jespino requested review from srkgupta and a team October 27, 2019 15:09
@ghost ghost removed their request for review October 27, 2019 15:09
marianunez
marianunez suggested changes Oct 28, 2019
View reviewed changes
Copy link
Member

@marianunez marianunez left a comment

Choose a reason for hiding this comment

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

This test should be updated if it is no longer going to goToLastViewedChannel but the Default Team after removal.

lieut-data
lieut-data suggested changes Oct 28, 2019
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Can we unit test the changes here?

cc @mgdelacroix re: #3960

@jespino
Copy link
Member Author

jespino commented Oct 29, 2019

@lieut-data done

@jespino
Copy link
Member Author

jespino commented Oct 29, 2019

@marianunez done

@srkgupta srkgupta added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 29, 2019
srkgupta
srkgupta suggested changes Oct 29, 2019
View reviewed changes
Copy link
Contributor

@srkgupta srkgupta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @jespino
When a user is removed from only one channel in a team, the user is not automatically redirected to the other channel. The user sees a loading error as shown in this screenshot. Also attaching few JS errors thrown in the console, when it tries to redirect.

Screenshot 2019-10-29 at 7 45 24 PM

Screenshot 2019-10-29 at 7 43 54 PM

srkgupta
srkgupta approved these changes Oct 30, 2019
View reviewed changes
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

I see that there is a seperate PR to fix the issue mentioned earlier:
#4070

So approving this PR since all the original issue reported is working fine on the PR instance.

@srkgupta srkgupta added QA Review Done and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 30, 2019
@mattermod
Copy link
Contributor

Test server destroyed

@amyblais amyblais added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Oct 30, 2019
@jespino jespino merged commit 94b192a into mattermost:master Oct 31, 2019
@jespino jespino deleted the MM-19040 branch October 31, 2019 12:11
@mattermod
Copy link
Contributor

@jespino
Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Fetching origin
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-#4068-upstream-release-5.17-1572523875
Branch 'automated-cherry-pick-of-#4068-upstream-release-5.17-1572523875' set up to track remote branch 'release-5.17' from 'upstream'.
+++ Downloading patch to /tmp/4068.patch (in case you need to do this again)

+++ About to attempt cherry pick of PR. To reattempt:
  $ git am -3 /tmp/4068.patch

Applying: Fixing redirect on removed from channel
Applying: adding tests
Patch failed at 0002 adding tests
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

+++ Conflicts detected:

!!! None. Did you git am --continue?

+++ Aborting in-progress git am.

+++ Returning you to the master branch and cleaning up.

jespino added a commit that referenced this pull request Oct 31, 2019
@jespino
Fixing redirect on removed from channel (#4068)
e9e2560 
* Fixing redirect on removed from channel

* adding tests

* Remove unnecesary broken test

* Fixing linting errors
@jespino jespino added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 31, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Oct 31, 2019
brewsterbhg pushed a commit to brewsterbhg/mattermost-webapp that referenced this pull request Nov 11, 2019
@jespino @brewsterbhg
Fixing redirect on removed from channel (mattermost#4068)
58eee96 
* Fixing redirect on removed from channel

* adding tests

* Remove unnecesary broken test

* Fixing linting errors
@ogi-m ogi-m added the Tests/Done Release tests have been written label Nov 13, 2019
@hmhealey hmhealey mentioned this pull request Oct 7, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marianunez marianunez marianunez approved these changes

@lieut-data lieut-data lieut-data approved these changes

@srkgupta srkgupta srkgupta approved these changes

Assignees

@jespino jespino

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation QA Review Done Tests/Done Release tests have been written
Projects
None yet
Milestone
v5.17.0
7 participants
@jespino @mattermod @marianunez @lieut-data @srkgupta @ogi-m @amyblais