feat(api): include entitlements v2 methods in JavaScript SDK #3375
Conversation
📝 WalkthroughWalkthroughAdds V2 entitlements surfaces alongside existing V1 in JavaScript client: new CustomerEntitlementsV2, EntitlementsV2, and GrantsV2 classes with /api/v2 endpoints; wires dual properties (entitlementsV1 and entitlements) in Customers and OpenMeter clients; retains V1 classes/methods for backward compatibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
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. ✨ Finishing touches
🧪 Generate unit tests
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
🧹 Nitpick comments (5)
api/client/javascript/src/client/entitlements.ts (3)
404-417: Minor ergonomics: destructureoptionsto avoid leaking unknown propsTo keep the request options clean and avoid passing stray fields, destructure
optionsto extractquery.- public async list( - options?: RequestOptions & { - query?: operations['listEntitlementsV2']['parameters']['query'] - } - ) { - const resp = await this.client.GET('/api/v2/entitlements', { - params: { - query: options?.query, - }, - ...options, - }) + public async list( + options?: RequestOptions & { + query?: operations['listEntitlementsV2']['parameters']['query'] + } + ) { + const { query, ...opts } = options ?? {} + const resp = await this.client.GET('/api/v2/entitlements', { + params: { query }, + ...opts, + })
453-466: Repeat theoptionsdestructuring pattern hereSame rationale as above for
GrantsV2.list.- public async list( - options?: RequestOptions & { - query?: operations['listGrantsV2']['parameters']['query'] - } - ) { - const resp = await this.client.GET('/api/v2/grants', { - params: { - query: options?.query, - }, - ...options, - }) + public async list( + options?: RequestOptions & { + query?: operations['listGrantsV2']['parameters']['query'] + } + ) { + const { query, ...opts } = options ?? {} + const resp = await this.client.GET('/api/v2/grants', { + params: { query }, + ...opts, + })
478-492: Consider renaming method tovoidGrant(keep alias for consistency)
voidis a keyword; while valid as a property,voidGrantreads clearer and aligns with the operation name. You can keepvoidas a thin alias to avoid surprises.- public async void( + public async voidGrant( grantId: operations['voidGrant']['parameters']['path']['grantId'], options?: RequestOptions ) { const resp = await this.client.DELETE('/api/v1/grants/{grantId}', { params: { path: { grantId, }, }, ...options, }) return transformResponse(resp) } + + /** @deprecated Use `voidGrant` */ + public async void( + grantId: operations['voidGrant']['parameters']['path']['grantId'], + options?: RequestOptions + ) { + return this.voidGrant(grantId, options) + }api/client/javascript/src/client/customers.ts (2)
20-22: Expose V2 by default but mark V1 as deprecatedGood dual-surface. Add deprecation JSDoc on V1 to steer users.
- public entitlementsV1: CustomerEntitlements + /** @deprecated Use `entitlements` (V2). */ + public entitlementsV1: CustomerEntitlementsAlso applies to: 26-28
392-419: Nit: destructureoptionsto avoid passing extraneous fieldsSame ergonomics pattern as elsewhere.
- public async list( + public async list( customerIdOrKey: operations['listCustomerEntitlementsV2']['parameters']['path']['customerIdOrKey'], options?: RequestOptions & { query?: operations['listCustomerEntitlementsV2']['parameters']['query'] } ) { - const resp = await this.client.GET( + const { query, ...opts } = options ?? {} + const resp = await this.client.GET( '/api/v2/customers/{customerIdOrKey}/entitlements', { params: { path: { customerIdOrKey }, - query: options?.query, + query, }, - ...options, + ...opts, } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/client/javascript/src/client/customers.ts(2 hunks)api/client/javascript/src/client/entitlements.ts(1 hunks)api/client/javascript/src/client/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
api/client/javascript/src/client/customers.ts (3)
api/client/javascript/src/client/schemas.ts (1)
operations(11775-26482)api/client/javascript/src/client/common.ts (1)
RequestOptions(6-6)api/client/javascript/src/client/utils.ts (1)
transformResponse(15-35)
api/client/javascript/src/client/entitlements.ts (3)
api/client/javascript/src/client/schemas.ts (2)
paths(1-2417)operations(11775-26482)api/client/javascript/src/client/common.ts (1)
RequestOptions(6-6)api/client/javascript/src/client/utils.ts (1)
transformResponse(15-35)
api/client/javascript/src/client/index.ts (1)
api/client/javascript/src/client/entitlements.ts (2)
Entitlements(17-269)EntitlementsV2(390-438)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Artifacts / Container image
- GitHub Check: Test
- GitHub Check: Code Generators
- GitHub Check: Build
- GitHub Check: Lint
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
api/client/javascript/src/client/index.ts (2)
11-11: Import looks goodBrings both V1 and V2 into scope cleanly.
56-58: Avoid breaking change: keepentitlementsas V1, addentitlementsV2for V2Changing the canonical
entitlementsto V2 will break existing consumers using V1 methods (create/delete/override/reset). Keepentitlementsas V1, addentitlementsV2, and mark V1 deprecated in JSDoc.- public entitlementsV1: Entitlements - public entitlements: EntitlementsV2 + /** @deprecated Use `entitlementsV2` moving forward. */ + public entitlementsV1: Entitlements + // Back-compat: keep V1 on the canonical name + public entitlements: Entitlements + // New V2 surface + public entitlementsV2: EntitlementsV2api/client/javascript/src/client/entitlements.ts (1)
425-437: LGTM:getendpoint wiring and typing are correctCorrect path, params, and return handling via
transformResponse.api/client/javascript/src/client/customers.ts (1)
617-641: LGTM: history endpoint models requiredwindowSizecleanlyClear separation of required
windowSizeand optional query. Path and params look correct.
| this.entitlementsV1 = new Entitlements(this.client) | ||
| this.entitlements = new EntitlementsV2(this.client) | ||
| this.events = new Events(this.client) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wire-up to preserve back-compat and expose V2
Initialize V1 once and alias it to both entitlements and entitlementsV1; expose V2 as entitlementsV2.
- this.entitlementsV1 = new Entitlements(this.client)
- this.entitlements = new EntitlementsV2(this.client)
+ const entitlementsV1 = new Entitlements(this.client)
+ this.entitlements = entitlementsV1
+ this.entitlementsV1 = entitlementsV1
+ this.entitlementsV2 = new EntitlementsV2(this.client)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.entitlementsV1 = new Entitlements(this.client) | |
| this.entitlements = new EntitlementsV2(this.client) | |
| this.events = new Events(this.client) | |
| const entitlementsV1 = new Entitlements(this.client) | |
| this.entitlements = entitlementsV1 | |
| this.entitlementsV1 = entitlementsV1 | |
| this.entitlementsV2 = new EntitlementsV2(this.client) | |
| this.events = new Events(this.client) |
🤖 Prompt for AI Agents
In api/client/javascript/src/client/index.ts around lines 94 to 96, you should
initialize the V1 Entitlements only once and alias it for backward
compatibility: create a single instance and assign it to both
this.entitlementsV1 and this.entitlements, then instantiate V2 and assign it to
this.entitlementsV2 (leave this.events as-is); ensure you do not instantiate
Entitlements twice and that property names are exactly entitlements,
entitlementsV1, and entitlementsV2.
2 participants
Summary by CodeRabbit