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

added the warning message when a user with bot account gets deleted #2879

Merged
merged 5 commits into from
Jun 19, 2019
Merged

added the warning message when a user with bot account gets deleted #2879

merged 5 commits into from
Jun 19, 2019

Conversation

pradeepmurugesan
Copy link
Contributor

@pradeepmurugesan pradeepmurugesan commented May 29, 2019 •
edited by hanzei
Loading

Summary

This adds a disclaimer message regarding the bot accounts when deactivating a user with at-least one bot accounts created.

Ticket Link

Fixes mattermost/mattermost#10863

Related Pull Requests

  • Has server changes - No
  • Has redux changes - No
  • Has mobile changes- No

Screenshots:

Without Bot accounts
With Bot accounts

@hanzei
Copy link
Contributor

hanzei commented May 29, 2019

@pradeepmurugesan Is this ready for review?

@hanzei hanzei added the Work in Progress Not yet ready for review label May 29, 2019
@pradeepmurugesan pradeepmurugesan changed the title [WIP] added the warning message when a user with bot account gets deleted added the warning message when a user with bot account gets deleted May 29, 2019
@hanzei hanzei added 1: PM Review Requires review by a product manager Setup Old Test Server Triggers the creation of a test server and removed Work in Progress Not yet ready for review labels May 29, 2019
@hanzei hanzei requested a review from jasonblais May 29, 2019 20:44
jasonblais
jasonblais reviewed May 30, 2019
View reviewed changes
Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

Functionally this works great.

For the case when the user does not manage bot accounts, wondering if we can keep the text without the bullet like in mattermost/mattermost#10863. It may feel odd to the System Admin given there is only one item in the list.

image

I'm okay having the question on a separate line, however, like

This action deactivates \{user\}. They will be logged out and not have access to any teams or channels on this system.

Are you sure you want to deactivate \{user\}?

@hanzei hanzei requested a review from jasonblais May 30, 2019 21:25
@hanzei hanzei added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels May 30, 2019
@jasonblais
Copy link
Contributor

Thanks @pradeepmurugesan for the update! Looks like the latest build fails - I'll re-test after you've had a chance to take a look at it

@saturninoabril saturninoabril removed the Setup Old Test Server Triggers the creation of a test server label May 31, 2019
@pradeepmurugesan pradeepmurugesan force-pushed the GH-10863 branch 2 times, most recently from e87fe7d to 639bf18 Compare May 31, 2019 09:17
@pradeepmurugesan
Copy link
Contributor Author

@jasonblais have modified the same. Kindly review

@jasonblais jasonblais added the Setup Old Test Server Triggers the creation of a test server label May 31, 2019
jasonblais
jasonblais approved these changes May 31, 2019
View reviewed changes
Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

Looks good! Re-tested functionality as well.

I'll ask @asaadmahmood to help review as well, including the line spacing for the text on the dialog
image

@jasonblais jasonblais added 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager labels May 31, 2019
@hanzei hanzei removed the Setup Old Test Server Triggers the creation of a test server label Jun 1, 2019
@pradeepmurugesan pradeepmurugesan force-pushed the GH-10863 branch 2 times, most recently from a2693c1 to 8f8f65d Compare June 3, 2019 08:37
@jasonblais
Copy link
Contributor

@sudheerDev @asaadmahmood Kind reminder to help review this PR when you have a chance

@asaadmahmood
Copy link
Contributor

Done @jasonblais. Updated the spacing.
Screenshot 2019-06-09 at 2 19 14 PM

sudheerDev
sudheerDev suggested changes Jun 9, 2019
View reviewed changes
Copy link
Contributor

@sudheerDev sudheerDev left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . Requested a minor change.

components/admin_console/system_users/system_users_dropdown/system_users_dropdown.jsx Outdated Show resolved Hide resolved
@sudheerDev
Copy link
Contributor

@jasonblais Need another dev reviewer on this.

@jasonblais
Copy link
Contributor

@asaadmahmood also, looks like the build fails

@pradeepmurugesan
Copy link
Contributor Author

@sudheerDev Fixed the review comments.

@asaadmahmood I am so sorry. I found that I have overwritten your commit during the process of fixing the review comments. Can you kindly push your changes. I am so sorry.

@asaadmahmood
Copy link
Contributor

@pradeepmurugesan No worries, will do in a while.

@asaadmahmood
Copy link
Contributor

@pradeepmurugesan Done.

@hanzei hanzei requested a review from sudheerDev June 12, 2019 18:06
sudheerDev
sudheerDev approved these changes Jun 13, 2019
View reviewed changes
Copy link
Contributor

@sudheerDev sudheerDev left a comment

Choose a reason for hiding this comment

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

LGTM :+1

@bradjcoughlin
Copy link
Contributor

@jasonblais is this PR slated for a release?

@jasonblais jasonblais added this to the v5.14.0 milestone Jun 18, 2019
@jasonblais jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Jun 18, 2019
@jasonblais
Copy link
Contributor

@bradjcoughlin This can go into the 5.14 release (so just merged to master)

@pradeepmurugesan Can you help rebase against the master branch? Looks like this is otherwise ready to merge 🎉

@hanzei hanzei merged commit 8dbde42 into mattermost:master Jun 19, 2019
@hanzei
Copy link
Contributor

hanzei commented Jun 19, 2019

Thanks @pradeepmurugesan for the awesome contribution 👍

@DHaussermann DHaussermann added the Tests/Done Release tests have been written label Jun 21, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jul 5, 2019
@cpoile cpoile mentioned this pull request Jul 10, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@asaadmahmood asaadmahmood asaadmahmood approved these changes

@jasonblais jasonblais jasonblais approved these changes

@sudheerDev sudheerDev sudheerDev approved these changes

@bradjcoughlin bradjcoughlin bradjcoughlin approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Done Release tests have been written
Projects
None yet
Milestone
v5.14.0
9 participants
@pradeepmurugesan @hanzei @jasonblais @asaadmahmood @sudheerDev @bradjcoughlin @saturninoabril @amyblais @DHaussermann