diff --git a/internal/review/service.go b/internal/review/service.go index 20e1a31..6304376 100644 --- a/internal/review/service.go +++ b/internal/review/service.go @@ -61,7 +61,7 @@ func (s Service) Approve(ctx context.Context, first, last string) (Result, error } store := Store{Path: view.Path} err = store.Update(func(state *State) error { - if err := approvalAllowed(view, *state); err != nil { + if err := approvalAllowed(selected, *state); err != nil { return err } if err := s.unchanged(ctx, view); err != nil { @@ -241,11 +241,11 @@ func activeAttempts(state State) []Attempt { return active } -func approvalAllowed(view Snapshot, state State) error { +func approvalAllowed(selected []Stack, state State) error { if len(activeAttempts(state)) > 0 { return fmt.Errorf("recover or replace the active review before approval") } - for _, stack := range view.Stacks { + for _, stack := range selected { a := state.latest(stack) if a == nil { continue diff --git a/internal/review/state_test.go b/internal/review/state_test.go index fe45552..33a1f8e 100644 --- a/internal/review/state_test.go +++ b/internal/review/state_test.go @@ -67,6 +67,43 @@ func TestApprovalBlocksCurrentFeedbackAndActiveAttempts(t *testing.T) { } } +func TestApprovalWithPendingAttemptsOutsideRange(t *testing.T) { + for _, status := range []string{"feedback", "reviewing"} { + t.Run(status, func(t *testing.T) { + r := testRepo(t) + for _, subject := range []string{"Before", "First", "Last", "After"} { + gitTest(t, r.Dir, "commit", "--allow-empty", "-m", subject) + } + view, err := r.Snapshot(t.Context()) + require.NoError(t, err) + store := Store{Path: view.Path} + require.NoError(t, store.Update(func(state *State) error { + state.Attempts = append(state.Attempts, + newAttempt(view.Stacks[0], status), + newAttempt(view.Stacks[3], status)) + return nil + })) + service := Service{Repository: r} + _, err = service.Approve(t.Context(), view.Stacks[1].Key, view.Stacks[2].Tip) + if status == "reviewing" { + require.ErrorContains(t, err, "active review") + state, err := store.Load() + require.NoError(t, err) + require.Len(t, state.Attempts, 2) + return + } + require.NoError(t, err) + result, err := service.Status(t.Context(), false) + require.NoError(t, err) + require.Len(t, result.Entries, 4) + require.Equal(t, "feedback", result.Entries[0].Status) + require.Equal(t, "approved", result.Entries[1].Status) + require.Equal(t, "approved", result.Entries[2].Status) + require.Equal(t, "feedback", result.Entries[3].Status) + }) + } +} + func TestStateLockAndCorruption(t *testing.T) { r := testRepo(t) gitTest(t, r.Dir, "commit", "--allow-empty", "-m", "One")