Fix 1086 add interactivity patterns in SocketModeClient docs#1139
Fix 1086 add interactivity patterns in SocketModeClient docs#1139seratch merged 4 commits intoslackapi:mainfrom ruberVulpes:fix-1086
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1139 +/- ##
=======================================
Coverage 86.61% 86.62%
=======================================
Files 110 110
Lines 10754 10754
=======================================
+ Hits 9315 9316 +1
+ Misses 1439 1438 -1
Continue to review full report at Codecov.
|
| * Go to **Settings** > **Socket Mode**, then turn on **Enable Socket Mode** | ||
| * Go to **Features** > **App Home**, look under **Show Tabs** > **Messages Tab** then turn on **Allow users to send Slash commands and messages from the messages tab** | ||
| * Go to **Features** > **Event Subscriptions**, then turn on **Enable Events** | ||
|
|
There was a problem hiding this comment.
I think this and the change on L:20 would make the generated page look like this
with the current file ./scripts/docs.sh generates the following for me
if it still looks good on your end with the proposed changes I'll commit them, but I wanted to bring it up since it looks like the formatting might be wonky to me
There was a problem hiding this comment.
@ruberVulpes Thanks for clarifying this! Your changes look reasonable here.
| * Go to **Features** > **Event Subscriptions**, then turn on **Enable Events** | ||
|
|
||
| * On the same page expand **Subscribe to bot events** click **Add Bot User Event** and select **message.im** | ||
|
|
docs-src/socket-mode/index.rst
Outdated
| timestamp=req.payload["event"]["ts"], | ||
| ) | ||
| if req.type == "interactive" \ | ||
| and req.payload["type"] == "shortcut": |
There was a problem hiding this comment.
The type can be None
| and req.payload["type"] == "shortcut": | |
| and req.payload.get("type") == "shortcut": |
There was a problem hiding this comment.
Ah, never mind. As long as req.type == "interactive" is checked beforehand, accessing req.payload["type"] should be safe
There was a problem hiding this comment.
oop, I just pushed the changes for .get
I'm fine with leaving it .get incase people decide to later copy/paste from it since it should be safe if other payload types might not have the same guarantee
| * Go to **Settings** > **Socket Mode**, then turn on **Enable Socket Mode** | ||
| * Go to **Features** > **App Home**, look under **Show Tabs** > **Messages Tab** then turn on **Allow users to send Slash commands and messages from the messages tab** | ||
| * Go to **Features** > **Event Subscriptions**, then turn on **Enable Events** | ||
|
|
There was a problem hiding this comment.
@ruberVulpes Thanks for clarifying this! Your changes look reasonable here.
|
Also I was thinking that when App Manifests go GA it might be cool to use those for the app configs. It won't give the users the experience of clicking through the UI but it should be a more concise guide |
|
@ruberVulpes Thanks for working on these improvements! As for the App Manifest updates, yes, we can consider updating the documents when it becomes GA. |
Summary
Fixes #1086
Adds examples for Shortcuts and Modal submissions to the Socket Mode documentation.
I've run the synchronous version of the code and tested. For some reason the async code was failing with
on
I'm not to familiar with Python's aysnc, I also get this error with the version on
mainas well. It could also be an issue with me writing this on Windows.Let me know if you'd like any changes to the non code documentation, I made it a bit verbose so that a first time user should be able to follow the guide but I'm unsure if that's the target audience.
Category (place an
xin each of the[ ])/docs-src(Documents, have you run./scripts/docs.sh?) 👍/docs-src-v2(Documents, have you run./scripts/docs-v2.sh?)/tutorial(PythOnBoardingBot tutorial)tests/integration_tests(Automated tests for this library)Requirements (place an
xin each[ ])python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.shafter making the changes.I've also run
./scripts/docs.shand confirmed the webpage matched what I expected