Fix GitLab repo-fetch parent derivation and its test mock - #703
Fix GitLab repo-fetch parent derivation and its test mock#703blockgroot wants to merge 1 commit into
Conversation
parent was re-derived from repo.fullPath via string splitting, even though the GraphQL query already fetches namespace.fullPath, the field that actually represents it. The derivation happens to be equivalent for well-formed data, but is redundant and was the source of a stale test fixture whose fullPath and namespace.fullPath were mutually inconsistent (and whose id wasn't shaped like a real GitLab GID), causing the GitLab case of the fetchRepos test to fail since it was introduced in middlewarehq#672. Use namespace.fullPath directly and fix the mock to match a real GitLab GraphQL response shape. Fixes middlewarehq#702
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe GitLab repository fixture now uses a global project ID and a consistent full path. Repository mapping now reads the parent from ChangesGitLab repository mapping
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The PR makes a localized GitLab parent-path correction and aligns its test mock with the implementation; no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Fixes #702.
fetchRepos's GitLab branch (web-server/pages/api/internal/[org_id]/git_org_repos.ts)re-derived
parentfromrepo.fullPathvia string splitting, even though the GraphQLquery already fetches
namespace.fullPath— the field that directly represents it. Thederivation is mathematically equivalent for well-formed data, so it wasn't a production
bug, but it was redundant, and it was the root cause of a stale test fixture: the mock's
fullPathandnamespace.fullPathwere mutually inconsistent with each other, and itsidwasn't shaped like a real GitLab GID (gid://gitlab/Project/...). Both caused theGitLab case of
fetchRepos's test to fail ever since it was introduced in #672.Changes
git_org_repos.ts: readparentfromrepo.namespace.fullPathdirectly instead ofre-deriving it from
repo.fullPath.git_org_repos.test.ts: use a realistic, internally-consistent GitLab GraphQL mock(GID-shaped
id,fullPath/namespace.fullPaththat agree with each other), andcorrect the assertion's expected
idto a number, matching what the implementationactually returns.
Test plan
cd web-server && npx jest --watchAll=false --passWithNoTests -t "fetchRepos"— bothGitHub and GitLab cases now pass (previously the GitLab case failed).
npx tsc --noEmit— clean.npx eslint "pages/api/internal/[org_id]/git_org_repos.ts"— no new findings (verifiedidentical pre-existing warning/error count on
mainbefore this change; this file haspre-existing lint debt unrelated to this fix, left untouched per scope).
Summary by CodeRabbit