Skip to content

Commit d0ef804

Browse files
authored
fix: don't allow emoji with mrkdwn (#1418)
Fixes #1414 Fixes #881
2 parents 93ad347 + 195c9bd commit d0ef804

File tree

6 files changed

+37
-23
lines changed

6 files changed

+37
-23
lines changed

attachments_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ func TestAttachment_UnmarshalMarshalJSON_WithBlocks(t *testing.T) {
1919
"text": {
2020
"type": "mrkdwn",
2121
"text": "Pick something:",
22-
"emoji": null,
2322
"verbatim": true
2423
},
2524
"accessory": {

block_object.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func unmarshalBlockObject(r json.RawMessage, object blockObject) (blockObject, e
122122
type TextBlockObject struct {
123123
Type string `json:"type"`
124124
Text string `json:"text"`
125-
Emoji *bool `json:"emoji"`
125+
Emoji *bool `json:"emoji,omitempty"`
126126
Verbatim bool `json:"verbatim,omitempty"`
127127
}
128128

@@ -142,9 +142,8 @@ func (s TextBlockObject) Validate() error {
142142
return errors.New("type must be either of plain_text or mrkdwn")
143143
}
144144

145-
// https://github.com/slack-go/slack/issues/881
146-
if s.Type == "mrkdwn" && s.Emoji != nil && *s.Emoji {
147-
return errors.New("emoji cannot be true in mrkdown")
145+
if s.Type == "mrkdwn" && s.Emoji != nil {
146+
return errors.New("emoji cannot be set for mrkdwn type")
148147
}
149148

150149
// https://api.slack.com/reference/block-kit/composition-objects#text__fields
@@ -161,11 +160,28 @@ func (s TextBlockObject) Validate() error {
161160
}
162161

163162
// NewTextBlockObject returns an instance of a new Text Block Object
163+
//
164+
// If you want to create a mrkdwn object, you should set the emoji parameter to false. The
165+
// reason is that Slack doesn't accept emoji in mrkdwn.
164166
func NewTextBlockObject(elementType, text string, emoji bool, verbatim bool) *TextBlockObject {
167+
// If we're trying to build a mrkdwn object, we can't send emoji at all. I think the
168+
// right approach here is to be a bit clever, and not break the function interface.
169+
//
170+
// So, here's the plan:
171+
// 1. If the type is mrkdwn, set emoji to nil, regardless of what the user passed in
172+
// 2. Else, set emoji to the value passed in
173+
var emojiPtr *bool
174+
175+
if elementType == "mrkdwn" {
176+
emojiPtr = nil
177+
} else {
178+
emojiPtr = &emoji
179+
}
180+
165181
return &TextBlockObject{
166182
Type: elementType,
167183
Text: text,
168-
Emoji: &emoji,
184+
Emoji: emojiPtr,
169185
Verbatim: verbatim,
170186
}
171187
}

block_object_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -117,43 +117,43 @@ func TestValidateTextBlockObject(t *testing.T) {
117117
input: TextBlockObject{
118118
Type: "mrkdwn",
119119
Text: "testText",
120-
Emoji: emojiFalse,
120+
Emoji: nil,
121121
Verbatim: false,
122122
},
123123
expected: nil,
124124
},
125125
{
126126
input: TextBlockObject{
127-
Type: "mrkdwn",
127+
Type: "invalid",
128128
Text: "testText",
129-
Emoji: nil,
129+
Emoji: emojiFalse,
130130
Verbatim: false,
131131
},
132-
expected: nil,
132+
expected: errors.New("type must be either of plain_text or mrkdwn"),
133133
},
134134
{
135135
input: TextBlockObject{
136-
Type: "invalid",
136+
Type: "mrkdwn",
137137
Text: "testText",
138-
Emoji: emojiFalse,
138+
Emoji: emojiTrue,
139139
Verbatim: false,
140140
},
141-
expected: errors.New("type must be either of plain_text or mrkdwn"),
141+
expected: errors.New("emoji cannot be set for mrkdwn type"),
142142
},
143143
{
144144
input: TextBlockObject{
145145
Type: "mrkdwn",
146146
Text: "testText",
147-
Emoji: emojiTrue,
147+
Emoji: emojiFalse,
148148
Verbatim: false,
149149
},
150-
expected: errors.New("emoji cannot be true in mrkdown"),
150+
expected: errors.New("emoji cannot be set for mrkdwn type"),
151151
},
152152
{
153153
input: TextBlockObject{
154154
Type: "mrkdwn",
155155
Text: "",
156-
Emoji: emojiFalse,
156+
Emoji: nil,
157157
Verbatim: false,
158158
},
159159
expected: errors.New("text must have a minimum length of 1"),
@@ -162,7 +162,7 @@ func TestValidateTextBlockObject(t *testing.T) {
162162
input: TextBlockObject{
163163
Type: "mrkdwn",
164164
Text: strings.Repeat("a", 3001),
165-
Emoji: emojiFalse,
165+
Emoji: nil,
166166
Verbatim: false,
167167
},
168168
expected: errors.New("text cannot be longer than 3000 characters"),
@@ -171,7 +171,7 @@ func TestValidateTextBlockObject(t *testing.T) {
171171

172172
for _, test := range tests {
173173
err := test.input.Validate()
174-
assert.Equal(t, err, test.expected)
174+
assert.Equal(t, test.expected, err)
175175
}
176176
}
177177

block_section_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestNewBlockSectionContainsAddedTextBlockAndAccessory(t *testing.T) {
3030
textBlockInSection := sectionBlock.Text
3131
assert.Equal(t, textBlockInSection.Text, textBlockObject.Text)
3232
assert.Equal(t, textBlockInSection.Type, textBlockObject.Type)
33-
assert.True(t, *textBlockInSection.Emoji)
33+
assert.Nil(t, textBlockInSection.Emoji)
3434
assert.False(t, textBlockInSection.Verbatim)
3535
assert.Equal(t, sectionBlock.Accessory.ImageElement, conflictImage)
3636
}

0 commit comments

Comments
 (0)