feat(s3): domain model — CollectionOrder aggregate, state machine, events, ports (STA-120)#118
Merged
Merged
Conversation
WalkthroughAdds a complete fiat-on-ramp domain: immutable value objects, enums, event records with TOPICs, repository/gateway ports, a generic state machine, and two aggregate roots (CollectionOrder, Refund) implementing lifecycle transitions and validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Service/Client
participant CO as CollectionOrder (Aggregate)
participant Repo as CollectionOrderRepository
participant Gateway as PspGateway
participant Pub as CollectionEventPublisher
Client->>CO: CollectionOrder.initiate(...)
CO-->>Client: CollectionOrder (PENDING)
Client->>Repo: save(order)
Repo-->>Client: saved
Client->>CO: initiatePayment()
CO-->>Client: order (PAYMENT_INITIATED)
Client->>Gateway: initiatePayment(PspPaymentRequest)
Gateway-->>Client: PspPaymentResult(pspRef,status)
Client->>CO: awaitConfirmation(pspRef)
CO-->>Client: order (AWAITING_CONFIRMATION)
Client->>Repo: save(order)
Note over Gateway,Pub: PSP webhook / poll triggers confirmation
Client->>CO: confirmCollection(collectedAmount)
CO-->>Client: order (COLLECTED)
Client->>Repo: save(order)
Client->>Pub: publish(CollectionCompletedEvent)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
…ents, ports (STA-120) Closes STA-120 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Puneethkumarck
force-pushed
the
feature/STA-120-s3-domain-model
branch
from
fccbaf8 to
e32d479
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionCompletedEvent.java`:
- Line 19: The TOPIC constant in CollectionCompletedEvent (public static final
String TOPIC) breaks the project's event naming convention by using
"fiat.collected"; change its value to follow the fiat.{entity}.{action} pattern
(e.g., "fiat.collection.completed") and update any references/consumers that use
CollectionCompletedEvent.TOPIC so they subscribe/publish to the new topic
string; ensure tests or config that assert topic names are updated accordingly.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`:
- Around line 93-129: The initiate factory currently uses Instant.now() and
UUID.randomUUID() directly and hardcodes the 1800s expiry, making tests
non-deterministic; modify CollectionOrder.initiate to accept a Clock (or
overload initiate with a Clock parameter) and a UUID supplier (or accept a UUID
collectionId param), replace Instant.now() with Instant.now(clock) and
UUID.randomUUID() with the supplied UUID, and extract the 1800 expiry into a
private static final Duration (or configurable parameter) so tests can pass a
fixed clock/UUID and a controlled expiry when building via
CollectionOrder.builder()/initiate.
- Around line 65-66: The TERMINAL_STATES set in CollectionOrder omits
COLLECTION_STATUS.COLLECTED even though COLLECTED is semantically an end state
(only allowed transition is to REFUND_INITIATED); either add COLLECTED to the
TERMINAL_STATES Set.of(...) declaration, or keep TERMINAL_STATES as-is but add a
clear query method on CollectionOrder (e.g., isCollected()) that returns status
== CollectionStatus.COLLECTED and update any callers to use this for logic that
treats collected as terminal, and ensure state-transition logic (wherever
transitions are enforced) explicitly allows only REFUND_INITIATED from
COLLECTED.
- Around line 45-63: CollectionOrder's builder/toBuilder can produce invalid
instances because the record lacks a compact (canonical) constructor to validate
critical invariants; add a compact canonical constructor for the record
CollectionOrder that performs null checks and domain validations (e.g., non-null
collectionId, paymentId, correlationId, amount, paymentRail, psp, senderAccount,
status, createdAt, updatedAt, and that status and money fields meet domain
constraints and collectedAmount/pspSettledAt/failureReason/errorCode are
consistent with status) so any instance created via the builder or direct
construction is validated; implement these checks inside the CollectionOrder
canonical constructor to throw appropriate exceptions on violation.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Money.java`:
- Around line 5-14: The Money compact constructor must canonicalize both fields
so equals/hashCode are stable: resolve the provided currency string via
Currency.getInstance(currency.trim().toUpperCase()) and replace the record's
currency parameter with currencyInstance.getCurrencyCode(); determine the
currency scale via currencyInstance.getDefaultFractionDigits() and set the
amount parameter to amount.setScale(scale, RoundingMode.UNNECESSARY) (or throw a
clear IllegalArgumentException if the amount has too many fraction digits), or
alternatively use amount.stripTrailingZeros() if you prefer flexible
normalization; perform null/blank checks first, then reassign the normalized
amount and currency in the compact constructor of the Money record so stored
values are canonical for equality and persistence.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.java`:
- Around line 5-11: In the PspIdentifier constructor, canonicalize pspId and
pspName by trimming whitespace before validation and assignment: call trim() on
the incoming pspId and pspName, validate the trimmed values (check
null/isBlank), and then assign the trimmed values to the record/fields so stored
and compared values are normalized; update any references in the constructor
body that use pspId/pspName to use the trimmed versions.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransaction.java`:
- Line 24: The PspTransaction field rawResponse must not persist raw PSP
payloads; replace it with a sanitized or encrypted reference: remove or
deprecate the rawResponse String and instead add a redactedResponse (e.g.,
redactedSnapshot) or encryptedResponseRef field plus metadata for sizeBytes and
retentionExpiry; update the PspTransaction constructor/builders and any factory
methods to call a sanitizer/encryptor (e.g., redactRawResponse() or
storeEncryptedResponseRef()) before persisting, enforce a maximum size limit and
persist only the blob reference/metadata rather than raw payload, and ensure
audit append-only behavior is preserved via these new safe fields.
- Around line 15-26: Add a compact record constructor to PspTransaction that
enforces the same invariants currently in the factory: validate non-null for
pspTxnId, collectionId, amount, receivedAt and non-blank for pspName,
pspReference, eventType, status, and rawResponse; throw appropriate exceptions
(e.g., NullPointerException/IllegalArgumentException) on failure so every
construction path (including the package-scoped builder and the public canonical
constructor) yields a valid instance. Keep the `@Builder` and record header as-is
and remove duplicated checks from the factory or delegate them to the new
compact constructor so validation is centralized in the PspTransaction record.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Refund.java`:
- Around line 39-63: The initiate method in Refund uses UUID.randomUUID() and
Instant.now() which makes tests non-deterministic; add an overloaded initiate
method that accepts explicit refundId and initiatedAt (or a Clock) and have the
existing initiate delegate to it (e.g., create initiate(UUID collectionId, UUID
paymentId, Money refundAmount, String reason, UUID refundId, Instant
initiatedAt) and keep the existing initiate(...) but delegate by calling
UUID.randomUUID() and Instant.now(clock) or Instant.now() to that new overload);
update Refund.builder() usage in the new overload to set refundId and
initiatedAt from parameters so tests can supply fixed values when needed (refer
to Refund.initiate, refundId, initiatedAt).
- Around line 20-32: The Refund record's `@Builder` currently bypasses invariants;
add a canonical (compact) constructor inside the Refund record to validate
required fields (refundId, collectionId, paymentId, refundAmount, status) are
non-null and that status is one of the allowed RefundStatus values (or
map/translate invalid values to a safe default or throw
IllegalArgumentException). In the canonical constructor (inside record
Refund(...)), also normalize/validate timestamps (initiatedAt/completedAt) and
failureReason/pspRefundRef (e.g., trim or null-check) so all construction paths
(including Refund.builder()) enforce the same invariants; throw clear
IllegalArgumentException on violations. Ensure constructor references the record
components by name (refundId, collectionId, paymentId, refundAmount, status,
pspRefundRef, initiatedAt, completedAt, failureReason) so the builder uses these
validations automatically.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.java`:
- Around line 3-5: The domain port CollectionEventPublisher currently exposes
publish(Object) which breaks compile-time safety; change the interface to accept
a domain event supertype (e.g., define or use an existing CollectionEvent
interface/class and replace publish(Object) with publish(CollectionEvent event))
and update all callers, adapters, and tests to import and publish that specific
supertype so only supported collection/refund events can cross the boundary
(also add any necessary mapping in adapters from infrastructure payloads to the
CollectionEvent subtype).
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.java`:
- Around line 3-8: Add an idempotency key to PSP operations by extending
PspPaymentRequest and PspRefundRequest to include a non-null idempotencyKey (or
requestId) field and update the PspGateway interface methods (initiatePayment
and initiateRefund) to expect requests that carry this key; ensure the PSP
client implementations propagate this idempotencyKey to the upstream provider
API, validate/generate a key at the service boundary if missing (e.g.,
collectionId + attempt counter or UUID), and include the key in request/response
logs and error messages to avoid duplicate transactions on retries.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentRequest.java`:
- Around line 9-16: Change the PspPaymentRequest record to enforce invariants
and stronger typing: replace the raw String pspName field with the domain type
PspIdentifier and add a canonical constructor for PspPaymentRequest that
validates collectionId and amount are non-null (and that amount is
positive/valid per Money rules) using Objects.requireNonNull or throwing
IllegalArgumentException on invalid values; ensure the constructor also requires
a non-null PspIdentifier so adapters receive a consistent PSP type.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentResult.java`:
- Around line 3-4: Replace the raw String status in the PspPaymentResult record
with a typed enum to avoid string-comparison bugs: add a new enum
PspPaymentStatus { PENDING, SUCCESS, FAILED, REQUIRES_ACTION } (adjust names to
match domain), change the PspPaymentResult declaration to use PspPaymentStatus
status, and update all callers/serializers/deserializers to construct/parse the
enum (provide a safe factory/parse method on PspPaymentStatus to map legacy
strings and handle unknown values). Ensure places that switch on status use the
enum for exhaustive handling.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.java`:
- Around line 7-13: The PspRefundRequest record currently exposes a raw String
pspName which bypasses the new PspIdentifier value object; update the record
signature to use PspIdentifier instead of String (i.e., change the pspName
parameter to type PspIdentifier) and update any call sites that construct
PspRefundRequest to pass a PspIdentifier; leave unwrapping (e.g., to String or
provider-specific id) to the integration adapter code so adapters convert
PspIdentifier to whatever external representation is required.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundResult.java`:
- Line 3: Change the PspRefundResult port contract to use the domain enum
RefundStatus instead of a raw String: replace the record field String status
with RefundStatus status (and optionally add a String rawStatus field to carry
PSP-native values if needed). Update the PspRefundResult declaration and any
constructors/usages (calls that construct or read pspRefundRef/status) to import
and use com.stablecoin.payments.onramp.domain.model.RefundStatus and map
PSP-native status strings to RefundStatus at the adapter/border layer rather
than leaking them through the port.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspTransactionRepository.java`:
- Line 12: The port method findByCollectionId(UUID collectionId) lacks a
deterministic ordering; update the contract to require chronological (append)
order by receivedAt—e.g., rename to findByCollectionIdOrderByReceivedAtAsc or
document/annotate that implementations must return results ordered ascending by
PspTransaction.receivedAt—so all adapters return a reproducible, append-order
sequence for replay and investigation.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java`:
- Around line 9-11: The constructor StateMachine(List<StateTransition<S, T>>
transitions) should validate that no two StateTransition entries share the same
(from, trigger) pair with different to states; iterate the provided transitions
and build a map keyed by the (from, trigger) tuple (or use a composite key), and
if you encounter a key already present with a different to state throw an
IllegalArgumentException (or similarly fail-fast) to avoid non-deterministic
findFirst() behavior; keep using List.copyOf(transitions) after validation
(refer to StateMachine constructor and StateTransition's
getFrom/getTrigger/getTo methods).
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateTransition.java`:
- Around line 3-4: The StateTransition record allows null values for its
components (from, trigger, to) which can lead to invalid transitions; add a
compact canonical constructor for StateTransition that validates each component
(from, trigger, to) using null checks (e.g., Objects.requireNonNull or explicit
checks) and throws a clear NPE with a descriptive message if any are null so
invalid transitions fail fast during construction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b3af8b7-c6aa-470e-a24e-18185665d1e4
📒 Files selected for processing (30)
fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionCompletedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionFailedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionInitiatedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundCompletedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundInitiatedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/AccountType.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/BankAccount.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionStatus.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionTrigger.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Money.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PaymentRail.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PaymentRailType.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransaction.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransactionDirection.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Refund.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/RefundStatus.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionOrderRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentRequest.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentResult.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundResult.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspTransactionRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/RefundRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachineException.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateTransition.java
| Instant collectedAt | ||
| ) { | ||
|
|
||
| public static final String TOPIC = "fiat.collected"; |
There was a problem hiding this comment.
Topic naming inconsistent with other events.
All other events use fiat.{entity}.{action} pattern:
fiat.collection.initiatedfiat.collection.failedfiat.refund.initiatedfiat.refund.completed
This one uses fiat.collected — breaks the convention and complicates consumer topic subscription patterns.
Proposed fix
- public static final String TOPIC = "fiat.collected";
+ public static final String TOPIC = "fiat.collection.completed";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static final String TOPIC = "fiat.collected"; | |
| public static final String TOPIC = "fiat.collection.completed"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionCompletedEvent.java`
at line 19, The TOPIC constant in CollectionCompletedEvent (public static final
String TOPIC) breaks the project's event naming convention by using
"fiat.collected"; change its value to follow the fiat.{entity}.{action} pattern
(e.g., "fiat.collection.completed") and update any references/consumers that use
CollectionCompletedEvent.TOPIC so they subscribe/publish to the new topic
string; ensure tests or config that assert topic names are updated accordingly.
Comment on lines
+45
to
+63
| @Builder(toBuilder = true, access = AccessLevel.PACKAGE) | ||
| public record CollectionOrder( | ||
| UUID collectionId, | ||
| UUID paymentId, | ||
| UUID correlationId, | ||
| Money amount, | ||
| PaymentRail paymentRail, | ||
| PspIdentifier psp, | ||
| BankAccount senderAccount, | ||
| CollectionStatus status, | ||
| Money collectedAmount, | ||
| String pspReference, | ||
| Instant pspSettledAt, | ||
| String failureReason, | ||
| String errorCode, | ||
| Instant createdAt, | ||
| Instant updatedAt, | ||
| Instant expiresAt | ||
| ) { |
There was a problem hiding this comment.
Builder bypasses aggregate invariants.
Same issue as Refund — no canonical constructor means toBuilder() or direct builder usage could create invalid aggregates (e.g., null collectionId, invalid status). Add compact constructor validation for critical fields.
🛡️ Proposed canonical constructor
public record CollectionOrder(
UUID collectionId,
UUID paymentId,
UUID correlationId,
Money amount,
PaymentRail paymentRail,
PspIdentifier psp,
BankAccount senderAccount,
CollectionStatus status,
Money collectedAmount,
String pspReference,
Instant pspSettledAt,
String failureReason,
String errorCode,
Instant createdAt,
Instant updatedAt,
Instant expiresAt
) {
+ public CollectionOrder {
+ if (collectionId == null) throw new IllegalArgumentException("collectionId is required");
+ if (paymentId == null) throw new IllegalArgumentException("paymentId is required");
+ if (status == null) throw new IllegalArgumentException("status is required");
+ if (createdAt == null) throw new IllegalArgumentException("createdAt is required");
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`
around lines 45 - 63, CollectionOrder's builder/toBuilder can produce invalid
instances because the record lacks a compact (canonical) constructor to validate
critical invariants; add a compact canonical constructor for the record
CollectionOrder that performs null checks and domain validations (e.g., non-null
collectionId, paymentId, correlationId, amount, paymentRail, psp, senderAccount,
status, createdAt, updatedAt, and that status and money fields meet domain
constraints and collectedAmount/pspSettledAt/failureReason/errorCode are
consistent with status) so any instance created via the builder or direct
construction is validated; implement these checks inside the CollectionOrder
canonical constructor to throw appropriate exceptions on violation.
Comment on lines
+65
to
+66
| private static final Set<CollectionStatus> TERMINAL_STATES = | ||
| Set.of(COLLECTION_FAILED, REFUNDED, MANUAL_REVIEW); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
COLLECTED is not terminal but can only transition to REFUND_INITIATED.
Semantically COLLECTED represents a successful end state for the collection flow. If refunds are optional/rare, consider documenting why it's not marked terminal, or add a query method like isCollected() for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`
around lines 65 - 66, The TERMINAL_STATES set in CollectionOrder omits
COLLECTION_STATUS.COLLECTED even though COLLECTED is semantically an end state
(only allowed transition is to REFUND_INITIATED); either add COLLECTED to the
TERMINAL_STATES Set.of(...) declaration, or keep TERMINAL_STATES as-is but add a
clear query method on CollectionOrder (e.g., isCollected()) that returns status
== CollectionStatus.COLLECTED and update any callers to use this for logic that
treats collected as terminal, and ensure state-transition logic (wherever
transitions are enforced) explicitly allows only REFUND_INITIATED from
COLLECTED.
Comment on lines
+93
to
+129
| public static CollectionOrder initiate(UUID paymentId, UUID correlationId, | ||
| Money amount, PaymentRail paymentRail, | ||
| PspIdentifier psp, BankAccount senderAccount) { | ||
| if (paymentId == null) { | ||
| throw new IllegalArgumentException("paymentId is required"); | ||
| } | ||
| if (correlationId == null) { | ||
| throw new IllegalArgumentException("correlationId is required"); | ||
| } | ||
| if (amount == null) { | ||
| throw new IllegalArgumentException("amount is required"); | ||
| } | ||
| if (paymentRail == null) { | ||
| throw new IllegalArgumentException("paymentRail is required"); | ||
| } | ||
| if (psp == null) { | ||
| throw new IllegalArgumentException("psp is required"); | ||
| } | ||
| if (senderAccount == null) { | ||
| throw new IllegalArgumentException("senderAccount is required"); | ||
| } | ||
|
|
||
| var now = Instant.now(); | ||
| return CollectionOrder.builder() | ||
| .collectionId(UUID.randomUUID()) | ||
| .paymentId(paymentId) | ||
| .correlationId(correlationId) | ||
| .amount(amount) | ||
| .paymentRail(paymentRail) | ||
| .psp(psp) | ||
| .senderAccount(senderAccount) | ||
| .status(PENDING) | ||
| .createdAt(now) | ||
| .updatedAt(now) | ||
| .expiresAt(now.plusSeconds(1800)) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same testability concern: Instant.now() and UUID.randomUUID().
Factory uses runtime values directly. Consider accepting a Clock or overloading for deterministic testing. The hardcoded 1800 second expiry could also be extracted to a constant or made configurable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`
around lines 93 - 129, The initiate factory currently uses Instant.now() and
UUID.randomUUID() directly and hardcodes the 1800s expiry, making tests
non-deterministic; modify CollectionOrder.initiate to accept a Clock (or
overload initiate with a Clock parameter) and a UUID supplier (or accept a UUID
collectionId param), replace Instant.now() with Instant.now(clock) and
UUID.randomUUID() with the supplied UUID, and extract the 1800 expiry into a
private static final Duration (or configurable parameter) so tests can pass a
fixed clock/UUID and a controlled expiry when building via
CollectionOrder.builder()/initiate.
Comment on lines
+5
to
+14
| public record Money(BigDecimal amount, String currency) { | ||
|
|
||
| public Money { | ||
| if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { | ||
| throw new IllegalArgumentException("Amount must be positive"); | ||
| } | ||
| if (currency == null || currency.isBlank()) { | ||
| throw new IllegalArgumentException("Currency must not be blank"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Canonicalize amount and currency inside the value object.
This record currently treats 1.0 USD and 1.00 USD as different values because record equality delegates to BigDecimal.equals. currency also accepts case/whitespace variants. Normalize both in the compact constructor, ideally via Currency.getInstance(...) plus currency-scale enforcement, so equality and persistence/event payloads stay stable.
Proposed change
import java.math.BigDecimal;
+import java.math.RoundingMode;
+import java.util.Currency;
+import java.util.Locale;
public record Money(BigDecimal amount, String currency) {
public Money {
if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
throw new IllegalArgumentException("Amount must be positive");
}
if (currency == null || currency.isBlank()) {
throw new IllegalArgumentException("Currency must not be blank");
}
+ Currency javaCurrency = Currency.getInstance(currency.trim().toUpperCase(Locale.ROOT));
+ amount = amount.setScale(javaCurrency.getDefaultFractionDigits(), RoundingMode.UNNECESSARY);
+ currency = javaCurrency.getCurrencyCode();
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Money.java`
around lines 5 - 14, The Money compact constructor must canonicalize both fields
so equals/hashCode are stable: resolve the provided currency string via
Currency.getInstance(currency.trim().toUpperCase()) and replace the record's
currency parameter with currencyInstance.getCurrencyCode(); determine the
currency scale via currencyInstance.getDefaultFractionDigits() and set the
amount parameter to amount.setScale(scale, RoundingMode.UNNECESSARY) (or throw a
clear IllegalArgumentException if the amount has too many fraction digits), or
alternatively use amount.stripTrailingZeros() if you prefer flexible
normalization; perform null/blank checks first, then reassign the normalized
amount and currency in the compact constructor of the Money record so stored
values are canonical for equality and persistence.
Comment on lines
+7
to
+13
| public record PspRefundRequest( | ||
| UUID collectionId, | ||
| String pspReference, | ||
| Money refundAmount, | ||
| String pspName, | ||
| String reason | ||
| ) { |
There was a problem hiding this comment.
Use the domain PSP identifier here instead of a raw string.
pspName bypasses the PspIdentifier value object added in this PR, so case drift / invalid provider ids can still cross the boundary. Keep the port typed with PspIdentifier and let the adapter unwrap it at the integration edge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.java`
around lines 7 - 13, The PspRefundRequest record currently exposes a raw String
pspName which bypasses the new PspIdentifier value object; update the record
signature to use PspIdentifier instead of String (i.e., change the pspName
parameter to type PspIdentifier) and update any call sites that construct
PspRefundRequest to pass a PspIdentifier; leave unwrapping (e.g., to String or
provider-specific id) to the integration adapter code so adapters convert
PspIdentifier to whatever external representation is required.
| @@ -0,0 +1,4 @@ | |||
| package com.stablecoin.payments.onramp.domain.port; | |||
|
|
|||
| public record PspRefundResult(String pspRefundRef, String status) { | |||
There was a problem hiding this comment.
Type the refund status in the port contract.
String status lets provider-specific or unexpected values leak into the domain boundary. The refund state machine already has RefundStatus, so this port should return the canonical domain status and keep any PSP-native status in a separate raw field if you still need it.
Proposed change
+import com.stablecoin.payments.onramp.domain.model.RefundStatus;
-public record PspRefundResult(String pspRefundRef, String status) {
+public record PspRefundResult(String pspRefundRef, RefundStatus status, String rawStatus) {
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public record PspRefundResult(String pspRefundRef, String status) { | |
| import com.stablecoin.payments.onramp.domain.model.RefundStatus; | |
| public record PspRefundResult(String pspRefundRef, RefundStatus status, String rawStatus) { | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundResult.java`
at line 3, Change the PspRefundResult port contract to use the domain enum
RefundStatus instead of a raw String: replace the record field String status
with RefundStatus status (and optionally add a String rawStatus field to carry
PSP-native values if needed). Update the PspRefundResult declaration and any
constructors/usages (calls that construct or read pspRefundRef/status) to import
and use com.stablecoin.payments.onramp.domain.model.RefundStatus and map
PSP-native status strings to RefundStatus at the adapter/border layer rather
than leaking them through the port.
|
|
||
| PspTransaction save(PspTransaction transaction); | ||
|
|
||
| List<PspTransaction> findByCollectionId(UUID collectionId); |
There was a problem hiding this comment.
Define chronological ordering for transaction reads.
For an append-only PSP audit log, findByCollectionId(...) without an order contract makes replay and investigation adapter-dependent. Specify ascending receivedAt/append order in the port contract or method name so all implementations return a deterministic sequence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspTransactionRepository.java`
at line 12, The port method findByCollectionId(UUID collectionId) lacks a
deterministic ordering; update the contract to require chronological (append)
order by receivedAt—e.g., rename to findByCollectionIdOrderByReceivedAtAsc or
document/annotate that implementations must return results ordered ascending by
PspTransaction.receivedAt—so all adapters return a reproducible, append-order
sequence for replay and investigation.
Comment on lines
+9
to
+11
| public StateMachine(List<StateTransition<S, T>> transitions) { | ||
| this.transitions = List.copyOf(transitions); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating no duplicate transitions in constructor.
If the transitions list contains two entries with the same (from, trigger) pair but different to states, findFirst() returns a non-deterministic result depending on list order. Add a constructor guard or document the first-match-wins semantics explicitly.
🛡️ Proposed validation
public StateMachine(List<StateTransition<S, T>> transitions) {
+ var duplicates = transitions.stream()
+ .collect(java.util.stream.Collectors.groupingBy(
+ t -> t.from() + ":" + t.trigger(),
+ java.util.stream.Collectors.counting()))
+ .entrySet().stream()
+ .filter(e -> e.getValue() > 1)
+ .map(java.util.Map.Entry::getKey)
+ .toList();
+ if (!duplicates.isEmpty()) {
+ throw new IllegalArgumentException("Duplicate transitions: " + duplicates);
+ }
this.transitions = List.copyOf(transitions);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public StateMachine(List<StateTransition<S, T>> transitions) { | |
| this.transitions = List.copyOf(transitions); | |
| } | |
| public StateMachine(List<StateTransition<S, T>> transitions) { | |
| var duplicates = transitions.stream() | |
| .collect(java.util.stream.Collectors.groupingBy( | |
| t -> t.from() + ":" + t.trigger(), | |
| java.util.stream.Collectors.counting())) | |
| .entrySet().stream() | |
| .filter(e -> e.getValue() > 1) | |
| .map(java.util.Map.Entry::getKey) | |
| .toList(); | |
| if (!duplicates.isEmpty()) { | |
| throw new IllegalArgumentException("Duplicate transitions: " + duplicates); | |
| } | |
| this.transitions = List.copyOf(transitions); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java`
around lines 9 - 11, The constructor StateMachine(List<StateTransition<S, T>>
transitions) should validate that no two StateTransition entries share the same
(from, trigger) pair with different to states; iterate the provided transitions
and build a map keyed by the (from, trigger) tuple (or use a composite key), and
if you encounter a key already present with a different to state throw an
IllegalArgumentException (or similarly fail-fast) to avoid non-deterministic
findFirst() behavior; keep using List.copyOf(transitions) after validation
(refer to StateMachine constructor and StateTransition's
getFrom/getTrigger/getTo methods).
Comment on lines
+3
to
+4
| public record StateTransition<S, T>(S from, T trigger, S to) { | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding null validation to the canonical constructor.
Allowing null values for from, trigger, or to would create invalid transition definitions that silently propagate until StateMachine lookup fails. Fail-fast validation improves debuggability.
🛡️ Optional: Add compact constructor with null checks
-public record StateTransition<S, T>(S from, T trigger, S to) {
-}
+public record StateTransition<S, T>(S from, T trigger, S to) {
+ public StateTransition {
+ java.util.Objects.requireNonNull(from, "from state must not be null");
+ java.util.Objects.requireNonNull(trigger, "trigger must not be null");
+ java.util.Objects.requireNonNull(to, "to state must not be null");
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public record StateTransition<S, T>(S from, T trigger, S to) { | |
| } | |
| public record StateTransition<S, T>(S from, T trigger, S to) { | |
| public StateTransition { | |
| java.util.Objects.requireNonNull(from, "from state must not be null"); | |
| java.util.Objects.requireNonNull(trigger, "trigger must not be null"); | |
| java.util.Objects.requireNonNull(to, "to state must not be null"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateTransition.java`
around lines 3 - 4, The StateTransition record allows null values for its
components (from, trigger, to) which can lead to invalid transitions; add a
compact canonical constructor for StateTransition that validates each component
(from, trigger, to) using null checks (e.g., Objects.requireNonNull or explicit
checks) and throws a clear NPE with a descriptive message if any are null so
invalid transitions fail fast during construction.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.java (1)
7-12:⚠️ Potential issue | 🟠 MajorUse
PspIdentifierhere instead of a raw PSP name.Line 11 bypasses the new value object and reintroduces provider identity drift at the gateway boundary. Keep the request typed with
PspIdentifier; let the adapter unwrap it to the PSP-specific representation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.java` around lines 7 - 12, The PspRefundRequest record is using a raw String field pspName which reintroduces provider identity drift; change the record to use the PspIdentifier value object instead of pspName (replace the pspName field with a PspIdentifier-typed field) and update any constructors/usages to accept/propagate PspIdentifier so adapters can unwrap it to PSP-specific representations when needed (refer to PspRefundRequest and PspIdentifier).fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.java (1)
5-11:⚠️ Potential issue | 🟠 MajorTrim and canonicalize before assignment.
" stripe "still passes here and becomes a distinct identifier from"stripe", which is risky for repository lookups and emitted events. Normalize both fields inside the canonical constructor before validating.Proposed fix
public record PspIdentifier(String pspId, String pspName) { public PspIdentifier { - if (pspId == null || pspId.isBlank()) { + pspId = pspId == null ? null : pspId.trim(); + pspName = pspName == null ? null : pspName.trim(); + + if (pspId == null || pspId.isEmpty()) { throw new IllegalArgumentException("PSP ID is required"); } - if (pspName == null || pspName.isBlank()) { + if (pspName == null || pspName.isEmpty()) { throw new IllegalArgumentException("PSP name is required"); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.java` around lines 5 - 11, In the PspIdentifier canonical constructor, normalize pspId and pspName before validation and assignment: trim whitespace (and apply consistent canonicalization, e.g., consistent case normalization such as toLowerCase if your system expects case-insensitivity) on the incoming pspId and pspName values, then validate the normalized values for null/blank and throw the existing IllegalArgumentException messages if invalid, and finally assign the normalized results to the record fields so inputs like " stripe " become the canonical "stripe" used throughout.fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.java (1)
3-5:⚠️ Potential issue | 🟠 MajorPublish a typed domain event instead of
Object.Line 5 removes compile-time safety from the port and lets arbitrary payloads cross the boundary. Narrow this to a common collection/refund domain-event supertype so callers and adapters stay aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.java` around lines 3 - 5, The publish(Object event) port loses type safety—change the signature in CollectionEventPublisher to accept a narrow domain-event supertype (e.g., CollectionEvent or CollectionDomainEvent) instead of Object; create or reference the common event interface/class (e.g., CollectionEvent) that all collection/refund events implement, update the publish method signature in CollectionEventPublisher.publish(CollectionEvent event), and then update all adapters/implementations and callers to accept/emit that typed event and import the new supertype.fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.java (1)
5-7:⚠️ Potential issue | 🟠 MajorModel idempotency at this port boundary.
These methods initiate non-idempotent PSP operations. If a timeout happens after provider acceptance, a retry can create duplicate payment/refund requests because neither request type carries a required idempotency key.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.java` around lines 5 - 7, The port methods initiatePayment(PspPaymentRequest) and initiateRefund(PspRefundRequest) must model idempotency at the boundary: add an idempotency key field to both PspPaymentRequest and PspRefundRequest (or require an IdempotencyKey parameter) and propagate it through initiatePayment and initiateRefund so callers supply a stable idempotency key; ensure the downstream gateway implementations use that key when calling PSPs and include it in PspPaymentResult/PspRefundResult metadata to correlate retries.fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java (1)
9-10: 🛠️ Refactor suggestion | 🟠 MajorFail fast on duplicate
(from, trigger)transitions.Because
transition()resolves withfindFirst(), duplicate transition keys become order-dependent and can route the aggregate to the wrong state. Validate uniqueness in the constructor before copying the list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java` around lines 9 - 10, In StateMachine's constructor, validate that there are no duplicate (from, trigger) keys among the provided List<StateTransition<S, T>> before calling List.copyOf(transitions); compute a set of pairing keys (e.g. combine transition.getFrom() and transition.getTrigger() or use Map.entry-style tuples) while iterating the incoming transitions, and if a duplicate is detected throw an IllegalArgumentException with a clear message; this ensures transition() (which uses findFirst()) cannot produce order-dependent routing due to duplicate keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundInitiatedEvent.java`:
- Line 13: The event currently exposes an unbounded String field named reason in
RefundInitiatedEvent; replace it with a bounded code/enum (e.g., add a
RefundReason enum and change the RefundInitiatedEvent constructor/field from
String reason to RefundReason reasonCode) so only predefined reason codes are
published, and move any free-form operator or customer notes into the
aggregate/persistence layer (or keep them only on the command/DB model). Update
serialization/consumers to use the new RefundReason type and consider a
migration/compatibility path (e.g., keep the old String as deprecated or map
legacy strings to enum values) to avoid breaking subscribers.
- Around line 7-15: The RefundInitiatedEvent record currently allows null or
invalid values; add a compact canonical constructor for RefundInitiatedEvent
that validates refundId, collectionId, paymentId, and initiatedAt are non-null,
refundAmount is non-null and > 0, and currency and reason are non-null/non-blank
(trimmed length > 0); throw IllegalArgumentException (or NullPointerException
for nulls) with clear messages when invariants fail so malformed events cannot
be created and emitted to Kafka.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`:
- Around line 181-189: The failCollection method lacks the aggregate
terminal-state guard used elsewhere; add a call to assertNotTerminal() at the
start of failCollection() before performing STATE_MACHINE.transition(status,
FAIL) so the method mirrors other transitions (use the existing
assertNotTerminal() helper on the current status), then proceed with the
transition and
toBuilder().status(nextState).failureReason(reason).errorCode(errorCode).updatedAt(Instant.now()).build().
- Around line 164-176: confirmCollection uses two separate Instant.now() calls
causing microsecond skew between pspSettledAt and updatedAt; fix by capturing
Instant.now() once into a local variable (e.g., var now = Instant.now()) at the
start of the method after validation and reuse that variable when building the
new object for pspSettledAt and updatedAt in the toBuilder() chain (method:
confirmCollection, fields: pspSettledAt, updatedAt; keep existing checks like
collectedAmount and STATE_MACHINE.transition).
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransaction.java`:
- Line 70: The factory currently sets receivedAt using Instant.now(), coupling
PspTransaction creation to the system clock; modify the PspTransaction creation
path (the factory/builder that calls .receivedAt(Instant.now())) to accept a
java.time.Clock or an explicit receivedAt Instant parameter (e.g. add parameter
Clock clock or Instant receivedAt to the factory/builder API), use
Instant.now(clock) or the provided receivedAt instead of Instant.now(), and
update production callers to pass Clock.systemUTC() (or omit and default to
Instant.now(Clock.systemUTC())). Ensure all test callers can inject a fixed
Clock or a deterministic Instant to make tests deterministic; update references
to the factory/builder where .receivedAt(Instant.now()) is used accordingly.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Refund.java`:
- Around line 101-113: The fail method in Refund currently doesn't record a
timestamp; update Refund.fail(String failureReason) to set a failure timestamp:
either populate the existing completedAt (consistent quick fix) or add a new
failedAt field to the Refund model and builder for clearer semantics; in
Refund.fail use
toBuilder().status(FAILED).failureReason(failureReason).completedAt(Instant.now())
(or .failedAt(Instant.now()) if you add the field) before build(), and ensure
the Refund class, builder, and any serializers/persistence mappings are updated
to include the new failedAt field if you choose that approach.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java`:
- Around line 13-23: Add explicit null guards at the start of both transition(S
currentState, T trigger) and canTransition(S currentState, T trigger): if either
currentState or trigger is null, throw the domain-specific
StateMachineException.invalidTransition(currentState, trigger) (or another
appropriate StateMachineException), then proceed with the existing stream logic
that uses StateTransition::from and StateTransition::trigger; this prevents
NullPointerException from equals() and ensures null inputs produce a controlled
domain error.
---
Duplicate comments:
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.java`:
- Around line 5-11: In the PspIdentifier canonical constructor, normalize pspId
and pspName before validation and assignment: trim whitespace (and apply
consistent canonicalization, e.g., consistent case normalization such as
toLowerCase if your system expects case-insensitivity) on the incoming pspId and
pspName values, then validate the normalized values for null/blank and throw the
existing IllegalArgumentException messages if invalid, and finally assign the
normalized results to the record fields so inputs like " stripe " become the
canonical "stripe" used throughout.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.java`:
- Around line 3-5: The publish(Object event) port loses type safety—change the
signature in CollectionEventPublisher to accept a narrow domain-event supertype
(e.g., CollectionEvent or CollectionDomainEvent) instead of Object; create or
reference the common event interface/class (e.g., CollectionEvent) that all
collection/refund events implement, update the publish method signature in
CollectionEventPublisher.publish(CollectionEvent event), and then update all
adapters/implementations and callers to accept/emit that typed event and import
the new supertype.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.java`:
- Around line 5-7: The port methods initiatePayment(PspPaymentRequest) and
initiateRefund(PspRefundRequest) must model idempotency at the boundary: add an
idempotency key field to both PspPaymentRequest and PspRefundRequest (or require
an IdempotencyKey parameter) and propagate it through initiatePayment and
initiateRefund so callers supply a stable idempotency key; ensure the downstream
gateway implementations use that key when calling PSPs and include it in
PspPaymentResult/PspRefundResult metadata to correlate retries.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.java`:
- Around line 7-12: The PspRefundRequest record is using a raw String field
pspName which reintroduces provider identity drift; change the record to use the
PspIdentifier value object instead of pspName (replace the pspName field with a
PspIdentifier-typed field) and update any constructors/usages to
accept/propagate PspIdentifier so adapters can unwrap it to PSP-specific
representations when needed (refer to PspRefundRequest and PspIdentifier).
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java`:
- Around line 9-10: In StateMachine's constructor, validate that there are no
duplicate (from, trigger) keys among the provided List<StateTransition<S, T>>
before calling List.copyOf(transitions); compute a set of pairing keys (e.g.
combine transition.getFrom() and transition.getTrigger() or use Map.entry-style
tuples) while iterating the incoming transitions, and if a duplicate is detected
throw an IllegalArgumentException with a clear message; this ensures
transition() (which uses findFirst()) cannot produce order-dependent routing due
to duplicate keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a44bbcf-5494-45f4-a955-7025e72712ac
📒 Files selected for processing (30)
fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionCompletedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionFailedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/CollectionInitiatedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundCompletedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundInitiatedEvent.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/AccountType.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/BankAccount.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionStatus.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionTrigger.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Money.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PaymentRail.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PaymentRailType.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspIdentifier.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransaction.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransactionDirection.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Refund.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/RefundStatus.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionEventPublisher.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/CollectionOrderRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspGateway.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentRequest.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspPaymentResult.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundRequest.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspRefundResult.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/PspTransactionRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/port/RefundRepository.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachineException.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateTransition.java
Comment on lines
+7
to
+15
| public record RefundInitiatedEvent( | ||
| UUID refundId, | ||
| UUID collectionId, | ||
| UUID paymentId, | ||
| BigDecimal refundAmount, | ||
| String currency, | ||
| String reason, | ||
| Instant initiatedAt | ||
| ) { |
There was a problem hiding this comment.
Enforce event invariants in the record constructor.
This record currently accepts null IDs/timestamp, blank currency/reason, and non-positive amounts. That allows malformed domain events to escape onto Kafka before any adapter-level validation catches them.
Proposed fix
package com.stablecoin.payments.onramp.domain.event;
import java.math.BigDecimal;
import java.time.Instant;
+import java.util.Objects;
import java.util.UUID;
public record RefundInitiatedEvent(
UUID refundId,
UUID collectionId,
@@
String reason,
Instant initiatedAt
) {
+ public RefundInitiatedEvent {
+ Objects.requireNonNull(refundId, "refundId must not be null");
+ Objects.requireNonNull(collectionId, "collectionId must not be null");
+ Objects.requireNonNull(paymentId, "paymentId must not be null");
+ Objects.requireNonNull(refundAmount, "refundAmount must not be null");
+ Objects.requireNonNull(currency, "currency must not be null");
+ Objects.requireNonNull(reason, "reason must not be null");
+ Objects.requireNonNull(initiatedAt, "initiatedAt must not be null");
+
+ if (refundAmount.signum() <= 0) {
+ throw new IllegalArgumentException("refundAmount must be positive");
+ }
+ if (currency.isBlank()) {
+ throw new IllegalArgumentException("currency must not be blank");
+ }
+ if (reason.isBlank()) {
+ throw new IllegalArgumentException("reason must not be blank");
+ }
+ }
public static final String TOPIC = "fiat.refund.initiated";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundInitiatedEvent.java`
around lines 7 - 15, The RefundInitiatedEvent record currently allows null or
invalid values; add a compact canonical constructor for RefundInitiatedEvent
that validates refundId, collectionId, paymentId, and initiatedAt are non-null,
refundAmount is non-null and > 0, and currency and reason are non-null/non-blank
(trimmed length > 0); throw IllegalArgumentException (or NullPointerException
for nulls) with clear messages when invariants fail so malformed events cannot
be created and emitted to Kafka.
| UUID paymentId, | ||
| BigDecimal refundAmount, | ||
| String currency, | ||
| String reason, |
There was a problem hiding this comment.
Avoid publishing free-form refund reasons on the event bus.
Line 13 exposes reason as an arbitrary String. If this carries operator notes or customer-entered text, it will be replicated to every subscriber and retained in Kafka. Prefer a bounded reason code/enum in the event payload and keep detailed notes inside the aggregate or persistence layer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/event/RefundInitiatedEvent.java`
at line 13, The event currently exposes an unbounded String field named reason
in RefundInitiatedEvent; replace it with a bounded code/enum (e.g., add a
RefundReason enum and change the RefundInitiatedEvent constructor/field from
String reason to RefundReason reasonCode) so only predefined reason codes are
published, and move any free-form operator or customer notes into the
aggregate/persistence layer (or keep them only on the command/DB model). Update
serialization/consumers to use the new RefundReason type and consider a
migration/compatibility path (e.g., keep the old String as deprecated or map
legacy strings to enum values) to avoid breaking subscribers.
Comment on lines
+164
to
+176
| public CollectionOrder confirmCollection(Money collectedAmount) { | ||
| assertNotTerminal(); | ||
| if (collectedAmount == null) { | ||
| throw new IllegalArgumentException("Collected amount is required"); | ||
| } | ||
| var nextState = STATE_MACHINE.transition(status, PAYMENT_CONFIRMED); | ||
| return toBuilder() | ||
| .status(nextState) | ||
| .collectedAmount(collectedAmount) | ||
| .pspSettledAt(Instant.now()) | ||
| .updatedAt(Instant.now()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Double Instant.now() call creates timestamp skew.
pspSettledAt and updatedAt are set from separate Instant.now() calls, potentially differing by microseconds. Capture once and reuse.
🔧 Proposed fix
public CollectionOrder confirmCollection(Money collectedAmount) {
assertNotTerminal();
if (collectedAmount == null) {
throw new IllegalArgumentException("Collected amount is required");
}
var nextState = STATE_MACHINE.transition(status, PAYMENT_CONFIRMED);
+ var now = Instant.now();
return toBuilder()
.status(nextState)
.collectedAmount(collectedAmount)
- .pspSettledAt(Instant.now())
- .updatedAt(Instant.now())
+ .pspSettledAt(now)
+ .updatedAt(now)
.build();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public CollectionOrder confirmCollection(Money collectedAmount) { | |
| assertNotTerminal(); | |
| if (collectedAmount == null) { | |
| throw new IllegalArgumentException("Collected amount is required"); | |
| } | |
| var nextState = STATE_MACHINE.transition(status, PAYMENT_CONFIRMED); | |
| return toBuilder() | |
| .status(nextState) | |
| .collectedAmount(collectedAmount) | |
| .pspSettledAt(Instant.now()) | |
| .updatedAt(Instant.now()) | |
| .build(); | |
| } | |
| public CollectionOrder confirmCollection(Money collectedAmount) { | |
| assertNotTerminal(); | |
| if (collectedAmount == null) { | |
| throw new IllegalArgumentException("Collected amount is required"); | |
| } | |
| var nextState = STATE_MACHINE.transition(status, PAYMENT_CONFIRMED); | |
| var now = Instant.now(); | |
| return toBuilder() | |
| .status(nextState) | |
| .collectedAmount(collectedAmount) | |
| .pspSettledAt(now) | |
| .updatedAt(now) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`
around lines 164 - 176, confirmCollection uses two separate Instant.now() calls
causing microsecond skew between pspSettledAt and updatedAt; fix by capturing
Instant.now() once into a local variable (e.g., var now = Instant.now()) at the
start of the method after validation and reuse that variable when building the
new object for pspSettledAt and updatedAt in the toBuilder() chain (method:
confirmCollection, fields: pspSettledAt, updatedAt; keep existing checks like
collectedAmount and STATE_MACHINE.transition).
Comment on lines
+181
to
+189
| public CollectionOrder failCollection(String reason, String errorCode) { | ||
| var nextState = STATE_MACHINE.transition(status, FAIL); | ||
| return toBuilder() | ||
| .status(nextState) | ||
| .failureReason(reason) | ||
| .errorCode(errorCode) | ||
| .updatedAt(Instant.now()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Missing assertNotTerminal() guard — inconsistent with other transitions.
failCollection() skips the terminal state check, unlike other transition methods. While the state machine will throw on invalid transitions, adding the guard provides consistent error messages and aligns with the aggregate's defensive coding pattern.
🔧 Proposed fix
public CollectionOrder failCollection(String reason, String errorCode) {
+ assertNotTerminal();
var nextState = STATE_MACHINE.transition(status, FAIL);
return toBuilder()
.status(nextState)
.failureReason(reason)
.errorCode(errorCode)
.updatedAt(Instant.now())
.build();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/CollectionOrder.java`
around lines 181 - 189, The failCollection method lacks the aggregate
terminal-state guard used elsewhere; add a call to assertNotTerminal() at the
start of failCollection() before performing STATE_MACHINE.transition(status,
FAIL) so the method mirrors other transitions (use the existing
assertNotTerminal() helper on the current status), then proceed with the
transition and
toBuilder().status(nextState).failureReason(reason).errorCode(errorCode).updatedAt(Instant.now()).build().
| .amount(amount) | ||
| .status(status) | ||
| .rawResponse(rawResponse) | ||
| .receivedAt(Instant.now()) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Instant.now() couples factory to system clock.
Hardcoding Instant.now() prevents deterministic unit testing. Inject a Clock or accept receivedAt as a parameter, defaulting to Instant.now(Clock.systemUTC()) in production callers.
♻️ Proposed fix
public static PspTransaction create(UUID collectionId, String pspName,
String pspReference, PspTransactionDirection direction,
String eventType, Money amount,
- String status, String rawResponse) {
+ String status, String rawResponse,
+ Instant receivedAt) {
// ... validation unchanged ...
+ if (receivedAt == null) {
+ throw new IllegalArgumentException("receivedAt is required");
+ }
return PspTransaction.builder()
.pspTxnId(UUID.randomUUID())
// ... other fields ...
- .receivedAt(Instant.now())
+ .receivedAt(receivedAt)
.build();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/PspTransaction.java`
at line 70, The factory currently sets receivedAt using Instant.now(), coupling
PspTransaction creation to the system clock; modify the PspTransaction creation
path (the factory/builder that calls .receivedAt(Instant.now())) to accept a
java.time.Clock or an explicit receivedAt Instant parameter (e.g. add parameter
Clock clock or Instant receivedAt to the factory/builder API), use
Instant.now(clock) or the provided receivedAt instead of Instant.now(), and
update production callers to pass Clock.systemUTC() (or omit and default to
Instant.now(Clock.systemUTC())). Ensure all test callers can inject a fixed
Clock or a deterministic Instant to make tests deterministic; update references
to the factory/builder where .receivedAt(Instant.now()) is used accordingly.
Comment on lines
+101
to
+113
| public Refund fail(String failureReason) { | ||
| if (status != PROCESSING) { | ||
| throw new IllegalStateException( | ||
| "Refund %s cannot fail from state %s".formatted(refundId, status)); | ||
| } | ||
| if (failureReason == null || failureReason.isBlank()) { | ||
| throw new IllegalArgumentException("Failure reason is required"); | ||
| } | ||
| return toBuilder() | ||
| .status(FAILED) | ||
| .failureReason(failureReason) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider setting completedAt (or a dedicated failedAt) on failure.
complete() records completedAt but fail() leaves no timestamp. For audit trails, capturing when the failure occurred can be valuable. If completedAt semantically implies success, consider adding failedAt to the record.
🔧 Quick fix using existing field
public Refund fail(String failureReason) {
if (status != PROCESSING) {
throw new IllegalStateException(
"Refund %s cannot fail from state %s".formatted(refundId, status));
}
if (failureReason == null || failureReason.isBlank()) {
throw new IllegalArgumentException("Failure reason is required");
}
return toBuilder()
.status(FAILED)
.failureReason(failureReason)
+ .completedAt(Instant.now()) // or introduce a failedAt field
.build();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Refund fail(String failureReason) { | |
| if (status != PROCESSING) { | |
| throw new IllegalStateException( | |
| "Refund %s cannot fail from state %s".formatted(refundId, status)); | |
| } | |
| if (failureReason == null || failureReason.isBlank()) { | |
| throw new IllegalArgumentException("Failure reason is required"); | |
| } | |
| return toBuilder() | |
| .status(FAILED) | |
| .failureReason(failureReason) | |
| .build(); | |
| } | |
| public Refund fail(String failureReason) { | |
| if (status != PROCESSING) { | |
| throw new IllegalStateException( | |
| "Refund %s cannot fail from state %s".formatted(refundId, status)); | |
| } | |
| if (failureReason == null || failureReason.isBlank()) { | |
| throw new IllegalArgumentException("Failure reason is required"); | |
| } | |
| return toBuilder() | |
| .status(FAILED) | |
| .failureReason(failureReason) | |
| .completedAt(Instant.now()) // or introduce a failedAt field | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/model/Refund.java`
around lines 101 - 113, The fail method in Refund currently doesn't record a
timestamp; update Refund.fail(String failureReason) to set a failure timestamp:
either populate the existing completedAt (consistent quick fix) or add a new
failedAt field to the Refund model and builder for clearer semantics; in
Refund.fail use
toBuilder().status(FAILED).failureReason(failureReason).completedAt(Instant.now())
(or .failedAt(Instant.now()) if you add the field) before build(), and ensure
the Refund class, builder, and any serializers/persistence mappings are updated
to include the new failedAt field if you choose that approach.
Comment on lines
+13
to
+23
| public S transition(S currentState, T trigger) { | ||
| return transitions.stream() | ||
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | ||
| .map(StateTransition::to) | ||
| .findFirst() | ||
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | ||
| } | ||
|
|
||
| public boolean canTransition(S currentState, T trigger) { | ||
| return transitions.stream() | ||
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); |
There was a problem hiding this comment.
Guard null state/trigger inputs explicitly.
Lines 15 and 23 dereference currentState and trigger through equals, so null inputs escape as NullPointerException instead of a domain-specific error. Reject nulls up front, or switch to Objects.equals(...) plus explicit validation.
Proposed fix
+import java.util.Objects;
import java.util.List;
@@
public S transition(S currentState, T trigger) {
+ Objects.requireNonNull(currentState, "currentState is required");
+ Objects.requireNonNull(trigger, "trigger is required");
return transitions.stream()
.filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger))
.map(StateTransition::to)
.findFirst()
.orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger));
}
public boolean canTransition(S currentState, T trigger) {
+ if (currentState == null || trigger == null) {
+ return false;
+ }
return transitions.stream()
.anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public S transition(S currentState, T trigger) { | |
| return transitions.stream() | |
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | |
| .map(StateTransition::to) | |
| .findFirst() | |
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | |
| } | |
| public boolean canTransition(S currentState, T trigger) { | |
| return transitions.stream() | |
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); | |
| import java.util.Objects; | |
| import java.util.List; | |
| public S transition(S currentState, T trigger) { | |
| Objects.requireNonNull(currentState, "currentState is required"); | |
| Objects.requireNonNull(trigger, "trigger is required"); | |
| return transitions.stream() | |
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | |
| .map(StateTransition::to) | |
| .findFirst() | |
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | |
| } | |
| public boolean canTransition(S currentState, T trigger) { | |
| if (currentState == null || trigger == null) { | |
| return false; | |
| } | |
| return transitions.stream() | |
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/statemachine/StateMachine.java`
around lines 13 - 23, Add explicit null guards at the start of both transition(S
currentState, T trigger) and canTransition(S currentState, T trigger): if either
currentState or trigger is null, throw the domain-specific
StateMachineException.invalidTransition(currentState, trigger) (or another
appropriate StateMachineException), then proceed with the existing stream logic
that uses StateTransition::from and StateTransition::trigger; this prevents
NullPointerException from equals() and ensures null inputs produce a controlled
domain error.
This was referenced Mar 8, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Money,PaymentRail,PspIdentifier,BankAccountTOPICconstantsTest plan
Closes STA-120
🤖 Generated with Claude Code
Summary by CodeRabbit