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

Fix bug decoding SlackPlainText #131

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Conversation

rampion
Copy link
Contributor

@rampion rampion commented Dec 20, 2023

The SlackPlainText constructor of SlackTextObject's ToJSON and FromJSON instances disagreed on whether its type should be "plain_text" or "text".

This failure to decode when the type was set to "plain_text" caused a bug when interacting with slack.

text <- obj .: "text"
pure . SlackPlainText . SlackText $ lines text
"mrkdwn" -> do
text <- obj .: "text"
pure . SlackMarkdownText . SlackText $ lines text
_ -> fail "Unknown SlackTextObject type, must be one of ['text', 'mrkdwn']"
_ -> fail "Unknown SlackTextObject type, must be one of ['plain_text', 'mrkdwn']"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note how previously the ToJSON instance encoded SlackPlainText values using "type" .= "plain_text" but FromJSON didn't accept that type.

@rampion rampion force-pushed the fix-slack-text-object-from-json branch from 4b4dc7a to de79452 Compare December 21, 2023 01:45
@rampion rampion changed the base branch from master to fix-nix December 21, 2023 01:45
Copy link

@levinean levinean left a comment

Choose a reason for hiding this comment

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

nice love the unit tests!

@rampion rampion merged commit 119ed19 into fix-nix Jan 9, 2024
4 checks passed
@rampion rampion deleted the fix-slack-text-object-from-json branch January 9, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants