Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #683 IgnoringSelfEvents middleware does not filter out Message sent via response_url #684

Merged
merged 4 commits into from
Jul 21, 2022
Merged

Fix #683 IgnoringSelfEvents middleware does not filter out Message sent via response_url #684

merged 4 commits into from
Jul 21, 2022

Conversation

seratch
Copy link
Member

@seratch seratch commented Jul 20, 2022 •
edited
Loading

This pull request resolves #683

I've confirmed that the same issue does not exist in bolt-js and bolt-java.

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 Jul 20, 2022
@seratch seratch added this to the 1.14.2 milestone Jul 20, 2022
@seratch seratch self-assigned this Jul 20, 2022
@seratch seratch mentioned this pull request Jul 20, 2022
Closed
@seratch
Copy link
Member Author

seratch commented Jul 20, 2022

Need to check the failing tests.

FAILED tests/scenario_tests/test_events_org_apps.py::TestEventsOrgApps::test_team_access_granted - assert False is True
FAILED tests/scenario_tests/test_message_bot.py::TestBotMessage::test_message_handler - assert 2 == 3
FAILED tests/scenario_tests/test_workflow_steps.py::TestWorkflowSteps::test_execute - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests/test_workflow_steps.py::TestWorkflowSteps::test_execute_process_before_response - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests/test_workflow_steps_decorator_simple.py::TestWorkflowStepsDecorator::test_execute - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests/test_workflow_steps_decorator_with_args.py::TestWorkflowStepsDecorator::test_execute - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests_async/test_events_org_apps.py::TestAsyncOrgApps::test_team_access_granted - assert False is True
FAILED tests/scenario_tests_async/test_message_bot.py::TestAsyncMessage::test_string_keyword - assert 2 == 3
FAILED tests/scenario_tests_async/test_workflow_steps.py::TestAsyncWorkflowSteps::test_execute - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests_async/test_workflow_steps.py::TestAsyncWorkflowSteps::test_execute_process_before_response - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests_async/test_workflow_steps_decorator_simple.py::TestAsyncWorkflowStepsDecorator::test_execute - KeyError: '/workflows.stepCompleted'
FAILED tests/scenario_tests_async/test_workflow_steps_decorator_with_args.py::TestAsyncWorkflowStepsDecorator::test_execute - KeyError: '/workflows.stepCompl...

@seratch
Copy link
Member Author

seratch commented Jul 20, 2022

The first commit was not a right solution and the failed tests successfully detected the problem. The latest commit should work for the bot_message events and the rest.

@codecov
Copy link

codecov bot commented Jul 20, 2022 •
edited
Loading

Codecov Report

Merging #684 (5b33e67) into main (398c900) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #684   +/-   ##
=======================================
  Coverage   92.05%   92.05%           
=======================================
  Files         172      172           
  Lines        5865     5867    +2     
=======================================
+ Hits         5399     5401    +2     
  Misses        466      466
Impacted Files Coverage Δ
...ignoring_self_events/async_ignoring_self_events.py 100.00% <100.00%> (ø)
...eware/ignoring_self_events/ignoring_self_events.py 100.00% <100.00%> (ø)

@seratch seratch merged commit e8136c0 into slackapi:main Jul 21, 2022
@seratch seratch deleted the issue-683-ignore-self branch July 21, 2022 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@seratch seratch

Labels
area:async area:sync bug Something isn't working
Projects
None yet
Milestone
1.14.2
Development

Successfully merging this pull request may close these issues.

IgnoringSelfEvents middleware does not filter out Message sent via response_url
1 participant
@seratch