feat(api): Add settlement mode to v1 api#4029
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:
📝 WalkthroughWalkthroughAdds a new exported Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/spec/packages/legacy/src/productcatalog/subscription.tsp (1)
130-135: Nice addition — consider documenting the default here too for consistencyThis field has a default in code, but the description doesn’t mention it (unlike the plan field). Adding that one line makes the API docs clearer for users.
✍️ Suggested doc tweak
/** * The settlement mode of the subscription, which determines how the invoice is generated and payments are applied. + * When not specified, defaults to `credit_then_invoice`. */ `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) `@summary`("Settlement mode") settlementMode?: PlanSettlementMode = PlanSettlementMode.creditThenInvoiceSettlementMode;As per coding guidelines
**/*.tsp: "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/legacy/src/productcatalog/subscription.tsp` around lines 130 - 135, The JSDoc for the subscription property settlementMode omits the default value; update the comment for settlementMode to explicitly state the default (PlanSettlementMode.creditThenInvoiceSettlementMode) so the API docs match the actual implementation and mirror the style used on the plan field; locate the settlementMode property and add one sentence like "Defaults to PlanSettlementMode.creditThenInvoiceSettlementMode." to the existing description.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/spec/packages/legacy/src/productcatalog/subscription.tsp`:
- Around line 130-135: The JSDoc for the subscription property settlementMode
omits the default value; update the comment for settlementMode to explicitly
state the default (PlanSettlementMode.creditThenInvoiceSettlementMode) so the
API docs match the actual implementation and mirror the style used on the plan
field; locate the settlementMode property and add one sentence like "Defaults to
PlanSettlementMode.creditThenInvoiceSettlementMode." to the existing
description.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b9b076e-da79-427c-8b03-4700c19b1de9
⛔ Files ignored due to path filters (8)
api/client/go/client.gen.gois excluded by!api/client/**api/client/javascript/src/client/schemas.tsis excluded by!api/client/**api/client/javascript/src/zod/index.tsis excluded by!api/client/**api/client/python/openmeter/_generated/models/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (3)
api/api.gen.goapi/spec/packages/legacy/src/productcatalog/plan.tspapi/spec/packages/legacy/src/productcatalog/subscription.tsp
815be88 to
a7b453c
Compare
a7b453c to
d7c0dde
Compare
d7c0dde to
c078160
Compare
c078160 to
12debf9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/spec/packages/legacy/src/productcatalog/subscription.tsp (1)
463-467: Add summary + enum behavior docs forPlanSubscriptionCreate.settlementMode.This works functionally, but the docs here are much thinner than the read model. Adding
@summaryand short value semantics will make generated OpenAPI/SDK docs clearer for users.Suggested change
/** * The settlement mode of the subscription. + * - credit_then_invoice: credits are applied first, then remaining balance is invoiced. + * - credit_only: only credits are generated and applied, no invoices are created. */ + `@summary`("Settlement mode") settlementMode?: SettlementMode;As per coding guidelines, "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/legacy/src/productcatalog/subscription.tsp` around lines 463 - 467, Add a short `@summary` and enum-value semantics to the PlanSubscriptionCreate.settlementMode property so the OpenAPI/SDK docs match the read model; update the JSDoc for the settlementMode field (symbol: PlanSubscriptionCreate.settlementMode) to include a one-line `@summary` and a brief description of each SettlementMode enum value (what each option means/behaves like at runtime), keeping wording concise and consistent with the read model semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/spec/packages/legacy/src/productcatalog/subscription.tsp`:
- Around line 130-138: Subscription.read model currently declares settlementMode
as optional which makes generated clients treat it as nullable; update the
declaration for the Subscription type so settlementMode is required (remove the
optional marker) while keeping its type SettlementMode and default value
SettlementMode.CreditThenInvoice, i.e., change the field declaration for
settlementMode in this file (the symbol to edit is settlementMode on the
Subscription read model, referencing the SettlementMode enum) so the read model
always exposes a non-null value.
---
Nitpick comments:
In `@api/spec/packages/legacy/src/productcatalog/subscription.tsp`:
- Around line 463-467: Add a short `@summary` and enum-value semantics to the
PlanSubscriptionCreate.settlementMode property so the OpenAPI/SDK docs match the
read model; update the JSDoc for the settlementMode field (symbol:
PlanSubscriptionCreate.settlementMode) to include a one-line `@summary` and a
brief description of each SettlementMode enum value (what each option
means/behaves like at runtime), keeping wording concise and consistent with the
read model semantics.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5293b0f3-0995-4e6c-9de9-47af0781bb77
⛔ Files ignored due to path filters (8)
api/client/go/client.gen.gois excluded by!api/client/**api/client/javascript/src/client/schemas.tsis excluded by!api/client/**api/client/javascript/src/zod/index.tsis excluded by!api/client/**api/client/python/openmeter/_generated/models/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (3)
api/api.gen.goapi/spec/packages/legacy/src/productcatalog/plan.tspapi/spec/packages/legacy/src/productcatalog/subscription.tsp
🚧 Files skipped from review as they are similar to previous changes (1)
- api/spec/packages/legacy/src/productcatalog/plan.tsp
12debf9 to
0cf2297
Compare
Add settlement mode to v1 api
Summary by CodeRabbit