slackapi / node-slack-interactive-messages

Slack Buttons, Menus, and Dialogs made simpler for Node
MIT License
133 stars 41 forks source link

Fix callback handling, add options #28

Closed aoberoi closed 6 years ago

aoberoi commented 6 years ago

Summary

fixes #23 fixes #25

adds two new configuration options for SlackMessageAdapter:

this impacts the table in #27 by making it look like the following:

Type of Request cb(): Message cb(): Promise<Message> respond(m: Message) respond(m: Promise<Message>) Notes
Message Action (Button, Select) incl. embeded in App Unfurl synchronous response if resolved under timeout, synchronous response; else response_url request response_url request (can be used several times) error: axios will likely throw
Dialog Submission synchronous response synchronous response response_url request (can be used several times) - not a replacement, used as follow up error: axios will likely throw A warning is logged if the synchronous response takes longer than the timeout
Menu Options synchronous response synchronous response - - A warning is logged if the synchronous response takes longer than the timeout

TODO:

Requirements (place an x in each [ ])

codecov[bot] commented 6 years ago

Codecov Report

Merging #28 into master will increase coverage by 46.55%. The diff coverage is 92.3%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #28       +/-   ##
===========================================
+ Coverage   32.51%   79.06%   +46.55%     
===========================================
  Files           3        3               
  Lines         163      172        +9     
===========================================
+ Hits           53      136       +83     
+ Misses        110       36       -74
Impacted Files Coverage Δ
src/express-middleware.js 14.63% <0%> (-0.76%) :arrow_down:
src/util.js 100% <100%> (+45.45%) :arrow_up:
src/adapter.js 99.14% <100%> (+62.86%) :arrow_up:

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 a4a1ac1...93558ad. Read the comment docs.

aoberoi commented 6 years ago

in the interest of building new features on top of this branch, and since there aren't yet any reviewers with specific expertise in this module, i'm going to merge. general JS code review still welcome!