feat(sidebar): add last activity task sorting (#2762)
This commit is contained in:
@@ -426,7 +426,7 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
|
||||
reviewState := summarizeReviewState(raw.Reviews.Nodes)
|
||||
checksState := ""
|
||||
if len(raw.Commits.Nodes) > 0 && raw.Commits.Nodes[0].Commit.StatusCheckRollup != nil {
|
||||
checksState = strings.ToLower(raw.Commits.Nodes[0].Commit.StatusCheckRollup.State)
|
||||
checksState = normalizeGraphQLCheckRollupState(raw.Commits.Nodes[0].Commit.StatusCheckRollup.State)
|
||||
}
|
||||
unresolved := 0
|
||||
for _, t := range raw.ReviewThreads.Nodes {
|
||||
@@ -447,6 +447,23 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
|
||||
}
|
||||
}
|
||||
|
||||
// normalizeGraphQLCheckRollupState converts GitHub's GraphQL status-rollup
|
||||
// enum to the smaller contract shared by REST and TaskPR persistence.
|
||||
func normalizeGraphQLCheckRollupState(raw string) string {
|
||||
switch strings.ToLower(strings.TrimSpace(raw)) {
|
||||
case "":
|
||||
return ""
|
||||
case checkStatusSuccess:
|
||||
return checkStatusSuccess
|
||||
case checkConclusionFail, "error":
|
||||
return checkConclusionFail
|
||||
case "expected", checkStatusPending:
|
||||
return checkStatusPending
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
// reviewNodesToSamples converts the GraphQL reviewNode shape to the shared
|
||||
// reviewSample slice used by the dedup helpers in client_helpers.go.
|
||||
func reviewNodesToSamples(nodes []reviewNode) []reviewSample {
|
||||
|
||||
@@ -220,6 +220,29 @@ func TestRunBatchedPRQuery_DecodesAliasesBackToRefs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeGraphQLCheckRollupState_CheckRollup(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
raw string
|
||||
want string
|
||||
}{
|
||||
{name: "success", raw: "SUCCESS", want: checkStatusSuccess},
|
||||
{name: "failure", raw: "FAILURE", want: checkConclusionFail},
|
||||
{name: "error", raw: "ERROR", want: checkConclusionFail},
|
||||
{name: "pending", raw: "PENDING", want: checkStatusPending},
|
||||
{name: "expected", raw: "EXPECTED", want: checkStatusPending},
|
||||
{name: "empty", raw: "", want: ""},
|
||||
{name: "unknown", raw: "NEUTRAL", want: ""},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := normalizeGraphQLCheckRollupState(tt.raw); got != tt.want {
|
||||
t.Fatalf("normalizeGraphQLCheckRollupState(%q) = %q, want %q", tt.raw, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunBatchedPRQuery_PaginatesResolvedReviewThreads(t *testing.T) {
|
||||
exec := &stubGraphQLExecutor{responses: []string{
|
||||
batchedPRReviewThreadsResponse(t, 102, resolvedReviewThreadNodes(100), true, "cursor-1"),
|
||||
|
||||
@@ -797,6 +797,49 @@ func (s *Service) prepareTaskPRSyncState(ctx context.Context, tp *TaskPR, status
|
||||
}
|
||||
}
|
||||
|
||||
func taskPRChangedFields(tp *TaskPR, status *PRStatus, next taskPRSyncState) []string {
|
||||
if tp == nil || status == nil || status.PR == nil {
|
||||
return nil
|
||||
}
|
||||
changed := make([]string, 0, 16)
|
||||
changed = appendChangedField(changed, "state", tp.State != next.state)
|
||||
changed = appendChangedField(changed, "pr_title", tp.PRTitle != status.PR.Title)
|
||||
changed = appendChangedField(changed, "additions", tp.Additions != status.PR.Additions)
|
||||
changed = appendChangedField(changed, "deletions", tp.Deletions != status.PR.Deletions)
|
||||
changed = appendChangedField(changed, "review_state", tp.ReviewState != status.ReviewState)
|
||||
changed = appendChangedField(changed, "checks_state", tp.ChecksState != status.ChecksState)
|
||||
changed = appendChangedField(changed, "mergeable_state", tp.MergeableState != next.mergeableState)
|
||||
changed = appendChangedField(changed, "review_count", tp.ReviewCount != next.reviewCount)
|
||||
changed = appendChangedField(
|
||||
changed,
|
||||
"pending_review_count",
|
||||
tp.PendingReviewCount != next.pendingReviewCount,
|
||||
)
|
||||
changed = appendChangedField(
|
||||
changed,
|
||||
"required_reviews",
|
||||
!intPtrEqual(tp.RequiredReviews, next.requiredReviews),
|
||||
)
|
||||
changed = appendChangedField(changed, "checks_total", tp.ChecksTotal != next.checksTotal)
|
||||
changed = appendChangedField(changed, "checks_passing", tp.ChecksPassing != next.checksPassing)
|
||||
changed = appendChangedField(
|
||||
changed,
|
||||
"unresolved_review_threads",
|
||||
tp.UnresolvedReviewThreads != next.unresolved,
|
||||
)
|
||||
changed = appendChangedField(changed, "base_branch", tp.BaseBranch != next.baseBranch)
|
||||
changed = appendChangedField(changed, "merged_at", !timeEqual(tp.MergedAt, next.mergedAt))
|
||||
changed = appendChangedField(changed, "closed_at", !timeEqual(tp.ClosedAt, next.closedAt))
|
||||
return changed
|
||||
}
|
||||
|
||||
func appendChangedField(changed []string, field string, isChanged bool) []string {
|
||||
if !isChanged {
|
||||
return changed
|
||||
}
|
||||
return append(changed, field)
|
||||
}
|
||||
|
||||
// SyncTaskPR updates a TaskPR record with the latest PR status. Multi-repo:
|
||||
// the row is found by (task_id, owner, repo, pr_number) since the same
|
||||
// task can have several PRs; the legacy GetTaskPR(taskID) "first match"
|
||||
@@ -815,22 +858,7 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
|
||||
}
|
||||
next := s.prepareTaskPRSyncState(ctx, tp, status)
|
||||
|
||||
changed := tp.State != next.state ||
|
||||
tp.PRTitle != status.PR.Title ||
|
||||
tp.Additions != status.PR.Additions ||
|
||||
tp.Deletions != status.PR.Deletions ||
|
||||
tp.ReviewState != status.ReviewState ||
|
||||
tp.ChecksState != status.ChecksState ||
|
||||
tp.MergeableState != next.mergeableState ||
|
||||
tp.ReviewCount != next.reviewCount ||
|
||||
tp.PendingReviewCount != next.pendingReviewCount ||
|
||||
!intPtrEqual(tp.RequiredReviews, next.requiredReviews) ||
|
||||
tp.ChecksTotal != next.checksTotal ||
|
||||
tp.ChecksPassing != next.checksPassing ||
|
||||
tp.UnresolvedReviewThreads != next.unresolved ||
|
||||
tp.BaseBranch != next.baseBranch ||
|
||||
!timeEqual(tp.MergedAt, next.mergedAt) ||
|
||||
!timeEqual(tp.ClosedAt, next.closedAt)
|
||||
changedFields := taskPRChangedFields(tp, status, next)
|
||||
|
||||
tp.State = next.state
|
||||
tp.PRTitle = status.PR.Title
|
||||
@@ -856,7 +884,18 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
|
||||
return fmt.Errorf("update task PR: %w", err)
|
||||
}
|
||||
|
||||
if changed && s.eventBus != nil {
|
||||
if len(changedFields) > 0 {
|
||||
if s.logger != nil {
|
||||
s.logger.Debug("task PR semantic state changed",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("pr_owner", tp.Owner),
|
||||
zap.String("pr_repo", tp.Repo),
|
||||
zap.Int("pr_number", tp.PRNumber),
|
||||
zap.Strings("changed_fields", changedFields),
|
||||
)
|
||||
}
|
||||
}
|
||||
if len(changedFields) > 0 && s.eventBus != nil {
|
||||
event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", tp)
|
||||
if err := s.eventBus.Publish(ctx, events.GitHubTaskPRUpdated, event); err != nil {
|
||||
s.logger.Debug("failed to publish task PR updated event", zap.Error(err))
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"reflect"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
@@ -952,6 +953,121 @@ func TestSyncTaskPR_SecondIdenticalSyncNoEvent(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncTaskPR_EquivalentRESTAndGraphQLStatusNoSecondEvent(t *testing.T) {
|
||||
svc, store, eb := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
createdAt := time.Date(2026, 8, 17, 10, 0, 0, 0, time.UTC)
|
||||
updatedAt := createdAt.Add(time.Hour)
|
||||
pr := &PR{
|
||||
Number: 1,
|
||||
Title: "Same status",
|
||||
URL: "https://github.com/owner/repo/pull/1",
|
||||
HTMLURL: "https://github.com/owner/repo/pull/1",
|
||||
State: "open",
|
||||
HeadBranch: "feat",
|
||||
HeadSHA: "abc",
|
||||
BaseBranch: "main",
|
||||
AuthorLogin: "alice",
|
||||
RepoOwner: "owner",
|
||||
RepoName: "repo",
|
||||
Mergeable: true,
|
||||
MergeableState: "clean",
|
||||
Additions: 3,
|
||||
Deletions: 1,
|
||||
CreatedAt: createdAt,
|
||||
UpdatedAt: updatedAt,
|
||||
}
|
||||
restStatus := newPRStatus(
|
||||
pr,
|
||||
[]PRReview{{Author: "alice", State: reviewStateApproved, CreatedAt: updatedAt}},
|
||||
[]CheckRun{{Status: checkStatusCompleted, Conclusion: checkConclusionSuccess}},
|
||||
)
|
||||
raw := &batchedPRResult{
|
||||
State: "OPEN",
|
||||
Title: pr.Title,
|
||||
URL: pr.URL,
|
||||
HeadRefName: pr.HeadBranch,
|
||||
BaseRefName: pr.BaseBranch,
|
||||
HeadRefOid: pr.HeadSHA,
|
||||
Additions: pr.Additions,
|
||||
Deletions: pr.Deletions,
|
||||
CreatedAt: pr.CreatedAt,
|
||||
UpdatedAt: pr.UpdatedAt,
|
||||
}
|
||||
raw.Author.Login = pr.AuthorLogin
|
||||
raw.Mergeable = "MERGEABLE"
|
||||
raw.MergeStatus = "CLEAN"
|
||||
raw.Reviews.Nodes = []reviewNode{{State: "APPROVED"}}
|
||||
raw.Reviews.Nodes[0].Author.Login = "alice"
|
||||
raw.Reviews.Nodes[0].SubmittedAt = updatedAt
|
||||
raw.Commits.Nodes = []struct {
|
||||
Commit struct {
|
||||
StatusCheckRollup *struct {
|
||||
State string `json:"state"`
|
||||
} `json:"statusCheckRollup"`
|
||||
} `json:"commit"`
|
||||
}{{}}
|
||||
raw.Commits.Nodes[0].Commit.StatusCheckRollup = &struct {
|
||||
State string `json:"state"`
|
||||
}{State: "SUCCESS"}
|
||||
graphqlStatus := convertBatchedPRResult(raw, "owner", "repo", 1)
|
||||
if graphqlStatus.ChecksState != restStatus.ChecksState {
|
||||
t.Fatalf("GraphQL checks state = %q, REST = %q", graphqlStatus.ChecksState, restStatus.ChecksState)
|
||||
}
|
||||
if graphqlStatus.ReviewState != restStatus.ReviewState || graphqlStatus.ReviewCount != restStatus.ReviewCount {
|
||||
t.Fatalf("GraphQL review summary = (%q, %d), REST = (%q, %d)", graphqlStatus.ReviewState, graphqlStatus.ReviewCount, restStatus.ReviewState, restStatus.ReviewCount)
|
||||
}
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1",
|
||||
Owner: "owner",
|
||||
Repo: "repo",
|
||||
PRNumber: 1,
|
||||
PRURL: pr.URL,
|
||||
PRTitle: "Old title",
|
||||
HeadBranch: pr.HeadBranch,
|
||||
BaseBranch: pr.BaseBranch,
|
||||
State: "open",
|
||||
}); err != nil {
|
||||
t.Fatalf("create task PR: %v", err)
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", restStatus); err != nil {
|
||||
t.Fatalf("REST sync: %v", err)
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", graphqlStatus); err != nil {
|
||||
t.Fatalf("GraphQL sync: %v", err)
|
||||
}
|
||||
if got := eb.publishedCount(); got != 1 {
|
||||
t.Fatalf("equivalent REST and GraphQL snapshots published %d events, want 1", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncTaskPR_ChangedFields(t *testing.T) {
|
||||
taskPR := &TaskPR{
|
||||
State: "open",
|
||||
PRTitle: "before",
|
||||
Additions: 1,
|
||||
Deletions: 2,
|
||||
ReviewState: "pending",
|
||||
ChecksState: "pending",
|
||||
MergeableState: "blocked",
|
||||
ReviewCount: 1,
|
||||
PendingReviewCount: 2,
|
||||
ChecksTotal: 3,
|
||||
ChecksPassing: 1,
|
||||
BaseBranch: "main",
|
||||
}
|
||||
status := &PRStatus{PR: &PR{Title: "after", State: "merged", Additions: 4, Deletions: 5, BaseBranch: "release", MergeableState: "clean"}, ReviewState: "approved", ChecksState: "success", ReviewCount: 2, PendingReviewCount: 0, ChecksTotal: 4, ChecksPassing: 4}
|
||||
next := taskPRSyncState{
|
||||
state: "merged", mergeableState: "clean", reviewCount: 2, pendingReviewCount: 0,
|
||||
checksTotal: 4, checksPassing: 4, baseBranch: "release",
|
||||
}
|
||||
want := []string{"state", "pr_title", "additions", "deletions", "review_state", "checks_state", "mergeable_state", "review_count", "pending_review_count", "checks_total", "checks_passing", "base_branch"}
|
||||
got := taskPRChangedFields(taskPR, status, next)
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("taskPRChangedFields() = %#v, want %#v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncTaskPR_PublishesEventOnMergeableStateChange(t *testing.T) {
|
||||
svc, store, eb := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
|
||||
Reference in New Issue
Block a user