Skip to content
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
21 changes: 21 additions & 0 deletions doc/includes/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1600,6 +1600,27 @@ with the current comment body pre-filled.
* `-m`, `--message=MSG`: New comment body. Opens editor if not provided.
* `-b`, `--branch=BRANCH`: Branch containing the draft. Defaults to the current branch.

### git-spice review delete {#gs-review-delete}

```
gs review delete (rm) <draft> ... [flags]
```

Delete draft comments

Deletes one or more local draft comments.

Use 'gs review list --draft-only'
to find the branch-local draft IDs.

**Arguments**

* `draft`: Draft comment IDs to delete.

**Flags**

* `-b`, `--branch=BRANCH`: Branch containing the drafts. Defaults to the current branch.

### git-spice review resolve {#gs-review-resolve}

```
Expand Down
28 changes: 28 additions & 0 deletions internal/handler/review/delete.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package review

import (
"context"
"fmt"
)

// DeleteDraftsRequest describes local review drafts to delete.
type DeleteDraftsRequest struct {
// Branch identifies the branch containing the drafts.
Branch string // required

// IDs identify drafts within the branch.
IDs []DraftID // required
}

// DeleteDrafts removes local review drafts.
func (h *DraftHandler) DeleteDrafts(
ctx context.Context,
req *DeleteDraftsRequest,
) error {
deleted, err := h.Store.DeleteReviewDrafts(ctx, req.Branch, req.IDs)
if err != nil {
return fmt.Errorf("delete draft comments: %w", err)
}
h.Log.Infof("Deleted %d draft comment(s).", deleted)
return nil
}
1 change: 1 addition & 0 deletions internal/handler/review/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ var _ Service = (*spice.Service)(nil)
// Store persists branch-local review drafts.
type Store interface {
AddReviewDraft(context.Context, string, review.Draft) (review.Draft, error)
DeleteReviewDrafts(context.Context, string, []review.DraftID) (int, error)
LoadReviewDrafts(context.Context, string) ([]review.Draft, error)
RemovePublishedReviewDrafts(context.Context, string, []review.Draft) error
UpdateReviewDraftBody(context.Context, string, review.DraftID, string) error
Expand Down
25 changes: 25 additions & 0 deletions internal/handler/review/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,31 @@ func TestDraftHandler_ReplaceDraftBody(t *testing.T) {
require.NoError(t, err)
}

func TestDraftHandler_DeleteDrafts(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockStore(ctrl)
handler := &DraftHandler{
Log: silog.Nop(),
Store: store,
Editor: nil,
}

store.
EXPECT().
DeleteReviewDrafts(
gomock.Any(),
"feature",
[]DraftID{1, 3},
).
Return(2, nil)

err := handler.DeleteDrafts(t.Context(), &DeleteDraftsRequest{
Branch: "feature",
IDs: []DraftID{1, 3},
})
require.NoError(t, err)
}

func TestHandler_LoadReviewData(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockStore(ctrl)
Expand Down
39 changes: 39 additions & 0 deletions internal/handler/review/mocks_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 54 additions & 0 deletions internal/spice/state/review_draft.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,60 @@ func (s *Store) AddReviewDraft(
return draft, nil
}

// DeleteReviewDrafts atomically removes drafts by branch-local ID.
func (s *Store) DeleteReviewDrafts(
ctx context.Context,
branch string,
ids []review.DraftID,
) (int, error) {
requested := make(map[review.DraftID]struct{}, len(ids))
for _, id := range ids {
requested[id] = struct{}{}
}

statements := make([]jsonmut.Statement, 0, len(requested))
for _, id := range slices.Sorted(maps.Keys(requested)) {
path := jsontext.Pointer("/drafts").AppendToken(id.String())
statements = append(statements,
jsonmut.Lookup(path).Then(func(value jsontext.Value) jsonmut.Statement {
if len(value) == 0 {
return jsonmut.Fail[struct{}](
&reviewDraftNotFoundError{ID: id},
)
}
return jsonmut.Delete(path)
}),
)
}

err := storage.UpdateJSON(
ctx,
s.db,
storage.JSONMutationRequest{
Key: reviewDraftsJSON(branch),
IfMissing: jsontext.Value(`{}`),
Requires: []string{branchKey(branch)},
Message: fmt.Sprintf("%v: delete review drafts", branch),
},
jsonmut.Block(statements...),
)
if notFound, ok := errors.AsType[*reviewDraftNotFoundError](err); ok {
return 0, notFound
}
if err != nil {
return 0, fmt.Errorf("delete review drafts: %w", err)
}
return len(requested), nil
}

type reviewDraftNotFoundError struct {
ID review.DraftID
}

func (e *reviewDraftNotFoundError) Error() string {
return fmt.Sprintf("draft comment %d not found", e.ID)
}

// UpdateReviewDraftBody atomically replaces one draft's body.
func (s *Store) UpdateReviewDraftBody(
ctx context.Context,
Expand Down
55 changes: 55 additions & 0 deletions internal/spice/state/review_draft_concurrency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,61 @@ func TestReviewDraftsConcurrentEdit(t *testing.T) {
assert.Equal(t, "Second edited", drafts[1].Body)
}

func TestReviewDraftsConcurrentDeleteAndAdd(t *testing.T) {
ctx := t.Context()
stores, repo := newConcurrentReviewDraftStores(t)
_, err := stores[0].AddReviewDraft(
ctx,
"feat",
review.Draft{
ID: 0,
Body: "First",
Anchor: review.Anchor{
Path: "first.go",
StartLine: 1,
EndLine: 1,
},
},
)
require.NoError(t, err)

var added review.Draft
repo.pauseNextRefUpdates(2)
errs := runConcurrently(
func() error {
_, err := stores[0].DeleteReviewDrafts(
ctx,
"feat",
[]review.DraftID{1},
)
return err
},
func() (err error) {
added, err = stores[1].AddReviewDraft(
ctx,
"feat",
review.Draft{
ID: 0,
Body: "Second",
Anchor: review.Anchor{
Path: "second.go",
StartLine: 2,
EndLine: 2,
},
},
)
return err
},
)
require.NoError(t, errs[0])
require.NoError(t, errs[1])

drafts, err := stores[0].LoadReviewDrafts(ctx, "feat")
require.NoError(t, err)
require.NotNil(t, drafts)
assert.Equal(t, []review.Draft{added}, drafts)
}

func newConcurrentReviewDraftStores(
t *testing.T,
) ([2]*state.Store, *pausingGitRepository) {
Expand Down
85 changes: 85 additions & 0 deletions internal/spice/state/review_draft_delete_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package state_test

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.abhg.dev/gs/internal/review"
"go.abhg.dev/gs/internal/spice/state"
"go.abhg.dev/gs/internal/spice/state/storage"
)

func TestReviewDraftsDelete(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: "feat",
Base: "main",
}))
require.NoError(t, tx.Commit(ctx, "track feat"))

var added []review.Draft
for _, body := range []string{"First", "Second", "Third"} {
draft, err := store.AddReviewDraft(
ctx,
"feat",
review.Draft{
ID: 0,
Body: body,
Anchor: review.Anchor{
Path: "main.go",
StartLine: 1,
EndLine: 1,
},
},
)
require.NoError(t, err)
added = append(added, draft)
}

deleted, err := store.DeleteReviewDrafts(
ctx,
"feat",
[]review.DraftID{1, 3, 3},
)
require.NoError(t, err)
assert.Equal(t, 2, deleted)
drafts, err := store.LoadReviewDrafts(ctx, "feat")
require.NoError(t, err)
require.NotNil(t, drafts)
assert.Equal(t, []review.Draft{added[1]}, drafts)

_, err = store.DeleteReviewDrafts(
ctx,
"feat",
[]review.DraftID{2, 99},
)
assert.EqualError(t, err, "draft comment 99 not found")
drafts, err = store.LoadReviewDrafts(ctx, "feat")
require.NoError(t, err)
assert.Equal(t, []review.Draft{added[1]}, drafts)
}

func TestReviewDraftsDeleteFromUntrackedBranch(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.DeleteReviewDrafts(
ctx,
"feat",
[]review.DraftID{1},
)
assert.ErrorIs(t, err, state.ErrNotExist)
}
2 changes: 2 additions & 0 deletions review.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type reviewCmd struct {
Publish reviewPublishCmd `cmd:"" help:"Publish draft comments as a review"`
List reviewListCmd `cmd:"" aliases:"ls" help:"List review comments"`
Edit reviewEditCmd `cmd:"" help:"Edit a draft comment"`
Delete reviewDeleteCmd `cmd:"" aliases:"rm" help:"Delete draft comments"`
Resolve reviewResolveCmd `cmd:"" help:"Resolve a review thread"`
Reopen reviewReopenCmd `cmd:"" help:"Reopen a resolved review thread"`
}
Expand Down Expand Up @@ -97,6 +98,7 @@ type ReviewHandler interface {
type ReviewDraftHandler interface {
SaveCommentDraft(context.Context, *review.CommentRequest) error
SaveReplyDraft(context.Context, *review.ReplyRequest) error
DeleteDrafts(context.Context, *review.DeleteDraftsRequest) error
ReplaceDraftBody(context.Context, *review.ReplaceDraftBodyRequest) error
}

Expand Down
Loading
Loading