Merge pull request 'fix(github): consolidate review.go and identity.go into reviews.go (#116)' (#119) from review-bot-issue-116 into feature/github-support
Reviewed-on: #119 Reviewed-by: Aaron Weiker <aaron@weiker.org> Reviewed-by: security-review-bot <10+security-review-bot@noreply.gitea.weiker.me>
This commit was merged in pull request #119.
This commit is contained in:
@@ -0,0 +1,284 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
const (
|
||||
// reviewsPerPage is the number of reviews to fetch per API page.
|
||||
reviewsPerPage = 100
|
||||
// maxReviewPages is the maximum number of pages to paginate through
|
||||
// when listing reviews. Acts as a safeguard against infinite pagination.
|
||||
maxReviewPages = 100
|
||||
)
|
||||
|
||||
// ErrCannotDeleteSubmittedReview is returned when DeleteReview is called on
|
||||
// a review that has already been submitted (APPROVED, REQUEST_CHANGES, COMMENT).
|
||||
// GitHub only allows deletion of PENDING reviews. Callers that need to replace
|
||||
// a submitted review should use DismissReview instead.
|
||||
var ErrCannotDeleteSubmittedReview = errors.New("cannot delete submitted review: use DismissReview instead")
|
||||
|
||||
// ErrConflictingCommitIDs is returned when PostReview receives comments with
|
||||
// differing non-empty CommitIDs. The GitHub API accepts only a single commit_id
|
||||
// per review submission; callers must ensure all comments target the same commit.
|
||||
var ErrConflictingCommitIDs = errors.New("comments contain conflicting commit IDs: all must target the same commit")
|
||||
|
||||
// postReviewRequest is the GitHub API request body for creating a review.
|
||||
type postReviewRequest struct {
|
||||
CommitID string `json:"commit_id,omitempty"`
|
||||
Body string `json:"body"`
|
||||
Event string `json:"event"`
|
||||
Comments []reviewCommentEntry `json:"comments,omitempty"`
|
||||
}
|
||||
|
||||
// reviewCommentEntry is a single inline comment in a review creation request.
|
||||
type reviewCommentEntry struct {
|
||||
Path string `json:"path"`
|
||||
Position int `json:"position"`
|
||||
Body string `json:"body"`
|
||||
}
|
||||
|
||||
// reviewResponse is the GitHub API response for a review.
|
||||
type reviewResponse struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
State string `json:"state"`
|
||||
CommitID string `json:"commit_id"`
|
||||
User struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"user"`
|
||||
}
|
||||
|
||||
// dismissReviewRequest is the GitHub API request body for dismissing a review.
|
||||
type dismissReviewRequest struct {
|
||||
Message string `json:"message"`
|
||||
Event string `json:"event"`
|
||||
}
|
||||
|
||||
// userResponse is the GitHub API response for the authenticated user.
|
||||
type userResponse struct {
|
||||
Login string `json:"login"`
|
||||
}
|
||||
|
||||
// translateGitHubReviewState translates a GitHub API review state to the
|
||||
// canonical vcs.Review.State value.
|
||||
func translateGitHubReviewState(state string) string {
|
||||
switch state {
|
||||
case "CHANGES_REQUESTED":
|
||||
return "REQUEST_CHANGES"
|
||||
case "COMMENTED":
|
||||
return "COMMENT"
|
||||
default:
|
||||
// States like APPROVED, DISMISSED, and PENDING pass through unchanged
|
||||
// as they already match the canonical vcs representation. PENDING appears
|
||||
// on draft reviews that have not yet been submitted via the GitHub UI or API.
|
||||
return state
|
||||
}
|
||||
}
|
||||
|
||||
// PostReview submits a review on a pull request.
|
||||
//
|
||||
// The vcs.ReviewEvent constants (ReviewEventApprove, ReviewEventRequestChanges,
|
||||
// ReviewEventComment) have string values that match GitHub's wire-format event
|
||||
// strings (APPROVE, REQUEST_CHANGES, COMMENT), so Event is cast directly to
|
||||
// string without translation.
|
||||
//
|
||||
// ReviewComment.Position maps directly to the GitHub API position field.
|
||||
// When req.Comments is empty, the payload omits the comments field entirely
|
||||
// (via the omitempty tag on postReviewRequest.Comments).
|
||||
//
|
||||
// The GitHub API accepts a single commit_id per review submission. PostReview
|
||||
// uses req.CommitID as the primary commit anchor. If req.CommitID is empty,
|
||||
// it falls back to extracting from the first comment with a non-empty CommitID.
|
||||
// If any subsequent comment specifies a different CommitID, PostReview returns
|
||||
// ErrConflictingCommitIDs. Comments with an empty CommitID are allowed and
|
||||
// inherit the review-level value.
|
||||
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
|
||||
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
|
||||
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
|
||||
|
||||
payload := postReviewRequest{
|
||||
Body: req.Body,
|
||||
Event: string(req.Event),
|
||||
CommitID: req.CommitID,
|
||||
}
|
||||
|
||||
// Build the payload in one pass. The GitHub API accepts a single commit_id
|
||||
// per review. req.CommitID is the primary source; if empty, we extract from
|
||||
// the first comment that supplies one. Reject if any comment disagrees with
|
||||
// the resolved commit_id.
|
||||
for _, comment := range req.Comments {
|
||||
if comment.CommitID != "" {
|
||||
if payload.CommitID == "" { // only reachable when req.CommitID is empty
|
||||
payload.CommitID = comment.CommitID
|
||||
} else if payload.CommitID != comment.CommitID {
|
||||
return nil, ErrConflictingCommitIDs
|
||||
}
|
||||
// else: matching SHA is a no-op by design
|
||||
}
|
||||
payload.Comments = append(payload.Comments, reviewCommentEntry{
|
||||
Path: comment.Path,
|
||||
Position: comment.Position,
|
||||
Body: comment.Body,
|
||||
})
|
||||
}
|
||||
|
||||
body, err := c.doJSONRequest(ctx, http.MethodPost, reqURL, payload)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("post review: %w", err)
|
||||
}
|
||||
|
||||
var resp reviewResponse
|
||||
if err := json.Unmarshal(body, &resp); err != nil {
|
||||
return nil, fmt.Errorf("parse review response: %w", err)
|
||||
}
|
||||
|
||||
return &vcs.Review{
|
||||
ID: resp.ID,
|
||||
Body: resp.Body,
|
||||
User: vcs.UserInfo{Login: resp.User.Login},
|
||||
State: translateGitHubReviewState(resp.State),
|
||||
CommitID: resp.CommitID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// ListReviews retrieves all reviews for a pull request with pagination.
|
||||
// GitHub review states are translated to canonical vcs values.
|
||||
func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) {
|
||||
perPage := reviewsPerPage
|
||||
if c.reviewPageSize > 0 {
|
||||
perPage = c.reviewPageSize
|
||||
}
|
||||
maxPages := maxReviewPages
|
||||
if c.reviewMaxPages > 0 {
|
||||
maxPages = c.reviewMaxPages
|
||||
}
|
||||
|
||||
var allReviews []vcs.Review
|
||||
truncated := false
|
||||
|
||||
for page := 1; page <= maxPages; page++ {
|
||||
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews?per_page=%d&page=%d",
|
||||
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page)
|
||||
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list reviews page %d: %w", page, err)
|
||||
}
|
||||
|
||||
var responses []reviewResponse
|
||||
if err := json.Unmarshal(body, &responses); err != nil {
|
||||
return nil, fmt.Errorf("parse reviews response: %w", err)
|
||||
}
|
||||
|
||||
if len(responses) == 0 {
|
||||
break
|
||||
}
|
||||
|
||||
for _, r := range responses {
|
||||
allReviews = append(allReviews, vcs.Review{
|
||||
ID: r.ID,
|
||||
Body: r.Body,
|
||||
User: vcs.UserInfo{Login: r.User.Login},
|
||||
State: translateGitHubReviewState(r.State),
|
||||
CommitID: r.CommitID,
|
||||
})
|
||||
}
|
||||
|
||||
if len(responses) < perPage {
|
||||
break
|
||||
}
|
||||
|
||||
// Truncation detection: this runs on the final allowed iteration
|
||||
// (page == maxPages) only when the page was full (the len < perPage
|
||||
// early-break above didn't fire). A full final page means additional
|
||||
// reviews likely exist beyond our pagination limit.
|
||||
if page == maxPages {
|
||||
truncated = true
|
||||
}
|
||||
}
|
||||
|
||||
if truncated {
|
||||
slog.Warn("ListReviews hit page limit; results may be truncated",
|
||||
"owner", owner, "repo", repo, "pr", number,
|
||||
"maxPages", maxPages, "reviewsFetched", len(allReviews))
|
||||
}
|
||||
|
||||
return allReviews, nil
|
||||
}
|
||||
|
||||
// DeleteReview deletes a pull request review.
|
||||
// Only PENDING reviews can be deleted; attempting to delete a submitted review
|
||||
// (APPROVED, CHANGES_REQUESTED, or COMMENTED per GitHub API naming) returns
|
||||
// ErrCannotDeleteSubmittedReview.
|
||||
func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error {
|
||||
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d",
|
||||
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
|
||||
|
||||
// nil body: the GitHub DELETE endpoint for reviews requires no request body.
|
||||
_, err := c.doRequestWithBody(ctx, http.MethodDelete, reqURL, nil)
|
||||
if err != nil {
|
||||
var apiErr *APIError
|
||||
if errors.As(err, &apiErr) && apiErr.StatusCode == 422 {
|
||||
return fmt.Errorf("delete review: %w", ErrCannotDeleteSubmittedReview)
|
||||
}
|
||||
return fmt.Errorf("delete review: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// DismissReview dismisses a submitted review on a pull request.
|
||||
// This is the correct way to "remove" a submitted review (APPROVED, REQUEST_CHANGES).
|
||||
// GitHub does not allow deleting submitted reviews — they must be dismissed.
|
||||
func (c *Client) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
|
||||
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d/dismissals",
|
||||
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
|
||||
|
||||
payload := dismissReviewRequest{
|
||||
Message: message,
|
||||
// Event is required by the GitHub API for dismissal requests, even though
|
||||
// "DISMISS" is the only valid value for this endpoint.
|
||||
Event: "DISMISS",
|
||||
}
|
||||
|
||||
_, err := c.doJSONRequest(ctx, http.MethodPut, reqURL, payload)
|
||||
if err != nil {
|
||||
return fmt.Errorf("dismiss review: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// SupersedeReviews marks prior reviews as superseded by dismissing them.
|
||||
// This implements vcs.ReviewSuperseder for the GitHub adapter.
|
||||
// The baseURL and sentinel parameters are unused for GitHub (dismissal is the mechanism).
|
||||
func (c *Client) SupersedeReviews(ctx context.Context, owner, repo string, prNumber int, oldReviews []vcs.Review, newReviewID int64, _, _ string) error {
|
||||
var errs []error
|
||||
for _, old := range oldReviews {
|
||||
if err := c.DismissReview(ctx, owner, repo, prNumber, old.ID, "Superseded by new review"); err != nil {
|
||||
errs = append(errs, fmt.Errorf("dismiss review %d: %w", old.ID, err))
|
||||
}
|
||||
}
|
||||
return errors.Join(errs...)
|
||||
}
|
||||
|
||||
// GetAuthenticatedUser returns the login name of the authenticated user.
|
||||
func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
|
||||
reqURL := fmt.Sprintf("%s/user", c.baseURL)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("get authenticated user: %w", err)
|
||||
}
|
||||
var resp userResponse
|
||||
if err := json.Unmarshal(body, &resp); err != nil {
|
||||
return "", fmt.Errorf("parse user response: %w", err)
|
||||
}
|
||||
return resp.Login, nil
|
||||
}
|
||||
Reference in New Issue
Block a user