Skip to content

Commit

Permalink
Fix bug in Slack Conversation adapter to make emails case-insensitive
Browse files Browse the repository at this point in the history
  • Loading branch information
mike-gregory-ovo committed Jun 27, 2023
1 parent 91f947f commit 1105aed
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
8 changes: 4 additions & 4 deletions adapters/slack/conversation/conversation.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ func (c *Conversation) Get(_ context.Context) ([]string, error) {

for _, user := range *users {
if !user.IsBot {
emails = append(emails, user.Profile.Email)
emails = append(emails, strings.ToLower(user.Profile.Email))

// Add the email -> ID map for use with Remove method.
c.cache[user.Profile.Email] = user.ID
c.cache[strings.ToLower(user.Profile.Email)] = user.ID
}
}

Expand All @@ -200,7 +200,7 @@ func (c *Conversation) Add(_ context.Context, emails []string) error {
slackIds := make([]string, len(emails))

for index, email := range emails {
user, err := c.client.GetUserByEmail(email)
user, err := c.client.GetUserByEmail(strings.ToLower(email))
if err != nil {
return fmt.Errorf("slack.conversation.add.getuserbyemail(%s) -> %w", email, err)
}
Expand Down Expand Up @@ -228,7 +228,7 @@ func (c *Conversation) Remove(_ context.Context, emails []string) error {
}

for _, email := range emails {
err := c.client.KickUserFromConversation(c.conversationName, c.cache[email])
err := c.client.KickUserFromConversation(c.conversationName, c.cache[strings.ToLower(email)])
if err != nil {
if c.MuteRestrictedErrOnKickFromPublic && strings.Contains(err.Error(), "restricted_action") {
c.Logger.Println("Cannot kick from public channel, but error is muted by configuration - continuing")
Expand Down
31 changes: 31 additions & 0 deletions adapters/slack/conversation/conversation_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func TestConversation_Add(t *testing.T) {
assert.NoError(t, err)
}

//nolint:funlen
func TestConversation_Remove(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -168,6 +169,36 @@ func TestConversation_Remove(t *testing.T) {

assert.NoError(t, err)
})

t.Run("Check case sensitivity", func(t *testing.T) {
t.Parallel()

slackClient := newMockISlackConversation(t)
adapter := New(&slack.Client{}, "test")
adapter.client = slackClient

slackClient.EXPECT().GetUsersInConversation(&slack.GetUsersInConversationParameters{
ChannelID: "test",
Cursor: "",
Limit: 50,
}).Return([]string{"foo", "bar"}, "", nil)

slackClient.EXPECT().GetUsersInfo("foo", "bar").Return(&[]slack.User{
// Capitalise the letter E in email.
{ID: "foo", IsBot: false, Profile: slack.UserProfile{Email: "foo@Email"}},
{ID: "bar", IsBot: false, Profile: slack.UserProfile{Email: "bar@Email"}},
}, nil)

_, _ = adapter.Get(ctx)

slackClient.EXPECT().KickUserFromConversation("test", "foo").Return(nil)
slackClient.EXPECT().KickUserFromConversation("test", "bar").Return(nil)

// Capitalise the first letter of each email.
err := adapter.Remove(ctx, []string{"Foo@email", "Bar@email"})

assert.NoError(t, err)
})
}

//nolint:funlen
Expand Down

0 comments on commit 1105aed

Please sign in to comment.