feat: enable custominvoicing in cloud #2716
Conversation
📝 WalkthroughWalkthroughThis change introduces new API capabilities for custom invoicing integration. Three POST endpoints are added under Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
api/openapi.cloud.yaml (2)
735-799: Same as above: ensure authentication and DRY error handling
This POST (issuing/synchronized) also needs asecurityblock and could reuse shared error‐response components rather than redefining the same schemas inline.
800-863: Same as above: ensure authentication and DRY error handling
The/payment/statusoperation likewise should enforce security and reference shared error responses undercomponents/responses.
🧹 Nitpick comments (6)
api/openapi.cloud.yaml (2)
12807-12879: Enhance mapping & sync result schemas
- Provide
exampleentries for mapping objects and theCustomInvoicingSyncResultto illustrate typical payload shapes.- Confirm whether key properties (
invoiceNumber,externalId,lineExternalIds,lineDiscountExternalIds) should be marked asrequired.
12893-12906: Add examples and clarifytriggerusage
TheCustomInvoicingUpdatePaymentStatusRequestschema would benefit from anexamplefor thetriggerenum (e.g.,paid,payment_failed). Also consider expanding the description block to include a full JSON example of the request.api/client/javascript/src/client/schemas.ts (4)
116-132: AddinvoiceIdto the path-levelparametersto retain full type-safetyThe three new path objects declare
path?: never, yet each corresponding operation correctly exposes
path: { invoiceId: string }.
Consumers that rely on the path-level type (e.g. when building a strongly-typed router or client factory) will silently lose the compile-time guarantee that aninvoiceIdmust be supplied.- parameters: { - query?: never - header?: never - path?: never - cookie?: never - } + parameters: { + query?: never + header?: never + /* required by the placeholder in the URL */ + path: { invoiceId: string } + cookie?: never + }(Same change applies to the
issuing/synchronizedandpayment/statuspaths).
Please update the source OpenAPI / TypeSpec instead of editing the generated file.Also applies to: 133-166
3310-3319: VerifysentToCustomerAttyping –Datevs ISO-stringThe field is declared as
Datebut will be serialized as an ISO-8601 string over the wire.
Unless the generator has custom (de)serializers, SDK users might accidentally send aDateinstance withouttoISOString(), producing"{ }"in JSON.Two options:
- Keep the transport type
stringand offer helpers that parse / stringify.- Provide a custom transformer in the generator that converts
Date↔︎ string.Please confirm which strategy the rest of the SDK follows and align this field.
3373-3383: Optional array fields can default to empty arrays
lineExternalIdsandlineDiscountExternalIdsare optional.
Down-stream code often needs to map / filter these collections – defaulting to[]in the backend or during deserialization removes a lot of?.checks.Consider documenting that these arrays will always be present (possibly empty) or update the schema:
- lineExternalIds?: CustomInvoicingLineExternalIdMapping[] + lineExternalIds: CustomInvoicingLineExternalIdMapping[](And same for
lineDiscountExternalIds).
11482-11503: Consider 409/422 responses for state-transition errorsWhen updating payment status you may attempt an invalid transition (e.g.
voidafterpaid).
Returning a generic 400 forces clients to inspect the problem details payload.
Adding explicit 409 Conflict or 422 Unprocessable Entity makes the contract clearer and aids automatic retry logic.If feasible, extend the spec with these codes.
Also applies to: 11540-11557
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/client/javascript/src/client/schemas.ts(5 hunks)api/openapi.cloud.yaml(3 hunks)api/spec/src/cloud/main.tsp(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/client/javascript/src/client/schemas.ts (2)
api/api.gen.go (10)
CustomInvoicingDraftSyncronizedRequest(1929-1932)CustomInvoicingFinalizedInvoicingRequest(1935-1941)CustomInvoicingFinalizedPaymentRequest(1944-1947)CustomInvoicingFinalizedRequest(1952-1958)CustomInvoicingLineDiscountExternalIdMapping(1961-1967)CustomInvoicingLineExternalIdMapping(1970-1976)CustomInvoicingPaymentTrigger(1979-1979)CustomInvoicingSyncResult(1984-2002)CustomInvoicingTaxConfig(2005-2010)CustomInvoicingUpdatePaymentStatusRequest(2015-2018)api/client/go/client.gen.go (10)
CustomInvoicingDraftSyncronizedRequest(1758-1761)CustomInvoicingFinalizedInvoicingRequest(1764-1770)CustomInvoicingFinalizedPaymentRequest(1773-1776)CustomInvoicingFinalizedRequest(1781-1787)CustomInvoicingLineDiscountExternalIdMapping(1790-1796)CustomInvoicingLineExternalIdMapping(1799-1805)CustomInvoicingPaymentTrigger(1808-1808)CustomInvoicingSyncResult(1813-1831)CustomInvoicingTaxConfig(1834-1839)CustomInvoicingUpdatePaymentStatusRequest(1844-1847)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Quickstart
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Developer environment
- GitHub Check: Commit hooks
- GitHub Check: Test
- GitHub Check: Migration Checks
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
api/spec/src/cloud/main.tsp (1)
140-142: Well-structured addition of the custom invoicing interfaceThe addition of the
AppCustomInvoicinginterface properly exposes the custom invoicing functionality to the cloud API by extending the base implementation fromOpenMeter.App.AppCustomInvoicing. This follows the same pattern as other app interfaces and is correctly tagged with "Apps".api/client/javascript/src/client/schemas.ts (1)
11308-11408: Operation uses “Finalized” payload on “issuing/synchronized” endpoint – double-check intent
/issuing/synchronizedconsumesCustomInvoicingFinalizedRequest.
If “issuing” is only one step before “finalized”, this might be correct; otherwise, it may be confusing that a finalized data structure is accepted by an issuing-stage endpoint.Please ensure the naming represents the actual lifecycle stage; otherwise introduce a more specific request DTO.
turip
force-pushed
the
feat/enable-custominvoicing-cloud
branch
from
10db0d4 to
3a598f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
api/client/javascript/src/client/schemas.ts (1)
9958-9977: Type exports correctly definedAll TypeScript type aliases for the new schema components are properly exported, making them available for consumers of this API client.
The spelling of "Synchronized" has been corrected as noted in the previous review.
🧹 Nitpick comments (7)
openmeter/app/custominvoicing/httpdriver/custominvoicing.go (3)
16-23: Consider updating type declarations to match the corrected namingWhile the request body type has been updated with the correct spelling, the type aliases and handler function still use the misspelled "Syncronized" variant. For consistency, consider updating these type declarations as well.
-type ( - DraftSyncronizedRequest = appcustominvoicing.SyncDraftInvoiceInput - DraftSyncronizedResponse = api.Invoice - DraftSyncronizedParams = struct { +type ( + DraftSynchronizedRequest = appcustominvoicing.SyncDraftInvoiceInput + DraftSynchronizedResponse = api.Invoice + DraftSynchronizedParams = struct { InvoiceID string `json:"invoiceId"` } - DraftSyncronizedHandler httptransport.HandlerWithArgs[DraftSyncronizedRequest, DraftSyncronizedResponse, DraftSyncronizedParams] + DraftSynchronizedHandler httptransport.HandlerWithArgs[DraftSynchronizedRequest, DraftSynchronizedResponse, DraftSynchronizedParams] )
25-65: Update function name and internal references for consistencyThe function name and internal references should be updated to use the corrected spelling "Synchronized" to match the request type.
-func (h *handler) DraftSyncronized() DraftSyncronizedHandler { +func (h *handler) DraftSynchronized() DraftSynchronizedHandler { return httptransport.NewHandlerWithArgs( - func(ctx context.Context, r *http.Request, params DraftSyncronizedParams) (DraftSyncronizedRequest, error) { + func(ctx context.Context, r *http.Request, params DraftSynchronizedParams) (DraftSynchronizedRequest, error) { namespace, err := h.resolveNamespace(ctx) if err != nil { - return DraftSyncronizedRequest{}, fmt.Errorf("failed to resolve namespace: %w", err) + return DraftSynchronizedRequest{}, fmt.Errorf("failed to resolve namespace: %w", err) } var body api.CustomInvoicingDraftSynchronizedRequest if err := commonhttp.JSONRequestBodyDecoder(r, &body); err != nil { - return DraftSyncronizedRequest{}, fmt.Errorf("failed to decode draft synchronized request: %w", err) + return DraftSynchronizedRequest{}, fmt.Errorf("failed to decode draft synchronized request: %w", err) } - return DraftSyncronizedRequest{ + return DraftSynchronizedRequest{ InvoiceID: billing.InvoiceID{ ID: params.InvoiceID, Namespace: namespace, }, UpsertInvoiceResults: mapUpsertInvoiceResultFromAPI(body.Invoicing), }, nil }, - func(ctx context.Context, request DraftSyncronizedRequest) (DraftSyncronizedResponse, error) { + func(ctx context.Context, request DraftSynchronizedRequest) (DraftSynchronizedResponse, error) { if err := request.Validate(); err != nil { - return DraftSyncronizedResponse{}, err + return DraftSynchronizedResponse{}, err } invoice, err := h.service.SyncDraftInvoice(ctx, request) if err != nil { - return DraftSyncronizedResponse{}, err + return DraftSynchronizedResponse{}, err } return billinghttpdriver.MapInvoiceToAPI(invoice) }, - commonhttp.JSONResponseEncoderWithStatus[DraftSyncronizedResponse](http.StatusOK), + commonhttp.JSONResponseEncoderWithStatus[DraftSynchronizedResponse](http.StatusOK), httptransport.AppendOptions( h.options, - httptransport.WithOperationName("DraftSyncronized"), + httptransport.WithOperationName("DraftSynchronized"), httptransport.WithErrorEncoder(errorEncoder()), )..., ) }
68-116: Similar spelling inconsistency in IssuingSyncronizedFor consistency with the corrected spelling throughout the codebase, consider updating the
IssuingSyncronizedfunction and related type declarations as well.type ( - IssuingSyncronizedRequest = appcustominvoicing.SyncIssuingInvoiceInput - IssuingSyncronizedResponse = api.Invoice - IssuingSyncronizedParams = struct { + IssuingSynchronizedRequest = appcustominvoicing.SyncIssuingInvoiceInput + IssuingSynchronizedResponse = api.Invoice + IssuingSynchronizedParams = struct { InvoiceID string `json:"invoiceId"` } - IssuingSyncronizedHandler httptransport.HandlerWithArgs[IssuingSyncronizedRequest, IssuingSyncronizedResponse, IssuingSyncronizedParams] + IssuingSynchronizedHandler httptransport.HandlerWithArgs[IssuingSynchronizedRequest, IssuingSynchronizedResponse, IssuingSynchronizedParams] ) -func (h *handler) IssuingSyncronized() IssuingSyncronizedHandler { +func (h *handler) IssuingSynchronized() IssuingSynchronizedHandler { // Update internal references similarly... httptransport.WithOperationName("IssuingSynchronized"),api/spec/src/app/custominvoicing.tsp (3)
53-53: Function name inconsistent with parameter typeWhile the parameter type has been corrected to
CustomInvoicingDraftSynchronizedRequest, the function namedraftSyncronizedstill uses the misspelled variant "Syncronized". For consistency, consider updating the function name as well.- draftSyncronized( + draftSynchronized( @path invoiceId: ULID, @body body: CustomInvoicingDraftSynchronizedRequest, ): void | CommonErrors;
51-51: Update operation ID for consistencyThe
@operationIdshould also be updated to match the corrected spelling pattern for consistency.- @operationId("appCustomInvoicingDraftSynchronized") + @operationId("appCustomInvoicingDraftSynchronized")
62-65: Function name inconsistent with its purposeThe function name
finalizeddoesn't follow the same naming pattern as the other functions in this interface and doesn't match the route which includes "issuing/synchronized". Consider renaming it toissuingSynchronizedfor consistency with both the route path and other function names.- finalized( + issuingSynchronized( @path invoiceId: ULID, @body body: CustomInvoicingFinalizedRequest, ): void | CommonErrors;api/client/javascript/src/client/schemas.ts (1)
3358-3364: Consider adding descriptions for payment trigger enum valuesThe
CustomInvoicingPaymentTriggerenum values would benefit from additional documentation explaining the meaning and use case for each value (paid, payment_failed, void, etc.)./** * @description Payment trigger to execute on a finalized invoice. + * - paid: Payment succeeded + * - payment_failed: Payment attempt failed + * - payment_uncollectible: Payment deemed uncollectible + * - payment_overdue: Payment is overdue + * - action_required: Customer action needed to complete payment + * - void: Invoice has been voided * @enum {string} */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/client/javascript/src/client/schemas.ts(5 hunks)api/openapi.cloud.yaml(3 hunks)api/openapi.yaml(2 hunks)api/spec/src/app/custominvoicing.tsp(2 hunks)api/spec/src/cloud/main.tsp(1 hunks)openmeter/app/custominvoicing/httpdriver/custominvoicing.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- api/spec/src/cloud/main.tsp
- api/openapi.cloud.yaml
🧰 Additional context used
🧬 Code Graph Analysis (1)
openmeter/app/custominvoicing/httpdriver/custominvoicing.go (3)
api/client/javascript/src/client/schemas.ts (1)
CustomInvoicingDraftSynchronizedRequest(9958-9959)api/api.gen.go (1)
CustomInvoicingDraftSynchronizedRequest(1929-1932)api/client/go/client.gen.go (1)
CustomInvoicingDraftSynchronizedRequest(1758-1761)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Quickstart
- GitHub Check: Lint
- GitHub Check: Developer environment
- GitHub Check: Commit hooks
- GitHub Check: Test
- GitHub Check: Analyze (go)
- GitHub Check: Build
🔇 Additional comments (7)
openmeter/app/custominvoicing/httpdriver/custominvoicing.go (1)
33-33: Type correction aligned with schema naming conventionThe change correctly updates the variable type to use the properly spelled
CustomInvoicingDraftSynchronizedRequest, fixing the typographical error (Syncronized → Synchronized). This aligns with the type definitions in the API schema and generated client code.api/spec/src/app/custominvoicing.tsp (2)
55-55: Type correction aligned with schema definitionThe parameter type has been correctly updated to use the properly spelled
CustomInvoicingDraftSynchronizedRequest, fixing the typographical error. This aligns with the model definition below.
157-158: Model name correction for consistent namingThe model declaration and
@friendlyNameannotation have been properly updated to use the correctly spelled "Synchronized" rather than "Syncronized", fixing the typographical error. This ensures consistency with the type used in the interface method and the generated API schema.api/client/javascript/src/client/schemas.ts (4)
116-166: New custom invoicing endpoints look goodThese three new API endpoints follow a consistent pattern and are well-structured:
- Draft synchronization
- Issuing synchronization
- Payment status updates
The POST method is correctly specified for all endpoints with clear documentation comments.
3304-3383: Well-structured schema definitions for custom invoicingThe schema definitions provide comprehensive data structures for:
- Draft synchronization request
- Finalized invoicing/payment requests
- External ID mappings
- Synchronization results
Good job on including detailed JSDoc descriptions that explain the purpose of each field and structure.
3394-3400: Schema for payment status updates is well-definedThe
CustomInvoicingUpdatePaymentStatusRequestschema is properly structured with a clear purpose and appropriate trigger field referencing the payment trigger enum.
11308-11568: API operation definitions are comprehensiveThe operation definitions include well-structured:
- Path parameters for invoice ID
- Request body specifications
- Detailed response types for success and error scenarios
Each operation consistently handles all standard HTTP status codes and appropriate problem+json responses.
2 participants
Overview
Add missing type exposure for cloud yaml.
Summary by CodeRabbit