elevate-project routes updated#345
Conversation
WalkthroughRouting updates: tenant read endpoints switched to GET, organizationExtension endpoints renamed/re-routed to createOrUpdate (eventListener removed), multi-path orchestration added for users’ solutions across project and survey, and package version bumped to 1.1.49. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant I as Interface Gateway
participant O as Orchestrator\n(getMergedProgramSolutions)
participant P as Project Service
participant S as Survey Service
Note over I,O: POST /interface/v1/users/solutions[/:id]\ninSequence=true, orchestrated=true
C->>I: Request solutions
I->>O: Invoke getMergedProgramSolutions
rect rgba(200,230,255,0.25)
note right of O: Calls two target paths\n(kept in sequence / orchestrated)
O->>P: POST /project/v1/users/solutions/:id
P-->>O: Project solutions
O->>S: POST /survey/v1/users/solutions/:id
S-->>O: Survey solutions
end
O-->>I: Merged solutions
I-->>C: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
elevate-project/constants/routes.js (1)
3075-3091: Bug: non-id interface route forwards to :id targets.
/interface/v1/users/solutionsshould not forward to.../solutions/:id. This will 404 or mis-bind params. Fix the paths and make theprojectservice explicit.Apply:
paths: [ - { - path: '/project/v1/users/solutions/:id', - type: 'POST', - }, - { service: 'survey', path: '/survey/v1/users/solutions/:id', type: 'POST' }, + { service: 'project', path: '/project/v1/users/solutions', type: 'POST' }, + { service: 'survey', path: '/survey/v1/users/solutions', type: 'POST' }, ],
🧹 Nitpick comments (4)
interface-routes/elevate-dev-routes.json (1)
13729-13741: ID variant (PATCH) also consistent.Symmetry with the POST routes is maintained; targetPackages and service are correct.
If clients may still call the old “update” endpoints, consider keeping temporary aliases with lower priority to ease rollout. Want a patch snippet for that?
elevate-project/constants/routes.js (3)
3031-3038: Tenant read: method switch to GET is correct; consider a backward-compatible POST alias.Many clients may still POST to these endpoints. Add temporary POST aliases (forwarding to target GET) for one release to avoid breaking callers, and update docs/SDKs.
Example aliases to add near these routes:
{ sourceRoute: "/user/v1/tenant/read", type: "POST", inSequence: false, orchestrated: false, targetRoute: { path: "/user/v1/tenant/read", type: "GET" }, service: "user" }, { sourceRoute: "/user/v1/tenant/read/:id", type: "POST", inSequence: false, orchestrated: false, targetRoute: { path: "/user/v1/tenant/read/:id", type: "GET" }, service: "user" }Also applies to: 3042-3050
3093-3109: Make service explicit for the project path (symmetry, no implicit defaults).Minor, but clearer and avoids surprises if the router doesn’t inherit
servicefrom the outer route.- paths: [ - { - path: '/project/v1/users/solutions/:id', - type: 'POST', - }, - { service: 'survey', path: '/survey/v1/users/solutions/:id', type: 'POST' }, - ], + paths: [ + { service: 'project', path: '/project/v1/users/solutions/:id', type: 'POST' }, + { service: 'survey', path: '/survey/v1/users/solutions/:id', type: 'POST' }, + ],
3235-3243: Rename to createOrUpdate: add deprecated /update aliases to avoid breaking clients.Good rename, but clients using
/updatewill break. Keep aliases for one version and announce deprecation.Example aliases:
{ sourceRoute: "/project/v1/organizationExtension/update", type: "POST", inSequence: false, targetRoute: { path: "/project/v1/organizationExtension/createOrUpdate", type: "POST" }, service: "project" }, { sourceRoute: "/project/v1/organizationExtension/update/:id", type: "POST", inSequence: false, targetRoute: { path: "/project/v1/organizationExtension/createOrUpdate/:id", type: "POST" }, service: "project" }Also applies to: 3245-3253
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
elevate-project/constants/routes.js(2 hunks)elevate-project/package.json(1 hunks)interface-routes/elevate-dev-routes.json(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: MallanagoudaB
PR: ELEVATE-Project/utils#329
File: elevate-project/constants/routes.js:0-0
Timestamp: 2025-08-05T09:46:50.071Z
Learning: In the elevate-project/constants/routes.js file, the `orchestrated` property is not compulsory for all route configurations. Many routes exist without this property, making it optional rather than required.
📚 Learning: 2025-08-05T09:46:50.071Z
Learnt from: MallanagoudaB
PR: ELEVATE-Project/utils#329
File: elevate-project/constants/routes.js:0-0
Timestamp: 2025-08-05T09:46:50.071Z
Learning: In the elevate-project/constants/routes.js file, the `orchestrated` property is not compulsory for all route configurations. Out of 293 total routes, 33 routes (about 11%) exist without this property, making it optional rather than required.
Applied to files:
interface-routes/elevate-dev-routes.json
🔇 Additional comments (6)
elevate-project/package.json (1)
3-3: Approve version bump; verify release artifacts and lockfileRoutes look updated: no /project/v1/organizationExtension/update found; new /project/v1/organizationExtension/createOrUpdate entries present (elevate-project/constants/routes.js and interface-routes/*.json); /interface/v1/users/solutions orchestration and getMergedProgramSolutions exist; /user/v1/tenant/read present. Missing package-lock.json under elevate-project — generate/commit package-lock.json and ensure changelog entry and git tag for v1.1.49 are created and pushed.
interface-routes/elevate-dev-routes.json (3)
13687-13699: Rename to createOrUpdate (POST) looks correct and consistent.Path, method, and target package/service align with the rest of the project routes.
13701-13713: ID variant (POST) correctly mirrors the non-ID route.Good parity between the two POST endpoints.
13715-13727: No lingering references to /project/v1/organizationExtension/update — PATCH variant approved.Search found no matches for '/project/v1/organizationExtension/update'; createOrUpdate (POST + PATCH) appears only in interface-routes/elevate-dev-routes.json and elevate-project/constants/routes.js.
elevate-project/constants/routes.js (2)
3255-3263: PATCH→POST translation on target: verify service contract or keep PATCH for parity.Both PATCH source routes forward as POST. If the project service’s
createOrUpdateaccepts PATCH, keep parity.If PATCH is supported downstream, switch target type:
- path: "/project/v1/organizationExtension/createOrUpdate", - type: "POST" + path: "/project/v1/organizationExtension/createOrUpdate", + type: "PATCH"- path: "/project/v1/organizationExtension/createOrUpdate/:id", - type: "POST" + path: "/project/v1/organizationExtension/createOrUpdate/:id", + type: "PATCH"Also applies to: 3265-3273
3075-3091: Controller already orchestrates targetRoute.paths — no router change required.getMergedProgramSolutions (elevate-project/controllers/project.js) validates config.targetRoute.paths (>=2), issues concurrent POSTs to both configured paths, and merges the responses; the router/gateway does not need to pre-aggregate or hand both responses.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
interface-routes/elevate-dev-routes.json (5)
13673-13685: Gate createOrUpdate behind internal rate limit.These endpoints mutate org extensions; they should not be publicly callable. Add an internal rate limit.
"targetPackages": [ { "basePackageName": "project", "packageName": "elevate-project" } ], - "service": "project" + "service": "project", + "rateLimit": { + "type": "internal" + }
13687-13699: Avoid dual POST/PATCH for the same resource path unless required.You expose both POST and PATCH for createOrUpdate/:id; this can cause client ambiguity and duplicate analytics. If backward compatibility isn’t mandatory, prefer PATCH for id-scoped updates and deprecate the POST variant; otherwise, document both clearly. Either way, add internal rate limiting.
"targetPackages": [ { "basePackageName": "project", "packageName": "elevate-project" } ], - "service": "project" + "service": "project", + "rateLimit": { + "type": "internal" + }
13701-13713: Protect PATCH createOrUpdate with internal rate limit.Same rationale as POST: this should be internal/admin-only.
"targetPackages": [ { "basePackageName": "project", "packageName": "elevate-project" } ], - "service": "project" + "service": "project", + "rateLimit": { + "type": "internal" + }
13715-13727: Rate-limit id-scoped PATCH as internal.Consistency with other orgExtension mutations.
"targetPackages": [ { "basePackageName": "project", "packageName": "elevate-project" } ], - "service": "project" + "service": "project", + "rateLimit": { + "type": "internal" + }
13729-13741: updateRelatedOrgs should be internal/admin-only and naming consistency nit.
- Add internal rate limit (likely admin action).
- Nit: we use camelCase here (
organizationExtension) but hyphen-case elsewhere under SCP (organization-extensions). Not blocking, just flagging cross-domain naming inconsistency for future cleanup."targetPackages": [ { "basePackageName": "project", "packageName": "elevate-project" } ], - "service": "project" + "service": "project", + "rateLimit": { + "type": "internal" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
elevate-project/constants/routes.js(2 hunks)interface-routes/elevate-dev-routes.json(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: MallanagoudaB
PR: ELEVATE-Project/utils#329
File: elevate-project/constants/routes.js:0-0
Timestamp: 2025-08-05T09:46:50.071Z
Learning: In the elevate-project/constants/routes.js file, the `orchestrated` property is not compulsory for all route configurations. Many routes exist without this property, making it optional rather than required.
📚 Learning: 2025-08-05T09:46:50.071Z
Learnt from: MallanagoudaB
PR: ELEVATE-Project/utils#329
File: elevate-project/constants/routes.js:0-0
Timestamp: 2025-08-05T09:46:50.071Z
Learning: In the elevate-project/constants/routes.js file, the `orchestrated` property is not compulsory for all route configurations. Out of 293 total routes, 33 routes (about 11%) exist without this property, making it optional rather than required.
Applied to files:
interface-routes/elevate-dev-routes.json
🔇 Additional comments (8)
interface-routes/elevate-dev-routes.json (1)
13673-13741: Sanity checks — stale routes absent; backend handlers not found
- No occurrences of /project/v1/organizationExtension/eventListener or /project/v1/organizationExtension/update found in the repo.
- interface-routes/elevate-dev-routes.json (lines 13673–13741) and elevate-project/constants/routes.js (≈3225–3272) contain: POST & PATCH /project/v1/organizationExtension/createOrUpdate and POST /project/v1/organizationExtension/updateRelatedOrgs.
- Repo search found no backend route/controller registrations referencing "organizationExtension" (no Express/Spring/Flask/.NET handler patterns). Confirm the Project service implements handlers for POST and PATCH on these paths; if the service only supports POST, remove/adjust the PATCH entries or add the missing handlers.
elevate-project/constants/routes.js (7)
3235-3243: organizationExtension: createOrUpdate/:id (POST) — confirm consumers updated.
- Route definitions exist in elevate-project/constants/routes.js (≈lines 3235–3241 and 3255–3261) and interface-routes/elevate-dev-routes.json (≈lines 13687 and 13715); repo search returned only these definitions and no consumer/client call sites. Verify all consumers were updated to call this path or update client code accordingly.
3245-3263: PATCH on source → POST on target: confirm method transform is intentional and documented.
- Confirm the PATCH→POST mapping is intentional (elevate-project/constants/routes.js:3245–3263).
- Verify the gateway allows PATCH (CORS Access-Control-Allow-Methods and routing must permit PATCH).
- Update API docs / client guidance to call out that the gateway accepts PATCH but the upstream receives POST.
Quick repo scan found no CORS config; manual verification of gateway/runtime CORS required.
3265-3273: Route moved: updateRelatedOrgs under organizationExtension — RBAC verification required.
- Found in elevate-project/constants/routes.js (lines 3265–3271) and interface-routes/elevate-dev-routes.json (~line 13729); grep output shows no authorizer/policy in the matched context — confirm authorizer/policies still reference /project/v1/organizationExtension/updateRelatedOrgs.
3042-3051: GET /user/v1/tenant/read/:id — LGTM; confirm parity with non-id endpoint.rg output shows :id variants in elevate-project/constants/routes.js (lines 3042–3048), elevate-survey/constants/routes.js (2455–2461) and multiple interface-routes/*.json; no non-id (/user/v1/tenant/read) match was found — verify the non-id route exists and that both endpoints return congruent response shapes or consolidate/remove duplicates.
3225-3233: organizationExtension: createOrUpdate (POST) added — verified; no legacy /organizationExtension/eventListener or /organizationExtension/update references found.New createOrUpdate routes exist in elevate-project/constants/routes.js (≈lines 3225–3269) and corresponding entries in interface-routes; unrelated /mentoring/v1/organization/eventListener endpoints remain in mentoring modules.
3075-3091: Bug: source has no :id but targets require :id (will 404 or mis-route).sourceRoute '/interface/v1/users/solutions' has no :id while targetRoute.paths include ':id' — update targets to non-:id paths (or add :id to the source). File: elevate-project/constants/routes.js (around lines 3075–3091).
sourceRoute: '/interface/v1/users/solutions', type: 'POST', inSequence: true, orchestrated: true, targetRoute: { type: 'POST', functionName: 'getMergedProgramSolutions', paths: [ { - path: '/project/v1/users/solutions/:id', + path: '/project/v1/users/solutions', type: 'POST', }, - { service: 'survey', path: '/survey/v1/users/solutions/:id', type: 'POST' }, + { service: 'survey', path: '/survey/v1/users/solutions', type: 'POST' }, ], }, service: 'project',Verified getMergedProgramSolutions exists (elevate-project/controllers/project.js) and routes currently include ':id'. Also confirm the orchestrator handler actually supports the
pathsarray shape and expected param substitution.
3093-3109: Orchestrated users/solutions/:id — mapping OK; verify it's safe to parallelize before changing inSequence.
- I located getMergedProgramSolutions in elevate-project/controllers/project.js (around line ~281) and the route entries in elevate-project/constants/routes.js; the controller validates there are >=2 target paths but the grep output doesn't prove whether it issues calls in parallel or depends on ordered results.
- Suggested change (if the controller is order-independent):
- inSequence: true, + inSequence: false,
- Quick checks to run locally:
- print the controller function: sed -n '270,380p' elevate-project/controllers/project.js
- search for parallel vs sequential patterns: rg -n "Promise.all|for await|await " elevate-project/controllers/project.js || true
| sourceRoute: "/user/v1/tenant/read", | ||
| type: "POST", | ||
| type: "GET", | ||
| inSequence: false, | ||
| orchestrated: false, | ||
| targetRoute: { | ||
| path: "/user/v1/tenant/read", | ||
| type: "POST" | ||
| type: "GET" | ||
| }, | ||
| service: "user" |
There was a problem hiding this comment.
🧩 Analysis chain
Switching tenant/read to GET — LGTM; confirm downstream usage.
- Aligns with REST semantics. Good.
- Please confirm all clients and docs were updated to call GET (no request body assumed).
🏁 Script executed:
#!/bin/bash
# Verify no lingering POST calls to tenant/read
rg -nP -C2 "/tenant/read\b" --type js --type ts --type jsonLength of output: 6031
Switch tenant/read to GET — update remaining POST references.
- elevate-project change is correct (GET) — elevate-project/constants/routes.js:3031–3039.
- Fix remaining POST entries: shiksha-project/constants/routes.js (lines ~1939–1941 — targetRoute.type: 'POST') and shiksha-survey/constants/routes.js (lines ~1856–1858 — targetRoute.type: 'POST').
- Change those to GET, ensure the readOrganization handler accepts GET/no request body, update clients/docs, then re-run the grep to confirm no POSTs remain: rg -nP '/user/v1/tenant/read\b' --type js --type ts --type json.
🤖 Prompt for AI Agents
In elevate-project/constants/routes.js around lines 3031–3039 the route is
correctly switched to GET; update the remaining incorrect POST references in
shiksha-project/constants/routes.js (around lines ~1939–1941) and
shiksha-survey/constants/routes.js (around lines ~1856–1858) by changing
targetRoute.type from 'POST' to 'GET', then update the readOrganization handler
to accept GET requests with no request body (remove/ignore body parsing), update
any client calls and API docs to use GET, and finally re-run the grep command rg
-nP '/user/v1/tenant/read\b' --type js --type ts --type json to confirm no POST
entries remain.
Summary by CodeRabbit
New Features
Refactor
Chores