[MM-12623] Create CLI command "config reset" #10296

joshuabezaleel · 2019-02-17T03:42:01Z

Summary

Add the CLI command config reset and also it's test

Ticket Link

Fixes #9596

Checklist

Added or updated unit tests (required for all new features)

…to the config_test.go

hanzei

Thanks for the contribution @joshuabezaleel! 👍

These two copy pasted error let the build fail.

cmd/mattermost/commands/config_test.go

joshuabezaleel · 2019-02-18T13:09:10Z

I am sorry that the errors slip through. To be honest I didn't know why the build failed. Changes submitted.
Thank you very much for the sharp review, @hanzei !

test fixed

joshuabezaleel · 2019-02-22T08:35:45Z

Any idea why is this failing? I am really sorry but I still can not seem to understand why 🙁

hanzei · 2019-02-22T11:06:18Z

It was a wanky test, not related to your PR. Green now.

jespino

I really like the PR, needs some changes, but in general looks good :)

cmd/mattermost/commands/config.go

joshuabezaleel · 2019-02-26T13:20:45Z

Hi, @jespino I am sorry if I got a really bad question.
First of all, I tried to change the code based on your review that you can see in my latest commit but it stops working to reset the particular config inputted. Another thing is, now all the Command Line doesn't show any logs (config get, config set, etc.) except the config reset command but with this log as an output.

Do you have any idea why?
Thank you very much. Have a good day.

…rint debug statement

hanzei · 2019-03-04T10:57:11Z

Hey @joshuabezaleel,

You need to merge master into your branch. This should fix your problem. Let me know, if you need further help.

joshuabezaleel · 2019-03-12T01:02:25Z

By the way, thank you very much @hanzei for the resolving recommendation and @jespino for the review. Those are really helpful! 🙂

jespino · 2019-03-14T09:46:19Z

cmd/mattermost/commands/config.go

+		CommandPrettyPrintln("Are you sure you want to reset all the configuration settings?(YES/NO): ")
+		fmt.Scanln(&confirmResetAll)
+		if confirmResetAll == "YES" {
+			currentConfigMap := configToMap(*currentConfig)


This option should be atomic and simpler. The app.UpdateConfig is already storing the configuration in each iteration, so, if something goes wrong in one iteration, you will end up with a partial reset of the configuration.

Here probably you simply need something like app.SaveConfig(defaultConfig, false).

jespino · 2019-03-14T09:47:53Z

cmd/mattermost/commands/config.go

+	}
+
+	for _, arg := range args {
+		f := resetConfigValue(arg, defaultConfig, currentConfig)


Here we have the same problem, you can end up with a partial reset of your config, and actually, with an invalid configuration. Would be great if the changes to the config are done to a temporary config object, and at the end, we validate the new config and save it using the app.SaveConfig function.

lindy65

Thanks @joshuabezaleel !

…config-reset

cmd/mattermost/commands/config_test.go

hanzei · 2019-10-26T19:15:05Z

@joshuabezaleel Would you be open on fixing the issue found by our new linter? Let me know, if you need help with this or if I should take care of this.

…config-reset

hanzei · 2019-11-01T18:30:05Z

Thanks for the contribution @joshuabezaleel! I finished the PR for you. I hope you are fine with this.

* master: (70 commits) Run unused against codebase (mattermost#12968) [MM-12623] Create CLI command "config reset" (mattermost#10296) Migrate tests from store/storetest/status_store.go to use test… (mattermost#12873) Fix golangci-lint target (mattermost#12985) [MM-16437] Plugin crashes the server when calling WriteHeader with an invalid http code (mattermost#11276) MM-18060: Include deleted posts in compliance export. (mattermost#12957) [MM-18331] When patching a bot send websocket notification (mattermost#12373) [MM-19473] Fix data race on user login (mattermost#12870) license, openGraph tests: convert to testify (mattermost#12919) oauth_test: use testify (mattermost#12949) [MM-18830] Unhelpful error message when adding bot to a channel before adding to team (mattermost#12844) emoji_test: update to use testify (mattermost#12932) MM-19310 - Wrong validation message when Bot name ends in a . (mattermost#12905) Migrate tests from store/storetest/oauth_store.go to use testify (mattermost#12875) Include extra metadata when clicking an interactive button (mattermost#12697) MM-19553: Generate valid passwords on bulk import. (mattermost#12871) Convert api4/webhook_test.go t.Fatal calls into require/assert calls (mattermost#12904) Fix golangci-lint target for non GOPATH installations (mattermost#12934) MM-17888 Check plugin Helpers minimum server version comments (mattermost#12663) [MM-18898] Stringify plugin.Log* parameters (mattermost#12700) ...

hanzei · 2019-11-04T09:56:57Z

No problem @joshuabezaleel, this was quickly done. Thanks again for the PR 👍

joshuabezaleel · 2019-11-04T10:00:28Z

Hey @hanzei , I am truly sorry that I've just saw this 🙁 Thank you for taking care of the linting issues for me, that's very kind of you!! 🙂

Nah, I am the one who should be thanking you folks for the opportunity to learn and contribute, and for being very patient with me for this long contribution. I hope you folks are still okay for me taking care of other issues in the future. This was a realy pleasant experience for me. Once again, thank you. Looking forward to contributing more to Mattermost!! 🎉

hanzei · 2019-11-04T10:03:19Z

Thanks for the kind words Joshua. It would be great if you continue contributing to Mattermost.

I'm looking forward to more awesome PR from you. Let me know, if you have any questions about tickets.

Docs for mattermost/mattermost#10296

* Update command-line-tools.rst Docs for mattermost/mattermost#10296 * Update source/administration/command-line-tools.rst Co-Authored-By: Jason Blais <13119842+jasonblais@users.noreply.github.com>

* Update conf.py (#3165) * Update organizing-conversations.rst (#3204) Docs for PR: mattermost/mattermost-webapp#3702 * Update guest-accounts.rst (#3207) * Update guest-accounts.rst docs for mattermost/mattermost#13007 * Update guest-accounts.rst * Update command-line-tools.rst (#3209) * Update command-line-tools.rst Docs for mattermost/mattermost#10296 * Update source/administration/command-line-tools.rst Co-Authored-By: Jason Blais <13119842+jasonblais@users.noreply.github.com> * Update open-source-components.rst (#3214) * Delete email_notification_chart.png * Update config-settings.rst (#3210) * Update config-settings.rst Docs for mattermost/mattermost#11944 * Update config-settings.rst * Update config-settings.rst * Update telemetry for 5.18 (#3232) * Update telemetry.rst * Update telemetry.rst * MM-19844 - Disallow plugin upload w/signature Added some information about why you cannot upload and enable signature simultaneously. * removed extra line * Update config-settings.rst (#3226) * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update config-settings.rst * Update source/administration/config-settings.rst Co-Authored-By: Jesse Hallam <jesse.hallam@gmail.com> * Update source/administration/config-settings.rst Co-Authored-By: Jesse Hallam <jesse.hallam@gmail.com> * Update source/administration/config-settings.rst Co-Authored-By: Jesse Hallam <jesse.hallam@gmail.com> * Update config-settings.rst * Update config-settings.rst * v5.18 docs (ES) (#3238) * Update email notification flow chart * Update signing-in.rst * Update sending-messages.rst * Update mobile-faq.rst * Update mobile-compile-yourself.rst * Delete mobile_push_contents.png * Add files via upload * Update mobile-faq.rst * Update config-settings.rst * Update mobile-appstore-install.rst * Update source/mobile/mobile-faq.rst Co-Authored-By: Amy Blais <amy_blais@hotmail.com> * Update source/mobile/mobile-appstore-install.rst Co-Authored-By: Amy Blais <amy_blais@hotmail.com> * Update source/administration/config-settings.rst Co-Authored-By: Amy Blais <amy_blais@hotmail.com> * Update source/mobile/mobile-compile-yourself.rst Co-Authored-By: Amy Blais <amy_blais@hotmail.com> * Update mobile-faq.rst * Add plugin marketplace telemetry (#3197) * Update data-retention.rst (#3240) * Update data-retention.rst Fixed procedure order and UI terms. * Update data-retention.rst (#3241) Reviewed process/steps and edited. Added some clarification comments for process. * Update data-retention.rst Clarified retention periods/deletion times. * v5.18 Changelog (#3164) * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update source/administration/changelog.md Co-Authored-By: Dennis Kittrell <dennis.kittrell@mattermost.com> * Update changelog.md * Update changelog.md * Update changelog.md * Update changelog.md * Update docs for archived channels (#3177) * Update config setting description for ViewArchivedChannels * Update organizing-conversations.rst * Update important-upgrade-notes.rst (#3180) * Update important-upgrade-notes.rst * Update important-upgrade-notes.rst * Update important-upgrade-notes.rst * Update important-upgrade-notes.rst * Update compliance.rst (#3239) * Update compliance.rst Confirm UI steps against 5.18 UI and fixed minor formatting/typos. * Update source/administration/compliance.rst Co-Authored-By: Amy Blais <amy_blais@hotmail.com> * Update changelog.md * Update changelog.md * Update changelog.md * Update important-upgrade-notes.rst * Update changelog.md * Update important-upgrade-notes.rst * Update changelog.md * Update changelog.md

add functions for config reset command at the config.go and add test …

4701e8a

…to the config_test.go

hanzei requested a review from wiersgallak February 17, 2019 07:08

hanzei added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a developer Hackfest labels Feb 17, 2019

hanzei requested review from jespino and mgdelacroix February 17, 2019 07:09

hanzei previously requested changes Feb 18, 2019

View reviewed changes

cmd/mattermost/commands/config_test.go Outdated Show resolved Hide resolved

cmd/mattermost/commands/config_test.go Outdated Show resolved Hide resolved

changes to config_test error

e1514d4

joshuabezaleel changed the title ~~Create CLI command config reset~~ [MM-12623] Create CLI command "config reset" Feb 22, 2019

jespino requested changes Feb 23, 2019

View reviewed changes

remove the newConfig parameter, delete else statement, and removing p…

8f575b8

…rint debug statement

esethna assigned wiersgallak Feb 26, 2019

hanzei added Hackfest and removed Hackfest labels Mar 4, 2019

lieut-data mentioned this pull request Mar 8, 2019

[MM-13799] Ignoring database timeout during migration database upgrade to version 5.10.0 #10350

Closed

1 task

merge master to branch and changes how persisconfig work to function

6b4777e

hanzei requested review from jespino and wiersgallak and removed request for wiersgallak March 12, 2019 12:19

amyblais removed the Hackfest label Mar 13, 2019

jespino reviewed Mar 14, 2019

View reviewed changes

hanzei added this to the v5.18.0 milestone Oct 25, 2019

lindy65 approved these changes Oct 26, 2019

View reviewed changes

lindy65 added QA/Review Done QA review is completed but other reviews are outstanding (exception to usual process) Tests/Not Needed New release tests not required and removed 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Oct 26, 2019

hanzei added the 4: Reviews Complete All reviewers have approved the pull request label Oct 26, 2019

hanzei self-assigned this Oct 26, 2019

Merge remote-tracking branch 'origin/master' into create-cli-command-…

d96b0f7

…config-reset

golangcibot reviewed Oct 26, 2019

View reviewed changes

cmd/mattermost/commands/config_test.go Outdated Show resolved Hide resolved

cmd/mattermost/commands/config_test.go Outdated Show resolved Hide resolved

cmd/mattermost/commands/config_test.go Outdated Show resolved Hide resolved

hanzei removed their assignment Oct 28, 2019

Merge remote-tracking branch 'origin/master' into create-cli-command-…

9e6e4f2

…config-reset

hanzei self-assigned this Nov 1, 2019

Fix linter issues

b53aa56

hanzei merged commit 63a2870 into mattermost:master Nov 1, 2019

amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels Nov 18, 2019

amyblais assigned wiersgallak and unassigned hanzei Nov 21, 2019

wiersgallak added a commit to mattermost/docs that referenced this pull request Dec 3, 2019

Update command-line-tools.rst

2f7a94b

Docs for mattermost/mattermost#10296

wiersgallak mentioned this pull request Dec 3, 2019

Update command-line-tools.rst mattermost/docs#3209

Merged

wiersgallak added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Dec 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-12623] Create CLI command "config reset" #10296

[MM-12623] Create CLI command "config reset" #10296

joshuabezaleel commented Feb 17, 2019 •

edited

hanzei left a comment

joshuabezaleel commented Feb 18, 2019

joshuabezaleel commented Feb 22, 2019 •

edited

hanzei commented Feb 22, 2019

jespino left a comment

joshuabezaleel commented Feb 26, 2019

hanzei commented Mar 4, 2019

joshuabezaleel commented Mar 12, 2019

jespino Mar 14, 2019

jespino Mar 14, 2019

lindy65 left a comment

hanzei commented Oct 26, 2019 •

edited

hanzei commented Nov 1, 2019

hanzei commented Nov 4, 2019

joshuabezaleel commented Nov 4, 2019

hanzei commented Nov 4, 2019

[MM-12623] Create CLI command "config reset" #10296

[MM-12623] Create CLI command "config reset" #10296

Conversation

joshuabezaleel commented Feb 17, 2019 • edited

Summary

Ticket Link

Checklist

hanzei left a comment

Choose a reason for hiding this comment

joshuabezaleel commented Feb 18, 2019

joshuabezaleel commented Feb 22, 2019 • edited

hanzei commented Feb 22, 2019

jespino left a comment

Choose a reason for hiding this comment

joshuabezaleel commented Feb 26, 2019

hanzei commented Mar 4, 2019

joshuabezaleel commented Mar 12, 2019

jespino Mar 14, 2019

Choose a reason for hiding this comment

jespino Mar 14, 2019

Choose a reason for hiding this comment

lindy65 left a comment

Choose a reason for hiding this comment

hanzei commented Oct 26, 2019 • edited

hanzei commented Nov 1, 2019

hanzei commented Nov 4, 2019

joshuabezaleel commented Nov 4, 2019

hanzei commented Nov 4, 2019

joshuabezaleel commented Feb 17, 2019 •

edited

joshuabezaleel commented Feb 22, 2019 •

edited

hanzei commented Oct 26, 2019 •

edited