Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments #394

seratch · 2021-07-07T08:14:40Z

This pull request fixes #370. The essential change in this pull request is to add next_ to kwargs_injection names. With this fix, developers can use next_ in addition to next in the middleware method arguments.

@app.middleware
def just_call_next(next_):
    next_()

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.

…ware arguments

seratch

comments for reviewers

seratch · 2021-07-07T08:15:25Z

tests/scenario_tests/test_middleware.py

@@ -87,6 +87,53 @@ def test_next_call(self):
        assert response.body == "acknowledged!"
        assert_auth_test_count(self, 1)

+    def test_decorator_next_call(self):


Checking these test patterns should be helpful for knowing the purpose of the changes.

seratch · 2021-07-07T08:16:23Z

slack_bolt/listener/listener.py

@@ -45,10 +45,10 @@ def run_middleware(
        for m in self.middleware:
            middleware_state = {"next_called": False}

-            def next():
+            def next_():


This is not an essential part of this PR. It's just a minor refactoring

seratch · 2021-07-07T08:16:52Z

slack_bolt/kwargs_injection/utils.py

@@ -52,6 +52,7 @@ def build_required_kwargs(
        "respond": request.context.respond,
        # middleware
        "next": next_func,
+        "next_": next_func,  # for the middleware using Python's built-in `next()` function


Adding this enables bolt-python to resolve the alias name as well.

codecov · 2021-07-07T08:29:25Z

Codecov Report

Merging #394 (4a4105d) into main (c61206d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #394   +/-   ##
=======================================
  Coverage   91.60%   91.61%           
=======================================
  Files         167      167           
  Lines        5384     5388    +4     
=======================================
+ Hits         4932     4936    +4     
  Misses        452      452

Impacted Files	Coverage Δ
slack_bolt/kwargs_injection/async_utils.py	`85.71% <ø> (ø)`
slack_bolt/kwargs_injection/utils.py	`85.71% <ø> (ø)`
slack_bolt/logger/messages.py	`90.62% <ø> (ø)`
slack_bolt/middleware/async_custom_middleware.py	`96.15% <ø> (ø)`
slack_bolt/middleware/async_middleware.py	`90.90% <ø> (ø)`
...e/authorization/async_multi_teams_authorization.py	`91.17% <ø> (ø)`
...e/authorization/async_single_team_authorization.py	`93.54% <ø> (ø)`
...dleware/authorization/single_team_authorization.py	`93.33% <ø> (ø)`
slack_bolt/middleware/custom_middleware.py	`100.00% <ø> (ø)`
...listener_matches/async_message_listener_matches.py	`100.00% <ø> (ø)`
... and 11 more

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 c61206d...4a4105d. Read the comment docs.

mwbrooks

Looks good to me! ✅

Just out of curiosity, is next_ a convention in Python? Rather than say nextMiddleware.

seratch · 2021-07-07T21:41:16Z

Just out of curiosity, is next_ a convention in Python? Rather than say nextMiddleware.

I think next_middleware is also fine but it may be a bit long (in comparison with other middleware args). As we use for syntax (for element in seq:) in most cases, it's rare to directly call next with an iterator. Having a kind of workaround name like this should make sense.

Fix slackapi#370 by adding an alias of next arg (next_) in middle…

4a4105d

…ware arguments

seratch added area:async improvement area:sync semver:minor labels Jul 7, 2021

seratch added this to the 1.7.0 milestone Jul 7, 2021

seratch commented Jul 7, 2021

View reviewed changes

seratch requested review from misscoded, mwbrooks, srajiang and stevengill July 7, 2021 09:59

mwbrooks approved these changes Jul 7, 2021

View reviewed changes

seratch merged commit dce45c5 into slackapi:main Jul 9, 2021

seratch deleted the issue-370-next_ branch July 9, 2021 04:09

seratch mentioned this pull request Nov 4, 2022

Fix #754 by adding the async version of Tornado adapter #758

Merged

8 tasks

seratch mentioned this pull request Feb 22, 2024

Rename logger to logger_ in slack_bolt.kwargs_injection.args.Args #1032

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments #394

Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments #394

seratch commented Jul 7, 2021

seratch left a comment

seratch Jul 7, 2021

seratch Jul 7, 2021

seratch Jul 7, 2021

codecov bot commented Jul 7, 2021

mwbrooks left a comment

seratch commented Jul 7, 2021

Fix #370 by adding an alias of next arg (next_) in middleware arguments #394

Fix #370 by adding an alias of next arg (next_) in middleware arguments #394

Conversation

seratch commented Jul 7, 2021

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

Requirements (place an x in each [ ])

seratch left a comment

Choose a reason for hiding this comment

seratch Jul 7, 2021

Choose a reason for hiding this comment

seratch Jul 7, 2021

Choose a reason for hiding this comment

seratch Jul 7, 2021

Choose a reason for hiding this comment

codecov bot commented Jul 7, 2021

Codecov Report

mwbrooks left a comment

Choose a reason for hiding this comment

seratch commented Jul 7, 2021

Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments #394

Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments #394

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

Requirements (place an `x` in each `[ ]`)