Add datetimepicker, url, email, number block elements

[WilliamBergamin]

Summary

This pull request adds four block elements, which were added in October 2022.

• datetimepicker
• url_text_input
• email_text_input
• number_input

Category

• slack_sdk.web.WebClient (sync/async) (Web API client)
• slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
• slack_sdk.socket_mode (Socket Mode client)
• slack_sdk.signature (Request Signature Verifier)
• slack_sdk.oauth (OAuth Flow Utilities)
• slack_sdk.models (UI component builders)
• slack_sdk.scim (SCIM API client)
• slack_sdk.audit_logs (Audit Logs API client)
• slack_sdk.rtm_v2 (RTM client)
• /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

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

[codecov]

Codecov Report

Merging #1288 (7fba5a1) into main (71745b0) will increase coverage by 0.22%.
The diff coverage is 96.72%.

@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   85.57%   85.79%   +0.22%     
==========================================
  Files          84       84              
  Lines        7827     7845      +18     
==========================================
+ Hits         6698     6731      +33     
+ Misses       1129     1114      -15     

Impacted Files	Coverage Δ
slack_sdk/models/blocks/block_elements.py	95.41% <96.49%> (+2.52%)	⬆️
slack_sdk/models/blocks/__init__.py	100.00% <100.00%> (ø)
slack_sdk/socket_mode/builtin/internals.py	72.53% <0.00%> (+2.14%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[seratch]

Looks great to me

[seratch]

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

[WilliamBergamin]

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

[seratch]

I've reported this error and it's already fixed on the original document side

Suggested change

	a UNUIX timestamp in seconds. This should be in the format of 10 digits, for example 1628633820
	a UNIX timestamp in seconds. This should be in the format of 10 digits, for example 1628633820

[seratch]

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,

[filmaj]

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.

[WilliamBergamin]

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?

[filmaj]

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.

[seratch]

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.

[seratch]

Thanks for improving this!

[filmaj]

LGTM, left a few minor comments.

[filmaj]

Nice!

[filmaj]

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.

[filmaj]

I believe this is a required property

[WilliamBergamin]

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?

[filmaj]

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 😄

[WilliamBergamin]

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?

[filmaj]

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.

[seratch]

@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.

[filmaj]

Suggested change

	is_decimal_allowed: Decimal numbers are allowed if is_decimal_allowed= true, set the value to false otherwise.
	is_decimal_allowed (required): Decimal numbers are allowed if is_decimal_allowed= true, set the value to false otherwise.

[WilliamBergamin]

Good catch 💯

[seratch]

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

[seratch]

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

[WilliamBergamin]

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

[WilliamBergamin]

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

[seratch]

Thank you! Looks good to me. Feel free to merge it once the CI builds pass. Also, please squash commits into one when merging.