Skip to content

Commit b67aad7

Browse files
JAORMXclaude
JAORMX
and
claude
committed
fix(github): carry _raw_data provenance and guard zero account ids
Review feedback on #8894: the rewritten ConvertAccounts dropped the _raw_data fields from converted accounts. Select them with COALESCE, preferring the enriched _tool_github_accounts row and falling back to the _tool_github_repo_accounts row for non-committers. The e2e fixture now carries the _raw_data columns like every other tool fixture, and the regenerated snapshot pins the pre-existing provenance for enriched accounts plus the issue-extractor provenance for the non-committer case. Also guard PR author and merged-by id generation against zero ids: an unmerged PR or a deleted user otherwise yields the domain id github:GithubAccount:<conn>:0, the same orphan-FK shape this PR fixes. issue_convertor already guards its AuthorId the same way. Verified with the full github e2e suite on both MySQL and PostgreSQL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 51b6419 commit b67aad7

4 files changed

Lines changed: 44 additions & 31 deletions

File tree

‎backend/plugins/github/e2e/raw_tables/_tool_github_repo_accounts.csv‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
connection_id,account_id,repo_github_id,login
2-
1,21979,134018330,appleboy
3-
1,964542,134018330,sarathsp06
4-
1,1052632,134018330,runner-mei
5-
1,3794113,134018330,shanhuhai5739
6-
1,3971390,134018330,ppmoon
7-
1,7496278,134018330,panjf2000
8-
1,8518239,134018330,gitter-badger
9-
1,145564,134018330,milichev
10-
1,11763614,2,Moonlight-Zhao
11-
1,12420699,2,shanghai-Jerry
12-
1,14950473,2,zqkgo
13-
1,22429695,2,codecov[bot]
14-
1,24841832,2,rikewang
15-
1,31087327,2,chensanle
16-
1,32893410,2,zhangyuanxue
17-
1,38849208,2,king526
1+
connection_id,account_id,repo_github_id,login,_raw_data_params,_raw_data_table,_raw_data_id,_raw_data_remark
2+
1,21979,134018330,appleboy,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,8,
3+
1,964542,134018330,sarathsp06,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,1,
4+
1,1052632,134018330,runner-mei,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,13,
5+
1,3794113,134018330,shanhuhai5739,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,2,
6+
1,3971390,134018330,ppmoon,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,14,
7+
1,7496278,134018330,panjf2000,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,5,
8+
1,8518239,134018330,gitter-badger,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,9,
9+
1,145564,134018330,milichev,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_issues,1,
10+
1,11763614,2,Moonlight-Zhao,,,0,
11+
1,12420699,2,shanghai-Jerry,,,0,
12+
1,14950473,2,zqkgo,,,0,
13+
1,22429695,2,codecov[bot],,,0,
14+
1,24841832,2,rikewang,,,0,
15+
1,31087327,2,chensanle,,,0,
16+
1,32893410,2,zhangyuanxue,,,0,
17+
1,38849208,2,king526,,,0,

‎backend/plugins/github/e2e/snapshot_tables/account.csv‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
id,email,full_name,user_name,avatar_url,organization,_raw_data_params,_raw_data_table,_raw_data_id,_raw_data_remark
2-
github:GithubAccount:1:1052632,runner.mei@,runner,runner-mei,https://avatars.githubusercontent.com/u/1052632?v=4,,,,0,
3-
github:GithubAccount:1:145564,,,milichev,,,,,0,
4-
github:GithubAccount:1:21979,appleboy.tw@gmail.com,Bo-Yi Wu,appleboy,https://avatars.githubusercontent.com/u/21979?v=4,"COSCUP,nodejs-tw,moztw,h5bp,CodeIgniter-TW,drone,Getmore,golangtw,laravel-taiwan,go-xorm,gin-gonic,PHPConf-TW,Mediatek-Cloud,SJFinder,go-gitea,laradock,gin-contrib,tagfans,maintainers,go-training,go-ggz,the-benchmarker,golang-queue",,,0,
5-
github:GithubAccount:1:3794113,shanhu5739@gmail.com,Derek,shanhuhai5739,https://avatars.githubusercontent.com/u/3794113?v=4,,,,0,
6-
github:GithubAccount:1:3971390,cnliuyunpeng@gmail.com,ppmoon,ppmoon,https://avatars.githubusercontent.com/u/3971390?v=4,,,,0,
7-
github:GithubAccount:1:7496278,i@andypan.me,Andy Pan,panjf2000,https://avatars.githubusercontent.com/u/7496278?v=4,,,,0,
8-
github:GithubAccount:1:8518239,badger@gitter.im,The Gitter Badger,gitter-badger,https://avatars.githubusercontent.com/u/8518239?v=4,,,,0,
9-
github:GithubAccount:1:964542,sarath.sp06@gmail.com,Sarath Sadasivan Pillai,sarathsp06,https://avatars.githubusercontent.com/u/964542?v=4,"exotel,leadmrktr,shellagilehub,odysseyhack,boodltech",,,0,
2+
github:GithubAccount:1:1052632,runner.mei@,runner,runner-mei,https://avatars.githubusercontent.com/u/1052632?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,13,
3+
github:GithubAccount:1:145564,,,milichev,,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_issues,1,
4+
github:GithubAccount:1:21979,appleboy.tw@gmail.com,Bo-Yi Wu,appleboy,https://avatars.githubusercontent.com/u/21979?v=4,"COSCUP,nodejs-tw,moztw,h5bp,CodeIgniter-TW,drone,Getmore,golangtw,laravel-taiwan,go-xorm,gin-gonic,PHPConf-TW,Mediatek-Cloud,SJFinder,go-gitea,laradock,gin-contrib,tagfans,maintainers,go-training,go-ggz,the-benchmarker,golang-queue","{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,8,
5+
github:GithubAccount:1:3794113,shanhu5739@gmail.com,Derek,shanhuhai5739,https://avatars.githubusercontent.com/u/3794113?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,2,
6+
github:GithubAccount:1:3971390,cnliuyunpeng@gmail.com,ppmoon,ppmoon,https://avatars.githubusercontent.com/u/3971390?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,14,
7+
github:GithubAccount:1:7496278,i@andypan.me,Andy Pan,panjf2000,https://avatars.githubusercontent.com/u/7496278?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,5,
8+
github:GithubAccount:1:8518239,badger@gitter.im,The Gitter Badger,gitter-badger,https://avatars.githubusercontent.com/u/8518239?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,9,
9+
github:GithubAccount:1:964542,sarath.sp06@gmail.com,Sarath Sadasivan Pillai,sarathsp06,https://avatars.githubusercontent.com/u/964542?v=4,"exotel,leadmrktr,shellagilehub,odysseyhack,boodltech","{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,1,

‎backend/plugins/github/tasks/account_convertor.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ func ConvertAccounts(taskCtx plugin.SubTaskContext) errors.Error {
8181
// merged-by user), and LEFT JOIN _tool_github_accounts for profile detail when it
8282
// was collected. This guarantees a domain `accounts` row for every CreatorId /
8383
// AuthorId the other convertors emit, instead of only for users who committed.
84+
// Raw-data provenance follows the same rule as the profile fields: the enriched
85+
// _tool_github_accounts row when we collected one, the repo_accounts row otherwise.
86+
// Note the consequence: fallback-provenance rows carry a _raw_data_table other than
87+
// _raw_github_api_accounts, so the batch-save divider's full-sync delete-then-reinsert
88+
// (keyed on this converter's raw table) never deletes them; they are reconciled by
89+
// upsert only. Scope deletion still covers them via _raw_data_params.
8490
// SQL is kept DB-agnostic (no backtick quoting, COALESCE not IFNULL) so it runs on
8591
// both MySQL and PostgreSQL.
8692
Input: func(stateManager *api.SubtaskStateManager) (dal.Rows, errors.Error) {
@@ -90,10 +96,10 @@ func ConvertAccounts(taskCtx plugin.SubTaskContext) errors.Error {
9096
COALESCE(ga.name, '') AS name,
9197
COALESCE(ga.email, '') AS email,
9298
COALESCE(ga.avatar_url, '') AS avatar_url,
93-
_tool_github_repo_accounts._raw_data_params AS _raw_data_params,
94-
_tool_github_repo_accounts._raw_data_table AS _raw_data_table,
95-
_tool_github_repo_accounts._raw_data_id AS _raw_data_id,
96-
_tool_github_repo_accounts._raw_data_remark AS _raw_data_remark`),
99+
COALESCE(ga._raw_data_params, _tool_github_repo_accounts._raw_data_params) AS _raw_data_params,
100+
COALESCE(ga._raw_data_table, _tool_github_repo_accounts._raw_data_table) AS _raw_data_table,
101+
COALESCE(ga._raw_data_id, _tool_github_repo_accounts._raw_data_id) AS _raw_data_id,
102+
COALESCE(ga._raw_data_remark, _tool_github_repo_accounts._raw_data_remark) AS _raw_data_remark`),
97103
dal.From(&models.GithubRepoAccount{}),
98104
dal.Join(`left join _tool_github_accounts ga on (
99105
ga.connection_id = _tool_github_repo_accounts.connection_id

‎backend/plugins/github/tasks/pr_convertor.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ func ConvertPullRequests(taskCtx plugin.SubTaskContext) errors.Error {
8787
OriginalStatus: pr.State,
8888
Title: pr.Title,
8989
Url: pr.Url,
90-
AuthorId: accountIdGen.Generate(data.Options.ConnectionId, pr.AuthorId),
9190
AuthorName: pr.AuthorName,
9291
Description: pr.Body,
9392
CreatedDate: pr.GithubCreatedAt,
@@ -104,9 +103,17 @@ func ConvertPullRequests(taskCtx plugin.SubTaskContext) errors.Error {
104103
Additions: pr.Additions,
105104
Deletions: pr.Deletions,
106105
MergedByName: pr.MergedByName,
107-
MergedById: accountIdGen.Generate(data.Options.ConnectionId, pr.MergedById),
108106
IsDraft: pr.IsDraft,
109107
}
108+
// Generate account ids only for real users (#8886): a zero AuthorId (deleted
109+
// user) or zero MergedById (unmerged PR) would otherwise produce an id like
110+
// github:GithubAccount:1:0 that no accounts row can ever match.
111+
if pr.AuthorId != 0 {
112+
domainPr.AuthorId = accountIdGen.Generate(data.Options.ConnectionId, pr.AuthorId)
113+
}
114+
if pr.MergedById != 0 {
115+
domainPr.MergedById = accountIdGen.Generate(data.Options.ConnectionId, pr.MergedById)
116+
}
110117
if pr.State == "open" || pr.State == "OPEN" {
111118
domainPr.Status = code.OPEN
112119
} else if pr.State == "MERGED" || (pr.State == "closed" && (pr.Merged || pr.MergedAt != nil)) {

0 commit comments

Comments
 (0)