Reclaim contributions - all#166
Conversation
By doing this, code can more easily extract the action_id from the payload without having to cast to every different block element type. Sadly, all of them except for Image have it, preventing us from just having getActionId() on the root BlockElement class. But perhaps doing that and making it optional is another way to do this?
…payloads Without this code, I was getting errors when Jackson tried to deserialize payloads that I had just sent containing initially selected options. The reason is that Jackson had no hints on how to deserialize the OptionOrOptionGroup shared interface. This resolves that, though likely not as elegant as it could be given the need to have two implementations, one for the raw class and one for when it is surrounded by Optional (for StaticSelectMenuIF).
Even though the Slack API docs say that when an user select's a menu, the interaction's value should arrive in the "value" field, I believe the docs are incorrect and reality is different. Specifically, what I am seeing in the real world is that select menus will have their value arrive instead as a "selected_option" field. Thus, this deserializer will inspect for this field and pull the value from there if present.
# Conflicts: # slack-base/src/main/java/com/hubspot/slack/client/models/events/SlackEventType.java
# Conflicts: # slack-base/src/main/java/com/hubspot/slack/client/models/blocks/elements/ExternalMultiSelectMenuIF.java # slack-base/src/main/java/com/hubspot/slack/client/models/blocks/elements/StaticMultiSelectMenuIF.java # slack-base/src/main/java/com/hubspot/slack/client/models/blocks/elements/StaticSelectMenuIF.java # slack-base/src/main/java/com/hubspot/slack/client/models/blocks/json/OptionOrOptionGroupDeserializer.java
Even though the Slack API docs say that when an user select's a menu, the interaction's value should arrive in the "value" field, I believe the docs are incorrect and reality is different. Specifically, what I am seeing in the real world is that select menus will have their value arrive instead as a "selected_option" field. Thus, this deserializer will inspect for this field and pull the value from there if present.
…payloads Without this code, I was getting errors when Jackson tried to deserialize payloads that I had just sent containing initially selected options. The reason is that Jackson had no hints on how to deserialize the OptionOrOptionGroup shared interface. This resolves that, though likely not as elegant as it could be given the need to have two implementations, one for the raw class and one for when it is surrounded by Optional (for StaticSelectMenuIF).
…payloads Without this code, I was getting errors when Jackson tried to deserialize payloads that I had just sent containing initially selected options. The reason is that Jackson had no hints on how to deserialize the OptionOrOptionGroup shared interface. This resolves that, though likely not as elegant as it could be given the need to have two implementations, one for the raw class and one for when it is surrounded by Optional (for StaticSelectMenuIF).
…ed out here: https://api.slack.com/legacy/interactive-message-field-guide#options_load_url Specifically, it seems that this different payload is what is sent when using BlockKit + external select menus. These classes are useful for deserializing the alternative incoming payload.
|
I just added to this PR a few more things: Alternative to
|
szabowexler
left a comment
szabowexler
left a comment
There was a problem hiding this comment.
honestly this looks fine. I'll just merge this in and we can deal with issues down the road.
2 participants
Hey there - sorry I've been MIA the last few months. Just heads down with our startup and hadn't had a chance to circle back on a couple PRs, plus some new stuff I recently needed.
Rather than letting them linger in my fork for months and get behind, I'm dropping it all here as one PR so that worst case they are on the record. I'll see if I can submit separate ones for each individual improvement, but given my spotty record who knows 🤣
First, some notes about the two existing PRs that this one encapsulates:
PR #145 is here, but also with the move from
HomeTabViewResponseIFtoHomeTabViewResponseper the comment there by @szabowexler.PR #134 is also here, with no meaningful changes. I haven't had a chance to resolved the question of testing. The issue I bumped into is that the existing tests in the suite, which I would normally crib from, seem to rely on serializing and deserializing and then comparing the JSON tree. The problem here is that
BlockElementActionIFreturns aStringforgetSelectedValue()but the actual JSON payload can sometimes be a string and other times be a complex object representing a "selected option". So the serialization going back ends up different, causing the test to fail. I haven't been able to spend enough time to come up with a more appropriate test for this class.OK, and now the new stuff captured in here:
SlackAppUninstalledEventIF,SlackTokensRevokedEventIF,TokensRevoked.StateActionValueto reflect the reality that the value can be something other than a string, depending on what blocks you submit. Without this change, the library will fail to parse the response of a modal submission that includes date or radio selections (and others). I needed date and radio blocks, so this patch includes added support for them, with tests.Apologies for dumping them in a group like this. By far the biggest inhibitor to being a better contributor is my noob-level git skills. I hope to borrow a teammate in the coming days and enlist his help in breaking these out and squashing them into a single commit, but in the meantime I wanted to get this on the record.