Skip to content

feat: implement external payment endpoints#4158

Merged
turip merged 3 commits intomainfrom
feat/implement-external-payment-endpoints
Apr 17, 2026
Merged

feat: implement external payment endpoints#4158
turip merged 3 commits intomainfrom
feat/implement-external-payment-endpoints

Conversation

@turip
Copy link
Copy Markdown
Member

@turip turip commented Apr 17, 2026

Overview

Implement the external payment handling endpoints.

Notes for reviewer

Summary by CodeRabbit

  • New Features

    • Added an endpoint to update external settlement status for credit grants, supporting Authorized and Settled transitions with validation and clear errors.
    • Service support for external settlement updates and a no-op implementation for environments without full billing.
  • Bug Fixes / Behavior

    • Improved request/response mapping for credit grant endpoints and unified status conversions.
  • Tests

    • Added unit tests and error-encoding checks for the external settlement conversion and endpoint.

@turip turip requested a review from a team as a code owner April 17, 2026 07:06
@turip turip added area/api release-note/feature Release note: Exciting New Features area/billing labels Apr 17, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 86fa4167-430f-4f74-8ce0-db2d44b9ed82

📥 Commits

Reviewing files that changed from the base of the PR and between ecefad3 and a9501ff.

📒 Files selected for processing (8)
  • api/v3/handlers/customers/credits/convert.go
  • api/v3/handlers/customers/credits/create_grant.go
  • api/v3/handlers/customers/credits/errors.go
  • api/v3/handlers/customers/credits/externalsettlement.go
  • api/v3/handlers/customers/credits/externalsettlement_test.go
  • api/v3/handlers/customers/credits/get_balance.go
  • api/v3/handlers/customers/credits/get_grant.go
  • api/v3/handlers/customers/credits/list_grants.go
✅ Files skipped from review due to trivial changes (1)
  • api/v3/handlers/customers/credits/externalsettlement_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • api/v3/handlers/customers/credits/convert.go
  • api/v3/handlers/customers/credits/errors.go
  • api/v3/handlers/customers/credits/get_balance.go
  • api/v3/handlers/customers/credits/externalsettlement.go

📝 Walkthrough

Walkthrough

Adds an external settlement update feature for credit grants: new HTTP handler, request/response conversions and errors, service interface and implementation, routing gating by feature flag, noop backing implementation, and unit tests for conversion and error encoding.

Changes

Cohort / File(s) Summary
API conversions & errors
api/v3/handlers/customers/credits/convert.go, api/v3/handlers/customers/credits/errors.go
Renamed conversion helpers (convert* → toAPIBilling*/fromAPIBilling*), added mapping for external purchase settlement statuses, and introduced errCodeCreditGrantExternalSettlementStatusInvalid + constructor.
External-settlement handler & tests
api/v3/handlers/customers/credits/externalsettlement.go, api/v3/handlers/customers/credits/externalsettlement_test.go
New HTTP handler UpdateCreditGrantExternalSettlement, request/response/params aliases, wiring to service call, and unit tests for conversion success/failure and error encoding.
Handler interface & routing
api/v3/handlers/customers/credits/handler.go, api/v3/server/routes.go
Added UpdateCreditGrantExternalSettlement() to Handler interface and routed it behind credits feature-flag and dependency checks.
Service API and implementation
openmeter/billing/creditgrant/service.go, openmeter/billing/creditgrant/service/service.go
Added UpdateExternalSettlement service method, new UpdateExternalSettlementInput type with validation, and implementation that loads charge, validates external settlement type, and delegates payment-state transition.
Noop service
openmeter/billing/creditgrant/noop.go
Noop service implements UpdateExternalSettlement, returning empty charge and nil error.
Handler usage updates
api/v3/handlers/customers/credits/create_grant.go, api/v3/handlers/customers/credits/get_grant.go, api/v3/handlers/customers/credits/list_grants.go, api/v3/handlers/customers/credits/get_balance.go
Switched internal conversion call sites to new toAPIBilling*/fromAPIBilling* naming and removed special-case generic-not-found handling in balance endpoint.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as HTTP Handler
    participant Service as CreditGrant Service
    participant ChargesService as Charges Service
    participant DB as Database/State

    Client->>Handler: POST /external-settlement (namespace, customerID, chargeID, status)
    Handler->>Handler: Parse JSON -> fromAPIUpdateCreditGrantExternalSettlementRequest
    Handler->>Service: UpdateExternalSettlement(ctx, input)
    Service->>Service: Validate input
    Service->>DB: Get charge & verify customer/grant
    DB-->>Service: Charge data
    Service->>Service: Ensure settlement type == external
    Service->>ChargesService: HandleCreditPurchaseExternalPaymentStateTransition(charge, targetStatus)
    ChargesService->>DB: Persist payment state transition
    DB-->>ChargesService: Updated charge
    ChargesService-->>Service: Updated creditpurchase.Charge
    Service-->>Handler: creditpurchase.Charge
    Handler->>Handler: toAPIBillingCreditGrant(charge)
    Handler-->>Client: 200 OK + BillingCreditGrant
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • tothandras
  • chrisgacsal
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: implement external payment endpoints' accurately captures the main objective of the changeset, which introduces new endpoint implementations for handling external credit grant settlement updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/implement-external-payment-endpoints

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
api/v3/handlers/customers/credits/errors.go (1)

13-30: The package-level errCreditGrantExternalSettlementStatusInvalid var is unused.

Small thing: the newCreditGrantExternalSettlementStatusInvalid function constructs a brand-new ValidationIssue from scratch, so the static var above it is effectively dead code today. The nearby api/v3/handlers/meters/query/errors.go pattern builds the base issue once and then calls .WithAttr("value", duration) to add dynamic fields — that pattern removes the duplication and keeps the message template in one place.

♻️ Possible refactor
-var errCreditGrantExternalSettlementStatusInvalid = models.NewValidationIssue(
-	errCodeCreditGrantExternalSettlementStatusInvalid,
-	"unsupported credit grant settlement status",
-	models.WithCriticalSeverity(),
-	commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest),
-	models.WithFieldString("status"),
-)
-
-func newCreditGrantExternalSettlementStatusInvalid(status string) error {
-	return models.NewValidationIssue(
-		errCodeCreditGrantExternalSettlementStatusInvalid,
-		fmt.Sprintf("unsupported credit grant settlement status: %s", status),
-		models.WithCriticalSeverity(),
-		commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest),
-		models.WithFieldString("status"),
-		models.WithAttribute("status", status),
-	)
-}
+var errCreditGrantExternalSettlementStatusInvalid = models.NewValidationIssue(
+	errCodeCreditGrantExternalSettlementStatusInvalid,
+	"unsupported credit grant settlement status",
+	models.WithCriticalSeverity(),
+	commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest),
+	models.WithFieldString("status"),
+)
+
+func newCreditGrantExternalSettlementStatusInvalid(status string) error {
+	return errCreditGrantExternalSettlementStatusInvalid.WithAttr("status", status)
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v3/handlers/customers/credits/errors.go` around lines 13 - 30, Remove the
dead duplicate by reusing the package-level template
errCreditGrantExternalSettlementStatusInvalid inside
newCreditGrantExternalSettlementStatusInvalid: build the base ValidationIssue
once with models.NewValidationIssue
(errCreditGrantExternalSettlementStatusInvalid) and in
newCreditGrantExternalSettlementStatusInvalid return that template augmented
with the dynamic attribute (call the instance method to add the "status"
attribute, e.g.
errCreditGrantExternalSettlementStatusInvalid.WithAttribute("status", status) or
the equivalent .WithAttr API), so you keep the single message template and only
add the runtime "status" value.
api/v3/handlers/customers/credits/externalsettlement_test.go (1)

49-54: Prefer t.Context() over context.Background() in tests.

Small one: since you already have t in scope, using t.Context() ties cancellation to the test lifecycle and matches the repo convention.

♻️ Proposed tweak
-	handled := apierrors.GenericErrorEncoder()(context.Background(), err, rec, req)
+	handled := apierrors.GenericErrorEncoder()(t.Context(), err, rec, req)

And drop the now-unused "context" import.

As per coding guidelines: "Use t.Context() when a testing.T or testing.TB is available instead of introducing context.Background() to keep cancellation and test-scoped lifecycle tied to the test harness".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v3/handlers/customers/credits/externalsettlement_test.go` around lines 49
- 54, Replace the use of context.Background() with t.Context() when calling
apierrors.GenericErrorEncoder() in the test (the call that assigns handled :=
apierrors.GenericErrorEncoder()(context.Background(), err, rec, req)), and
remove the now-unused "context" import; this ensures the request context is tied
to the test lifecycle.
🤖 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/v3/handlers/customers/credits/create_grant.go`:
- Around line 46-52: Add a short inline comment near the GenericNotFoundError
handling in the Create handler explaining the current assumption:
models.IsGenericNotFoundError(err) originates from creditGrantService.Create's
customer lookup (see Create in creditGrantService/service.go), and that if the
service later validates other entities (features, tax codes, etc.) this mapping
should be revisited; place the comment adjacent to the error branch that returns
apierrors.NewNotFoundError(ctx, err, "customer") to guide future maintainers.

---

Nitpick comments:
In `@api/v3/handlers/customers/credits/errors.go`:
- Around line 13-30: Remove the dead duplicate by reusing the package-level
template errCreditGrantExternalSettlementStatusInvalid inside
newCreditGrantExternalSettlementStatusInvalid: build the base ValidationIssue
once with models.NewValidationIssue
(errCreditGrantExternalSettlementStatusInvalid) and in
newCreditGrantExternalSettlementStatusInvalid return that template augmented
with the dynamic attribute (call the instance method to add the "status"
attribute, e.g.
errCreditGrantExternalSettlementStatusInvalid.WithAttribute("status", status) or
the equivalent .WithAttr API), so you keep the single message template and only
add the runtime "status" value.

In `@api/v3/handlers/customers/credits/externalsettlement_test.go`:
- Around line 49-54: Replace the use of context.Background() with t.Context()
when calling apierrors.GenericErrorEncoder() in the test (the call that assigns
handled := apierrors.GenericErrorEncoder()(context.Background(), err, rec,
req)), and remove the now-unused "context" import; this ensures the request
context is tied to the test lifecycle.
🪄 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: e9965c45-2a93-42bd-a6f9-0d8cd460e8a4

📥 Commits

Reviewing files that changed from the base of the PR and between bc4528f and edc1dd4.

📒 Files selected for processing (11)
  • api/v3/handlers/customers/credits/convert.go
  • api/v3/handlers/customers/credits/create_grant.go
  • api/v3/handlers/customers/credits/errors.go
  • api/v3/handlers/customers/credits/externalsettlement.go
  • api/v3/handlers/customers/credits/externalsettlement_test.go
  • api/v3/handlers/customers/credits/get_grant.go
  • api/v3/handlers/customers/credits/handler.go
  • api/v3/server/routes.go
  • openmeter/billing/creditgrant/noop.go
  • openmeter/billing/creditgrant/service.go
  • openmeter/billing/creditgrant/service/service.go

Comment thread api/v3/handlers/customers/credits/create_grant.go
@turip turip requested a review from tothandras April 17, 2026 07:12
…ntion

Rename all conversion functions in the customers/credits handler to
follow the FromAPI/ToAPI naming convention and remove unused error var.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@turip turip enabled auto-merge (squash) April 17, 2026 08:22
@turip turip merged commit d0886c8 into main Apr 17, 2026
34 of 36 checks passed
@turip turip deleted the feat/implement-external-payment-endpoints branch April 17, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api area/billing release-note/feature Release note: Exciting New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants