Update Devportal API Spec#2046
Conversation
ShakyaPr
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
|
Warning Review limit reached
More reviews will be available in 50 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements two coordinated changes: (1) restructures the OpenAPI specification to consolidate organization-scoped resources under a unified Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/developer-portal/docs/devportal-openapi-spec-v1.yaml (2)
686-715:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMigrate subscription policy delete/get callers to
policyIdsemantics.Line 686 changes the contract to
{policyId}for lookup/delete. Current integrations still include name-based delete usage (for example,portals/developer-portal/src/routes/devportalRoute.js:103andplatform-api/src/internal/client/devportal_client/subscription_policies_service.go:68-95).Update callers/routes to use
policyIdconsistently, or temporarily keep a deprecated name-based delete contract during migration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/docs/devportal-openapi-spec-v1.yaml` around lines 686 - 715, The OpenAPI changed the delete/get path to use {policyId} (operationId: getSubscriptionPolicy and deleteSubscriptionPolicy) but existing callers still pass a name; update all callers to use the new policyId semantics (e.g., change the route usage in portals/developer-portal/src/routes/devportalRoute.js and the client calls in platform-api/src/internal/client/devportal_client/subscription_policies_service.go to send policyId instead of name), or if immediate migration is risky add a temporary backward-compatible handler in the subscription policies server that accepts the name-based parameter and resolves it to policyId before delegating to the existing deleteSubscriptionPolicy/getSubscriptionPolicy logic; ensure parameter names and generated client calls match the OpenAPI parameter name policyId.
176-2574:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAlign runtime routes with the new org-scoped path contract.
Line 176 starts a broad move to
/o/{orgId}/devportal/v1/..., but current route wiring still shows/organizations/:orgId/...patterns (for example,portals/developer-portal/src/routes/devportalRoute.js:100-110). This leaves the published spec ahead of served routes.Please migrate runtime route registrations in the same release, or keep deprecated legacy paths in the spec until runtime migration is live.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/docs/devportal-openapi-spec-v1.yaml` around lines 176 - 2574, The OpenAPI spec was changed to use the org-scoped path contract (/o/{orgId}/devportal/v1/...), but runtime route registrations (e.g., in the devportalRoute router that currently registers routes like /organizations/:orgId/...) still use the legacy /organizations/:orgId/... pattern; update the route wiring in the devportalRoute registration (and any helper that builds route strings) to register the new paths matching the spec, update middleware/handlers that read the param (orgId vs organizationId) to use the same param name, run/update route tests, and if backward compatibility is required add explicit redirects or duplicate legacy routes rather than changing only the spec.
🧹 Nitpick comments (1)
portals/developer-portal/src/openapi/handlers/_multipart.js (1)
37-43: ⚡ Quick winUse a null-prototype accumulator for
grouped(and||=is safe on the service’s Node runtime).
groupedis keyed byreq.files[*].fieldname; switching from{}to a null-prototype object avoids collisions with inherited properties. The||=usage is fine since the developer-portal container runs onnode:23-bookworm-slim.♻️ Proposed change
- const grouped = {}; + const grouped = Object.create(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/src/openapi/handlers/_multipart.js` around lines 37 - 43, The accumulator object `grouped` is created with a normal object literal which can inherit properties and cause key collisions when using `req.files[*].fieldname`; change its initialization to a null-prototype object (use Object.create(null)) and keep the existing grouping logic inside the loop (the block that iterates over req.files and uses (grouped[f.fieldname] ||= []).push(f)) so keys are stored on a clean map with the same ||= shorthand.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/developer-portal/src/openapi/handlers/_multipart.js`:
- Around line 38-47: The handler currently blocks the event loop by using
fs.readFileSync when f.path is present and also uses a plain object for grouped
that can collide with prototype keys; change the handler to be async and await
fs.promises.readFile(f.path) (or use util.promisify) inside a try/catch and call
next(err) on failures, and replace the grouped plain object with
Object.create(null) or a Map so fieldname keys cannot collide; locate the loop
that iterates req.files and the grouped variable initialization in _multipart.js
(the block that sets f.buffer and pushes into grouped[f.fieldname]) and update
those lines to use asynchronous reads and a safe map-like container while
preserving the existing error routing via next.
---
Outside diff comments:
In `@portals/developer-portal/docs/devportal-openapi-spec-v1.yaml`:
- Around line 686-715: The OpenAPI changed the delete/get path to use {policyId}
(operationId: getSubscriptionPolicy and deleteSubscriptionPolicy) but existing
callers still pass a name; update all callers to use the new policyId semantics
(e.g., change the route usage in
portals/developer-portal/src/routes/devportalRoute.js and the client calls in
platform-api/src/internal/client/devportal_client/subscription_policies_service.go
to send policyId instead of name), or if immediate migration is risky add a
temporary backward-compatible handler in the subscription policies server that
accepts the name-based parameter and resolves it to policyId before delegating
to the existing deleteSubscriptionPolicy/getSubscriptionPolicy logic; ensure
parameter names and generated client calls match the OpenAPI parameter name
policyId.
- Around line 176-2574: The OpenAPI spec was changed to use the org-scoped path
contract (/o/{orgId}/devportal/v1/...), but runtime route registrations (e.g.,
in the devportalRoute router that currently registers routes like
/organizations/:orgId/...) still use the legacy /organizations/:orgId/...
pattern; update the route wiring in the devportalRoute registration (and any
helper that builds route strings) to register the new paths matching the spec,
update middleware/handlers that read the param (orgId vs organizationId) to use
the same param name, run/update route tests, and if backward compatibility is
required add explicit redirects or duplicate legacy routes rather than changing
only the spec.
---
Nitpick comments:
In `@portals/developer-portal/src/openapi/handlers/_multipart.js`:
- Around line 37-43: The accumulator object `grouped` is created with a normal
object literal which can inherit properties and cause key collisions when using
`req.files[*].fieldname`; change its initialization to a null-prototype object
(use Object.create(null)) and keep the existing grouping logic inside the loop
(the block that iterates over req.files and uses (grouped[f.fieldname] ||=
[]).push(f)) so keys are stored on a clean map with the same ||= shorthand.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 140413b5-3f2f-4ef0-a1f7-f7e558d68ab3
📒 Files selected for processing (9)
portals/developer-portal/docs/devportal-openapi-spec-v1.yamlportals/developer-portal/src/openapi/handlers/_multipart.jsportals/developer-portal/src/openapi/handlers/apiContent.jsportals/developer-portal/src/openapi/handlers/apis.jsportals/developer-portal/src/openapi/handlers/applications.jsportals/developer-portal/src/openapi/handlers/identityProviders.jsportals/developer-portal/src/openapi/handlers/organizationContent.jsportals/developer-portal/src/openapi/handlers/organizations.jsportals/developer-portal/src/openapi/handlers/subscriptionPolicies.js
ShakyaPr
force-pushed
the
devportal_spec_change
branch
from
7821530 to
45a6d33
Compare
| "404": | ||
| $ref: "#/components/responses/TextMessageResponse" | ||
| /organizations/{orgId}/provider: | ||
| /o/{orgId}/devportal/v1/provider: |
There was a problem hiding this comment.
| /o/{orgId}/devportal/v1/provider: | |
| /o/{orgId}/devportal/v1/providers: |
2 participants
Purpose
devportal-openapi-spec-v1.yamlwith below things:/o/{orgId}/devportal/v1/{resource}(was/devportal/organizations/{orgId}/{resource})URL shape
servers.urlis nowhttps://devportal.api-platform.io(prod) plus the local HTTP/HTTPS variants. The/devportalbase path is gone fromservers.urland reintroduced after{orgId}in each path./organizations/{orgId}/X→/o/{orgId}/devportal/v1/X(~50 paths).POST /organizations,GET/PATCH/DELETE /organizations/{orgId}) kept at the root — they manage the org collection itself, not resources within an org./login,/temp-arazzo-file,/applications/*) untouched; these will be dropped from the spec in a follow-up when the legacy-only routes are removed.Naming changes
/identityProvider(singular, camelCase)/identity-providers(plural, kebab)/events,/events/{eventId}/webhook-events,/webhook-events/{eventId}/deliveries/{deliveryId}/retry/webhook-deliveries/{deliveryId}/retry{subscriptionId}(1 path, 1 param ref){subId}— consolidated on existingSubIdcomponent{name}(view name, 3 paths){viewName}— consolidated on existingViewNamecomponent{policyIdOrName}(overloaded){policyId}(see subscription policies redesign)Component cleanup: removed unused
Nameand duplicateSubscriptionIdparameter components; renamedPolicyIdOrName→PolicyId.OperationIds normalized so
APIis rendered asApi(14 IDs across API metadata, API content, API flows): e.g.createAPIMetadata→createApiMetadata,getAPIFlow→getApiFlow. The PUT API-content endpoint is renamed fromupdateAPIContenttoreplaceApiContentto better reflect its replace-semantics.OAuth*operationIds left as-is (proper noun).Subscription policies — lookup-by-name redesign
The old {policyIdOrName} path overloaded one segment to mean either UUID or name (with GET/DELETE interpreting it differently). Replaced with the standard REST split:
/subscription-policies— new listSubscriptionPolicies operation with optional ?name= filter. Returns an array (0 or 1 item when filtered). New SubscriptionPolicyListResponse component added./subscription-policies/{policyId}— ID-only, single-resource fetch./subscription-policies/{policyId}— ID-only, single-resource delete.Callers that only have a name now do list?name=X then operate on the returned policyId. New endpoint will need a matching controller method when the spec-driven router is enabled.