refactor(rotation): update calcAdvance to return error and improve st…

…ate handling (#4292)

engine/rotationmanager/advance.go

@@ -17,41 +17,49 @@ type advance struct {
 }
 
 type rotState struct {
-	rotation.State
-	Version int
+	ShiftStart time.Time
+	Position   int
+	Version    int
 }
 
 // calcAdvance will calculate rotation advancement if it is required. If not, nil is returned
-func calcAdvance(ctx context.Context, t time.Time, rot *rotation.Rotation, state rotState, partCount int) *advance {
+func calcAdvance(ctx context.Context, t time.Time, rot *rotation.Rotation, state rotState, partCount int) (*advance, error) {
 	var mustUpdate bool
-	origPos := state.Position
 
-	// get next shift start time
-	newStart := rot.EndTime(state.ShiftStart)
-	if state.Version == 1 {
-		newStart = calcVersion1EndTime(rot, state.ShiftStart)
+	if state.Position >= partCount {
 		mustUpdate = true
+		state.Position = 0
 	}
 
-	if state.Position >= partCount {
-		// deleted last participant
-		state.Position = 0
+	endTimeFunc := rot.EndTime
+
+	switch state.Version {
+	case 1:
+		// for a V1 state, use the old calculation method
 		mustUpdate = true
+		endTimeFunc = func(t time.Time) time.Time {
+			return calcVersion1EndTime(rot, t)
+		}
+	case 2:
+		// no-op
+	default:
+		return nil, fmt.Errorf("unknown rotation state version (supported: 1,2): %d", state.Version)
 	}
 
-	if newStart.After(t) || state.Version == 1 {
+	// get next shift start time
+	newStart := endTimeFunc(state.ShiftStart)
+	if newStart.After(t) {
 		if mustUpdate {
+			// we need to update the rotation state to v2, which will reset the start time and make it compatible with v2, but we
+			// don't need to change the position (unless it was due to participant deletion).
 			return &advance{
 				id:          rot.ID,
 				newPosition: state.Position,
-
-				// If migrating from version 1 to 2 without changing
-				// who's on-call do so silently.
-				silent: state.Version == 1 && state.Position == origPos,
-			}
+			}, nil
 		}
+
 		// in the future, so nothing to do yet
-		return nil
+		return nil, nil
 	}
 
 	if !newStart.After(t.Add(-15 * time.Minute)) {
@@ -68,7 +76,7 @@ func calcAdvance(ctx context.Context, t time.Time, rot *rotation.Rotation, state
 		}
 
 		state.Position = (state.Position + 1) % partCount
-		end := rot.EndTime(state.ShiftStart)
+		end := endTimeFunc(state.ShiftStart)
 		if end.After(t) {
 			break
 		}
@@ -78,5 +86,5 @@ func calcAdvance(ctx context.Context, t time.Time, rot *rotation.Rotation, state
 	return &advance{
 		id:          rot.ID,
 		newPosition: state.Position,
-	}
+	}, nil
 }

engine/rotationmanager/update.go

@@ -122,7 +122,10 @@ func (db *DB) calcAdvances(ctx context.Context, tx *sql.Tx, all bool, rotID *str
 			return nil, errors.Wrap(err, "load timezone")
 		}
 		rot.Start = rot.Start.In(loc)
-		adv = calcAdvance(ctx, t, &rot, state, partCount)
+		adv, err = calcAdvance(ctx, t, &rot, state, partCount)
+		if err != nil {
+			return nil, errors.Wrap(err, "calculate rotation advance")
+		}
 		if adv != nil {
 			needsAdvance = append(needsAdvance, *adv)
 			if len(needsAdvance) == 150 {