Fix #1450 slack_file in image block/element #1452
Conversation
seratch
added
enhancement
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1452 +/- ##
==========================================
+ Coverage 85.34% 85.36% +0.01%
==========================================
Files 111 111
Lines 12273 12289 +16
==========================================
+ Hits 10475 10491 +16
Misses 1798 1798 ☔ View full report in Codecov by Sentry. |
| @@ -559,3 +559,25 @@ def to_dict(self) -> Dict[str, Any]: # skipcq: PYL-W0221 | |||
| else: | |||
| json["trigger"] = self._trigger | |||
| return json | |||
|
|
|||
|
|
|||
| class SlackFile(JsonObject): | |||
There was a problem hiding this comment.
Maybe add a docstring linking to https://api.slack.com/reference/block-kit/composition-objects#slack_file here?
There was a problem hiding this comment.
I've added the comment for this class!
| if self._id is not None: | ||
| json["id"] = self._id | ||
| if self._url is not None: | ||
| json["url"] = self._url |
There was a problem hiding this comment.
If a user sets both, then both properties would be set. According to the docs on this object, this would result in the payload being rejected:
This file must be an image and you must provide either the URL or ID. In addition, the user posting these blocks must have access to this file. If both are provided then the payload will be rejected.
There was a problem hiding this comment.
that's true but runtime error with server side error should be okay. i would like to avoid having duplicated logic on the client side for this (we already have a lot in this sdk but maintaining them is a concern I've been thinking about)
| alt_text (required): A plain-text summary of the image. This should not contain any markup. | ||
| image_url: The URL of the image to be displayed. | ||
| slack_file: A Slack image file object that defines the source of the image. |
There was a problem hiding this comment.
Should we state that either image_url or slack_file is required somewhere here?
There was a problem hiding this comment.
yeah, but in this sdk we don't mention such in comments
Summary
This pull request resolves #1450
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.