From 81dfb3f9175e9462f997bf691920484825b43f21 Mon Sep 17 00:00:00 2001 From: Ali Date: Sun, 20 Oct 2024 08:18:26 +0400 Subject: [PATCH] Skip queuing ADD_MEMBER job for the chat-roulette bot --- internal/bot/bot_user.go | 5 ++--- internal/bot/bot_user_test.go | 4 ++-- internal/bot/job_add_member.go | 16 +++++++--------- internal/bot/job_add_member_test.go | 2 +- internal/bot/job_greet_admin.go | 1 + internal/bot/job_sync_members.go | 23 ++++++++++++++++++++++- internal/bot/job_sync_members_test.go | 3 +++ internal/server/server.go | 12 ++++++++---- 8 files changed, 46 insertions(+), 20 deletions(-) diff --git a/internal/bot/bot_user.go b/internal/bot/bot_user.go index 787e012..7d031a3 100644 --- a/internal/bot/bot_user.go +++ b/internal/bot/bot_user.go @@ -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 diff --git a/internal/bot/bot_user_test.go b/internal/bot/bot_user_test.go index 551f0d8..5917995 100644 --- a/internal/bot/bot_user_test.go +++ b/internal/bot/bot_user_test.go @@ -41,7 +41,7 @@ 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) }) @@ -49,7 +49,7 @@ func Test_IsUserABot(t *testing.T) { 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) }) } diff --git a/internal/bot/job_add_member.go b/internal/bot/job_add_member.go index 4badf4c..a56316d 100644 --- a/internal/bot/job_add_member.go +++ b/internal/bot/job_add_member.go @@ -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 } @@ -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 diff --git a/internal/bot/job_add_member_test.go b/internal/bot/job_add_member_test.go index 1e33555..cf6acfe 100644 --- a/internal/bot/job_add_member_test.go +++ b/internal/bot/job_add_member_test.go @@ -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() { diff --git a/internal/bot/job_greet_admin.go b/internal/bot/job_greet_admin.go index f5d78a6..56849d2 100644 --- a/internal/bot/job_greet_admin.go +++ b/internal/bot/job_greet_admin.go @@ -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 { diff --git a/internal/bot/job_sync_members.go b/internal/bot/job_sync_members.go index df3e695..d1ccb99 100644 --- a/internal/bot/job_sync_members.go +++ b/internal/bot/job_sync_members.go @@ -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) @@ -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, diff --git a/internal/bot/job_sync_members_test.go b/internal/bot/job_sync_members_test.go index 455045d..bcf9044 100644 --- a/internal/bot/job_sync_members_test.go +++ b/internal/bot/job_sync_members_test.go @@ -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 @@ -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() { diff --git a/internal/server/server.go b/internal/server/server.go index dd5682c..e95bedd 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -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), )