-
Notifications
You must be signed in to change notification settings - Fork 248
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.
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
Fix #664 Django example installation store improvement #665
Conversation
Codecov Report
@@ Coverage Diff @@
## main #665 +/- ##
=======================================
Coverage 92.05% 92.05%
=======================================
Files 172 172
Lines 5864 5864
=======================================
Hits 5398 5398
Misses 466 466 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I added one suggestion to a comment in the example to help potential future maintainers understand the workaround introduced here.
@@ -145,6 +139,24 @@ def find_installation( | |||
|
|||
if len(rows) > 0: | |||
i = rows[0] | |||
if user_id is not None: | |||
# Fetch the latest bot token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we add a bit more context to this comment to help remind future maintainers about this workaround.
# Fetch the latest bot token | |
# Above, when fetching applicable installations, we retrieved relevant installations but filtered on matching user ID | |
# In the case that this app uses both bot and user scopes, we also now fetch installations that match only on team/enterprise ID and contain a bot token | |
# If any such results are retrieved, we merged them into the user-id-filtered results. | |
# This is a fix for the situation described in https://github.com/slackapi/bolt-python/issues/664 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR fixes #664 for me
This pull request resolves #664 by improving the installation store in Django example app.
Category (place an
x
in each of the[ ]
)slack_bolt.App
and/or its core componentsslack_bolt.async_app.AsyncApp
and/or its core componentsslack_bolt.adapter
/docs
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.
./scripts/install_all_and_run_tests.sh
after making the changes.