feat(portal-sdk): add approval API#14
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a complete approval workflow API to the portal SDK. It extends the OpenAPI specification with three new endpoints under ChangesApproval Workflow Implementation
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/portal-sdk/openapi.yaml (1)
1124-1153: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSeparate secret-bearing credential payloads from general credential responses.
At Line [1125], Line [1152], and Line [1182], secret fields are part of shared credential response schemas, and that shared schema is reused by list/get responses at Line [1845] and Line [1860]. This broadens the contract surface for secrets and increases accidental leakage risk in downstream logging/telemetry.
Use dedicated create/regenerate response schemas for secret-bearing fields, and keep list/get schemas secret-free.
Proposed contract split (illustrative)
- ApplicationCredentialResponse: - content: - application/json: - schema: - $ref: '`#/components/schemas/ApplicationCredential`' + ApplicationCredentialResponse: + content: + application/json: + schema: + $ref: '`#/components/schemas/ApplicationCredentialPublic`' + ApplicationCredentialCreateOrRegenerateResponse: + content: + application/json: + schema: + $ref: '`#/components/schemas/ApplicationCredentialWithSecrets`'Also applies to: 1182-1184, 1840-1861
🤖 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 `@packages/portal-sdk/openapi.yaml` around lines 1124 - 1153, The shared credential response schema (ApplicationCredentialBasicsCommon / BasicAuthApplicationCredentialBasics) currently includes secret fields (e.g., properties.key and basic-auth.password); refactor by splitting into two schemas: keep a secret-free common response (e.g., ApplicationCredentialBasicsPublic or keep ApplicationCredentialBasicsCommon) used by list/get responses, and add a separate creation/regeneration response schema (e.g., ApplicationCredentialWithSecret or BasicAuthApplicationCredentialSecret) that contains secret-bearing properties (key, basic-auth.password). Update all references so list/get endpoints reference the public schema and only create/regenerate endpoints reference the secret schema, and ensure enum/type/name definitions remain shared as needed.
🧹 Nitpick comments (1)
packages/portal-sdk/openapi.yaml (1)
641-647: ⚡ Quick winAvoid positional
$refpointers for response field enums.At Line [641]-Line [647], response properties depend on
/paths/.../parameters/{index}references. This is brittle: reordering parameters can silently change response typing. Prefer named component schemas and reference those from both parameters and response properties.🤖 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 `@packages/portal-sdk/openapi.yaml` around lines 641 - 647, The response schema uses positional parameter $ref pointers (e.g., refs to /paths/~1api~1approvals/get/parameters/0/schema, /parameters/1/schema, /parameters/2/schema, /parameters/3/schema) for properties like status, result, and resource_type which is brittle; instead add named component schemas (for example components/schemas/ApprovalStatus, ApprovalResult, ResourceType) and update both the parameters in the GET /api/approvals operation and the response properties (status, result, resource_type) to reference those new component schema names so the enums are stable and independent of parameter ordering.
🤖 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.
Outside diff comments:
In `@packages/portal-sdk/openapi.yaml`:
- Around line 1124-1153: The shared credential response schema
(ApplicationCredentialBasicsCommon / BasicAuthApplicationCredentialBasics)
currently includes secret fields (e.g., properties.key and basic-auth.password);
refactor by splitting into two schemas: keep a secret-free common response
(e.g., ApplicationCredentialBasicsPublic or keep
ApplicationCredentialBasicsCommon) used by list/get responses, and add a
separate creation/regeneration response schema (e.g.,
ApplicationCredentialWithSecret or BasicAuthApplicationCredentialSecret) that
contains secret-bearing properties (key, basic-auth.password). Update all
references so list/get endpoints reference the public schema and only
create/regenerate endpoints reference the secret schema, and ensure
enum/type/name definitions remain shared as needed.
---
Nitpick comments:
In `@packages/portal-sdk/openapi.yaml`:
- Around line 641-647: The response schema uses positional parameter $ref
pointers (e.g., refs to /paths/~1api~1approvals/get/parameters/0/schema,
/parameters/1/schema, /parameters/2/schema, /parameters/3/schema) for properties
like status, result, and resource_type which is brittle; instead add named
component schemas (for example components/schemas/ApprovalStatus,
ApprovalResult, ResourceType) and update both the parameters in the GET
/api/approvals operation and the response properties (status, result,
resource_type) to reference those new component schema names so the enums are
stable and independent of parameter ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82bb55e0-513b-41d0-868b-90d19fd24373
⛔ Files ignored due to path filters (15)
packages/portal-sdk/src/generated/client.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/client.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/index.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/types.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/utils.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/auth.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/params.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/pathSerializer.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/queryKeySerializer.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/serverSentEvents.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/utils.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/index.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/sdk.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/transformers.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/types.gen.tsis excluded by!**/generated/**
📒 Files selected for processing (4)
packages/portal-sdk/openapi.yamlpackages/portal-sdk/src/approval.tspackages/portal-sdk/src/browser.tspackages/portal-sdk/src/index.ts
Add an `approval` resource to the portal SDK exposing the developer-portal
approval workflow endpoints:
- client.approval.list(query?) -> GET /api/approvals
- client.approval.accept(id, body?) -> POST /api/approvals/{approval_id}/accept
- client.approval.reject(id, body?) -> POST /api/approvals/{approval_id}/reject
openapi.yaml gains only the three approval paths plus the once-only credential
secret fields (key-auth.key, basic-auth.username/password) on the credential
response schemas — existing formatting is left untouched. src/generated carries
only the corresponding additions (no unrelated regeneration churn).
Verified: nx lint / build / typecheck pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ba451c9 to
31f136a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/portal-sdk/openapi.yaml (1)
1040-1072:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't put one-time secrets on the shared credential read model.
ApplicationCredentialis reused by the generic credential GET/list responses, so addingkey-auth.keyandbasic-auth.passwordhere makes those broad read endpoints advertise API keys/passwords as ordinary response fields. That is a security contract regression and causes the generated SDK to treat secret material as normal read data instead of create/regenerate-only output. Split secret-bearing create/regenerate responses from the redacted read/list schema.🤖 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 `@packages/portal-sdk/openapi.yaml` around lines 1040 - 1072, The current shared read/list model exposes one-time secrets (key-auth.key and basic-auth.password) via ApplicationCredential/BasicAuthApplicationCredentialBasics; remove those secret properties from the public/read schemas and create separate create/regenerate response schemas that include the secrets. Concretely: delete key-auth.key and basic-auth.password from the read/list schema (ApplicationCredential / BasicAuthApplicationCredentialBasics), introduce new response schemas (e.g., ApplicationCredentialCreateResponse and BasicAuthApplicationCredentialCreateResponse) that extend the common ApplicationCredentialBasics and add the secret fields for creation/regeneration only, and update the POST/regenerate endpoints to return the new secret-bearing schemas while keeping GET/list endpoints returning the redacted read schemas. Ensure enum/type names (BasicAuthApplicationCredentialBasics, key-auth, basic-auth, ApplicationCredential) are used to locate and refactor the definitions.
🧹 Nitpick comments (2)
packages/portal-sdk/openapi.yaml (2)
1480-1485: ⚡ Quick winClose
ApprovalActionReqto unsupported fields.This schema currently allows arbitrary extra JSON members on accept/reject because
additionalPropertiesdefaults totrue. For a workflow-changing endpoint, it is better to fail fast on unexpected payloads.🔧 Suggested schema tweak
ApprovalActionReq: type: object + additionalProperties: false properties: metadata: type: string description: Opaque JSON string stored on the approval. The developer portal uses it to carry the acting platform admin's identity.🤖 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 `@packages/portal-sdk/openapi.yaml` around lines 1480 - 1485, The ApprovalActionReq schema currently allows arbitrary extra properties; update the ApprovalActionReq definition to disallow unknown fields by adding additionalProperties: false under the ApprovalActionReq object and explicitly declare any intended fields (e.g., metadata) so unexpected payload members cause validation errors; if you plan future optional fields, add them to the properties list rather than relying on open objects.
1949-1989: 🏗️ Heavy liftModel pending and finished approvals as separate shapes.
statusis coupled to fields likeresultandoperated_at, but this response flattens everything into one mostly-optional object. The generated TypeScript type therefore permits impossible states such as a"finished"approval with no outcome, which leaks avoidable ambiguity into the SDK surface. AoneOffor pending vs finished approvals would make the contract much more accurate.🤖 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 `@packages/portal-sdk/openapi.yaml` around lines 1949 - 1989, Split the flattened approval object into two explicit shapes and use oneOf to distinguish them: create an ApprovalPending (allOf ResourceBasics + object requiring event, status (value 'pending'), resource_type, applied_at and excluding finished-only fields like result and operated_at) and an ApprovalFinished (allOf ResourceBasics + object requiring status (value 'finished'), result, operated_at, resource_type and the operator_* fields as appropriate); then replace the current combined schema with oneOf: [`#/components/schemas/ApprovalPending`, `#/components/schemas/ApprovalFinished`]. Ensure you keep refs to ApprovalEvent, ApprovalStatus, ApprovalResult and preserve metadata, resource_id, resource_name, applicant_id/applicant_name but move required constraints (result, operated_at) into ApprovalFinished and applied_at into ApprovalPending so impossible states are no longer representable.
🤖 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 `@test.ts`:
- Around line 7-8: Remove the hardcoded bearer token string in test.ts and load
it from an environment variable instead (e.g., process.env.API_SEVEN_TOKEN);
update the code that currently includes the literal
'a7prt-...294c60bb05f988072fee33' to read the token from env, validate presence
at startup and fail with a clear error if missing, and ensure the token is never
logged or committed—also add a note to docs/.env example and confirm secrets are
excluded from VCS (.gitignore, and rotate the exposed token immediately).
- Around line 39-49: The catch block currently only handles APIError via
APIError.isAPIError(error) and silently ignores all other exceptions; update the
catch to handle non-APIError cases as well — for any error that is not APIError,
log the error (including stack) and rethrow it (or return a rejected promise) so
failures aren't swallowed; keep the existing APIError handling (including usage
of error.rawError()) and ensure the catch block always either handles or
rethrows the error.
- Around line 6-9: The code is calling new API7Portal with positional args but
the class defines constructor(opts: Options); change the instantiation to pass a
single options object to match the constructor contract (e.g. new API7Portal({
baseUrl: 'https://developer.apiseven.com', apiKey: 'a7prt-...'})) so the
API7Portal constructor(opts: Options) receives the expected properties; update
any tests/usages that construct API7Portal to use the same object form.
- Around line 14-38: The test prints full SDK responses (console.log(res),
console.log(res1), console.log('Created application:', res2),
console.log('Updated application:', res3), console.log('Deleted application:',
res2.id), console.log(res4), console.log(res5.raw_openapis)) which can leak
secrets; change these to log only minimal/redacted fields (e.g., id, name,
status) or use a shared redactResponse helper before logging, and ensure you do
not print raw_openapis or full response bodies from client.application.* and
client.apiProduct.* — instead log safe properties (res2.id, res2.name,
res3.id/status, res1.length or ids, and a redacted/boolean indicator for
apiProduct content).
---
Outside diff comments:
In `@packages/portal-sdk/openapi.yaml`:
- Around line 1040-1072: The current shared read/list model exposes one-time
secrets (key-auth.key and basic-auth.password) via
ApplicationCredential/BasicAuthApplicationCredentialBasics; remove those secret
properties from the public/read schemas and create separate create/regenerate
response schemas that include the secrets. Concretely: delete key-auth.key and
basic-auth.password from the read/list schema (ApplicationCredential /
BasicAuthApplicationCredentialBasics), introduce new response schemas (e.g.,
ApplicationCredentialCreateResponse and
BasicAuthApplicationCredentialCreateResponse) that extend the common
ApplicationCredentialBasics and add the secret fields for creation/regeneration
only, and update the POST/regenerate endpoints to return the new secret-bearing
schemas while keeping GET/list endpoints returning the redacted read schemas.
Ensure enum/type names (BasicAuthApplicationCredentialBasics, key-auth,
basic-auth, ApplicationCredential) are used to locate and refactor the
definitions.
---
Nitpick comments:
In `@packages/portal-sdk/openapi.yaml`:
- Around line 1480-1485: The ApprovalActionReq schema currently allows arbitrary
extra properties; update the ApprovalActionReq definition to disallow unknown
fields by adding additionalProperties: false under the ApprovalActionReq object
and explicitly declare any intended fields (e.g., metadata) so unexpected
payload members cause validation errors; if you plan future optional fields, add
them to the properties list rather than relying on open objects.
- Around line 1949-1989: Split the flattened approval object into two explicit
shapes and use oneOf to distinguish them: create an ApprovalPending (allOf
ResourceBasics + object requiring event, status (value 'pending'),
resource_type, applied_at and excluding finished-only fields like result and
operated_at) and an ApprovalFinished (allOf ResourceBasics + object requiring
status (value 'finished'), result, operated_at, resource_type and the operator_*
fields as appropriate); then replace the current combined schema with oneOf:
[`#/components/schemas/ApprovalPending`, `#/components/schemas/ApprovalFinished`].
Ensure you keep refs to ApprovalEvent, ApprovalStatus, ApprovalResult and
preserve metadata, resource_id, resource_name, applicant_id/applicant_name but
move required constraints (result, operated_at) into ApprovalFinished and
applied_at into ApprovalPending so impossible states are no longer
representable.
🪄 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: 065255f2-ecf2-4fdc-aa87-7e4da2c79c19
⛔ Files ignored due to path filters (14)
packages/portal-sdk/src/generated/client.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/client.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/index.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/types.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/client/utils.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/auth.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/params.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/pathSerializer.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/queryKeySerializer.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/serverSentEvents.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/core/utils.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/index.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/sdk.gen.tsis excluded by!**/generated/**packages/portal-sdk/src/generated/types.gen.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
packages/portal-sdk/eslint.config.mjspackages/portal-sdk/openapi.yamlpackages/portal-sdk/src/approval.tspackages/portal-sdk/src/unstable/typing.tstest.ts
✅ Files skipped from review due to trivial changes (1)
- packages/portal-sdk/src/unstable/typing.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/portal-sdk/src/approval.ts
ae61481 to
5e6a385
Compare
432f98c to
ee7b7f3
Compare
What
Adds an
approvalresource to the portal SDK, wrapping the developer-portal approval workflow endpoints:client.approval.list(query?)GET /api/approvalslistApprovalsclient.approval.accept(id, body?)POST /api/approvals/{approval_id}/acceptacceptApprovalclient.approval.reject(id, body?)POST /api/approvals/{approval_id}/rejectrejectApprovalWired into both the server (
src/index.ts) and browser (src/browser.ts)API7Portalclients.Scope kept minimal
openapi.yamlis not wholesale re-bundled — every existing line (and its double-quoteformatting) is left byte-for-byte unchanged. The only additions are:
key-auth.key,basic-auth.username/basic-auth.password(returned only on create/regenerate).→
openapi.yaml: 206 insertions, 0 deletions.src/generated/**likewise carries only the approval additions (new operations, types,transformer, exports) grafted onto the existing generated output — no unrelated regeneration
churn (no generator-version reformatting, no touched
core/orclient/files).Verification
nx lint portal-sdk✅nx build portal-sdk✅ (ESM + CJS + d.ts)nx typecheck portal-sdk✅nx test portal-sdk— no test files in repo (pre-existing)🤖 Generated with Claude Code
Summary by CodeRabbit