mattermost-plugin-jira icon indicating copy to clipboard operation
mattermost-plugin-jira copied to clipboard

Remove /jira settings command

Open ayusht2810 opened this issue 1 year ago • 7 comments

Summary

  • Removed /jira settings from slash commands as the actual command for settings is /jira instance settings [setting] [value] as stated in the help command

Ticket Link

Fixes #1037

ayusht2810 avatar Apr 30 '24 06:04 ayusht2810

@ayusht2810 I do not think we need to remove the command here. Can you revert this change and add the command to autocomplete cc: @Kshitij-Katiyar

raghavaggarwal2308 avatar May 06 '24 07:05 raghavaggarwal2308

@mickmister Regarding the issue https://github.com/mattermost/mattermost-plugin-jira/issues/1037, the command /jira settings is used when we only have a single instance present and the command is used for that single instance (this is something like an enhancement in which we don't need to write the whole instance). The command is missing from the help command and is also not present in the readme as well. So, should we remove the command, or should we keep everything as it is, considering it is an enhancement only and not a proper feature? Also, should we add other missing commands to the autocomplete (like, connect, disconnect)?

ayusht2810 avatar May 07 '24 11:05 ayusht2810

Regarding the issue https://github.com/mattermost/mattermost-plugin-jira/issues/1037, the command /jira settings is used when we only have a single instance present

@ayusht2810 In almost all cases, there will be only one Jira instance installed, so I think we should cater to that scenario. I think a settings command is expected for the plugins to implement if there are any user settings. I think we should also add autocomplete for connect and disconnect as you mention. Maybe we can conditionally show these based on if there are multiple Jira instances set up?


Thinking about it some more, if a command doesn't work correctly when there are multiple Jira instances installed, we can just return an error when someone tries to use it on a server with more than one Jira instance installed. Something like:

Please run this command instead: `/jira instance [jira url] settings`

If the given command is compatible with multiple instances (I think it works correctly for connect and disconnect), then I think we should just add them to autocomplete. If settings is not compatible with multiple instances, we can just return an error when the user tries to run the command. What do you think?

mickmister avatar May 07 '24 19:05 mickmister

@mickmister I checked the current flow for the code. If we have a single instance present, then the slash commands /jira connect, /jira disconnect and /jira settings work fine for that single instance (no autocomplete). But where there are multiple instances present, the user is provided with a modal in both the /jira connect and /jira disconnect commands (still no auto complete): image But for the /jira settings command, we simply get help command in the response.

I don't think we need to provide auto complete for the above commands. We can certainly add a modal for the /jira settings command similarly to connect and disconnect commands. What are your thoughts on this? Please let me know if we should still add auto complete for the commands and return responses on the basis of the number of instances.

ayusht2810 avatar May 08 '24 10:05 ayusht2810

I don't think we need to provide auto complete for the above commands

I don't understand this though. Why not provide autocomplete for /jira connect and /jira disconnect? It's always a bit weird on a customer call when they run /jira connect, but it doesn't show up in the autocomplete. Same with /jira create (though that doesn't work on mobile in any case. We should cater to desktop though as far as autocomplete in this case).

I think /jira settings is the outlier here, and I'm thinking we should just return an error message when the user runs it in an invalid context, which in this case is when there are multiple Jira instances connected, and just suggest to run the full multi-instance version of the command. What's the drawback of including /jira connect etc. in the autocomplete?

mickmister avatar May 08 '24 15:05 mickmister

@mickmister I am fine with adding /jira connect and /jira disconnect commands in the auto-complete. It was missing from documentation so I thought it was incorrect for those commands to be added to auto-complete. But I think what you said is correct, and we can add them to the help command as well. The changes for the /jira settings command also look good to me. I will start working on them.

ayusht2810 avatar May 09 '24 08:05 ayusht2810

@mickmister updated the commands and added a demo video screen-capture (6).webm

Please let me know if anything else needs to be updated here.

ayusht2810 avatar May 09 '24 16:05 ayusht2810

@mickmister Added new test cases. Please re-review

ayusht2810 avatar Jul 02 '24 05:07 ayusht2810