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

Add configuration via system console #92

Closed
wants to merge 4 commits into from
Closed

Conversation

jwilander
Copy link

@jwilander jwilander commented Jun 13, 2022

Summary

I wanted to make this plugin configurable via the System Console, since doing it with mmctl was causing me issues and I wanted to update it often. But, because the plugin manifest settings schema only offers strings and other primitive types as options, I needed the configuration for WelcomeMessages to support being either a string or a JSON array. Hence, the custom UnmarshalJSON for Configuration.

It's not an ideal solution but it works decently well. The other method I considered taking was using the custom settings schema type but then I'd have to re-implement the text field component and add a web app component to this plugin, which I didn't want to do. Thoughts?

@jwilander jwilander added the 2: Dev Review Requires review by a core committer label Jun 13, 2022
@jwilander jwilander requested a review from levb June 13, 2022 20:26
@jwilander jwilander requested a review from jfrerich as a code owner June 13, 2022 20:26
@levb levb requested a review from DHaussermann June 21, 2022 11:42
server/manifest.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

Approving with one nit suggestion.

README.md Outdated Show resolved Hide resolved
@hanzei hanzei added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Sep 18, 2022
Co-authored-by: Jason Frerich <[email protected]>
@hanzei
Copy link
Contributor

hanzei commented Sep 1, 2023

@AayushChaudhary0001 Would you be open to reviewing this PR?

@hanzei hanzei requested review from AayushChaudhary0001 and removed request for DHaussermann October 16, 2023 20:20
@jwilander jwilander requested a review from mickmister as a code owner January 4, 2024 12:04
@mickmister
Copy link
Contributor

Note that there is a similar PR being finished up that implements a custom admin component for this Brightscout#15. I think this PR can/should be closed in favor of the custom component implementation. Are you okay with this @jwilander?

@jwilander
Copy link
Author

jwilander commented Jan 10, 2024

Yes, let's close in favour of that.

@jwilander jwilander closed this Jan 10, 2024
@Kshitij-Katiyar
Copy link
Contributor

Opening this PR due to comments #108 (comment) and #108 (comment).

Copy link

@AayushChaudhary0001 AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

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

@Kshitij-Katiyar While testing this PR, it was found that this was not working for MM cloud server. Any new user added to a team was not getting any welcome message nor any action button. Please verify it once.

@Kshitij-Katiyar
Copy link
Contributor

Closing the PR because of the PR #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants