refactor(git): de-duplicate Bitbucket PR/issue field mappings - #1780
Conversation
Extract mapPullRequestItem/mapIssueItem (and a prSharedFields helper for prToPullRequestInfo) so the raw->view-model mapping exists once instead of three near-identical copies. getBitbucketPullRequestList/getBitbucketIssueList now map fetchAllPages results through the same mappers the unit tests exercise via parsePullRequests/parseIssues, closing the gap where tested code and production code had diverged.
![gfargo-horizon-agent[bot]](https://avatars.githubusercontent.com/in/4083186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 Agent review (sonnet) — LGTM
REVIEW: LGTM
RESOLVES: full
The PR extracts mapPullRequestItem/mapIssueItem/prSharedFields exactly per the plan, wires getBitbucketPullRequestList/getBitbucketIssueList/parsePullRequests/parseIssues/prToPullRequestInfo through them, and keeps the __test export delegating to parsePullRequests/parseIssues so existing tests now exercise the real production mapping path. Diff is scoped to a single file, all mapping logic is byte-identical to the prior triplicated code, and tests/lint/typecheck pass locally.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise consolidates the triplicated PR mapping into prSharedFields/mapPullRequestItem and the duplicated issue mapping into mapIssueItem, with production paths (getBitbucketPullRequestList/getBitbucketIssueList/prToPullRequestInfo) now routed through the same functions the tests exercise via parsePullRequests/parseIssues. Tests, lint, and typecheck all pass with no behavioral changes or scope creep detected.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise correctly extracts mapPullRequestItem/mapIssueItem/prSharedFields and routes all three call sites (parsePullRequests/parseIssues, getBitbucketPullRequestList/getBitbucketIssueList, prToPullRequestInfo) through them, eliminating the triplicated mapping and making the existing unit tests exercise the real production mapping code. Tests (43/43) and typecheck pass with no new issues introduced.
…715-high-bitbucketlistdata-ts-fiel # Conflicts: # src/git/bitbucketListData.ts
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The only change since the last review is a merge-from-main commit that resolved a conflict in bitbucketListData.ts; comparing origin/main to HEAD shows it's a clean rebase of the dedup refactor (prSharedFields/mapPullRequestItem/mapIssueItem) on top of main's independently-added @me-resolution, bbqlQuote escaping, and paginate helper, with nothing dropped or duplicated back in. No new production or test regressions were introduced.
What
De-duplicates the Bitbucket raw→view-model field mapping in
src/git/bitbucketListData.ts, which previously existed three times for PRs and twice for issues.Why
Closes #1715
Plane: OSS-957
parsePullRequests/parseIssueswere exercised only by unit tests, whilegetBitbucketPullRequestList/getBitbucketIssueListpaginated viafetchAllPagesand re-mapped inline — so a mapping fix applied to one copy could silently miss the others, and the tests wouldn't catch it.How
mapPullRequestItem/mapIssueItemas the single source of truth for raw→view-model mapping.prSharedFieldsfor the 11 fields common toPullRequestListItemandPullRequestInfo.parsePullRequests/parseIssuesnow just callparsePage(...).values.map(mapX).getBitbucketPullRequestList/getBitbucketIssueListnow mapfetchAllPagesresults through the same mappers instead of inline closures.prToPullRequestInfonow reusesprSharedFields+bodyinstead of a third hand-written copy.normalizeState,nicknameOf,nicknamesOf, etc. all unchanged;sanitize*still applied at call sites.Testing
bitbucketListData.test.tssuite, 33 tests, now exercises production mapping code)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Closes #1715