Add support for section fields #123
Conversation
|
I am not going to be able to finish this PR. Here's the context: CircleCI jobs send messages which only have "fields" but no "text". Currently we require "text" to exist, so we get a parse error when we parse such a message. This PR attempts to rectify this, but it involves some API changes, and more generally I think that slack-web needs more of the pattern Persistent and http-client use where they don't allow you to construct a SlackSection or whatever, but instead provide a default which you override. This would reduce the API breakage of fixing things like this. |
maxhallinan
changed the title
| , slackSectionFields :: Maybe [SlackText] | ||
| -- ^ Required if 'slackSectionText' is not provided. | ||
| , slackSectionAccessory :: Maybe SlackAccessory | ||
| } |
There was a problem hiding this comment.
slackSectionText and slackSectionFields should really be SlackTextObject, but using that type would break the library's API even further.
The consequence of using SlackText is that the library only supports mrkdwn text. This is an incomplete implementation of Slack's API, which accepts both plain_text and mrkdwn for the section text and fields attributes.
| Error _ -> | ||
| Nothing | ||
| Success slackAccessory -> | ||
| Just slackAccessory |
There was a problem hiding this comment.
There are no good options. This seems like the least bad.
| @@ -374,9 +413,7 @@ data SlackBlock | |||
| deriving stock (Eq) | |||
|
|
|||
| instance Show SlackBlock where | |||
There was a problem hiding this comment.
I briefly reformed this Show instance but realized that doing so invalidates 8 golden files. In the interest of keeping this PR small, I reverted.
| @@ -163,22 +163,43 @@ truncateSlackMessage (SlackMessage blocks) = | |||
| in (SlackMessage truncatedBlocks, or isTruncateds) | |||
|
|
|||
| truncateSlackBlock :: SlackBlock -> (SlackBlock, Bool) | |||
| truncateSlackBlock sb@(SlackBlockSection (SlackText texts) mAccessory) = | |||
| let messageLength = sum $ map T.length texts | |||
| truncateSlackBlock sb@(SlackBlockSection SlackSection {..}) = | |||
There was a problem hiding this comment.
The entire premise of this function is bizarre and I think it should be removed from the library. Userland should be responsible for handling messages that exceed the length limit.
For that reason, I am not taking the fields text into account here. It would be more trouble than it's worth, imo.
| prefixFirstSlackBlockSection prefix (SlackBlockSection text mAccessory : sbs) = (SlackBlockSection (message prefix <> text) mAccessory : sbs, True) | ||
| prefixFirstSlackBlockSection prefix (sb : sbs) = let (prefixedSbs, match) = prefixFirstSlackBlockSection prefix sbs in (sb : prefixedSbs, match) | ||
| prefixFirstSlackBlockSection _ [] = ([], False) | ||
| prefixFirstSlackBlockSection prefix (SlackBlockSection SlackSection {..} : sbs) = |
There was a problem hiding this comment.
This helper is also bizarre in its specificity and seems to belong to some application's use-case. Not a good fit for a library.
There was a problem hiding this comment.
@maxhallinan Is there any way to test this pr with our backend? Or does it have to be merged before it can be used?
You would have to install |
|
|
@levinean I can add a test that it encodes to valid Slack block JSON. |
@levinean you can copy/paste tests/golden/BlockKitBuilderMessage/simple_blocks.golden.json into the Block Kit Builder to see the fields JSON above. |
Before submitting your PR, check that you've:
@sincedeclarations to the HaddockAfter submitting your PR:
(unreleased)on the Changelog