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

[GH-9961] Fix boolean setting not updating properly for plugins #2141

Merged
merged 1 commit into from
Dec 10, 2018
Merged

[GH-9961] Fix boolean setting not updating properly for plugins #2141

merged 1 commit into from
Dec 10, 2018

Conversation

chetanyakan
Copy link
Contributor

@chetanyakan chetanyakan commented Dec 5, 2018 •
edited by lindy65
Loading

Summary

In Admin Console page for plugin settings, boolean setting in plugins cannot be updated if their default value is true.
This PR fixes the above bug.

Ticket Link

mattermost/mattermost#9961
https://mattermost.atlassian.net/browse/MM-13475

Checklist

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed

@esethna esethna added the 1: PM Review Requires review by a product manager label Dec 5, 2018
@jasonblais jasonblais added 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager labels Dec 7, 2018
@jasonblais jasonblais removed their assignment Dec 7, 2018
@jasonblais jasonblais added the Setup Old Test Server Triggers the creation of a test server label Dec 7, 2018
@mattermost mattermost deleted a comment from mattermod Dec 7, 2018
@mattermost mattermost deleted a comment from mattermod Dec 7, 2018
@jasonblais jasonblais 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 Dec 7, 2018
@jasonblais
Copy link
Contributor

@cpanato Looks like the spinmint isn't firing for this PR.

@cpanato cpanato removed the Setup Old Test Server Triggers the creation of a test server label Dec 7, 2018
@mattermost mattermost deleted a comment from mattermod Dec 7, 2018
@mattermost mattermost deleted a comment from mattermod Dec 7, 2018
@cpanato cpanato added the Setup Old Test Server Triggers the creation of a test server label Dec 7, 2018
@cpanato
Copy link
Contributor

cpanato commented Dec 7, 2018

@jasonblais done

@jasonblais jasonblais self-requested a review December 7, 2018 13:48
@jasonblais jasonblais added the 1: PM Review Requires review by a product manager label Dec 7, 2018
@jasonblais
Copy link
Contributor

Thanks Carlos, @chetanyakan - would you happen to have a sample plugin I can use to verify the changes?

I checked with our existing plugins, and changing the settings work, but I didn't find one with a boolean value set to true by default.

@jasonblais jasonblais self-assigned this Dec 8, 2018
@chetanyakan
Copy link
Contributor Author

@jasonblais
mattermost-plugin-sample-plugin-darwin-amd64.tar.gz
mattermost-plugin-sample-plugin-linux-amd64.tar.gz
mattermost-plugin-sample-plugin-windows-amd64.tar.gz

Please find the plugin files attached.

jasonblais
jasonblais approved these changes Dec 10, 2018
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.

Thanks! Works for me

@jasonblais jasonblais removed 1: PM Review Requires review by a product manager Setup Old Test Server Triggers the creation of a test server labels Dec 10, 2018
@jasonblais jasonblais removed their assignment Dec 10, 2018
@mattermost mattermost deleted a comment from mattermod Dec 10, 2018
@mattermost mattermost deleted a comment from mattermod Dec 10, 2018
@mattermost mattermost deleted a comment from mattermod Dec 10, 2018
@jwilander jwilander added 4: Reviews Complete All reviewers have approved the pull request 2: Dev Review Requires review by a core commiter and removed 2: Dev Review Requires review by a core commiter 4: Reviews Complete All reviewers have approved the pull request labels Dec 10, 2018
jespino
jespino approved these changes Dec 10, 2018
View reviewed changes
Copy link
Member

@jespino jespino left a comment

Choose a reason for hiding this comment

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

LGTM

@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 Dec 10, 2018
@jasonblais
Copy link
Contributor

Thanks for the quick reviews. @chetanyakan can you help rebase and then this PR is ready to merge.

@jwilander jwilander merged commit 79b0bac into mattermost:master Dec 10, 2018
@jwilander
Copy link
Member

I merged it, that check is optional and there shouldn't be any problems with not rebasing

@jwilander
Copy link
Member

Thanks @chetanyakan !

@chetanyakan chetanyakan deleted the GH-9961 branch December 11, 2018 04:17
@amyblais amyblais added this to the v5.7.0 milestone Dec 12, 2018
@lindy65 lindy65 removed the 4: Reviews Complete All reviewers have approved the pull request label Dec 28, 2018
@amyblais amyblais removed this from the v5.7.0 milestone Jan 1, 2019
@lindy65 lindy65 modified the milestone: v5.8.0 Jan 10, 2019
@lindy65 lindy65 added the Tests/Not Needed Does not require new release tests label Jan 10, 2019
@JtheBAB JtheBAB mentioned this pull request Jan 15, 2019
Closed
9 tasks
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jespino jespino jespino approved these changes

@jwilander jwilander jwilander approved these changes

@jasonblais jasonblais jasonblais approved these changes

Assignees
No one assigned
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
No milestone
8 participants
@chetanyakan @jasonblais @cpanato @jwilander @jespino @lindy65 @esethna @amyblais