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

MM-60420: treat bots as participants #1939

Merged
merged 1 commit into from
Sep 13, 2024
Merged
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
19 changes: 10 additions & 9 deletions server/sqlstore/playbook_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,23 +156,26 @@ func NewPlaybookRunStore(pluginAPI PluginAPIClient, sqlStore *SQLStore) app.Play
// the user is not a member of the channel they won't have permissions to get the user list
participantsCol := `
COALESCE(
(SELECT string_agg(rp.UserId, ',')
(SELECT string_agg(x.UserId, ',') FROM (
SELECT rp.UserId
FROM IR_Incident as i2
JOIN IR_Run_Participants as rp on rp.IncidentID = i2.ID
LEFT JOIN Bots b ON (b.UserId = rp.UserId)
WHERE i2.Id = i.Id
AND rp.IsParticipant = true
AND rp.UserId NOT IN (SELECT UserId FROM Bots)
), ''
ORDER BY (CASE WHEN b.UserId IS NULL THEN 0 ELSE 1 END), rp.UserId
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still JOIN so we can order bots after human participants.

) x), ''
) AS ConcatenatedParticipantIDs`
if sqlStore.db.DriverName() == model.DatabaseDriverMysql {
participantsCol = `
COALESCE(
(SELECT group_concat(rp.UserId separator ',')
FROM IR_Incident as i2
JOIN IR_Run_Participants as rp on rp.IncidentID = i2.ID
LEFT JOIN Bots b ON (b.UserId = rp.UserId)
WHERE i2.Id = i.Id
AND rp.IsParticipant = true
AND rp.UserId NOT IN (SELECT UserId FROM Bots)
ORDER BY (CASE WHEN b.UserId IS NULL THEN 0 ELSE 1 END), rp.UserId
), ''
) AS ConcatenatedParticipantIDs`
}
Expand Down Expand Up @@ -891,13 +894,12 @@ func (s *playbookRunStore) GetPlaybookRunIDsForChannel(channelID string) ([]stri
}

// GetHistoricalPlaybookRunParticipantsCount returns the count of all members of a playbook run's channel
// since the beginning of the playbook run, excluding bots.
// since the beginning of the playbook run.
Comment on lines 896 to +897
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is effectively "retroactive."

func (s *playbookRunStore) GetHistoricalPlaybookRunParticipantsCount(channelID string) (int64, error) {
query := s.queryBuilder.
Select("COUNT(DISTINCT cmh.UserId)").
From("ChannelMemberHistory AS cmh").
Where(sq.Eq{"cmh.ChannelId": channelID}).
Where(sq.Expr("cmh.UserId NOT IN (SELECT UserId FROM Bots)"))
Where(sq.Eq{"cmh.ChannelId": channelID})

var numParticipants int64
err := s.store.getBuilder(s.store.db, &numParticipants, query)
Expand Down Expand Up @@ -1380,8 +1382,7 @@ func (s *playbookRunStore) GetParticipantsActiveTotal() (int64, error) {
From("IR_Run_Participants as rp").
Join("IR_Incident AS i ON i.ID = rp.IncidentID").
Where(sq.Eq{"i.CurrentStatus": app.StatusInProgress}).
Where(sq.Eq{"rp.IsParticipant": true}).
Where(sq.Expr("rp.UserId NOT IN (SELECT UserId FROM Bots)"))
Where(sq.Eq{"rp.IsParticipant": true})

if err := s.store.getBuilder(s.store.db, &count, query); err != nil {
return 0, errors.Wrap(err, "failed to count active participants")
Expand Down
2 changes: 1 addition & 1 deletion server/sqlstore/playbook_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,7 @@ func TestGetParticipantsActiveTotal(t *testing.T) {
createRuns(store, playbookRunStore, playbook1ID, []userInfo{alice, bob, bot1}, team1ID, 3, app.StatusInProgress)
createRuns(store, playbookRunStore, playbook2ID, []userInfo{tom, bob}, team2ID, 5, app.StatusInProgress)

expected := 2*3 + 2*5 // ignore bots
expected := 3*3 + 2*5
actual, err := playbookRunStore.GetParticipantsActiveTotal()
require.NoError(t, err)
require.Equal(t, int64(expected), actual)
Expand Down
6 changes: 2 additions & 4 deletions server/sqlstore/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ func (s *StatsStore) TotalActiveParticipants(filters *StatsFilters) int {
From("IR_Run_Participants as rp").
Join("IR_Incident AS i ON i.ID = rp.IncidentID").
Where("i.EndAt = 0").
Where("rp.IsParticipant = true").
Where(sq.Expr("rp.UserId NOT IN (SELECT UserId FROM Bots)"))
Where("rp.IsParticipant = true")

query = applyFilters(query, filters)

Expand Down Expand Up @@ -316,8 +315,7 @@ func (s *StatsStore) ActiveParticipantsPerDayLastXDays(x int, filters *StatsFilt

q = q.
From("IR_Incident as i").
InnerJoin("ChannelMemberHistory as cmh ON i.ChannelId = cmh.ChannelId").
Where(sq.Expr("cmh.UserId NOT IN (SELECT UserId FROM Bots)"))
InnerJoin("ChannelMemberHistory as cmh ON i.ChannelId = cmh.ChannelId")
q = applyFilters(q, filters)

counts, err := s.performQueryForXCols(q, x)
Expand Down
14 changes: 7 additions & 7 deletions server/sqlstore/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,41 +196,41 @@ func TestTotalInProgressPlaybookRuns(t *testing.T) {
}

addUsersToRuns(t, store, []userInfo{bob, lucy, phil}, []string{playbookRuns[0].ID, playbookRuns[1].ID, playbookRuns[2].ID, playbookRuns[3].ID, playbookRuns[5].ID, playbookRuns[6].ID, playbookRuns[7].ID, playbookRuns[8].ID})
addUsersToRuns(t, store, []userInfo{bob, quincy}, []string{playbookRuns[4].ID})
addUsersToRuns(t, store, []userInfo{john}, []string{playbookRuns[0].ID})
addUsersToRuns(t, store, []userInfo{bob, quincy, bot1}, []string{playbookRuns[4].ID})
addUsersToRuns(t, store, []userInfo{john, bot2}, []string{playbookRuns[0].ID})
addUsersToRuns(t, store, []userInfo{jane}, []string{playbookRuns[0].ID, playbookRuns[1].ID})

t.Run(driverName+" Active Participants - team1", func(t *testing.T) {
result := statsStore.TotalActiveParticipants(&StatsFilters{
TeamID: team1id,
})
assert.Equal(t, 5, result)
assert.Equal(t, 6, result)
})

t.Run(driverName+" Active Participants - team2", func(t *testing.T) {
result := statsStore.TotalActiveParticipants(&StatsFilters{
TeamID: team2id,
})
assert.Equal(t, 4, result)
assert.Equal(t, 5, result)
})

t.Run(driverName+" Active Participants, playbook1", func(t *testing.T) {
result := statsStore.TotalActiveParticipants(&StatsFilters{
PlaybookID: "playbook1",
})
assert.Equal(t, 5, result)
assert.Equal(t, 6, result)
})

t.Run(driverName+" Active Participants, playbook2", func(t *testing.T) {
result := statsStore.TotalActiveParticipants(&StatsFilters{
PlaybookID: "playbook2",
})
assert.Equal(t, 4, result)
assert.Equal(t, 5, result)
})

t.Run(driverName+" Active Participants, all", func(t *testing.T) {
result := statsStore.TotalActiveParticipants(&StatsFilters{})
assert.Equal(t, 6, result)
assert.Equal(t, 8, result)
})

t.Run(driverName+" In-progress Playbook Runs - team1", func(t *testing.T) {
Expand Down
Loading