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 user settings registry #172

Merged
merged 5 commits into from
Dec 19, 2023
Merged

Add user settings registry #172

merged 5 commits into from
Dec 19, 2023

Conversation

larkox
Copy link
Contributor

@larkox larkox commented Dec 4, 2023

Summary

Add user settings functionality to the demo plugin

Ticket Link

https://mattermost.atlassian.net/browse/MM-55755

Related to:

mattermost/mattermost#25517

@larkox larkox added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Dec 4, 2023
@larkox larkox requested a review from marianunez December 4, 2023 13:57
@larkox larkox requested a review from hanzei as a code owner December 4, 2023 13:57
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I wasn't able to build the webapp part of the plugin, so I took the liberty to bump the dependencies.

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.

Nice one 👍

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.

Can we add the documentation in the webapp Readme of how this new registry would look like? here -> https://github.com/mattermost/mattermost-plugin-demo/tree/master/webapp#main-menu-action

@larkox larkox requested a review from marianunez December 5, 2023 09:45
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.

@larkox
Copy link
Contributor Author

larkox commented Dec 13, 2023

@marianunez Not really. The code checks whether the registry exist, and use it if it exist, and doesn't use it if not. Therefore... the plugin can work with previous versions (it won't show the settings, thought).

@larkox larkox removed the 2: Dev Review Requires review by a core committer label Dec 13, 2023
Copy link

@yasserfaraazkhan yasserfaraazkhan left a comment

Choose a reason for hiding this comment

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

able to save options
image

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d6a03f3) 3.10% compared to head (17e8931) 3.10%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #172   +/-   ##
======================================
  Coverage    3.10%   3.10%           
======================================
  Files          15      15           
  Lines        1869    1869           
======================================
  Hits           58      58           
  Misses       1809    1809           
  Partials        2       2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Dec 18, 2023
@hanzei hanzei added this to the v0.10.0 milestone Dec 18, 2023
@larkox
Copy link
Contributor Author

larkox commented Dec 19, 2023

/update-branch

@larkox larkox merged commit c2a7d9d into master Dec 19, 2023
4 checks passed
@larkox larkox deleted the addUserSettings branch December 19, 2023 12:41
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants