feat(api): add accounting periods to spec#4365
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds accounting period types and enum, operation contracts (list/get/close) with filters and request models, registers accounting tag metadata and endpoint wiring in konnect/openmeter TypeSpec packages, imports accounting index, and adds stubbed server route handlers for the new endpoints. ChangesAccounting Periods API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
f038588 to
e459570
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/spec/packages/aip/src/accounting/operations.tsp (1)
27-27: ⚡ Quick winType the
statusfilter with the accounting status enum.Using
Common.StringFieldFilterExactmakesstatusaccept arbitrary strings in generated contracts. It’s better to use an enum-based filter shape so clients get strict typing foropen|closing|closed.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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip/src/accounting/operations.tsp` at line 27, Replace the loose string filter on the status property (currently declared as status?: Common.StringFieldFilterExact) with a strict enum-based filter so generated contracts only allow the accounting states; define or import an AccountStatus enum with values "open" | "closing" | "closed" and change the type to use the enum filter (for example Common.EnumFieldFilterExact<AccountStatus> or an equivalent EnumFieldFilter type your Common library provides), and update any imports/exports to reference AccountStatus so the operations.tsp status property is strongly typed against that enum.
🤖 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 `@api/spec/packages/aip/src/accounting/operations.tsp`:
- Around line 116-119: The `close` operation's return type is currently a
create-style response but should be a 200-style resource response; update the
declaration of close(`@body` body: CloseAccountingPeriodRequest) to return a
200-style response (e.g. replace Shared.CreateResponse<AccountingPeriod> with
Shared.ResourceResponse<AccountingPeriod> or your project's standard 200
resource response type) while keeping Common.ErrorResponses, so the signature
reads close(`@body` body: CloseAccountingPeriodRequest):
Shared.ResourceResponse<AccountingPeriod> | Common.ErrorResponses.
- Around line 107-109: Update the docs to use the deepObject-shaped query
example instead of the current flat example: replace the example
`filter[status]=open` with the deep-object form `filter[status][]=open` in the
comment/block so it matches the API’s deepObject filter handling (search for the
comment containing `Returns the period transitioned to \`closing\`` and the
`filter[status]=open` text and update it).
---
Nitpick comments:
In `@api/spec/packages/aip/src/accounting/operations.tsp`:
- Line 27: Replace the loose string filter on the status property (currently
declared as status?: Common.StringFieldFilterExact) with a strict enum-based
filter so generated contracts only allow the accounting states; define or import
an AccountStatus enum with values "open" | "closing" | "closed" and change the
type to use the enum filter (for example
Common.EnumFieldFilterExact<AccountStatus> or an equivalent EnumFieldFilter type
your Common library provides), and update any imports/exports to reference
AccountStatus so the operations.tsp status property is strongly typed against
that enum.
🪄 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: b0761e8d-8da3-49c4-a395-f379de953e87
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (8)
api/spec/packages/aip/src/accounting/index.tspapi/spec/packages/aip/src/accounting/operations.tspapi/spec/packages/aip/src/accounting/period.tspapi/spec/packages/aip/src/konnect.tspapi/spec/packages/aip/src/openmeter.tspapi/spec/packages/aip/src/shared/consts.tspapi/v3/api.gen.goapi/v3/server/routes.go
Summary by CodeRabbit