feat: v3 invoice api actions#4671
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 invoice deletion and four workflow actions—advance, approve, retry, and snapshot quantities—across API contracts, Go server routing and handlers, billing validation, the JavaScript SDK, and end-to-end tests. ChangesInvoice lifecycle operations
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant BillingInvoiceHandler
participant InvoiceService
Client->>Server: POST /billing/invoices/{invoiceId}/approve
Server->>BillingInvoiceHandler: ProgressInvoice(approve)
BillingInvoiceHandler->>InvoiceService: ApproveInvoice(invoiceId)
InvoiceService-->>BillingInvoiceHandler: Updated invoice
BillingInvoiceHandler->>BillingInvoiceHandler: ToAPIStandardInvoice(invoice)
BillingInvoiceHandler-->>Client: 200 JSON invoice
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/spec/packages/aip/src/invoices/operations.tsp (1)
154-238: 📐 Maintainability & Code Quality | 🔵 TrivialNice and consistent op definitions — one doc nit.
All four new actions (
advance,approve,retry,snapshotQuantities) follow the existing pattern nicely (route/operationId/summary/extensions all lining up with what ends up inapi.gen.go). One thing worth calling out to users: the actual handler (action.go) rejects non-standard invoices with a validation error, but none of these four doc comments mention that these actions only apply to standard invoices. Might be worth a quick line in each doc comment so API consumers aren't surprised.As per path instructions, "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/invoices/operations.tsp` around lines 154 - 238, Add a brief note to each of the four invoice action doc comments on advance, approve, retry, and snapshotQuantities to state that these actions only apply to standard invoices. Update the comments in the invoice operations definition so they match the behavior enforced by action.go and make the API contract clearer for consumers.Source: Path instructions
🤖 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-client-javascript/README.md`:
- Line 206: Update the snapshotQuantities operation description in
operations.tsp so it consistently uses “usage-based” instead of “usage based” in
both places; this will flow through to the generated README entry for
client.invoices.snapshotQuantities and keep the docs wording consistent.
In `@api/v3/handlers/billinginvoices/action.go`:
- Around line 71-99: The ProgressInvoice action handler is doing an unnecessary
preflight invoice fetch and type check before dispatching to the service. Remove
the GetInvoiceById lookup and the standard-invoice guard from the action flow in
billinginvoices/action.go, and let the existing service methods (ApproveInvoice,
RetryInvoice, AdvanceInvoice, ForceCollectInvoice) own invoice loading and
validation so the handler only switches on action and returns the service
result.
---
Nitpick comments:
In `@api/spec/packages/aip/src/invoices/operations.tsp`:
- Around line 154-238: Add a brief note to each of the four invoice action doc
comments on advance, approve, retry, and snapshotQuantities to state that these
actions only apply to standard invoices. Update the comments in the invoice
operations definition so they match the behavior enforced by action.go and make
the API contract clearer for consumers.
🪄 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: 2c508cdb-c323-40f5-a42d-d964ff3c760c
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (11)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/invoices.tsapi/spec/packages/aip-client-javascript/src/models/operations/invoices.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/sdk/invoices.tsapi/spec/packages/aip/src/invoices/operations.tspapi/v3/api.gen.goapi/v3/handlers/billinginvoices/action.goapi/v3/handlers/billinginvoices/convert.goapi/v3/handlers/billinginvoices/handler.goapi/v3/server/routes.go
1e40dba to
08677be
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/billinginvoices_v3_test.go (2)
1696-1699: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the typed status constant instead of hardcoded strings.
Line 1636 correctly uses
string(billing.StandardInvoiceStatusDraftWaitingAutoApproval), but the manual-approval-needed check (line 1698) and the waiting-for-collection check (line 1959) fall back to raw string literals. Using the equivalent typed constants keeps these assertions resilient to any future renaming of the extended-status values.♻️ Suggested fix (pending confirming the exact constant names)
- require.Equal(t, "draft.manual_approval_needed", invoice.StatusDetails.ExtendedStatus) + require.Equal(t, string(billing.StandardInvoiceStatusDraftManualApprovalNeeded), invoice.StatusDetails.ExtendedStatus)- require.Equal(t, "draft.waiting_for_collection", invoice.StatusDetails.ExtendedStatus) + require.Equal(t, string(billing.StandardInvoiceStatusDraftWaitingForCollection), invoice.StatusDetails.ExtendedStatus)Also applies to: 1957-1960
🤖 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 `@e2e/billinginvoices_v3_test.go` around lines 1696 - 1699, Replace the hardcoded extended-status strings in the manual-approval assertion near invoice creation and the waiting-for-collection assertion with the corresponding typed constants from the billing package, converting them with string(...) as needed. Use the existing StandardInvoiceStatusDraftWaitingAutoApproval pattern to identify the appropriate constants and update both checks.
1058-1081: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese two blocks could reuse the new
pinBillingProfileFromDefaulthelper.
TestV3DeleteBillingInvoiceandTestV3AdvanceBillingInvoiceboth manually inline the create-profile-then-pin-to-customer sequence, while the later tests in this same file (Approve/Retry/Snapshot) use thepinBillingProfileFromDefaulthelper added at line 1487. Worth retrofitting these two for consistency and less duplication, since the helper exists precisely to avoid this.Also applies to: 1292-1310
🤖 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 `@e2e/billinginvoices_v3_test.go` around lines 1058 - 1081, The TestV3DeleteBillingInvoice and TestV3AdvanceBillingInvoice setup blocks duplicate the create-and-pin billing profile flow. Replace each inline profile creation and UpdateCustomerBilling call with the existing pinBillingProfileFromDefault helper, passing the appropriate customer and configuration callback to disable AutoAdvance and set SendInvoice collection behavior.
🤖 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.
Nitpick comments:
In `@e2e/billinginvoices_v3_test.go`:
- Around line 1696-1699: Replace the hardcoded extended-status strings in the
manual-approval assertion near invoice creation and the waiting-for-collection
assertion with the corresponding typed constants from the billing package,
converting them with string(...) as needed. Use the existing
StandardInvoiceStatusDraftWaitingAutoApproval pattern to identify the
appropriate constants and update both checks.
- Around line 1058-1081: The TestV3DeleteBillingInvoice and
TestV3AdvanceBillingInvoice setup blocks duplicate the create-and-pin billing
profile flow. Replace each inline profile creation and UpdateCustomerBilling
call with the existing pinBillingProfileFromDefault helper, passing the
appropriate customer and configuration callback to disable AutoAdvance and set
SendInvoice collection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 820e9446-ac65-4bea-a050-94d9ae9cc52c
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (18)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/invoices.tsapi/spec/packages/aip-client-javascript/src/models/operations/invoices.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/sdk/invoices.tsapi/spec/packages/aip/src/invoices/operations.tspapi/v3/api.gen.goapi/v3/handlers/billinginvoices/action.goapi/v3/handlers/billinginvoices/convert.goapi/v3/handlers/billinginvoices/delete.goapi/v3/handlers/billinginvoices/handler.goapi/v3/server/routes.goe2e/billinginvoices_v3_test.goe2e/v3helpers_test.goopenmeter/billing/httpdriver/invoice.goopenmeter/billing/httpdriver/invoice_test.goopenmeter/billing/invoice_validator.goopenmeter/billing/service/invoice.go
✅ Files skipped from review due to trivial changes (2)
- api/spec/packages/aip-client-javascript/src/models/operations/invoices.ts
- api/v3/api.gen.go
🚧 Files skipped from review as they are similar to previous changes (9)
- api/v3/handlers/billinginvoices/handler.go
- openmeter/billing/service/invoice.go
- api/v3/handlers/billinginvoices/convert.go
- api/v3/server/routes.go
- api/spec/packages/aip-client-javascript/src/sdk/invoices.ts
- api/spec/packages/aip-client-javascript/src/models/schemas.ts
- api/v3/handlers/billinginvoices/action.go
- e2e/v3helpers_test.go
- api/spec/packages/aip/src/invoices/operations.tsp
c9a845d to
822cf2a
Compare
8ff7ddb to
0f98e64
Compare
Overview
Fixes #(issue)
Notes for reviewer
Summary by CodeRabbit
Greptile Summary
This PR adds v3 invoice workflow actions and exposes them through the API clients. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (13): Last reviewed commit: "fix: response descriptions" | Re-trigger Greptile
Context used (3)