diff --git a/github/github-accessors.go b/github/github-accessors.go index c2a158f119..3e2e03400a 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -2972,6 +2972,14 @@ func (d *DraftReviewComment) GetBody() string { return *d.Body } +// GetLine returns the Line field if it's non-nil, zero value otherwise. +func (d *DraftReviewComment) GetLine() int { + if d == nil || d.Line == nil { + return 0 + } + return *d.Line +} + // GetPath returns the Path field if it's non-nil, zero value otherwise. func (d *DraftReviewComment) GetPath() string { if d == nil || d.Path == nil { @@ -2988,6 +2996,30 @@ func (d *DraftReviewComment) GetPosition() int { return *d.Position } +// GetSide returns the Side field if it's non-nil, zero value otherwise. +func (d *DraftReviewComment) GetSide() string { + if d == nil || d.Side == nil { + return "" + } + return *d.Side +} + +// GetStartLine returns the StartLine field if it's non-nil, zero value otherwise. +func (d *DraftReviewComment) GetStartLine() int { + if d == nil || d.StartLine == nil { + return 0 + } + return *d.StartLine +} + +// GetStartSide returns the StartSide field if it's non-nil, zero value otherwise. +func (d *DraftReviewComment) GetStartSide() string { + if d == nil || d.StartSide == nil { + return "" + } + return *d.StartSide +} + // GetAvatarURL returns the AvatarURL field if it's non-nil, zero value otherwise. func (e *Enterprise) GetAvatarURL() string { if e == nil || e.AvatarURL == nil { diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go index 96d2baf285..0d64461b5e 100644 --- a/github/github-stringify_test.go +++ b/github/github-stringify_test.go @@ -297,11 +297,15 @@ func TestDiscussionComment_String(t *testing.T) { func TestDraftReviewComment_String(t *testing.T) { v := DraftReviewComment{ - Path: String(""), - Position: Int(0), - Body: String(""), + Path: String(""), + Position: Int(0), + Body: String(""), + StartSide: String(""), + Side: String(""), + StartLine: Int(0), + Line: Int(0), } - want := `github.DraftReviewComment{Path:"", Position:0, Body:""}` + want := `github.DraftReviewComment{Path:"", Position:0, Body:"", StartSide:"", Side:"", StartLine:0, Line:0}` if got := v.String(); got != want { t.Errorf("DraftReviewComment.String = %v, want %v", got, want) } diff --git a/github/pulls_reviews.go b/github/pulls_reviews.go index 5bceb61c4c..42af0a0b8a 100644 --- a/github/pulls_reviews.go +++ b/github/pulls_reviews.go @@ -7,10 +7,13 @@ package github import ( "context" + "errors" "fmt" "time" ) +var ErrMixedCommentStyles = errors.New("cannot use both position and side/line form comments") + // PullRequestReview represents a review of a pull request. type PullRequestReview struct { ID *int64 `json:"id,omitempty"` @@ -36,6 +39,12 @@ type DraftReviewComment struct { Path *string `json:"path,omitempty"` Position *int `json:"position,omitempty"` Body *string `json:"body,omitempty"` + + // The new comfort-fade-preview fields + StartSide *string `json:"start_side,omitempty"` + Side *string `json:"side,omitempty"` + StartLine *int `json:"start_line,omitempty"` + Line *int `json:"line,omitempty"` } func (c DraftReviewComment) String() string { @@ -55,6 +64,32 @@ func (r PullRequestReviewRequest) String() string { return Stringify(r) } +func (r PullRequestReviewRequest) isComfortFadePreview() (bool, error) { + var isCF *bool + for _, comment := range r.Comments { + if comment == nil { + continue + } + hasPos := comment.Position != nil + hasComfortFade := (comment.StartSide != nil) || (comment.Side != nil) || + (comment.StartLine != nil) || (comment.Line != nil) + + switch { + case hasPos && hasComfortFade: + return false, ErrMixedCommentStyles + case hasPos && isCF != nil && *isCF: + return false, ErrMixedCommentStyles + case hasComfortFade && isCF != nil && !*isCF: + return false, ErrMixedCommentStyles + } + isCF = &hasComfortFade + } + if isCF != nil { + return *isCF, nil + } + return false, nil +} + // PullRequestReviewDismissalRequest represents a request to dismiss a review. type PullRequestReviewDismissalRequest struct { Message *string `json:"message,omitempty"` @@ -175,6 +210,38 @@ func (s *PullRequestsService) ListReviewComments(ctx context.Context, owner, rep // Read more about it here - https://github.com/google/go-github/issues/540 // // GitHub API docs: https://developer.github.com/v3/pulls/reviews/#create-a-pull-request-review +// +// In order to use multi-line comments, you must use the "comfort fade" preview. +// This replaces the use of the "Position" field in comments with 4 new fields: +// [Start]Side, and [Start]Line. +// These new fields must be used for ALL comments (including single-line), +// with the following restrictions (empirically observed, so subject to change). +// +// For single-line "comfort fade" comments, you must use: +// +// Path: &path, // as before +// Body: &body, // as before +// Side: &"RIGHT" (or "LEFT") +// Line: &123, // NOT THE SAME AS POSITION, this is an actual line number. +// +// If StartSide or StartLine is used with single-line comments, a 422 is returned. +// +// For multi-line "comfort fade" comments, you must use: +// +// Path: &path, // as before +// Body: &body, // as before +// StartSide: &"RIGHT" (or "LEFT") +// Side: &"RIGHT" (or "LEFT") +// StartLine: &120, +// Line: &125, +// +// Suggested edits are made by commenting on the lines to replace, and including the +// suggested edit in a block like this (it may be surrounded in non-suggestion markdown): +// +// ```suggestion +// Use this instead. +// It is waaaaaay better. +// ``` func (s *PullRequestsService) CreateReview(ctx context.Context, owner, repo string, number int, review *PullRequestReviewRequest) (*PullRequestReview, *Response, error) { u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, number) @@ -183,6 +250,15 @@ func (s *PullRequestsService) CreateReview(ctx context.Context, owner, repo stri return nil, nil, err } + // Detect which style of review comment is being used. + if isCF, err := review.isComfortFadePreview(); err != nil { + return nil, nil, err + } else if isCF { + // If the review comments are using the comfort fade preview fields, + // then pass the comfort fade header. + req.Header.Set("Accept", mediaTypeMultiLineCommentsPreview) + } + r := new(PullRequestReview) resp, err := s.client.Do(ctx, req, r) if err != nil { diff --git a/github/pulls_reviews_test.go b/github/pulls_reviews_test.go index 25aaca1c01..ae3fb7c57f 100644 --- a/github/pulls_reviews_test.go +++ b/github/pulls_reviews_test.go @@ -73,7 +73,7 @@ func TestReviewers_marshall(t *testing.T) { "created_at": ` + referenceTimeStr + `, "url": "u" } - ], + ], "teams" : [ { "id": 1, @@ -228,6 +228,121 @@ func TestPullRequestsService_ListReviewComments_withOptions(t *testing.T) { } } +func TestPullRequestReviewRequest_isComfortFadePreview(t *testing.T) { + path := "path/to/file.go" + body := "this is a comment body" + left, right := "LEFT", "RIGHT" + pos1, pos2, pos3 := 1, 2, 3 + line1, line2, line3 := 11, 22, 33 + + tests := []struct { + name string + review *PullRequestReviewRequest + wantErr error + wantBool bool + }{{ + name: "empty review", + review: &PullRequestReviewRequest{}, + wantBool: false, + }, { + name: "old-style review", + review: &PullRequestReviewRequest{ + Comments: []*DraftReviewComment{{ + Path: &path, + Body: &body, + Position: &pos1, + }, { + Path: &path, + Body: &body, + Position: &pos2, + }, { + Path: &path, + Body: &body, + Position: &pos3, + }}, + }, + wantBool: false, + }, { + name: "new-style review", + review: &PullRequestReviewRequest{ + Comments: []*DraftReviewComment{{ + Path: &path, + Body: &body, + Side: &right, + Line: &line1, + }, { + Path: &path, + Body: &body, + Side: &left, + Line: &line2, + }, { + Path: &path, + Body: &body, + Side: &right, + Line: &line3, + }}, + }, + wantBool: true, + }, { + name: "blended comment", + review: &PullRequestReviewRequest{ + Comments: []*DraftReviewComment{{ + Path: &path, + Body: &body, + Position: &pos1, // can't have both styles. + Side: &right, + Line: &line1, + }}, + }, + wantErr: ErrMixedCommentStyles, + }, { + name: "position then line", + review: &PullRequestReviewRequest{ + Comments: []*DraftReviewComment{{ + Path: &path, + Body: &body, + Position: &pos1, + }, { + Path: &path, + Body: &body, + Side: &right, + Line: &line1, + }}, + }, + wantErr: ErrMixedCommentStyles, + }, { + name: "line then position", + review: &PullRequestReviewRequest{ + Comments: []*DraftReviewComment{{ + Path: &path, + Body: &body, + Side: &right, + Line: &line1, + }, { + Path: &path, + Body: &body, + Position: &pos1, + }}, + }, + wantErr: ErrMixedCommentStyles, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotBool, gotErr := tc.review.isComfortFadePreview() + if tc.wantErr != nil { + if gotErr != tc.wantErr { + t.Errorf("isComfortFadePreview() = %v, wanted %v", gotErr, tc.wantErr) + } + } else { + if gotBool != tc.wantBool { + t.Errorf("isComfortFadePreview() = %v, wanted %v", gotBool, tc.wantBool) + } + } + }) + } +} + func TestPullRequestsService_ListReviewComments_invalidOwner(t *testing.T) { client, _, _, teardown := setup() defer teardown()