Implement view submission and view closed handling on @slack/interactive-messages

[aoberoi]

Summary

Implements view submission handling as described in #288

Requirements (place an x in each [ ])

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[codecov]

Codecov Report

Merging #904 into master will decrease coverage by 0.46%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
- Coverage   95.63%   95.17%   -0.47%     
==========================================
  Files          12       12              
  Lines         687      725      +38     
  Branches      147      164      +17     
==========================================
+ Hits          657      690      +33     
- Misses         11       13       +2     
- Partials       19       22       +3

Flag	Coverage Δ
#eventsapi	89.61% <ø> (ø)	⬆️
#interactivemessages	95.22% <ø> (-1.12%)	⬇️
#webapi	98.51% <ø> (ø)	⬆️
#webhook	87.5% <ø> (ø)	⬆️

Impacted Files	Coverage Δ
src/adapter.ts	95.52% <0%> (-1.59%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 039e137...5fcba28. Read the comment docs.

[shaydewael]

The types for these are in Bolt if you want them - https://github.com/slackapi/bolt/blob/master/src/types/view/index.ts

[aoberoi]

Great idea!

[aoberoi]

On second thought - the options are to depend on @slack/bolt to get these types, to duplicate those types in @slack/interactive-messages, or to move those types into @slack/types (and then refactor Bolt and this package to grab those types from that package).

I think the ideal solution is the last one, but I don't think its wise to increase scope of this feature to get that done. In the meantime, I think we can proceed with this approach and revisit the refactoring of all payload types into @slack/types at a later time. Many other packages might also benefit from this.

[shaydewael]

These checks seem correct, but this and the ViewClosed constraints are the same. Is there a reason we can't combine them to reduce code duplication?

[aoberoi]

There's a slight difference which is the first conditional within each section. The first checks for view_submission and the second checks for view_closed. But now that you point it out there's likely a way to reduce the code duplication - thanks!

[shaydewael]

Should we allow RegExp for externalId too? Since they are unique on a per-user basis, it would be really hard to setup a handler for them with a type of string.

[aoberoi]

Earlier I was trying not to expand scope so I didn't add RegExp matching to anything new - but now that you point it out that totally makes sense as a use case. It's not too much work to add that in now, so I might as well.

[pitininja]

Hello,
I have been testing this feature on this branch.
When I do a view_submission with debug, the condition in adapter.ts on line 454 is always matched because callback_id is not in payload but in payload.view.

// from matchCallback function
if (isString(constraints.callbackId) && payload.callback_id !== constraints.callbackId) {
    return false;
}

Maybe I am doing something wrong?
Sorry if I should not be posted here 🙏
Thank you for your work

[aoberoi]

@pitininja you're totally right! thanks for catching that.

[aoberoi]

@pitininja fixed the issue you pointed out (and the ones @shanedewael pointed out too).

we're likely releasing this change tomorrow.