Node-slack-sdk: View Submission support for @slack/interactive-messages

[aoberoi]

Description

Now that view submissions are available in the Slack Platform, let's build that feature into the @slack/interactive-messages package.

The API

slackInteractions.viewSubmission('my_callback_id', (payload) => {
  // ...
});

// using constraints other than callback_id
slackInteractions.viewSubmission({ externalId: 'other_id' }, (payload) => {
  // ...
});

// handling view_closed events
slackInteractions.viewClosed('my_callback_id', (payload) => {
});

The possible constraints are:

• callbackId (string or RegExp) - default: undefined (matches all)
• externalId (string) - default: undefined (matches all)
• viewId (string) - default undefined (matches all)

By using separate methods (.viewSubmission() and .viewClosed()) instead of adding more functionality to .action(), this solution allows handlers to be registered with a simple string instead of a constraints object more often. It also allows the submission handlers, which have meaningful return values, to be separated from closed handlers, which don't. This makes simple use cases look simple.

We're not interested in re-routing how dialog submissions are handled at the time, as that would be a semver major change. We should revisit this when we are more likely to introduce a breaking change (early 2020) or if there's substantial user confusion evident after the release.

Requirements (place an x in each of the [ ])

• [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
• [x] I've read and agree to the Code of Conduct.
• [x] I've searched for any related issues and avoided creating a duplicate issue.

[Startouf]

Right now submission can be confusing, as there are both dialog_submission and view_submission that have completely different payloads (and dialog_submission has a response_url). I understand dialog are being deprecated in favor of block kit, but who knows for how long... but I agree having a clear method exposed by the adapter would be nice.

Although this may be a bit off topic, but when looking at the inside of this lib, the adapter#dispatch code will just become messier anyways with the addition of extra cases. The right way to go about it would be to have some sort of strategy/factory pattern to have a clear mapping of which slack action type requires which type of response (ie just like what you have in bolt)

[aoberoi]

@Startouf are you opposed to naming the method .submission(...) because of the ambiguity between dialog submission and view submission? If so, do you have a suggestion for an alternative? One thought: .viewSubmission(...) and .viewClosed(...). I'll admit, I don't personally think that's an improvement, but I'm open to hearing what others think.

I wouldn't say that dialogs are deprecated, but I do think that modals can cover all of the use cases of dialogs (and more). We call this out-moded instead of deprecated. This package should continue to support both and allow developers to choose which they prefer.

PS. Regarding your off-topic topic (😄), I think that in the long term the @slack/interactive-messages package will also become out-moded as compared to Bolt. But we aren't announcing a deprecation for it right now. I actually can't think of a good reason to use this package over Bolt besides the fact that its a little more clear how you might mount the adapter it into an existing HTTP server. That will likely be addressed pretty soon with Bolt.

[Startouf]

If I remember well, the payload from dialogs and from a modal submission is different enough that it would make a single method .submission harder to maintain. We could expose a single submission which under the hood would call . viewSubmission or .dialogSubmission to better adapt the interface though.

I just think it would be better to just stick to what user will see on the slack documentation. At first I wasn't even aware there were both dialog and modals, and it's I think easy to mix both and get confused because things are not working. So it could avoid confusion to expose both viewSubmission and dialogSubmission() so the reader understands both exist and are meant to be used differently.

For instance, the bolt app decided to put all event handlers under /slack/events for "better simplicty", while the slack documentation said otherwise (slack/actions,options,...), and it got me stuck for quite some time.

I think the closer we stay to the slack REST api, the better.

[aoberoi]

I agree that the closer we stick to the platform's documentation at api.slack.com, the better the experience will be for people using this package.

It seems like .viewSubmission(...) and .viewClosed(...) are preferable to .submission(...) and .closed(...). The documentation describes modals as submittable and dismissable, but the actual payloads describe a 'view_submission' and 'view_closed'. I believe the payloads represent a more strong source of truth, since they cannot be changed without breaking the API in a backwards incompatible manner. However, I still think these names are clumsy for someone who is simply looking for how to use Modals (its not 100% clear that a Modal submission is the same thing as a View submission by reading the most common docs). Should we also consider .modalSubmission(...) and .modalClosed(...)?

Speaking of backwards compatibility, I don't think we can touch how dialog submissions are routed until we're willing to make a major version revision. I'd rather get a release out that supports view submission sooner than to necessitate a major version release. In order to do the major version release, we'd need to be able to support both the current major version and the next one at least until April 2020 (according to our support schedule), and that is a long time to carry that burden as compared to the value we would get out of it.

What this means is we're looking to add a couple new methods that only deal with view/modal submission. I'll update the top comment to reflect the latest on this topic.

[seratch]

@slack/interactive-messages package's APIs are similar to Bolt's one (e.g., action, options). So .view(...) looks another possible option and it may be better in terms of consistency among libraries under slackapi GitHub org.

I also feel comfortable with viewSubmission and modalSubmission. But if we go with modalSubmission and view will have another submittable type apart from modal in the future, we may need to rename it then.

[aoberoi]

@seratch i agree that modalSubmission has a drawback of potentially needing to be renamed if some other kind of view gains the ability to be submitted.

if we use .view(...) it would be slightly easier to migrate from @slack/interactive-messages to @slack/bolt - but the handlers still behave completely differently (return value is not significant in Bolt, and it is in this package). on the other hand, if we separate the methods, understanding and documenting the handlers becomes slightly easier. handlers attached with the viewSubmission() method have a meaningful return value, while handlers attached with viewClosed() don't have a meaningful return value. today, the .action() method is harder to document (it requires a matrix of possibilities) because the return value does not matter with block actions.

I think we have consensus on .viewSubmission() and .viewClosed() - it has the least number of downsides/objections in this discussion.

I'll update the top-level comment with this decision, and transition this issue to an enhancement.

[enochcheung]

Since this issue is closed, is there a new issue tracking the progress of the implementation of .viewSubmission() API?