Adding /setup to autocomplete (GitHub issue mattermost#892) #898
Conversation
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
There was a problem hiding this comment.
@bobf7 Thanks so much for this improvement 👍
Sorry for the delay on this review. Mostly looks good, just a few non-blocking suggestions.
changing to suggested correct spelling Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
Codecov ReportBase: 31.27% // Head: 31.27% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #898 +/- ##
==========================================
- Coverage 31.27% 31.27% -0.01%
==========================================
Files 49 49
Lines 6017 6018 +1
==========================================
Hits 1882 1882
- Misses 3946 3947 +1
Partials 189 189
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
Many thanks @mickmister ! Your suggestions look great so fixed now ✔️ |
server/command.go
Outdated
| // Help and info | ||
| jira.AddCommand(model.NewAutocompleteData("setup", "", "Start Jira plugin setup flow")) | ||
| jira.AddCommand(model.NewAutocompleteData("info", "", "Display information about the current user and the Jira plug-in")) | ||
| jira.AddCommand(model.NewAutocompleteData("help", "", "Display help for `/jira` command")) |
There was a problem hiding this comment.
Since this new command is an admin command, I think this should instead be placed into the section above called Admin Commands
Make command show for system admins Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
There was a problem hiding this comment.
@bobf7 Thanks for addressing the feedback 👍
I have one last request to remove the call to withFlagInstance for the /setup command. It should be good to go after this change 🚀
server/command.go
Outdated
| func createSetupCommand(optInstance bool) *model.AutocompleteData { | ||
| setup := model.NewAutocompleteData( | ||
| "setup", "", "Start Jira plugin setup flow") | ||
| setup.RoleID = model.SystemAdminRoleId | ||
| withFlagInstance(setup, optInstance, routeAutocompleteInstalledInstanceWithAlias) | ||
| return setup |
There was a problem hiding this comment.
In the case if the /setup command, we actually don't want to include the instance flag. I think this should work along with changing the calling function:
| func createSetupCommand(optInstance bool) *model.AutocompleteData { | |
| setup := model.NewAutocompleteData( | |
| "setup", "", "Start Jira plugin setup flow") | |
| setup.RoleID = model.SystemAdminRoleId | |
| withFlagInstance(setup, optInstance, routeAutocompleteInstalledInstanceWithAlias) | |
| return setup | |
| func createSetupCommand() *model.AutocompleteData { | |
| setup := model.NewAutocompleteData( | |
| "setup", "", "Start Jira plugin setup flow") | |
| setup.RoleID = model.SystemAdminRoleId | |
| return setup |
Summary
Adding /setup to autocomplete
Ticket Link
#892