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.

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

Add datetimepicker, url, email, number block elements #1288

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions slack/web/classes/elements.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from slack_sdk.models.blocks import ConversationMultiSelectElement # noqa
from slack_sdk.models.blocks import ConversationSelectElement # noqa
from slack_sdk.models.blocks import DatePickerElement # noqa
from slack_sdk.models.blocks import DateTimePickerElement # noqa
from slack_sdk.models.blocks import ExternalDataMultiSelectElement # noqa
from slack_sdk.models.blocks import ExternalDataSelectElement # noqa
from slack_sdk.models.blocks import ImageElement # noqa
Expand All @@ -15,6 +16,9 @@
from slack_sdk.models.blocks import LinkButtonElement # noqa
from slack_sdk.models.blocks import OverflowMenuElement # noqa
from slack_sdk.models.blocks import PlainTextInputElement # noqa
from slack_sdk.models.blocks import EmailInputElement # noqa
from slack_sdk.models.blocks import UrlInputElement # noqa
from slack_sdk.models.blocks import NumberInputElement # noqa
from slack_sdk.models.blocks import RadioButtonsElement # noqa
from slack_sdk.models.blocks import SelectElement # noqa
from slack_sdk.models.blocks import StaticMultiSelectElement # noqa
Expand Down
8 changes: 8 additions & 0 deletions slack_sdk/models/blocks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .block_elements import ConversationSelectElement
from .block_elements import DatePickerElement
from .block_elements import TimePickerElement
from .block_elements import DateTimePickerElement
from .block_elements import ExternalDataMultiSelectElement
from .block_elements import ExternalDataSelectElement
from .block_elements import ImageElement
Expand All @@ -32,6 +33,9 @@
from .block_elements import LinkButtonElement
from .block_elements import OverflowMenuElement
from .block_elements import PlainTextInputElement
from .block_elements import EmailInputElement
from .block_elements import UrlInputElement
from .block_elements import NumberInputElement
from .block_elements import RadioButtonsElement
from .block_elements import SelectElement
from .block_elements import StaticMultiSelectElement
Expand Down Expand Up @@ -69,6 +73,7 @@
"ConversationSelectElement",
"DatePickerElement",
"TimePickerElement",
"DateTimePickerElement",
"ExternalDataMultiSelectElement",
"ExternalDataSelectElement",
"ImageElement",
Expand All @@ -77,6 +82,9 @@
"LinkButtonElement",
"OverflowMenuElement",
"PlainTextInputElement",
"EmailInputElement",
"UrlInputElement",
"NumberInputElement",
"RadioButtonsElement",
"SelectElement",
"StaticMultiSelectElement",
Expand Down
298 changes: 251 additions & 47 deletions slack_sdk/models/blocks/block_elements.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import re
import warnings
from abc import ABCMeta
from typing import List, Optional, Set, Union, Sequence
from typing import Iterator, List, Optional, Set, Type, Union, Sequence

from slack_sdk.models import show_unknown_key_warning
from slack_sdk.models.basic_objects import (
Expand Down Expand Up @@ -64,64 +64,31 @@ def parse(cls, block_element: Union[dict, "BlockElement"]) -> Optional[Union["Bl
if "type" in block_element:
d = copy.copy(block_element)
t = d.pop("type")
for subclass in cls._get_sub_block_elements():
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

if t == subclass.type: # type: ignore
return subclass(**d)
if t == PlainTextObject.type: # skipcq: PYL-R1705
return PlainTextObject(**d)
elif t == MarkdownTextObject.type:
return MarkdownTextObject(**d)
elif t == ImageElement.type:
return ImageElement(**d)
elif t == ButtonElement.type:
return ButtonElement(**d)
elif t == StaticSelectElement.type:
return StaticSelectElement(**d)
elif t == StaticMultiSelectElement.type:
return StaticMultiSelectElement(**d)
elif t == ExternalDataSelectElement.type:
return ExternalDataSelectElement(**d)
elif t == ExternalDataMultiSelectElement.type:
return ExternalDataMultiSelectElement(**d)
elif t == UserSelectElement.type:
return UserSelectElement(**d)
elif t == UserMultiSelectElement.type:
return UserMultiSelectElement(**d)
elif t == ConversationSelectElement.type:
return ConversationSelectElement(**d)
elif t == ConversationMultiSelectElement.type:
return ConversationMultiSelectElement(**d)
elif t == ChannelSelectElement.type:
return ChannelSelectElement(**d)
elif t == ChannelMultiSelectElement.type:
return ChannelMultiSelectElement(**d)
elif t == PlainTextInputElement.type:
return PlainTextInputElement(**d)
elif t == RadioButtonsElement.type:
return RadioButtonsElement(**d)
elif t == CheckboxesElement.type:
return CheckboxesElement(**d)
elif t == OverflowMenuElement.type:
return OverflowMenuElement(**d)
elif t == DatePickerElement.type:
return DatePickerElement(**d)
elif t == TimePickerElement.type:
return TimePickerElement(**d)
else:
cls.logger.warning(f"Unknown element detected and skipped ({block_element})")
return None
else:
cls.logger.warning(f"Unknown element detected and skipped ({block_element})")
return None
elif isinstance(block_element, (TextObject, BlockElement)):
return block_element
else:
cls.logger.warning(f"Unknown element detected and skipped ({block_element})")
return None
cls.logger.warning(f"Unknown element detected and skipped ({block_element})")
return None

@classmethod
def parse_all(
cls, block_elements: Sequence[Union[dict, "BlockElement", TextObject]]
) -> List[Union["BlockElement", TextObject]]:
return [cls.parse(e) for e in block_elements or []]

@classmethod
def _get_sub_block_elements(cls: Type["BlockElement"]) -> Iterator[Type["BlockElement"]]:
Copy link
Member

Choose a reason for hiding this comment

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

Nice refactoring. If we observe slower runtime performance, we may consider revert this (but probably no)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was also concerned about the runtime performance of this, so I ran some tests,

  • best case is about the same as before since this is using a generator
  • worst case when all the subclasses must be found, is less then 0.1 ms slower on my local

But I agree if this does not scale properly and causes issues we can remove it

for subclass in cls.__subclasses__():
if hasattr(subclass, "type"):
yield subclass
yield from subclass._get_sub_block_elements()


# -------------------------------------------------
# Interactive Block Elements
Expand Down Expand Up @@ -514,6 +481,63 @@ def _validate_initial_time_valid(self) -> bool:
return self.initial_time is None or re.match(r"([0-1][0-9]|2[0-3]):([0-5][0-9])", self.initial_time) is not None


# -------------------------------------------------
# DateTimePicker
# -------------------------------------------------


class DateTimePickerElement(InputInteractiveElement):
type = "datetimepicker"

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

def __init__(
self,
*,
action_id: Optional[str] = None,
initial_date_time: Optional[int] = None,
confirm: Optional[Union[dict, ConfirmObject]] = None,
focus_on_load: Optional[bool] = None,
**others: dict,
):
"""
An element that allows the selection of a time of day formatted as a UNIX timestamp.
On desktop clients, this time picker will take the form of a dropdown list and the
date picker will take the form of a dropdown calendar. Both options will have free-text
entry for precise choices. On mobile clients, the time picker and date
picker will use native UIs.
https://api.slack.com/reference/block-kit/block-elements#datetimepicker

Args:
action_id (required): An identifier for the action triggered when a time is selected. You can use this
when you receive an interaction payload to identify the source of the action. Should be unique among
all other action_ids in the containing block. Maximum length for this field is 255 characters.
initial_date_time: The initial date and time that is selected when the element is loaded, represented as
a UNIX timestamp in seconds. This should be in the format of 10 digits, for example 1628633820
represents the date and time August 10th, 2021 at 03:17pm PST.
and mm is minutes with leading zeros (00 to 59), for example 22:25 for 10:25pm.
confirm: A confirm object that defines an optional confirmation dialog
that appears after a time is selected.
focus_on_load: Indicates whether the element will be set to auto focus within the view object.
Only one element can be set to true. Defaults to false.
"""
super().__init__(
type=self.type,
action_id=action_id,
confirm=ConfirmObject.parse(confirm),
focus_on_load=focus_on_load,
)
show_unknown_key_warning(self, others)

self.initial_date_time = initial_date_time

@JsonValidator("initial_date_time attribute must be between 0 and 99999999 seconds")
def _validate_initial_date_time_valid(self) -> bool:
return self.initial_date_time is None or (0 <= self.initial_date_time <= 9999999999)


# -------------------------------------------------
# Image
# -------------------------------------------------
Expand Down Expand Up @@ -1308,7 +1332,7 @@ def __init__(


# -------------------------------------------------
# Input Elements
# Plain Text Input Element
# -------------------------------------------------


Expand Down Expand Up @@ -1381,6 +1405,186 @@ def __init__(
self.dispatch_action_config = dispatch_action_config


# -------------------------------------------------
# Email Input Element
# -------------------------------------------------


class EmailInputElement(InputInteractiveElement):
type = "email_text_input"

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

def __init__(
self,
*,
action_id: Optional[str] = None,
initial_value: Optional[str] = None,
dispatch_action_config: Optional[Union[dict, DispatchActionConfig]] = None,
focus_on_load: Optional[bool] = None,
placeholder: Optional[Union[str, dict, TextObject]] = None,
**others: dict,
):
"""
https://api.slack.com/reference/block-kit/block-elements#email

Args:
action_id (required): An identifier for the input value when the parent modal is submitted.
You can use this when you receive a view_submission payload to identify the value of the input element.
Should be unique among all other action_ids in the containing block.
Maximum length for this field is 255 characters.
initial_value: The initial value in the email input when it is loaded.
dispatch_action_config: dispatch configuration object that determines when during
text input the element returns a block_actions payload.
focus_on_load: Indicates whether the element will be set to auto focus within the view object.
Only one element can be set to true. Defaults to false.
placeholder: A plain_text only text object that defines the placeholder text shown in the
email input. Maximum length for the text in this field is 150 characters.
"""
super().__init__(
type=self.type,
action_id=action_id,
placeholder=TextObject.parse(placeholder, PlainTextObject.type),
focus_on_load=focus_on_load,
)
show_unknown_key_warning(self, others)

self.initial_value = initial_value
self.dispatch_action_config = dispatch_action_config


# -------------------------------------------------
# Url Input Element
# -------------------------------------------------


class UrlInputElement(InputInteractiveElement):
type = "url_text_input"

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

def __init__(
self,
*,
action_id: Optional[str] = None,
initial_value: Optional[str] = None,
dispatch_action_config: Optional[Union[dict, DispatchActionConfig]] = None,
focus_on_load: Optional[bool] = None,
placeholder: Optional[Union[str, dict, TextObject]] = None,
**others: dict,
):
"""
A URL input element, similar to the Plain-text input element,
creates a single line field where a user can enter URL-encoded data.
https://api.slack.com/reference/block-kit/block-elements#url

Args:
action_id (required): An identifier for the input value when the parent modal is submitted.
You can use this when you receive a view_submission payload to identify the value of the input element.
Should be unique among all other action_ids in the containing block.
Maximum length for this field is 255 characters.
initial_value: The initial value in the URL input when it is loaded.
dispatch_action_config: A dispatch configuration object that determines when during text input
the element returns a block_actions payload.
focus_on_load: Indicates whether the element will be set to auto focus within the view object.
Only one element can be set to true. Defaults to false.
placeholder: A plain_text only text object that defines the placeholder text shown in the URL input.
Maximum length for the text in this field is 150 characters.
"""
super().__init__(
type=self.type,
action_id=action_id,
placeholder=TextObject.parse(placeholder, PlainTextObject.type),
focus_on_load=focus_on_load,
)
show_unknown_key_warning(self, others)

self.initial_value = initial_value
self.dispatch_action_config = dispatch_action_config


# -------------------------------------------------
# Number Input Element
# -------------------------------------------------


class NumberInputElement(InputInteractiveElement):
type = "number_input"

@property
def attributes(self) -> Set[str]:
return super().attributes.union(
{
"initial_value",
"is_decimal_allowed",
"min_value",
"max_value",
"dispatch_action_config",
}
)

def __init__(
self,
*,
action_id: Optional[str] = None,
is_decimal_allowed: Optional[bool] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is a required property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yess it is required in the docs but the python implementation of other required fields was not done, so I followed the trend of the existing code.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, when I look at, for example, the Button element's text property, that is required, and is not marked as Optional, so that is why I asked / pointed it out. I am also confused 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this file it seems like all action_id are optional but should be required, not sure what the standard is here @seratch what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

The action_id one, I think, is a unique situation, as technically (you can try it out yourself) it is not required. You can create a block kit block with elements without action_id, and it is fine. However, you should provide it. So our public facing documentation says it is required, while in reality, it is not really required.

Copy link
Member

Choose a reason for hiding this comment

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

@filmaj @WilliamBergamin First off, is_decimal_allowed is not required actually. So, I am pretty sure that the document is wrong.

action_id

Fil's understanding is correct. For most real-world use cases, the property can be necessary but there is a rare case such as "We have only one interactive block in the app! so we'd like to skip setting action_id to distinguish the button." or something like that. So the platform does not technically require it.

initial_value: Optional[str] = None,
min_value: Optional[str] = None,
max_value: Optional[str] = None,
Copy link
Member

Choose a reason for hiding this comment

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

nice-to-have:
We can accept these three types and call str() when assigning to instance fields.

Suggested change
initial_value: Optional[str] = None,
min_value: Optional[str] = None,
max_value: Optional[str] = None,
initial_value: Optional[Union[str, int, float]] = None,
min_value: Optional[Union[str, int, float]] = None,
max_value: Optional[Union[str, int, float]] = None,

Copy link
Contributor

Choose a reason for hiding this comment

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

This is interesting to me as it's the same suggestion I had in the Java PR: slackapi/java-slack-sdk#1076 (comment)

So should we provide this kind of 'helpful' interface to developers in all our SDKs? The fact that underlying HTTP API uses strings is more coincidence and a backend implementation/runtime constraint than actually a consideration for external developers.

For what it is worth IMO I think it is a nice addition, though it does increase the complexity a little bit in the SDK side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could leave the type as Optional[str] since this is what is in the docs, but still call str() when assigning to instance fields.

This way our types follow the docs and a dev that passes an int or float won't get a runtime error from the slack api.

what do you think @filmaj @seratch?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good to me and was what I hinted at yesterday in our discussion with the backend team as to what we could do on the SDK side, but I would want to make sure @seratch (or, for that matter, anyone else on our team) has no issues with that approach.

Copy link
Member

Choose a reason for hiding this comment

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

My suggestion here is that, regardless of the types given by a developer, this SDK eventually sends str values to the server-side. When it comes to the use cases where a developer directly manipulates dict objects, we should not (and actually are unable to) support int/float types for sure. However, these classes are supposed to be provide better developer experience. So, enabling developers to easily use more appropriate types should make sense.

dispatch_action_config: Optional[Union[dict, DispatchActionConfig]] = None,
focus_on_load: Optional[bool] = None,
placeholder: Optional[Union[str, dict, TextObject]] = None,
**others: dict,
):
"""
https://api.slack.com/reference/block-kit/block-elements#number

Args:
action_id (required): An identifier for the input value when the parent modal is submitted.
You can use this when you receive a view_submission payload to identify the value of the input element.
Should be unique among all other action_ids in the containing block.
Maximum length for this field is 255 characters.
is_decimal_allowed (required): Decimal numbers are allowed if is_decimal_allowed= true, set the value to
false otherwise.
initial_value: The initial value in the number input when it is loaded.
min_value: The minimum value, cannot be greater than max_value.
max_value: The maximum value, cannot be less than min_value.
dispatch_action_config: A dispatch configuration object that determines when
during text input the element returns a block_actions payload.
focus_on_load: Indicates whether the element will be set to auto focus within the view object.
Only one element can be set to true. Defaults to false.
placeholder: A plain_text only text object that defines the placeholder text shown
in the plain-text input. Maximum length for the text in this field is 150 characters.
"""
super().__init__(
type=self.type,
action_id=action_id,
placeholder=TextObject.parse(placeholder, PlainTextObject.type),
focus_on_load=focus_on_load,
)
show_unknown_key_warning(self, others)

self.initial_value = str(initial_value) if initial_value else None
self.is_decimal_allowed = is_decimal_allowed
self.min_value = str(min_value) if min_value else None
self.max_value = str(max_value) if max_value else None
Copy link
Member

Choose a reason for hiding this comment

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

0 is falsy in Python. These parts should be more explicit:

>>> if 0:
...     print("true")
... else:
...     print("false")
...
false
Suggested change
self.min_value = str(min_value) if min_value else None
self.max_value = str(max_value) if max_value else None
self.min_value = str(min_value) if min_value is not None else None
self.max_value = str(max_value) if max_value is not None else None

Copy link
Member

Choose a reason for hiding this comment

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

Also, if we accept non-string values here, the type hints for arguments need to be updated too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yess god catch 🥇 did not think about the 0 falsy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added type Optional[Union[int, float, str]] for initial_value, min_value, max_value

self.dispatch_action_config = dispatch_action_config


# -------------------------------------------------
# Radio Buttons Select
# -------------------------------------------------
Expand Down
Loading