Files
kandev/apps/backend/internal/github/client_helpers_test.go
T
João Salavisa 85aa3ac0aa feat(github): add /github page for PRs and issues (#687)
* feat(github): add /github page for PRs and issues

Dedicated page under /github lists pull requests and issues across the
authenticated user's repos with preset filters, custom GitHub search
queries, per-repo narrowing, saved queries, and one-click task launching
for any PR or issue.

Performance:
- Cache user PR/issue search responses for 30s with singleflight to
  coalesce concurrent misses, so preset switching and pagination avoid
  upstream round trips.
- Replace the per-row PR status fetch (N+1) with a batch endpoint
  POST /api/v1/github/prs/statuses that fans out concurrently on the
  backend with a semaphore and caches per-PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: apply Prettier formatting

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address review feedback on PR cache + status batching

- Harden searchCacheKey against separator collisions from
  user-controllable customQuery (length-prefix encoding).
- Drop repositories.loaded hydration when inner workspace fetch fails
  so the client refetches instead of trusting a partial snapshot.
- Guard useGitHubSearch with a monotonic request counter to drop
  stale responses when filter+page changes overlap.
- Fix usePRStatuses teardown: depend only on composed key so a new
  array identity per render doesn't cancel the in-flight batch.
- Correct godocs (duplicate, outdated state:open claim).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address additional review feedback

- Restore state:open for issue watcher: buildIssueSearchQuery no
  longer injects it as a default, so existing watches were starting
  to surface closed issues. Inject state:open in buildIssueFilter so
  only the watcher path is affected; the /github page presets already
  set their own state qualifiers.
- countCheckResults now excludes in-progress checks from the denominator
  so a `1/3` badge reflects pass rate on completed checks, not a mix of
  running and failed. The pending state is still carried via ChecksState.
- usePRStatuses retries after a transient fetch error (only mark the
  key as completed once the fetch resolves). This also fixes missing
  badges under React Strict Mode's intentional double-mount, since the
  first effect's cleanup fires before the response lands and the guard
  stays unset for the second mount to retry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(github): add tests for my-github hooks

Covers use-committed-query, buildParams from use-github-search,
use-saved-presets readStorage validation, use-known-repos recordForKey
accumulator semantics, and use-pr-statuses batch fetch lifecycle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address review feedback on search cache and repo accumulator

- Clamp perPage before composing the search cache key so identical
  pages (e.g. perPage=150 and =100) share a cache entry instead of
  creating duplicates keyed on the raw value.
- Add resetKnownReposStore and call it on /github unmount so the
  module-level repo accumulator doesn't carry across navigations.
- Rename use-pr-statuses test to match its assertion (it verifies
  stale statuses are cleared when the key changes and fetch fails).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: apply Prettier formatting

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(web): distinguish blocked from dirty in PR mergeable badge

GitHub's blocked state (required reviews/checks/branch protection)
is not a merge conflict, so don't label it "Conflicts". Surface it
as "Blocked" with the pending tone; keep "Conflicts" for dirty.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: address remaining /github code review comments

- Route PR status + batch through handleSearchError so ErrNoClient
  surfaces as 503 instead of 500 (matches the search endpoints).
- Sort workflow steps by position in the quick-task-launcher default
  pick, matching the convention in issue-watch-dialog.
- Make PresetItem keyboard-accessible: role=button, tabIndex, and
  Enter/Space handling on the wrapping div (it contains a nested
  delete button, so it can't itself be a button).
- Extract useRepoOptions out of useGitHubPageState to bring the hook
  back under the 100-line complexity limit.
- Honor ctx.Done() when acquiring the PR-status batch semaphore so a
  cancelled HTTP request releases queued goroutines instead of
  running each upstream fetch to completion.
- Clarify parsePaginationQuery doc: the upper bound (100) is enforced
  by clampSearchPage in the client layer, not here.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(web): address /github review follow-ups

- Merge duplicate @/lib/utils imports in pr-list.
- Use rel="noopener noreferrer" on external PR/issue links (codebase
  convention) and add cursor-pointer per the UI guidelines.
- Rewrite onOpenSaveDialog as an explicit if-block so the callback
  returns void rather than boolean | void.
- Use PaginationEllipsis in its own PaginationItem so the ellipsis
  and the adjacent page link are separate list items instead of
  visually fused into a single cell.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(web): derive QuickTaskLauncher dialog state from props

Replaces the useEffect+setState anti-pattern with useMemo-derived state
driven directly by the payload prop. Drops the eslint suppression that
was needed for react-hooks/set-state-in-effect.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-24 10:38:49 +01:00

474 lines
13 KiB
Go

package github
import (
"testing"
"time"
)
func TestBuildReviewSearchQuery(t *testing.T) {
tests := []struct {
name string
scope string
filter string
customQuery string
want string
}{
{
name: "customQuery overrides everything",
scope: ReviewScopeUser,
filter: "repo:owner/name",
customQuery: "type:pr is:open assignee:@me",
want: "type:pr is:open assignee:@me",
},
{
name: "user scope without filter",
scope: ReviewScopeUser,
want: "type:pr state:open user-review-requested:@me -is:draft",
},
{
name: "user scope with repo filter",
scope: ReviewScopeUser,
filter: "repo:owner/repo",
want: "type:pr state:open user-review-requested:@me -is:draft repo:owner/repo",
},
{
name: "user_and_teams scope without filter",
scope: ReviewScopeUserAndTeams,
want: "type:pr state:open review-requested:@me -is:draft",
},
{
name: "user_and_teams scope with org filter",
scope: ReviewScopeUserAndTeams,
filter: "org:myorg",
want: "type:pr state:open review-requested:@me -is:draft org:myorg",
},
{
name: "empty scope defaults to user_and_teams",
scope: "",
want: "type:pr state:open review-requested:@me -is:draft",
},
{
name: "empty customQuery falls through to scope logic",
scope: ReviewScopeUserAndTeams,
customQuery: "",
want: "type:pr state:open review-requested:@me -is:draft",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := buildReviewSearchQuery(tt.scope, tt.filter, tt.customQuery)
if got != tt.want {
t.Errorf("buildReviewSearchQuery(%q, %q, %q) = %q, want %q",
tt.scope, tt.filter, tt.customQuery, got, tt.want)
}
})
}
}
func TestConvertRawCheckRuns(t *testing.T) {
conclusion := "success"
summary := "All tests passed"
startedAt := "2025-01-15T10:00:00Z"
completedAt := "2025-01-15T10:05:00Z"
raw := []ghCheckRun{
{
Name: "ci/test",
Status: "completed",
Conclusion: &conclusion,
HTMLURL: "https://github.com/owner/repo/runs/1",
StartedAt: startedAt,
CompletedAt: completedAt,
Output: struct {
Title *string `json:"title"`
Summary *string `json:"summary"`
}{Summary: &summary},
},
{
Name: "ci/lint",
Status: "in_progress",
Conclusion: nil,
HTMLURL: "https://github.com/owner/repo/runs/2",
},
}
checks := convertRawCheckRuns(raw)
if len(checks) != 2 {
t.Fatalf("expected 2 checks, got %d", len(checks))
}
// Check first (completed)
if checks[0].Name != "ci/test" {
t.Errorf("expected name ci/test, got %s", checks[0].Name)
}
if checks[0].Source != checkSourceCheckRun {
t.Errorf("expected source %q, got %q", checkSourceCheckRun, checks[0].Source)
}
if checks[0].Conclusion != "success" {
t.Errorf("expected conclusion success, got %s", checks[0].Conclusion)
}
if checks[0].Output != "All tests passed" {
t.Errorf("expected output 'All tests passed', got %s", checks[0].Output)
}
if checks[0].StartedAt == nil {
t.Error("expected non-nil StartedAt")
}
// Check second (in progress, nil conclusion)
if checks[1].Conclusion != "" {
t.Errorf("expected empty conclusion, got %s", checks[1].Conclusion)
}
if checks[1].StartedAt != nil {
t.Error("expected nil StartedAt for empty string")
}
}
func TestConvertRawCheckRunsEmpty(t *testing.T) {
checks := convertRawCheckRuns(nil)
if len(checks) != 0 {
t.Errorf("expected empty slice, got %d", len(checks))
}
}
func TestConvertRawComments(t *testing.T) {
now := time.Now()
raw := []ghComment{
{
ID: 1,
Path: "main.go",
Line: 42,
Side: "RIGHT",
Body: "Looks good",
CreatedAt: now,
UpdatedAt: now,
User: struct {
Login string `json:"login"`
AvatarURL string `json:"avatar_url"`
Type string `json:"type"`
}{Login: "alice", AvatarURL: "https://avatar.example.com/alice", Type: "User"},
},
}
comments := convertRawComments(raw)
if len(comments) != 1 {
t.Fatalf("expected 1 comment, got %d", len(comments))
}
if comments[0].Author != "alice" {
t.Errorf("expected author alice, got %s", comments[0].Author)
}
if comments[0].Path != "main.go" {
t.Errorf("expected path main.go, got %s", comments[0].Path)
}
if comments[0].Line != 42 {
t.Errorf("expected line 42, got %d", comments[0].Line)
}
if comments[0].CommentType != commentTypeReview {
t.Errorf("expected comment type %q, got %q", commentTypeReview, comments[0].CommentType)
}
if comments[0].AuthorIsBot {
t.Error("expected non-bot comment")
}
}
func TestConvertRawIssueComments(t *testing.T) {
now := time.Now()
raw := []ghIssueComment{
{
ID: 10,
Body: "Snyk report",
CreatedAt: now,
UpdatedAt: now,
User: struct {
Login string `json:"login"`
AvatarURL string `json:"avatar_url"`
Type string `json:"type"`
}{
Login: "snyk-io",
AvatarURL: "https://avatar.example.com/snyk",
Type: "Bot",
},
},
}
comments := convertRawIssueComments(raw)
if len(comments) != 1 {
t.Fatalf("expected 1 comment, got %d", len(comments))
}
if comments[0].CommentType != commentTypeIssue {
t.Errorf("expected comment type %q, got %q", commentTypeIssue, comments[0].CommentType)
}
if !comments[0].AuthorIsBot {
t.Error("expected bot comment")
}
}
func TestMergeAndSortPRComments(t *testing.T) {
t1 := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
t2 := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC)
t3 := time.Date(2025, 1, 3, 0, 0, 0, 0, time.UTC)
review := []PRComment{
{ID: 2, CommentType: commentTypeReview, CreatedAt: t2},
{ID: 3, CommentType: commentTypeReview, CreatedAt: t3},
}
issue := []PRComment{
{ID: 1, CommentType: commentTypeIssue, CreatedAt: t1},
}
got := mergeAndSortPRComments(review, issue)
if len(got) != 3 {
t.Fatalf("expected 3 comments, got %d", len(got))
}
if got[0].ID != 1 || got[1].ID != 2 || got[2].ID != 3 {
t.Errorf("unexpected merge/sort order: %#v", got)
}
}
func TestConvertRawStatusContextsAndMergeChecks(t *testing.T) {
conclusion := "success"
checkRuns := []ghCheckRun{
{
Name: "ci/test",
Status: "completed",
Conclusion: &conclusion,
HTMLURL: "https://github.com/owner/repo/runs/1",
},
}
statuses := []ghStatusContext{
{
Context: "ci/test",
State: "pending",
TargetURL: "https://github.com/owner/repo/runs/1",
},
{
Context: "license/snyk",
State: "success",
TargetURL: "https://app.snyk.io/check/2",
},
}
merged := mergeChecks(convertRawCheckRuns(checkRuns), convertRawStatusContexts(statuses))
if len(merged) != 2 {
t.Fatalf("expected 2 checks after dedupe, got %d", len(merged))
}
if merged[0].Name != "ci/test" || merged[0].Source != checkSourceCheckRun {
t.Errorf("expected check_run to win dedupe, got %#v", merged[0])
}
if merged[1].Name != "license/snyk" || merged[1].Source != checkSourceStatusContext {
t.Errorf("expected status_context entry, got %#v", merged[1])
}
if merged[1].Conclusion != checkStatusSuccess {
t.Errorf("expected success conclusion, got %q", merged[1].Conclusion)
}
}
func TestMergeChecksDuplicateCheckRunsByName(t *testing.T) {
t1 := time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC)
t2 := time.Date(2025, 6, 1, 11, 0, 0, 0, time.UTC)
// Two check-runs with the same name but different URLs (different check suites).
checkRuns := []CheckRun{
{Name: "Analyze (go)", Source: checkSourceCheckRun, Status: "in_progress", HTMLURL: "https://github.com/o/r/runs/1", StartedAt: &t1},
{Name: "Analyze (go)", Source: checkSourceCheckRun, Status: "in_progress", HTMLURL: "https://github.com/o/r/runs/2", StartedAt: &t2},
}
merged := mergeChecks(checkRuns, nil)
if len(merged) != 1 {
t.Fatalf("expected 1 check after dedupe, got %d", len(merged))
}
if merged[0].HTMLURL != "https://github.com/o/r/runs/2" {
t.Errorf("expected newer check-run to win, got URL %s", merged[0].HTMLURL)
}
}
func TestMergeChecksCheckRunWinsOverStatusDifferentURL(t *testing.T) {
t1 := time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC)
checkRuns := []CheckRun{
{Name: "ci/test", Source: checkSourceCheckRun, Status: "completed", Conclusion: "success", HTMLURL: "https://github.com/o/r/runs/1", StartedAt: &t1},
}
statuses := []CheckRun{
{Name: "ci/test", Source: checkSourceStatusContext, Status: "in_progress", HTMLURL: "https://other-url.com/status/1", StartedAt: &t1},
}
merged := mergeChecks(checkRuns, statuses)
if len(merged) != 1 {
t.Fatalf("expected 1 check after dedupe, got %d", len(merged))
}
if merged[0].Source != checkSourceCheckRun {
t.Errorf("expected check_run to win over status_context, got source %s", merged[0].Source)
}
}
func TestIsNewerCheck(t *testing.T) {
t1 := time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC)
t2 := time.Date(2025, 6, 1, 11, 0, 0, 0, time.UTC)
if isNewerCheck(CheckRun{StartedAt: &t1}, CheckRun{StartedAt: &t2}) {
t.Error("older should not be newer")
}
if !isNewerCheck(CheckRun{StartedAt: &t2}, CheckRun{StartedAt: &t1}) {
t.Error("newer should be newer")
}
if isNewerCheck(CheckRun{StartedAt: nil}, CheckRun{StartedAt: &t1}) {
t.Error("nil StartedAt should not be newer")
}
if !isNewerCheck(CheckRun{StartedAt: &t1}, CheckRun{StartedAt: nil}) {
t.Error("non-nil should be newer than nil")
}
}
func TestConvertSearchItemToPR(t *testing.T) {
now := time.Now()
pr := convertSearchItemToPR(
42, "Fix bug", "https://github.com/owner/repo/pull/42", "open",
"alice", "https://api.github.com/repos/myorg/myrepo", false,
now, now,
)
if pr.Number != 42 {
t.Errorf("expected number 42, got %d", pr.Number)
}
if pr.Title != "Fix bug" {
t.Errorf("expected title 'Fix bug', got %s", pr.Title)
}
if pr.RepoOwner != "myorg" {
t.Errorf("expected owner myorg, got %s", pr.RepoOwner)
}
if pr.RepoName != "myrepo" {
t.Errorf("expected repo myrepo, got %s", pr.RepoName)
}
}
func TestLatestReviewByAuthor(t *testing.T) {
t1 := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
t2 := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC)
reviews := []PRReview{
{Author: "alice", State: "COMMENTED", CreatedAt: t1},
{Author: "alice", State: "APPROVED", CreatedAt: t2},
{Author: "bob", State: "CHANGES_REQUESTED", CreatedAt: t1},
}
latest := latestReviewByAuthor(reviews)
if len(latest) != 2 {
t.Fatalf("expected 2 authors, got %d", len(latest))
}
if latest["alice"].State != "APPROVED" {
t.Errorf("expected alice's latest to be APPROVED, got %s", latest["alice"].State)
}
if latest["bob"].State != "CHANGES_REQUESTED" {
t.Errorf("expected bob's latest to be CHANGES_REQUESTED, got %s", latest["bob"].State)
}
}
func TestLatestReviewByAuthorEmpty(t *testing.T) {
latest := latestReviewByAuthor(nil)
if len(latest) != 0 {
t.Errorf("expected empty map, got %d entries", len(latest))
}
}
func TestHasFailingChecks(t *testing.T) {
tests := []struct {
name string
checks []CheckRun
want bool
}{
{"empty", nil, false},
{"all passing", []CheckRun{
{Status: "completed", Conclusion: "success"},
}, false},
{"one failing", []CheckRun{
{Status: "completed", Conclusion: "success"},
{Status: "completed", Conclusion: "failure"},
}, true},
{"in progress", []CheckRun{
{Status: "in_progress", Conclusion: ""},
}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := hasFailingChecks(tt.checks)
if got != tt.want {
t.Errorf("hasFailingChecks() = %v, want %v", got, tt.want)
}
})
}
}
func TestCountCheckResults(t *testing.T) {
tests := []struct {
name string
checks []CheckRun
wantTotal, wantPass int
}{
{
name: "empty",
wantTotal: 0, wantPass: 0,
},
{
name: "in-progress checks excluded from total",
checks: []CheckRun{
{Status: checkStatusCompleted, Conclusion: checkConclusionSuccess},
{Status: "in_progress", Conclusion: ""},
{Status: "queued", Conclusion: ""},
},
wantTotal: 1, wantPass: 1,
},
{
name: "skipped and neutral are excluded",
checks: []CheckRun{
{Status: checkStatusCompleted, Conclusion: checkConclusionSuccess},
{Status: checkStatusCompleted, Conclusion: checkConclusionSkipped},
{Status: checkStatusCompleted, Conclusion: checkConclusionNeutral},
},
wantTotal: 1, wantPass: 1,
},
{
name: "failures count toward total but not passing",
checks: []CheckRun{
{Status: checkStatusCompleted, Conclusion: checkConclusionSuccess},
{Status: checkStatusCompleted, Conclusion: checkConclusionFail},
{Status: checkStatusCompleted, Conclusion: checkConclusionTimedOut},
},
wantTotal: 3, wantPass: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
total, passing := countCheckResults(tt.checks)
if total != tt.wantTotal || passing != tt.wantPass {
t.Errorf("countCheckResults = (%d, %d), want (%d, %d)",
total, passing, tt.wantTotal, tt.wantPass)
}
})
}
}
func TestHasChangesRequested(t *testing.T) {
tests := []struct {
name string
reviews []PRReview
want bool
}{
{"empty", nil, false},
{"approved", []PRReview{{State: "APPROVED"}}, false},
{"changes requested", []PRReview{
{State: "APPROVED"},
{State: "CHANGES_REQUESTED"},
}, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := hasChangesRequested(tt.reviews)
if got != tt.want {
t.Errorf("hasChangesRequested() = %v, want %v", got, tt.want)
}
})
}
}