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

MM-16114 Preventing plugin startup failure if bot account already exists as regular user. #11077

Merged
merged 1 commit into from Jun 10, 2019

Conversation

crspeller
Copy link
Member

Summary

Allows the plugin to takeover a regular user account with EnsureBot while logging an error letting the admin know what happened.

@crspeller crspeller added 2: Dev Review Requires review by a developer CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Jun 7, 2019
@crspeller crspeller added this to the v5.12.0 milestone Jun 7, 2019
@cpoile
Copy link
Member

cpoile commented Jun 7, 2019

So, no in-MM messages? I really don't mean to make work for you... But:

  1. I wonder if many sysadmins look at the logs
  2. If they don't realize the plugin failed to start, wait a few days, then realize, will they think to look in the log? Will they easily find that message if it's buried?

@crspeller
Copy link
Member Author

@cpoile Ya. We figured the value of those where low and there isn't a good system admin notification mechanism.

Part of this change is the plugin won't fail to start https://github.com/mattermost/mattermost-server/pull/11077/files#diff-7c191d59bfe26e51b1828e40ccfc46f8R52 it will just start using the account even though it's not a bot.

The plan is to improve this for 5.14 to better encourage migration to bot accounts.

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

I like this solution 👍

@crspeller crspeller merged commit 31d695f into master Jun 10, 2019
@crspeller crspeller deleted the mm-16114 branch June 10, 2019 14:39
crspeller added a commit that referenced this pull request Jun 10, 2019
@crspeller crspeller 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 Jun 10, 2019
@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Jun 10, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jun 10, 2019
Wipeout55 pushed a commit to Wipeout55/mattermost-server that referenced this pull request Jun 11, 2019
hanzei added a commit to mattermost/mattermost-plugin-zoom that referenced this pull request Jun 13, 2019
hanzei added a commit to mattermost/mattermost-plugin-zoom that referenced this pull request Jun 13, 2019
crspeller pushed a commit to mattermost/mattermost-plugin-zoom that referenced this pull request Jun 13, 2019
* Refactor zoom package

* Update dependency for mattermost/mattermost#11077
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants