Skip to content

Fix #488 app.message listeners do not catch messages with subtype: thread_broadcast - #489

Merged
seratch merged 3 commits into
slackapi:mainfrom
seratch:issue-488-broadcast
Oct 13, 2021
Merged

seratch merged 3 commits into
slackapi:mainfrom
seratch:issue-488-broadcast

Conversation

@seratch

@seratch seratch commented Oct 8, 2021

Copy link
Copy Markdown
Contributor

This pull request fixes #488. Refer to the issue for details.

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

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • 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.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@seratch seratch added bug Something isn't working area:async area:sync labels Oct 8, 2021
@seratch seratch added this to the 1.9.3 milestone Oct 8, 2021
@seratch seratch self-assigned this Oct 8, 2021
@codecov

codecov Bot commented Oct 8, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #489 (98fb478) into main (7b692e8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #489   +/-   ##
=======================================
  Coverage   91.43%   91.43%           
=======================================
  Files         167      167           
  Lines        5533     5533           
=======================================
  Hits         5059     5059           
  Misses        474      474           
Impacted Files Coverage Δ
slack_bolt/app/app.py 87.64% <100.00%> (ø)
slack_bolt/app/async_app.py 94.16% <100.00%> (ø)

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 7b692e8...98fb478. Read the comment docs.

Comment thread slack_bolt/app/app.py
# As of Jan 2021, most bot messages no longer have the subtype bot_message.
# By contrast, messages posted using classic app's bot token still have the subtype.
constraints = {"type": "message", "subtype": (None, "bot_message")}
constraints = {

@eddyg eddyg Oct 8, 2021 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be opposed to adding a comment here (and in async_app.py) that says something like:

# None processes messages without a subtype. thread_broadcast is for a
# message sent in a thread with "Also send to #channel" checked.

to add a bit of clarity as to why these subtypes need to be explicitly "constrained"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddyg Thanks for the great feedback (as always!) - I just added more comments for these constraints.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new comments are excellent. 🎉 I remember wondering about this exact section of code when I was learning Bolt. So thank you!

Comment thread slack_bolt/app/async_app.py Outdated
@seratch
seratch merged commit d2d9ffd into slackapi:main Oct 13, 2021
@seratch
seratch deleted the issue-488-broadcast branch October 13, 2021 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:async area:sync bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app.message listeners do not catch messages with subtype: thread_broadcast

3 participants