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

Fix #1450 slack_file in image block/element #1452

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Fix #1450 slack_file in image block/element #1452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
beee68b
Fix #1450 slack_file in image block/element
seratch Jan 26, 2024
1dd159c
Format tests
seratch Jan 26, 2024
42d2bc0
Update slack_sdk/models/blocks/basic_components.py
seratch Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions slack_sdk/models/blocks/basic_components.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,3 +559,32 @@ def to_dict(self) -> Dict[str, Any]: # skipcq: PYL-W0221
else:
json["trigger"] = self._trigger
return json


class SlackFile(JsonObject):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a docstring linking to https://api.slack.com/reference/block-kit/composition-objects#slack_file here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the comment for this class!

attributes = {"id", "url"}

def __init__(
self,
*,
id: Optional[str] = None,
url: Optional[str] = None,
filmaj marked this conversation as resolved.
Show resolved Hide resolved
):
"""An object containing Slack file information to be used in an image block or image element.
https://api.slack.com/reference/block-kit/composition-objects#slack_file

Args:
id: Slack ID of the file.
url: This URL can be the url_private or the permalink of the Slack file.
"""
self._id = id
seratch marked this conversation as resolved.
Show resolved Hide resolved
self._url = url

def to_dict(self) -> Dict[str, Any]: # skipcq: PYL-W0221
self.validate_json()
json = {}
if self._id is not None:
json["id"] = self._id
if self._url is not None:
json["url"] = self._url
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

return json
13 changes: 8 additions & 5 deletions slack_sdk/models/blocks/block_elements.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
JsonValidator,
EnumValidator,
)
from .basic_components import ButtonStyles, Workflow
from .basic_components import ButtonStyles, Workflow, SlackFile
from .basic_components import ConfirmObject
from .basic_components import DispatchActionConfig
from .basic_components import MarkdownTextObject
Expand Down Expand Up @@ -551,13 +551,14 @@ class ImageElement(BlockElement):

@property
def attributes(self) -> Set[str]:
return super().attributes.union({"alt_text", "image_url"})
return super().attributes.union({"alt_text", "image_url", "slack_file"})

def __init__(
self,
*,
image_url: Optional[str] = None,
alt_text: Optional[str] = None,
image_url: Optional[str] = None,
slack_file: Optional[Union[Dict[str, Any], SlackFile]] = None,
**others: dict,
):
"""An element to insert an image - this element can be used in section and
Expand All @@ -566,18 +567,20 @@ def __init__(
https://api.slack.com/reference/block-kit/block-elements#image

Args:
image_url (required): The URL of the image to be displayed.
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we state that either image_url or slack_file is required somewhere here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but in this sdk we don't mention such in comments

"""
super().__init__(type=self.type)
show_unknown_key_warning(self, others)

self.image_url = image_url
self.alt_text = alt_text
self.slack_file = slack_file if slack_file is None or isinstance(slack_file, SlackFile) else SlackFile(**slack_file)

@JsonValidator(f"image_url attribute cannot exceed {image_url_max_length} characters")
def _validate_image_url_length(self) -> bool:
return len(self.image_url) <= self.image_url_max_length
return self.image_url is None or len(self.image_url) <= self.image_url_max_length

@JsonValidator(f"alt_text attribute cannot exceed {alt_text_max_length} characters")
def _validate_alt_text_length(self) -> bool:
Expand Down
18 changes: 12 additions & 6 deletions slack_sdk/models/blocks/blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
JsonObject,
JsonValidator,
)
from .basic_components import MarkdownTextObject
from .basic_components import MarkdownTextObject, SlackFile
from .basic_components import PlainTextObject
from .basic_components import TextObject
from .block_elements import BlockElement, RichTextElement
Expand Down Expand Up @@ -208,7 +208,7 @@ class ImageBlock(Block):

@property
def attributes(self) -> Set[str]:
return super().attributes.union({"alt_text", "image_url", "title"})
return super().attributes.union({"alt_text", "image_url", "title", "slack_file"})

image_url_max_length = 3000
alt_text_max_length = 2000
Expand All @@ -217,8 +217,9 @@ def attributes(self) -> Set[str]:
def __init__(
self,
*,
image_url: str,
alt_text: str,
image_url: Optional[str] = None,
slack_file: Optional[Union[Dict[str, Any], SlackFile]] = None,
title: Optional[Union[str, dict, PlainTextObject]] = None,
block_id: Optional[str] = None,
**others: dict,
Expand All @@ -227,10 +228,11 @@ def __init__(
https://api.slack.com/reference/block-kit/blocks#image

Args:
image_url (required): The URL of the image to be displayed.
Maximum length for this field is 3000 characters.
alt_text (required): A plain-text summary of the image. This should not contain any markup.
Maximum length for this field is 2000 characters.
image_url: The URL of the image to be displayed.
Maximum length for this field is 3000 characters.
slack_file: A Slack image file object that defines the source of the image.
title: An optional title for the image in the form of a text object that can only be of type: plain_text.
Maximum length for the text in this field is 2000 characters.
block_id: A string acting as a unique identifier for a block. If not specified, one will be generated.
Expand All @@ -255,11 +257,15 @@ def __init__(
parsed_title = title
else:
raise SlackObjectFormationError(f"Unsupported type for title in an image block: {type(title)}")
if slack_file is not None:
self.slack_file = (
slack_file if slack_file is None or isinstance(slack_file, SlackFile) else SlackFile(**slack_file)
)
self.title = parsed_title

@JsonValidator(f"image_url attribute cannot exceed {image_url_max_length} characters")
def _validate_image_url_length(self):
return len(self.image_url) <= self.image_url_max_length
return self.image_url is None or len(self.image_url) <= self.image_url_max_length

@JsonValidator(f"alt_text attribute cannot exceed {alt_text_max_length} characters")
def _validate_alt_text_length(self):
Expand Down
47 changes: 41 additions & 6 deletions tests/slack_sdk/models/test_blocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
RichTextPreformattedElement,
RichTextElementParts,
)
from slack_sdk.models.blocks.basic_components import SlackFile
from . import STRING_3001_CHARS


Expand Down Expand Up @@ -151,7 +152,7 @@ def test_json(self):
SectionBlock(text="some text", fields=[f"field{i}" for i in range(5)]).to_dict(),
)

button = LinkButtonElement(text="Click me!", url="http://google.com")
button = LinkButtonElement(text="Click me!", url="https://example.com")
self.assertDictEqual(
{
"type": "section",
Expand Down Expand Up @@ -314,11 +315,11 @@ def test_issue_1369_title_type(self):
def test_json(self):
self.assertDictEqual(
{
"image_url": "http://google.com",
"image_url": "https://example.com",
"alt_text": "not really an image",
"type": "image",
},
ImageBlock(image_url="http://google.com", alt_text="not really an image").to_dict(),
ImageBlock(image_url="https://example.com", alt_text="not really an image").to_dict(),
)

def test_image_url_length(self):
Expand All @@ -327,11 +328,45 @@ def test_image_url_length(self):

def test_alt_text_length(self):
with self.assertRaises(SlackObjectFormationError):
ImageBlock(image_url="http://google.com", alt_text=STRING_3001_CHARS).to_dict()
ImageBlock(image_url="https://example.com", alt_text=STRING_3001_CHARS).to_dict()

def test_title_length(self):
with self.assertRaises(SlackObjectFormationError):
ImageBlock(image_url="http://google.com", alt_text="text", title=STRING_3001_CHARS).to_dict()
ImageBlock(image_url="https://example.com", alt_text="text", title=STRING_3001_CHARS).to_dict()

def test_slack_file(self):
self.assertDictEqual(
{
"slack_file": {"url": "https://example.com"},
"alt_text": "not really an image",
"type": "image",
},
ImageBlock(slack_file=SlackFile(url="https://example.com"), alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"id": "F11111"},
"alt_text": "not really an image",
"type": "image",
},
ImageBlock(slack_file=SlackFile(id="F11111"), alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"url": "https://example.com"},
"alt_text": "not really an image",
"type": "image",
},
ImageBlock(slack_file={"url": "https://example.com"}, alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"id": "F11111"},
"alt_text": "not really an image",
"type": "image",
},
ImageBlock(slack_file={"id": "F11111"}, alt_text="not really an image").to_dict(),
)


# ----------------------------------------------
Expand Down Expand Up @@ -446,7 +481,7 @@ def test_document_2(self):
def test_json(self):
self.elements = [
ButtonElement(text="Click me", action_id="reg_button", value="1"),
LinkButtonElement(text="URL Button", url="http://google.com"),
LinkButtonElement(text="URL Button", url="https://example.com"),
]
self.dict_elements = []
for e in self.elements:
Expand Down
45 changes: 40 additions & 5 deletions tests/slack_sdk/models/test_elements.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
InteractiveElement,
PlainTextObject,
)
from slack_sdk.models.blocks.basic_components import SlackFile
from slack_sdk.models.blocks.block_elements import (
DateTimePickerElement,
EmailInputElement,
Expand Down Expand Up @@ -186,11 +187,11 @@ def test_action_id(self):

class LinkButtonElementTests(unittest.TestCase):
def test_json(self):
button = LinkButtonElement(action_id="test", text="button text", url="http://google.com")
button = LinkButtonElement(action_id="test", text="button text", url="https://example.com")
self.assertDictEqual(
{
"text": {"emoji": True, "text": "button text", "type": "plain_text"},
"url": "http://google.com",
"url": "https://example.com",
"type": "button",
"action_id": button.action_id,
},
Expand Down Expand Up @@ -456,11 +457,11 @@ def test_document(self):
def test_json(self):
self.assertDictEqual(
{
"image_url": "http://google.com",
"image_url": "https://example.com",
"alt_text": "not really an image",
"type": "image",
},
ImageElement(image_url="http://google.com", alt_text="not really an image").to_dict(),
ImageElement(image_url="https://example.com", alt_text="not really an image").to_dict(),
)

def test_image_url_length(self):
Expand All @@ -469,7 +470,41 @@ def test_image_url_length(self):

def test_alt_text_length(self):
with self.assertRaises(SlackObjectFormationError):
ImageElement(image_url="http://google.com", alt_text=STRING_3001_CHARS).to_dict()
ImageElement(image_url="https://example.com", alt_text=STRING_3001_CHARS).to_dict()

def test_slack_file(self):
self.assertDictEqual(
{
"slack_file": {"id": "F11111"},
"alt_text": "not really an image",
"type": "image",
},
ImageElement(slack_file=SlackFile(id="F11111"), alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"url": "https://example.com"},
"alt_text": "not really an image",
"type": "image",
},
ImageElement(slack_file=SlackFile(url="https://example.com"), alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"id": "F11111"},
"alt_text": "not really an image",
"type": "image",
},
ImageElement(slack_file={"id": "F11111"}, alt_text="not really an image").to_dict(),
)
self.assertDictEqual(
{
"slack_file": {"url": "https://example.com"},
"alt_text": "not really an image",
"type": "image",
},
ImageElement(slack_file={"url": "https://example.com"}, alt_text="not really an image").to_dict(),
)


# -------------------------------------------------
Expand Down
Loading