Skip to content

Commit

Permalink
Skip queuing ADD_MEMBER job for the chat-roulette bot
Browse files Browse the repository at this point in the history
  • Loading branch information
bincyber committed Oct 20, 2024
1 parent 80bc645 commit 81dfb3f
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 20 deletions.
5 changes: 2 additions & 3 deletions internal/bot/bot_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ func GetBotUserID(ctx context.Context, client *slack.Client) (string, error) {
return resp.UserID, nil
}

// IsUserABot uses Slack's users.info API method
// to check if the given user is actually a bot.
func IsUserABot(ctx context.Context, client *slack.Client, userID string) (bool, error) {
// isUserASlackBot uses Slack's users.info API method to check if the given user is actually a bot.
func isUserASlackBot(ctx context.Context, client *slack.Client, userID string) (bool, error) {
resp, err := client.GetUserInfoContext(ctx, userID)
if err != nil {
return false, err
Expand Down
4 changes: 2 additions & 2 deletions internal/bot/bot_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ func Test_IsUserABot(t *testing.T) {

client := slack.New("xoxb-test-token-here", slack.OptionAPIURL(slackServer.GetAPIURL()))

boolean, err := IsUserABot(context.Background(), client, "W012A3CDE")
boolean, err := isUserASlackBot(context.Background(), client, "W012A3CDE")
assert.Nil(t, err)
assert.False(t, boolean)
})

t.Run("failure", func(t *testing.T) {
client := slack.New("xoxb-invalid-slack-authtoken")

_, err := IsUserABot(context.Background(), client, "W012A3CDE")
_, err := isUserASlackBot(context.Background(), client, "W012A3CDE")
assert.NotNil(t, err)
})
}
Expand Down
16 changes: 7 additions & 9 deletions internal/bot/job_add_member.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,14 @@ func AddMember(ctx context.Context, db *gorm.DB, client *slack.Client, p *AddMem
attributes.SlackUserID, p.UserID,
)

// Skip bot users as they should not participate in chat-roulette
isBot, err := IsUserABot(ctx, client, p.UserID)
if err != nil {
message := "failed to check if Slack user is a bot"
// Skip bot users as they cannot participate in chat-roulette
// This checks for all bot users in the channel and not only the chat-roulette bot
if isBot, err := isUserASlackBot(ctx, client, p.UserID); err != nil {
message := "failed to check if this Slack user is a bot"
logger.Error(message, "error", err)
return errors.Wrap(err, message)
}

if isBot {
logger.Debug("skipping because Slack user is a bot")
} else if isBot {
logger.Debug("skipping because this Slack user is a bot")
return nil
}

Expand Down Expand Up @@ -90,7 +88,7 @@ func AddMember(ctx context.Context, db *gorm.DB, client *slack.Client, p *AddMem
return errors.Wrap(result.Error, message)
}

logger.Info("queued GREET_MEMBER job for this match")
logger.Info("queued GREET_MEMBER job for this user")
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/bot/job_add_member_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (s *AddMemberSuite) Test_AddMember() {
err := AddMember(s.ctx, s.db, client, p)
r.NoError(err)
r.Contains(s.buffer.String(), "added Slack user to the database")
r.Contains(s.buffer.String(), "queued GREET_MEMBER job for this match")
r.Contains(s.buffer.String(), "queued GREET_MEMBER job for this user")
}

func (s *AddMemberSuite) Test_QueueAddMemberJob() {
Expand Down
1 change: 1 addition & 0 deletions internal/bot/job_greet_admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func UpsertChannelSettings(ctx context.Context, db *gorm.DB, interaction *slack.
Weekday: firstRound.Weekday().String(),
Hour: firstRound.Hour(),
NextRound: firstRound,
// BotUserID: "", // TODO
}

if err := QueueAddChannelJob(ctx, db, p); err != nil {
Expand Down
23 changes: 22 additions & 1 deletion internal/bot/job_sync_members.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func SyncMembers(ctx context.Context, db *gorm.DB, client *slack.Client, p *Sync
attributes.SlackChannelID, p.ChannelID,
)

logger.Info("syncing Slack members")
logger.Info("syncing members of Slack channel with database")

// Get the list of channel members from Slack
slackMembers, err := getChannelMembers(ctx, client, p.ChannelID, 100)
Expand Down Expand Up @@ -57,12 +57,33 @@ func SyncMembers(ctx context.Context, db *gorm.DB, client *slack.Client, p *Sync

logger.Debug("retrieved the list of members from the database", "members_count", len(dbMembers))

// Retrieve the userID of the chat-roulette bot
logger.Debug("retrieving the user ID of the Slack bot")

slackCtx, cancel := context.WithTimeout(ctx, 3000*time.Millisecond)
defer cancel()

botUserID, err := GetBotUserID(slackCtx, client)
if err != nil {
message := "failed to retrieve the user ID of the chat-roulette Slack bot"
logger.Error(message, "error", err)
return errors.Wrap(err, message)
}

logger.Debug("retrieved the user ID of the chat-roulette Slack bot")

// Reconcile between the two lists
members := reconcileMembers(ctx, slackMembers, dbMembers)

for _, member := range members {
switch {
case member.Create:
if member.UserID == botUserID {
// Skip as chat-roulette bot cannot participate in chat-roulette
logger.Debug("skipping chat-roulette bot user")
continue
}

// Queue an ADD_MEMBER job for the new member of the Slack channel.
p := &AddMemberParams{
ChannelID: p.ChannelID,
Expand Down
3 changes: 3 additions & 0 deletions internal/bot/job_sync_members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ func (s *SyncMembersSuite) Test_SyncMembers() {

addMember := "U5555555555" // this user will be created
deleteMember := "U9999999999" // this user will be deleted
botUser := "W012A3CDE" // this bot user will be skipped

slackMembers := []string{
"U0123456789",
"U9876543210",
"U1111111111",
addMember,
botUser,
}

// Mock Slack API call
Expand Down Expand Up @@ -106,6 +108,7 @@ func (s *SyncMembersSuite) Test_SyncMembers() {

err := SyncMembers(s.ctx, s.db, client, p)
r.NoError(err)
r.Contains(s.buffer.String(), "skipping chat-roulette bot user")
}

func (s *SyncMembersSuite) Test_SyncMembersJob() {
Expand Down
12 changes: 8 additions & 4 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,16 @@ func New(ctx context.Context, logger hclog.Logger, c *config.Config) (*Server, e
slackClient, httpClient := slackclient.New(logger, c.Bot.AuthToken)

// Retrieve the user_id of the chat-roulette Slack bot
logger.Debug("retrieving the user ID of the Slack bot")
slackBotUserID, err := bot.GetBotUserID(ctx, slackClient)
logger.Debug("retrieving the user ID of the chat-roulette Slack bot")

slackCtx, cancel := context.WithTimeout(ctx, 3000*time.Millisecond)
defer cancel()

slackBotUserID, err := bot.GetBotUserID(slackCtx, slackClient)
if err != nil {
return nil, errors.Wrap(err, "failed to retrieve the user ID of the Slack bot")
return nil, errors.Wrap(err, "failed to retrieve the user ID of the chat-roulette Slack bot")
}
logger.Debug("retrieved the user_id of the Slack bot", "slack_bot_user_id", slackBotUserID)
logger.Debug("retrieved the user ID of the chat-roulette Slack bot", "slack_bot_user_id", slackBotUserID)
span.SetAttributes(
attribute.String("slack_bot_user_id", slackBotUserID),
)
Expand Down

0 comments on commit 81dfb3f

Please sign in to comment.