refactor(db): extract shared query builder in prWorkItemsRepository#982
refactor(db): extract shared query builder in prWorkItemsRepository#982
Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM — Clean DRY refactoring that eliminates ~120 lines of repeated Drizzle query code without changing any observable behavior. The extraction of getPRSummarySelect(), getPRSummaryGroupBy(), queryPRSummaries(), and resolveOrgProjectIds() is well-motivated, correctly preserves all column selections and groupings, and the lazy-evaluation approach is a smart choice for test compatibility. All CI checks pass.
One minor note: the as SQL cast in listPRsForWorkItem (line 317) is a pragmatic trade-off introduced by the refactoring. It's safe at runtime since and() with two concrete SQL arguments always returns SQL, but it's worth knowing this is a type assertion rather than a type-safe narrowing. Not blocking — just documenting the trade-off. An alternative would be to widen queryPRSummaries to accept SQL | undefined, but that would weaken the contract and allow accidentally unfiltered queries, so the current approach is the better trade-off.
🕵️ claude-code · claude-opus-4-6 · run details
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Refactors
src/db/repositories/prWorkItemsRepository.tsto eliminate ~120 lines of repeated Drizzle query code across 3 near-identical PR listing functions.getPRSummarySelect()— lazy getter for the shared 7-column select map used by all PR summary queriesgetPRSummaryGroupBy()— lazy getter for the matching groupBy columns, also spread intolistUnifiedWorkForProject's extended groupByqueryPRSummaries(whereCondition)— internal query builder that encapsulates the full select/from/leftJoin/where/groupBy/orderBy pattern;listPRsForProject,listPRsForOrg, andlistPRsForWorkItemare now single-line call-throughsresolveOrgProjectIds(orgId)— DRY helper for fetching project IDs by org, removing duplication betweenlistPRsForOrgandlistWorkItemsThe helpers use lazy getter functions (not module-level constants) to avoid breaking tests that mock
schema/index.jswithout includingprWorkItems.Test plan
prWorkItemsRepositorypass without modificationtests/unit/api/routers/prs.test.ts) passrouter.test.ts,routers/projects.test.ts) now passnpm run typecheck— zero errorsnpm run lint— zero errorsRelated
https://trello.com/c/cZaMf79A/507-find-top-candidate-for-refactoring-and-plan-clean-refactoring-of-it-look-for-god-classes-modules-functions-files-code-duplicatio
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details