mattermost / mattermost-plugin-servicenow

ServiceNow plugin for Mattermost
Apache License 2.0
5 stars 5 forks source link

ServiceNow Slash Commands which launch dialogs do not run inside Playbooks #167

Open asatkinson opened 1 year ago

asatkinson commented 1 year ago

If I run the slash command /servicenow create incident in the channel the slash command works fine and shows the dialog.

However if I run the same slash command inside a playbook the dialog does not open.

Dialogs used to open, but doesn't anymore.

The playbooks team says the slash commands with dialogs they have tried work correctly.

I'm wondering if the change in how dialogs get launched might be causing an issue.

I am running ServiceNow plugin 3.2.0, Mattermost 7.7.1 Here is a screenshot of two commands embedded in a playbook. The first one should launch a dialog - but does not The second one only sends content to the channel and works fine.

image

mickmister commented 1 year ago

@asatkinson This is because the first command is implemented as webapp plugin. The playbooks plugin only runs server-side commands, so this ends up being a no-op for webapp plugin commands.

The alternative would be to use interactive dialogs. App framework commands are currently unsupported with Playbooks, but would be supported if webapp plugin ones were also supported. The App framework command parsing happens client-side atm, which is essentially the same issue with the webapp plugins.

mkdbns commented 1 year ago

That is a good summary @mickmister. The team has been looking at some options. This is one we are investigating:

Option A:

Here are others but require changes outside our plugin so not ideal:

Option B. Code changes in Mattermost server - Store the trigger id received in the command arguments without encrypting it so we can match that id at the client side by dispatching a WebSocket event.

Option C. Code changes in the playbook plugin - Update the code of the playbook plugin such that it somehow calls the slashCommandWillBePostedHook before the call of the API.

mickmister commented 1 year ago

@mkdbns I'm not sure if Option A is possible. There is no way for the ServiceNow webapp plugin to hook into the action of clicking that "Run slash command" button in the Playbooks UI. It's sort of a "fire and forget" action.

Using a websocket message as a workaround makes sense to me, though I'm not sure how you would have that work exclusively with the user's current Mattermost tab, since I don't think we will have the chance to generate or store these ids you mention.

I'm not sure if Option B would work either, for the same reason. I could be missing something about the flow of the feature though.

I think Option C is the only way to go. There are likely good reasons why the Playbooks feature works the way it currently works today, so changing it to support webapp plugin commands may not be as trivial as it sounds. I would reach out to them in the Playbooks channel for a deeper discussion on this.

asatkinson commented 1 year ago

@mickmister there is already a discussion in the CE Brightscout channel on this with the playbooks team. @mkdbns is moving back to the approach used to launch the modal in the 2.X stream an option - at least temporarily while this is worked out?

ayusht2810 commented 1 year ago

@mickmister @mkdbns @asatkinson we have shifted the logic to launch modals as it was before to resolve this issue in the PR https://github.com/Brightscout/mattermost-plugin-servicenow/pull/44 for now.

Here is another approach we were thinking to implement. This will resolve the issue of modals opening on multiple clients while running the slash command through ServiceNow, but will open the modal on multiple clients while running the slash command inside the playbook.

Approach:

  1. Call an API when a slash command is posted using slashCommandWillBeHostedHook which will return a unique ID (let's say clientID) in response and execute a function (let's say ExecuteCommandHelper) in a goroutine.
  2. The clientID returned from the API response will be stored in the redux store.
  3. ExecuteCommandHelper function further calls command handlers that dispatch WebSocket event with necessary information and clientID.
  4. Add a condition to not trigger the ExecuteCommand hook of the server to avoid dispatching of WebSocket event twice.
  5. WebSocket handler on webapp dispatch an action to open the modal after comparing the clientID received from the WebSocket event (if received) with clientID present in the store.

In case when playbook plugin executes a slash command:

  1. It calls execute function which further calls command handlers that dispatch WebSocket events with no clientID.
  2. As no clientID is received from the WebSocket event we will directly open the corresponding slash's command modal (This will lead to multiple modal issues on multiple clients)

The flow diagram for the above approach: image

Please let me know your thoughts on this.

mickmister commented 1 year ago

@ayusht2810 To make the diagram simpler for the ServiceNow command case, we can use slashCommandWillBePostedHook to short-circuit the frontend's communication to the server by returning an empty object like here. We can then just open the modal like normal, so we won't need to do any websocket message in that case. Does this make sense @ayusht2810?

So it seems the conclusion of the Playbooks command case, is that it will open in all clients. Is this correct?

ayusht2810 commented 1 year ago

@mickmister Yes, the above approach makes sense. But we still prefer to keep the slash command logic on the server side. By moving the logic to the webapp side, it will increase the code length and complexity as we need to make some extra API calls to check if the user is connected or not to ServiceNow and is authorized to configure subscriptions to avoid opening of modal and to show certain errors. Also, we need to make websocket events to resolve the Playbooks issue, so we can reuse them for the ServiceNow as well.

So it seems the conclusion of the Playbooks command case, is that it will open in all clients. Is this correct? Yes, it will open modals in all clients.