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: Allow emoji value in plain text blocks to be null #1354

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions attachments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func TestAttachment_UnmarshalMarshalJSON_WithBlocks(t *testing.T) {
"text": {
"type": "mrkdwn",
"text": "Pick something:",
"emoji": null,
"verbatim": true
},
"accessory": {
Expand Down
16 changes: 9 additions & 7 deletions block_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package slack
import (
"encoding/json"
"errors"

"gopkg.in/guregu/null.v4"
)

// Block Objects are also known as Composition Objects
Expand Down Expand Up @@ -120,10 +122,10 @@ func unmarshalBlockObject(r json.RawMessage, object blockObject) (blockObject, e
//
// More Information: https://api.slack.com/reference/messaging/composition-objects#text
type TextBlockObject struct {
Type string `json:"type"`
Text string `json:"text"`
Emoji bool `json:"emoji,omitempty"`
Verbatim bool `json:"verbatim,omitempty"`
Type string `json:"type"`
Text string `json:"text"`
Emoji null.Bool `json:"emoji,omitempty"`
Verbatim bool `json:"verbatim,omitempty"`
}

// validateType enforces block objects for element and block parameters
Expand All @@ -143,7 +145,7 @@ func (s TextBlockObject) Validate() error {
}

// https://github.com/slack-go/slack/issues/881
if s.Type == "mrkdwn" && s.Emoji {
if s.Type == "mrkdwn" && s.Emoji.ValueOrZero() == true {
return errors.New("emoji cannot be true in mrkdown")
}

Expand All @@ -161,11 +163,11 @@ func (s TextBlockObject) Validate() error {
}

// NewTextBlockObject returns an instance of a new Text Block Object
func NewTextBlockObject(elementType, text string, emoji, verbatim bool) *TextBlockObject {
func NewTextBlockObject(elementType, text string, emoji bool, verbatim bool) *TextBlockObject {
return &TextBlockObject{
Type: elementType,
Text: text,
Emoji: emoji,
Emoji: null.BoolFrom(emoji),
Verbatim: verbatim,
}
}
Expand Down
119 changes: 112 additions & 7 deletions block_object_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package slack

import (
"encoding/json"
"errors"
"strings"
"testing"

"github.com/go-test/deep"
"github.com/stretchr/testify/assert"
"gopkg.in/guregu/null.v4"
)

func TestNewImageBlockObject(t *testing.T) {
Expand All @@ -24,7 +27,7 @@ func TestNewTextBlockObject(t *testing.T) {

assert.Equal(t, textObject.Type, "plain_text")
assert.Equal(t, textObject.Text, "test")
assert.True(t, textObject.Emoji, "Emoji property should be true")
assert.True(t, textObject.Emoji.ValueOrZero(), "Emoji property should be true")
assert.False(t, textObject.Verbatim, "Verbatim should be false")

}
Expand Down Expand Up @@ -95,7 +98,34 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "plain_text",
Text: "testText",
Emoji: false,
Emoji: null.BoolFrom(false),
Verbatim: false,
},
expected: nil,
},
{
input: TextBlockObject{
Type: "plain_text",
Text: "testText",
Emoji: null.BoolFrom(true),
Verbatim: false,
},
expected: nil,
},
{
input: TextBlockObject{
Type: "plain_text",
Text: "testText",
Emoji: null.BoolFromPtr(nil),
Verbatim: false,
},
expected: nil,
},
{
input: TextBlockObject{
Type: "mrkdwn",
Text: "testText",
Emoji: null.BoolFrom(false),
Verbatim: false,
},
expected: nil,
Expand All @@ -104,7 +134,7 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "mrkdwn",
Text: "testText",
Emoji: false,
Emoji: null.BoolFromPtr(nil),
Verbatim: false,
},
expected: nil,
Expand All @@ -113,7 +143,7 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "invalid",
Text: "testText",
Emoji: false,
Emoji: null.BoolFrom(false),
Verbatim: false,
},
expected: errors.New("type must be either of plain_text or mrkdwn"),
Expand All @@ -122,7 +152,7 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "mrkdwn",
Text: "testText",
Emoji: true,
Emoji: null.BoolFrom(true),
Verbatim: false,
},
expected: errors.New("emoji cannot be true in mrkdown"),
Expand All @@ -131,7 +161,7 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "mrkdwn",
Text: "",
Emoji: false,
Emoji: null.BoolFrom(false),
Verbatim: false,
},
expected: errors.New("text must have a minimum length of 1"),
Expand All @@ -140,7 +170,7 @@ func TestValidateTextBlockObject(t *testing.T) {
input: TextBlockObject{
Type: "mrkdwn",
Text: strings.Repeat("a", 3001),
Emoji: false,
Emoji: null.BoolFrom(false),
Verbatim: false,
},
expected: errors.New("text cannot be longer than 3000 characters"),
Expand All @@ -152,3 +182,78 @@ func TestValidateTextBlockObject(t *testing.T) {
assert.Equal(t, err, test.expected)
}
}

func TestTextBlockObject_UnmarshalJSON(t *testing.T) {
cases := []struct {
raw []byte
expected TextBlockObject
err error
}{
{
[]byte(`{"type":"plain_text","text":"testText"}`),
TextBlockObject{
Type: "plain_text",
Text: "testText",
Emoji: null.BoolFromPtr(nil),
Verbatim: false,
},
nil,
},
{
[]byte(`{"type":"plain_text","text":":+1:","emoji":true}`),
TextBlockObject{
Type: "plain_text",
Text: ":+1:",
Emoji: null.BoolFrom(true),
Verbatim: false,
},
nil,
},
{
[]byte(`{"type":"plain_text","text":"No emojis allowed :(","emoji":false}`),
TextBlockObject{
Type: "plain_text",
Text: "No emojis allowed :(",
Emoji: null.BoolFrom(false),
Verbatim: false,
},
nil,
},
{
[]byte(`{"type":"mrkdwn","text":"testText"}`),
TextBlockObject{
Type: "mrkdwn",
Text: "testText",
Emoji: null.BoolFromPtr(nil),
Verbatim: false,
},
nil,
},
{
[]byte(`{"type":"mrkdwn","text":"No emojis allowed :(","emoji":false}`),
TextBlockObject{
Type: "mrkdwn",
Text: "No emojis allowed :(",
Emoji: null.BoolFrom(false),
Verbatim: false,
},
nil,
},
}
for _, tc := range cases {
var actual TextBlockObject
err := json.Unmarshal(tc.raw, &actual)
if err != nil {
if tc.err == nil {
t.Errorf("unexpected error: %s", err)
}
t.Errorf("expected error is %v, but got %v", tc.err, err)
}
if tc.err != nil {
t.Errorf("expected to raise an error %v", tc.err)
}
if diff := deep.Equal(actual, tc.expected); diff != nil {
t.Errorf("actual value does not match expected one\n%s", diff)
}
}
}
2 changes: 1 addition & 1 deletion block_section_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestNewBlockSectionContainsAddedTextBlockAndAccessory(t *testing.T) {
textBlockInSection := sectionBlock.Text
assert.Equal(t, textBlockInSection.Text, textBlockObject.Text)
assert.Equal(t, textBlockInSection.Type, textBlockObject.Type)
assert.True(t, textBlockInSection.Emoji)
assert.True(t, textBlockInSection.Emoji.ValueOrZero())
assert.False(t, textBlockInSection.Verbatim)
assert.Equal(t, sectionBlock.Accessory.ImageElement, conflictImage)
}
2 changes: 1 addition & 1 deletion chat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestPostMessage(t *testing.T) {
}

blocks := []Block{NewContextBlock("context", NewTextBlockObject(PlainTextType, "hello", false, false))}
blockStr := `[{"type":"context","block_id":"context","elements":[{"type":"plain_text","text":"hello"}]}]`
blockStr := `[{"type":"context","block_id":"context","elements":[{"type":"plain_text","text":"hello","emoji":false}]}]`

tests := map[string]messageTest{
"OnlyBasicProperties": {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ require (
github.com/gorilla/websocket v1.4.2
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/testify v1.2.2
gopkg.in/guregu/null.v4 v4.0.0
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/guregu/null.v4 v4.0.0 h1:1Wm3S1WEA2I26Kq+6vcW+w0gcDo44YKYD7YIEJNHDjg=
gopkg.in/guregu/null.v4 v4.0.0/go.mod h1:YoQhUrADuG3i9WqesrCmpNRwm1ypAgSHYqoOcTu/JrI=
18 changes: 12 additions & 6 deletions interactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,24 @@ const (
"type": "modal",
"title": {
"type": "plain_text",
"text": "launch project"
"text": "launch project",
"emoji": false
},
"blocks": [{
"type": "section",
"text": {
"text": "*Sally* has requested you set the deadline for the Nano launch project",
"type": "mrkdwn"
"type": "mrkdwn",
"emoji": false
},
"accessory": {
"type": "datepicker",
"action_id": "datepicker123",
"initial_date": "1990-04-28",
"placeholder": {
"type": "plain_text",
"text": "Select a date"
"text": "Select a date",
"emoji": false
}
}
}],
Expand All @@ -92,15 +95,17 @@ const (
"type": "modal",
"title": {
"type": "plain_text",
"text": "meal choice"
"text": "meal choice",
"emoji": false
},
"blocks": [
{
"type": "input",
"block_id": "multi-line",
"label": {
"type": "plain_text",
"text": "dietary restrictions"
"text": "dietary restrictions",
"emoji": false
},
"element": {
"type": "plain_text_input",
Expand All @@ -113,7 +118,8 @@ const (
"block_id": "target_channel",
"label": {
"type": "plain_text",
"text": "Select a channel to post the result on"
"text": "Select a channel to post the result on",
"emoji": false
},
"element": {
"type": "conversations_select",
Expand Down
9 changes: 6 additions & 3 deletions views_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,8 @@ func TestSlack_UpdateViewSubmissionResponse(t *testing.T) {
"type": "modal",
"title": {
"type": "plain_text",
"text": "Test update view submission response"
"text": "Test update view submission response",
"emoji": false
},
"blocks": [
{
Expand Down Expand Up @@ -803,7 +804,8 @@ func TestSlack_PushViewSubmissionResponse(t *testing.T) {
"type": "modal",
"title": {
"type": "plain_text",
"text": "Test update view submission response"
"text": "Test update view submission response",
"emoji": false
},
"blocks": [
{
Expand All @@ -812,7 +814,8 @@ func TestSlack_PushViewSubmissionResponse(t *testing.T) {
"elements": [
{
"type": "plain_text",
"text": "Context text"
"text": "Context text",
"emoji": false
},
{
"type": "image",
Expand Down
4 changes: 2 additions & 2 deletions webhooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ func TestWebhookMessage_WithBlocks(t *testing.T) {
assert.Equal(t, 1, len(msgSingleBlock.Blocks.BlockSet))

msgJsonSingleBlock, _ := json.Marshal(msgSingleBlock)
assert.Equal(t, `{"blocks":[{"type":"section","text":{"type":"plain_text","text":"text"}}],"replace_original":false,"delete_original":false}`, string(msgJsonSingleBlock))
assert.Equal(t, `{"blocks":[{"type":"section","text":{"type":"plain_text","text":"text","emoji":false}}],"replace_original":false,"delete_original":false}`, string(msgJsonSingleBlock))

msgTwoBlocks := WebhookMessage{Blocks: twoBlocks}
assert.Equal(t, 2, len(msgTwoBlocks.Blocks.BlockSet))

msgJsonTwoBlocks, _ := json.Marshal(msgTwoBlocks)
assert.Equal(t, `{"blocks":[{"type":"section","text":{"type":"plain_text","text":"text"}},{"type":"section","text":{"type":"plain_text","text":"text"}}],"replace_original":false,"delete_original":false}`, string(msgJsonTwoBlocks))
assert.Equal(t, `{"blocks":[{"type":"section","text":{"type":"plain_text","text":"text","emoji":false}},{"type":"section","text":{"type":"plain_text","text":"text","emoji":false}}],"replace_original":false,"delete_original":false}`, string(msgJsonTwoBlocks))

msgNoBlocks := WebhookMessage{Text: "foo"}
msgJsonNoBlocks, _ := json.Marshal(msgNoBlocks)
Expand Down