diff --git a/internal/spice/state/branch.go b/internal/spice/state/branch.go index 6edca787..9d50e40d 100644 --- a/internal/spice/state/branch.go +++ b/internal/spice/state/branch.go @@ -354,6 +354,7 @@ func (tx *BranchTx) Delete(ctx context.Context, name string) error { } // Rename changes a tracked branch name while preserving its stored state. +// Commit also moves state owned by the branch, such as review drafts. func (tx *BranchTx) Rename( ctx context.Context, oldName, newName string, @@ -582,16 +583,28 @@ func (s *Store) updateBranches(ctx context.Context, req updateBranchesRequest) e var dels []string for _, del := range req.Deletes { if _, renamed := req.Renames[del]; !renamed { - dels = append(dels, branchKey(del)) + dels = append( + dels, + branchKey(del), + reviewDraftsJSON(del), + ) } } var moves []storage.MoveRequest for _, oldName := range slices.Sorted(maps.Keys(req.Renames)) { - moves = append(moves, storage.MoveRequest{ - From: branchKey(oldName), - To: branchKey(req.Renames[oldName]), - }) + newName := req.Renames[oldName] + moves = append( + moves, + storage.MoveRequest{ + From: branchKey(oldName), + To: branchKey(newName), + }, + storage.MoveRequest{ + From: reviewDraftsJSON(oldName), + To: reviewDraftsJSON(newName), + }, + ) } updReq := storage.UpdateRequest{ diff --git a/internal/spice/state/review_draft.go b/internal/spice/state/review_draft.go index dba7b3ad..753581da 100644 --- a/internal/spice/state/review_draft.go +++ b/internal/spice/state/review_draft.go @@ -45,6 +45,7 @@ func (s *Store) AddReviewDraft( storage.JSONMutationRequest{ Key: reviewDraftsJSON(branch), IfMissing: jsontext.Value(`{}`), + Requires: []string{branchKey(branch)}, Message: fmt.Sprintf("%v: add review draft", branch), }, jsonmut.InsertAutoIncrement( @@ -77,6 +78,7 @@ func (s *Store) UpdateReviewDraftBody( storage.JSONMutationRequest{ Key: reviewDraftsJSON(branch), IfMissing: jsontext.Value(`{}`), + Requires: []string{branchKey(branch)}, Message: fmt.Sprintf("%v: update review draft", branch), }, jsonmut.Replace( diff --git a/internal/spice/state/review_draft_concurrency_test.go b/internal/spice/state/review_draft_concurrency_test.go index ad66d598..352db850 100644 --- a/internal/spice/state/review_draft_concurrency_test.go +++ b/internal/spice/state/review_draft_concurrency_test.go @@ -11,6 +11,7 @@ import ( "go.abhg.dev/gs/internal/review" "go.abhg.dev/gs/internal/silog/silogtest" "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/spice/state/statetest" "go.abhg.dev/gs/internal/spice/state/storage" ) @@ -159,6 +160,14 @@ func newConcurrentReviewDraftStores( stores[i], err = state.OpenStore(ctx, db, silogtest.New(t)) require.NoError(t, err) } + require.NoError(t, statetest.UpdateBranch( + ctx, + stores[0], + &statetest.UpdateRequest{ + Upserts: []state.UpsertRequest{{Name: "feat", Base: "main"}}, + Message: "track feat", + }, + )) return stores, pausingRepo } diff --git a/internal/spice/state/review_draft_test.go b/internal/spice/state/review_draft_test.go index a6c875e3..5cfb3711 100644 --- a/internal/spice/state/review_draft_test.go +++ b/internal/spice/state/review_draft_test.go @@ -20,6 +20,12 @@ func TestReviewDrafts(t *testing.T) { Trunk: "main", }) require.NoError(t, err) + tx := store.BeginBranchTx() + require.NoError(t, tx.Upsert(ctx, state.UpsertRequest{ + Name: "feature", + Base: "main", + })) + require.NoError(t, tx.Commit(ctx, "track feature")) comment, err := store.AddReviewDraft( ctx, @@ -62,3 +68,137 @@ func TestReviewDrafts(t *testing.T) { require.NoError(t, err) assert.Nil(t, drafts) } + +func TestReviewDraftsFollowBranchLifecycle(t *testing.T) { + t.Parallel() + + t.Run("Delete", func(t *testing.T) { + ctx := t.Context() + db := storage.NewDB(make(storage.MapBackend)) + store, err := state.InitStore(ctx, state.InitStoreRequest{ + DB: db, + Trunk: "main", + }) + require.NoError(t, err) + + tx := store.BeginBranchTx() + require.NoError(t, tx.Upsert(ctx, state.UpsertRequest{ + Name: "feature", + Base: "main", + })) + require.NoError(t, tx.Commit(ctx, "track feature")) + + _, err = store.AddReviewDraft( + ctx, + "feature", + review.Draft{ + ID: 0, + Body: "comment body", + Anchor: review.Anchor{ + Path: "main.go", + StartLine: 42, + EndLine: 42, + }, + }, + ) + require.NoError(t, err) + + tx = store.BeginBranchTx() + require.NoError(t, tx.Delete(ctx, "feature")) + require.NoError(t, tx.Commit(ctx, "untrack feature")) + + drafts, err := store.LoadReviewDrafts(ctx, "feature") + require.NoError(t, err) + assert.Nil(t, drafts) + }) + + t.Run("Rename", func(t *testing.T) { + ctx := t.Context() + db := storage.NewDB(make(storage.MapBackend)) + store, err := state.InitStore(ctx, state.InitStoreRequest{ + DB: db, + Trunk: "main", + }) + require.NoError(t, err) + + tx := store.BeginBranchTx() + require.NoError(t, tx.Upsert(ctx, state.UpsertRequest{ + Name: "feature", + Base: "main", + })) + require.NoError(t, tx.Commit(ctx, "track feature")) + + _, err = store.AddReviewDraft( + ctx, + "feature", + review.Draft{ + ID: 0, + Body: "comment body", + Anchor: review.Anchor{ + Path: "main.go", + StartLine: 42, + EndLine: 42, + }, + }, + ) + require.NoError(t, err) + + tx = store.BeginBranchTx() + require.NoError(t, tx.Rename(ctx, "feature", "renamed")) + require.NoError(t, tx.Commit(ctx, "rename feature")) + + drafts, err := store.LoadReviewDrafts(ctx, "feature") + require.NoError(t, err) + assert.Nil(t, drafts) + + drafts, err = store.LoadReviewDrafts(ctx, "renamed") + require.NoError(t, err) + require.Len(t, drafts, 1) + assert.Equal(t, "comment body", drafts[0].Body) + + added, err := store.AddReviewDraft( + ctx, + "renamed", + review.Draft{ + ID: 0, + Body: "second comment", + Anchor: review.Anchor{ + Path: "main.go", + StartLine: 42, + EndLine: 42, + }, + }, + ) + require.NoError(t, err) + assert.Equal(t, review.DraftID(2), added.ID) + }) + + t.Run("Untracked", func(t *testing.T) { + ctx := t.Context() + db := storage.NewDB(make(storage.MapBackend)) + store, err := state.InitStore(ctx, state.InitStoreRequest{ + DB: db, + Trunk: "main", + }) + require.NoError(t, err) + + _, err = store.AddReviewDraft( + ctx, + "feature", + review.Draft{ + ID: 0, + Body: "comment body", + Anchor: review.Anchor{ + Path: "main.go", + StartLine: 42, + EndLine: 42, + }, + }, + ) + assert.ErrorIs(t, err, state.ErrNotExist) + + drafts, err := store.LoadReviewDrafts(ctx, "feature") + require.NoError(t, err) + assert.Nil(t, drafts) + }) +}