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

rotation: maintain active user when deleting a user on the rotation #2375

Merged
merged 2 commits into from
May 18, 2022
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
63 changes: 63 additions & 0 deletions smoketest/rotationdeluser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package smoketest

import (
"encoding/json"
"fmt"
"testing"

"github.com/target/goalert/smoketest/harness"
)

// RotationDelUser tests that rotations preserve the active user when a user is deleted.
func TestRotationDelUser(t *testing.T) {
t.Parallel()

const sql = `
insert into users (id, name, email)
values
({{uuid "uid1"}}, 'bob', 'joe'),
({{uuid "uid2"}}, 'ben', 'frank'),
({{uuid "uid3"}}, 'joe', 'bob');

insert into rotations (id, name, type, start_time, shift_length, time_zone)
values
({{uuid "rot1"}}, 'default rotation', 'daily', now(), 1, 'UTC');

insert into rotation_participants (rotation_id, user_id, position)
values
({{uuid "rot1"}}, {{uuid "uid1"}}, 0),
({{uuid "rot1"}}, {{uuid "uid2"}}, 1),
({{uuid "rot1"}}, {{uuid "uid3"}}, 2);
`
h := harness.NewHarness(t, sql, "add-daily-alert-metrics")
defer h.Close()

h.GraphQLQuery2(fmt.Sprintf(
`mutation{updateRotation(input:{id:"%s", activeUserIndex: 1})}`,
h.UUID("rot1"),
))

h.GraphQLQuery2(fmt.Sprintf(
`mutation{deleteAll(input:[{id:"%s", type: user}])}`,
h.UUID("uid1"),
))

resp := h.GraphQLQuery2(fmt.Sprintf(
`query{rotation(id:"%s"){activeUserIndex}}`,
h.UUID("rot1"),
))
var data struct {
Rotation struct {
ActiveUserIndex int
}
}
err := json.Unmarshal(resp.Data, &data)
if err != nil {
t.Fatal(err)
return
}
// 2nd user is now first, so index should be zero
if data.Rotation.ActiveUserIndex != 0 {
t.Errorf("expected activeUserIndex to be 0, got %d", data.Rotation.ActiveUserIndex)
}
}
23 changes: 22 additions & 1 deletion user/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ type Store struct {
rotationParts *sql.Stmt
updateRotationPart *sql.Stmt
deleteRotationPart *sql.Stmt
rotActiveIndex *sql.Stmt
rotSetActive *sql.Stmt

findOneForUpdate *sql.Stmt

Expand Down Expand Up @@ -85,6 +87,9 @@ func NewStore(ctx context.Context, db *sql.DB) (*Store, error) {
WHERE id = $1
`),

rotActiveIndex: p.P(`SELECT position FROM rotation_state WHERE rotation_id = $1 FOR UPDATE`),
rotSetActive: p.P(`UPDATE rotation_state SET position = $2, rotation_participant_id = $3 WHERE rotation_id = $1`),

setUserRole: p.P(`UPDATE users SET role = $2 WHERE id = $1`),
findAuthSubjects: p.P(`
select subject_id, user_id, provider_id
Expand Down Expand Up @@ -343,6 +348,7 @@ func withTx(ctx context.Context, tx *sql.Tx, stmt *sql.Stmt) *sql.Stmt {

return tx.StmtContext(ctx, stmt)
}

func (s *Store) requireTx(ctx context.Context, tx *sql.Tx, fn func(*sql.Tx) error) error {
return nil
}
Expand Down Expand Up @@ -456,12 +462,22 @@ func (s *Store) removeUserFromRotation(ctx context.Context, tx *sql.Tx, userID,
participants = append(participants, p)
}

var activeIndex int
err = tx.StmtContext(ctx, s.rotActiveIndex).QueryRowContext(ctx, rotationID).Scan(&activeIndex)
if err != nil {
return fmt.Errorf("query active index: %w", err)
}

// update participant user IDs
var skipped bool
curIndex := -1
updatePart := tx.StmtContext(ctx, s.updateRotationPart)
for _, p := range participants {
for i, p := range participants {
if p.UserID == userID {
if i < activeIndex {
activeIndex--
}

skipped = true
continue
}
Expand All @@ -483,6 +499,11 @@ func (s *Store) removeUserFromRotation(ctx context.Context, tx *sql.Tx, userID,
}
}

_, err = tx.StmtContext(ctx, s.rotSetActive).ExecContext(ctx, rotationID, activeIndex, participants[activeIndex].ID)
if err != nil {
return fmt.Errorf("set active index: %w", err)
}

return nil
}

Expand Down