diff --git a/.gitea/actions/review/action.yml b/.gitea/actions/review/action.yml index d005697..0fdccdc 100644 --- a/.gitea/actions/review/action.yml +++ b/.gitea/actions/review/action.yml @@ -67,7 +67,7 @@ inputs: required: false default: 'false' update-existing: - description: 'Delete previous review from same bot before posting (default true)' + description: 'Delete previous review from same bot before posting. Accepts: true/1/yes or false/0/no (default true)' required: false default: 'true' diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 39865b4..c9e414f 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -359,7 +359,7 @@ func envOrDefaultInt(key string, defaultVal int) int { } func envOrDefaultBool(key string, defaultVal bool) bool { - v := os.Getenv(key) + v := strings.TrimSpace(strings.ToLower(os.Getenv(key))) if v == "" { return defaultVal } diff --git a/gitea/client.go b/gitea/client.go index f21b1d5..e8e457c 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -297,21 +297,32 @@ func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) { } // ListReviews returns all reviews on a pull request. +// Paginates through all pages to ensure no reviews are missed. func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) { - reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", - c.baseURL, - url.PathEscape(owner), - url.PathEscape(repo), - number) - body, err := c.doGet(ctx, reqURL) - if err != nil { - return nil, fmt.Errorf("list reviews: %w", err) + const pageSize = 50 + var all []Review + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews?limit=%d&page=%d", + c.baseURL, + url.PathEscape(owner), + url.PathEscape(repo), + number, + pageSize, + page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list reviews (page %d): %w", page, err) + } + var batch []Review + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse reviews (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < pageSize { + break + } } - var reviews []Review - if err := json.Unmarshal(body, &reviews); err != nil { - return nil, fmt.Errorf("parse reviews: %w", err) - } - return reviews, nil + return all, nil } // DeleteReview deletes a review by ID. The token must belong to the review author. diff --git a/gitea/client_test.go b/gitea/client_test.go index fc93473..514348d 100644 --- a/gitea/client_test.go +++ b/gitea/client_test.go @@ -357,11 +357,17 @@ func TestGetAuthenticatedUser_EmptyLogin(t *testing.T) { } func TestListReviews(t *testing.T) { + pageCount := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews" { t.Errorf("unexpected path: %s", r.URL.Path) } + if r.URL.Query().Get("limit") != "50" { + t.Errorf("expected limit=50, got %s", r.URL.Query().Get("limit")) + } + pageCount++ w.Header().Set("Content-Type", "application/json") + // Return 2 results (less than page size) to signal end w.Write([]byte(`[{"id":10,"user":{"login":"bot-a"},"state":"APPROVED","stale":false},{"id":11,"user":{"login":"bot-b"},"state":"REQUEST_CHANGES","stale":true}]`)) })) defer server.Close() @@ -377,8 +383,45 @@ func TestListReviews(t *testing.T) { if reviews[0].User.Login != "bot-a" { t.Errorf("expected bot-a, got %s", reviews[0].User.Login) } - if reviews[1].ID != 11 { - t.Errorf("expected id 11, got %d", reviews[1].ID) + if pageCount != 1 { + t.Errorf("expected 1 page fetch (results < page size), got %d", pageCount) + } +} + +func TestListReviews_Pagination(t *testing.T) { + pageCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pageCount++ + page := r.URL.Query().Get("page") + w.Header().Set("Content-Type", "application/json") + if page == "1" { + // Return exactly 50 items to trigger next page fetch + items := "[" + for i := 0; i < 50; i++ { + if i > 0 { + items += "," + } + items += fmt.Sprintf(`{"id":%d,"user":{"login":"bot"},"state":"APPROVED","stale":false}`, i+1) + } + items += "]" + w.Write([]byte(items)) + } else { + // Page 2: return fewer than 50 to signal end + w.Write([]byte(`[{"id":51,"user":{"login":"bot"},"state":"APPROVED","stale":false}]`)) + } + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reviews) != 51 { + t.Fatalf("expected 51 reviews across 2 pages, got %d", len(reviews)) + } + if pageCount != 2 { + t.Errorf("expected 2 page fetches, got %d", pageCount) } }