Skip to content

Commit

Permalink
Remove confusing warning messages #1030 (#1032)
Browse files Browse the repository at this point in the history
* Remove confusing warning messages #1030

* Fix test comments
  • Loading branch information
seratch committed Jun 10, 2021
1 parent 0a7918a commit 0ed7672
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
19 changes: 16 additions & 3 deletions slack_sdk/models/blocks/block_elements.py
Expand Up @@ -149,10 +149,17 @@ def __init__(
subtype: Optional[str] = None,
**others: dict,
):
"""An interactive block element.
We generally recommend using the concrete subclasses for better supports of available properties.
"""
if subtype:
self._subtype_warning()
super().__init__(type=type or subtype)
show_unknown_key_warning(self, others)

# Note that we don't intentionally have show_unknown_key_warning for the unknown key warnings here.
# It's fine to pass any kwargs to the held dict here although the class does not do any validation.
# show_unknown_key_warning(self, others)

self.action_id = action_id

Expand Down Expand Up @@ -190,11 +197,17 @@ def __init__(
confirm: Optional[Union[dict, ConfirmObject]] = None,
**others: dict,
):
"""InteractiveElement that is usable in input blocks"""
"""InteractiveElement that is usable in input blocks
We generally recommend using the concrete subclasses for better supports of available properties.
"""
if subtype:
self._subtype_warning()
super().__init__(action_id=action_id, type=type or subtype)
show_unknown_key_warning(self, others)

This comment has been minimized.

Copy link
@weallwegot

weallwegot Jul 9, 2021

just curious about why this is? I've had a couple of times where i misspell a kwarg like initial_option vs initial_options for different kinds of select elements and it takes me a while to figure out what i did, especially when i first start using something. curious if this warning was there to help lead developers in the right direction during that type of situation and if so, im just curious about the reasoning for moving away from it?

This comment has been minimized.

Copy link
@seratch

seratch Jul 9, 2021

Author Member

@weallwegot We removed this warning only from the InteractiveElement, which is an abstract class that has only action_id and type as known ones. This causes irrelevant warnings for any other fields.


# Note that we don't intentionally have show_unknown_key_warning for the unknown key warnings here.
# It's fine to pass any kwargs to the held dict here although the class does not do any validation.
# show_unknown_key_warning(self, others)

self.placeholder = TextObject.parse(placeholder)
self.confirm = ConfirmObject.parse(confirm)
Expand Down
22 changes: 22 additions & 0 deletions tests/slack_sdk/models/test_elements.py
Expand Up @@ -23,6 +23,8 @@
ChannelSelectElement,
ConfirmObject,
Option,
InputInteractiveElement,
InteractiveElement,
)
from . import STRING_3001_CHARS, STRING_301_CHARS

Expand All @@ -32,6 +34,26 @@
# -------------------------------------------------


class InteractiveElementTests(unittest.TestCase):
def test_with_interactive_element(self):
input = {
"type": "plain_text_input",
"action_id": "plain_input",
"placeholder": {"type": "plain_text", "text": "Enter some plain text"},
}
# Any properties should not be lost
self.assertDictEqual(input, InteractiveElement(**input).to_dict())

def test_with_input_interactive_element(self):
input = {
"type": "plain_text_input",
"action_id": "plain_input",
"placeholder": {"type": "plain_text", "text": "Enter some plain text"},
}
# Any properties should not be lost
self.assertDictEqual(input, InputInteractiveElement(**input).to_dict())


class InteractiveElementTests(unittest.TestCase):
def test_action_id(self):
with self.assertRaises(SlackObjectFormationError):
Expand Down

0 comments on commit 0ed7672

Please sign in to comment.