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
4 changes: 0 additions & 4 deletions internal/handler/review/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ func (h *DraftHandler) SaveCommentDraft(
ctx context.Context,
req *CommentRequest,
) error {
if !req.Anchor.IsLine() {
return errors.New("draft comments require a single-line file:line anchor")
}

body, err := h.commentBody(ctx, req.Message)
if err != nil {
return err
Expand Down
9 changes: 8 additions & 1 deletion internal/handler/review/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,14 @@ func (h *Handler) PublishDrafts(
continue
}

if !patch.ContainsLineRange(
if draft.Anchor.IsFile() && !patch.ContainsFile(draft.Anchor.Path) {
return fmt.Errorf(
"draft %s: review diff does not contain file %q",
draft.ID,
draft.Anchor.Path,
)
}
if !draft.Anchor.IsFile() && !patch.ContainsLineRange(
draft.Anchor.Path,
draft.Anchor.StartLine,
draft.Anchor.EndLine,
Expand Down
8 changes: 7 additions & 1 deletion internal/spice/state/review_draft.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type reviewDraftState struct {
type storedReviewDraft struct {
File string `json:"file"`
Line int `json:"line"`
EndLine int `json:"endLine,omitempty"`
Body string `json:"body"`
ThreadID string `json:"threadID,omitempty"`
}
Expand Down Expand Up @@ -220,13 +221,17 @@ func (s *Store) LoadReviewDrafts(
continue
}

endLine := stored.EndLine
if stored.Line != 0 && endLine == 0 {
endLine = stored.Line
}
drafts[i] = review.Draft{
ID: id,
Body: stored.Body,
Anchor: review.Anchor{
Path: stored.File,
StartLine: stored.Line,
EndLine: stored.Line,
EndLine: endLine,
},
}
}
Expand Down Expand Up @@ -258,6 +263,7 @@ func storeReviewDraft(draft review.Draft) storedReviewDraft {

stored.File = draft.Anchor.Path
stored.Line = draft.Anchor.StartLine
stored.EndLine = draft.Anchor.EndLine
return stored
}

Expand Down
46 changes: 46 additions & 0 deletions internal/spice/state/review_draft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,49 @@ func TestReviewDraftsFollowBranchLifecycle(t *testing.T) {
assert.Nil(t, drafts)
})
}

func TestReviewDraftScopes(t *testing.T) {
t.Parallel()

ctx := t.Context()
store, err := state.InitStore(ctx, state.InitStoreRequest{
DB: storage.NewDB(make(storage.MapBackend)),
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"))

fileDraft, err := store.AddReviewDraft(
ctx,
"feature",
review.Draft{
ID: 0,
Body: "file body",
Anchor: review.Anchor{Path: "main.go"},
},
)
require.NoError(t, err)
rangeDraft, err := store.AddReviewDraft(
ctx,
"feature",
review.Draft{
ID: 0,
Body: "range body",
Anchor: review.Anchor{
Path: "main.go",
StartLine: 2,
EndLine: 4,
},
},
)
require.NoError(t, err)

drafts, err := store.LoadReviewDrafts(ctx, "feature")
require.NoError(t, err)
assert.Equal(t, []review.Draft{fileDraft, rangeDraft}, drafts)
}
2 changes: 2 additions & 0 deletions review_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/alecthomas/kong"
"go.abhg.dev/gs/internal/forge"
"go.abhg.dev/gs/internal/git"
"go.abhg.dev/gs/internal/handler/review"
"go.abhg.dev/gs/internal/silog"
Expand Down Expand Up @@ -193,6 +194,7 @@ func reviewDraftToJSON(draft review.Draft) jsonComment {
comment.Scope = "file"
} else {
comment.Scope = "line"
comment.Side = forge.ReviewThreadSideRight.String()
if !draft.Anchor.IsLine() {
comment.Range = &jsonCommentRange{
Start: draft.Anchor.StartLine,
Expand Down
82 changes: 82 additions & 0 deletions testdata/script/review_draft_scopes.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Draft and publish file-level and line-range review comments.

as 'Test <test@example.com>'
at '2024-04-05T16:40:32Z'

cd repo
git init
git commit --allow-empty -m 'Initial commit'

gs repo init
shamhub-setup
shamhub new origin alice/example.git
shamhub register alice
git push origin main

env SHAMHUB_USERNAME=alice
gs auth login

git add main.go
gs bc -m 'Add main' feature1
gs branch submit --fill
stderr 'Created #'

# Draft the same file and range anchors accepted by immediate comments.
gs review comment main.go:4-5 -m 'Range draft.'
stderr 'Drafted comment 1 on main.go:4-5'
gs review comment main.go -m 'File draft.'
stderr 'Drafted comment 2 on main.go'

# Text and JSON output preserve each draft's complete anchor.
gs review list --draft-only
stderr 'main\.go:4-5'
stderr 'main\.go$'
gs review list --draft-only --json
cmp stdout $WORK/golden/drafts.ndjson

# Publishing retains the file and inclusive range scopes.
gs review publish
stderr 'Published 2 comment'
shamhub dump reviews 1
cmp stdout $WORK/golden/reviews.yaml

-- repo/main.go --
package main

func main() {
println("hello")
println("world")
}
-- golden/drafts.ndjson --
{"kind":"draft","id":"1","scope":"line","path":"main.go","line":4,"range":{"start":4,"end":5},"side":"right","body":"Range draft."}
{"kind":"draft","id":"2","scope":"file","path":"main.go","body":"File draft."}
-- golden/reviews.yaml --
changes:
- change: 1
submissions:
- submitter: alice
disposition: comment
commentIDs:
- 2
- 3
threads:
- id: thread-2
path: main.go
range:
start: 4
end: 5
side: right
resolved: false
outdated: false
comments:
- id: 2
author: alice
body: Range draft.
- id: thread-3
path: main.go
resolved: false
outdated: false
comments:
- id: 3
author: alice
body: File draft.
2 changes: 1 addition & 1 deletion testdata/script/review_list_json.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ func main() {
}

-- golden/draft_only.json --
{"kind":"draft","id":"1","scope":"line","path":"main.go","line":5,"body":"Add error handling here."}
{"kind":"draft","id":"1","scope":"line","path":"main.go","line":5,"side":"right","body":"Add error handling here."}
Loading