Skip to content

fix(github_graphql): set the merged flag on pull requests - #9087

Closed
zyv4yk wants to merge 1 commit into
apache:mainfrom
zyv4yk:fix/graphql-pr-merged-flag
Closed

fix(github_graphql): set the merged flag on pull requests#9087
zyv4yk wants to merge 1 commit into
apache:mainfrom
zyv4yk:fix/graphql-pr-merged-flag

Conversation

@zyv4yk

@zyv4yk zyv4yk commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Pre Checklist

  • I have read through the Contributing Documentation.
  • I have added relevant tests.
  • I have added relevant documentation.
  • I will add labels to the PR, such as pr-type/bug-fix, pr-type/feature-development, etc.

Summary

_tool_github_pull_requests.merged is false for every pull request collected through GraphQL, merged ones included.

convertGithubPullRequest in backend/plugins/github_graphql/tasks/pr_extractor.go fills MergedAt, MergedBy and MergeCommitSha, but never Merged, so the column keeps its zero value:

ClosedAt:        pull.ClosedAt,
MergedAt:        pull.MergedAt,
Body:            pull.Body,

#8573 closed the same gap on the REST side — it added Merged: pull.Merged to plugins/github/tasks/pr_extractor.go — but the GraphQL extractor was not part of that change, so a connection running on GraphQL still stores the flag as false.

The flag is derived from mergedAt rather than collected as a separate merged field, for a practical reason: mergedAt is already present in _raw_github_graphql_prs for existing installs, so re-running the extractor repairs rows that are already collected. Querying merged would only fix data going forward and would need a full re-collect for the rest.

The two are equivalent by construction — GitHub sets mergedAt exactly when a pull request is merged. Checked against the API over the 50 most recent pull requests in grafana/grafana (18 merged): zero rows where bool(merged) != (mergedAt != null).

gh api graphql -f query='{ repository(owner:"grafana",name:"grafana"){
  pullRequests(first:50, states:[MERGED,CLOSED,OPEN], orderBy:{field:CREATED_AT,direction:DESC}){
    nodes { number state merged mergedAt } } } }'

Does this close any open issues?

No open issue. It is the GraphQL half of #8571, which was closed by #8573 for the REST collector only.

Other Information

Impact is limited today because the domain layer does not read this column for status: pr_convertor.go computes Status from pr.State == "MERGED" || (pr.State == "closed" && (pr.Merged || pr.MergedAt != nil)), and the GraphQL state is already MERGED. Any consumer that reads _tool_github_pull_requests.merged directly, or any future query that keys on merged = true, gets nothing for GraphQL-collected data.

go build, go vet and golangci-lint run ./plugins/github_graphql/... are clean. pr_extractor_test.go covers merged, closed-unmerged and open. The e2e package needs E2E_DB_URL and was not run locally; the only github_graphql e2e fixture covers deployments, which this PR does not touch.

I do not have permission to set labels; this is pr-type/bug-fix.

The GraphQL extractor filled MergedAt, MergedBy and MergeCommit but never
Merged, so every pull request collected through GraphQL is stored with
merged = false, including the merged ones. apache#8573 fixed the same gap on the
REST side; the GraphQL extractor was not part of it.

The flag is derived from mergedAt, which GitHub sets exactly when a pull
request is merged, so rows already sitting in the raw layer are repaired by
re-running the extractor and no re-collect is needed.

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

zyv4yk commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Closing this one — the domain layer does not read this column today (pr_convertor.go derives status from the state and mergedAt), so the practical impact is limited, and I would rather not split review attention away from #9086. Happy to reopen if a maintainer wants the flag stored for parity with the REST extractor.

@zyv4yk zyv4yk closed this Aug 31, 2026
@zyv4yk
zyv4yk deleted the fix/graphql-pr-merged-flag branch August 31, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant