feat(s3): application layer tests — 19 controller ITs + FK bug fix (STA-128)#138
Merged
Merged
Conversation
…TA-128) Add CollectionControllerIT (14 tests) and StripeWebhookControllerIT (5 tests) covering all HTTP scenarios: POST 201/200/400, GET 200/404/400, refund 201/409/422/404, webhook 200/400/404. Fix CollectionCommandHandler save ordering — collection order must be persisted before PspTransaction to satisfy FK constraint. Fix PspTransactionPersistenceMapper null rawResponse handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdds integration tests for collections and Stripe webhooks, introduces test-fixtures dependency and API request fixtures, sets PSP provider to "dev" for integration tests, adds idempotencyKey to PspPaymentRequest and propagates it to Stripe adapter, persists collection order earlier, and defaults null PspTransaction.rawResponse to "{}". Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/build.gradle.kts`:
- Around line 114-116: The test-fixtures module pulls in the whole API via
testFixturesImplementation(project(":fiat-on-ramp:fiat-on-ramp-api")), coupling
shared fixtures; to fix, move the two fixtures that depend on API
types—CollectionOrderFixtures and RefundFixtures (which reference
CollectionRequest and RefundRequest)—out of the shared fixtures and into
src/integration-test of the module that needs them, update their package/imports
accordingly, and then remove the
testFixturesImplementation(project(":fiat-on-ramp:fiat-on-ramp-api"))
declaration so the testFixtures module no longer depends on the API.
In
`@fiat-on-ramp/fiat-on-ramp/src/integration-test/java/com/stablecoin/payments/onramp/application/controller/CollectionControllerIT.java`:
- Around line 64-81: The test shouldReturn200ForIdempotentReplay currently only
asserts the paymentId on the second (replay) response; capture and parse the
first response to extract its collectionId and then assert the second response
returns the exact same collectionId to prove idempotency. After performing the
first mockMvc.post to "/v1/collections" with the request, store the response
body and extract the collectionId (e.g., from the JSON), then perform the second
post and assert both jsonPath("$.paymentId", is(PAYMENT_ID.toString())) and
jsonPath("$.collectionId", is(firstCollectionId)) where firstCollectionId is the
value extracted from the first response. Ensure you reference the existing
variables/methods request, mockMvc, objectMapper, PAYMENT_ID and the endpoint
"/v1/collections".
In
`@fiat-on-ramp/fiat-on-ramp/src/integration-test/java/com/stablecoin/payments/onramp/application/controller/StripeWebhookControllerIT.java`:
- Around line 37-71: Update the two tests in StripeWebhookControllerIT
(shouldReturn200ForSucceededWebhook and shouldReturn200ForFailedWebhook) to
assert the persisted state transition: after performing the POST (using
anAwaitingConfirmationOrder(), PSP_REFERENCE and
collectionOrderRepository.save(...)), reload the saved order from
collectionOrderRepository (e.g., via findById or findByCollectionId using
order.collectionId()) and assert its terminal status changed to the expected
value for each case (succeeded → expected terminal success status, failed →
expected terminal failure status) in addition to the existing 200/"received"
assertions.
In
`@fiat-on-ramp/fiat-on-ramp/src/integration-test/resources/application-integration-test.yml`:
- Around line 32-33: The integration test profile currently sets psp provider to
dev which disables real Stripe HMAC verification; for StripeWebhookControllerIT
either switch the test to a Stripe-specific profile instead of using
app.psp.provider=dev or override that property in the test setup to use the
Stripe provider and ensure requests are signed by computing and adding valid
signatures using WebhookFixtures.computeSignature(...); update
StripeWebhookControllerIT to toggle the profile or set the property and sign
payloads so the webhook path performs real HMAC validation.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/service/CollectionCommandHandler.java`:
- Around line 78-79: The code currently calls the PSP after saving the order
inside the same `@Transactional` boundary (collectionOrderRepository.save(order)
in CollectionCommandHandler), which risks PSP side effects being visible if the
transaction later rolls back; either persist/commit the collection order before
calling the PSP or make the PSP call idempotent. Fix by moving the commit point:
either (A) persist and flush/commit the order before the PSP call (e.g.
save+flush or perform the save inside a separate transaction using
Propagation.REQUIRES_NEW for the method that calls
collectionOrderRepository.save(order)) so the FK is durable prior to
initiatePayment(), or (B) generate and pass a stable idempotency key derived
from paymentId/collectionId into the PSP request so retrying initiatePayment()
is safe; update the code paths in CollectionCommandHandler and the PSP
invocation to use the chosen approach.
In
`@fiat-on-ramp/fiat-on-ramp/src/testFixtures/java/com/stablecoin/payments/onramp/fixtures/CollectionOrderFixtures.java`:
- Around line 11-12: The import block change introduced by adding import
com.stablecoin.payments.onramp.api.CollectionRequest in CollectionOrderFixtures
caused a Spotless ordering/spacing violation; restore the expected import
grouping/ordering and spacing (or simply run ./gradlew spotlessApply) to
reformat the imports in CollectionOrderFixtures so they match the project's
Spotless rules before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0765d4f1-f3e6-4e8a-bd0d-c5ad2339f4ad
📒 Files selected for processing (8)
fiat-on-ramp/fiat-on-ramp/build.gradle.ktsfiat-on-ramp/fiat-on-ramp/src/integration-test/java/com/stablecoin/payments/onramp/application/controller/CollectionControllerIT.javafiat-on-ramp/fiat-on-ramp/src/integration-test/java/com/stablecoin/payments/onramp/application/controller/StripeWebhookControllerIT.javafiat-on-ramp/fiat-on-ramp/src/integration-test/resources/application-integration-test.ymlfiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/domain/service/CollectionCommandHandler.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/persistence/mapper/PspTransactionPersistenceMapper.javafiat-on-ramp/fiat-on-ramp/src/testFixtures/java/com/stablecoin/payments/onramp/fixtures/CollectionOrderFixtures.javafiat-on-ramp/fiat-on-ramp/src/testFixtures/java/com/stablecoin/payments/onramp/fixtures/RefundFixtures.java
Comment on lines
114
to
116
| // Test Fixtures | ||
| testFixturesImplementation(project(":fiat-on-ramp:fiat-on-ramp-api")) | ||
| testFixturesImplementation("org.assertj:assertj-core") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show which shared fixtures import API-layer DTOs.
rg -n --glob 'fiat-on-ramp/fiat-on-ramp/src/testFixtures/java/**/*.java' \
'import com\.stablecoin\.payments\.onramp\.api\.'Repository: Puneethkumarck/stablebridge-platform
Length of output: 427
Narrow testFixtures scope to domain-only fixtures.
The fiat-on-ramp-api dependency in line 115 couples your published test-fixture module to the API layer. Verification shows only CollectionOrderFixtures and RefundFixtures require CollectionRequest/RefundRequest from the API package. Move these two fixture classes to src/integration-test and remove the API dependency from shared testFixtures to keep the fixture module focused and decoupled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fiat-on-ramp/fiat-on-ramp/build.gradle.kts` around lines 114 - 116, The
test-fixtures module pulls in the whole API via
testFixturesImplementation(project(":fiat-on-ramp:fiat-on-ramp-api")), coupling
shared fixtures; to fix, move the two fixtures that depend on API
types—CollectionOrderFixtures and RefundFixtures (which reference
CollectionRequest and RefundRequest)—out of the shared fixtures and into
src/integration-test of the module that needs them, update their package/imports
accordingly, and then remove the
testFixturesImplementation(project(":fiat-on-ramp:fiat-on-ramp-api"))
declaration so the testFixtures module no longer depends on the API.
Comment on lines
+64
to
+81
| @Test | ||
| @DisplayName("should return 200 OK for idempotent replay with same paymentId") | ||
| void shouldReturn200ForIdempotentReplay() throws Exception { | ||
| // given | ||
| var request = aCollectionRequest(); | ||
|
|
||
| // first request — 201 Created | ||
| mockMvc.perform(post("/v1/collections") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isCreated()); | ||
|
|
||
| // when — second request with same paymentId | ||
| mockMvc.perform(post("/v1/collections") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.paymentId", is(PAYMENT_ID.toString()))); |
There was a problem hiding this comment.
Assert the same collectionId on replay.
This only proves the second request echoes the same paymentId. A broken handler could still create a second order and return 200 OK. Capture the first response and assert that the replay returns the exact same collectionId.
Suggested tightening
- mockMvc.perform(post("/v1/collections")
+ var firstResponse = mockMvc.perform(post("/v1/collections")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
- .andExpect(status().isCreated());
+ .andExpect(status().isCreated())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+ var firstCollectionId = objectMapper.readTree(firstResponse).get("collectionId").asText();
// when — second request with same paymentId
mockMvc.perform(post("/v1/collections")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isOk())
- .andExpect(jsonPath("$.paymentId", is(PAYMENT_ID.toString())));
+ .andExpect(jsonPath("$.paymentId", is(PAYMENT_ID.toString())))
+ .andExpect(jsonPath("$.collectionId", is(firstCollectionId)));🤖 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/integration-test/java/com/stablecoin/payments/onramp/application/controller/CollectionControllerIT.java`
around lines 64 - 81, The test shouldReturn200ForIdempotentReplay currently only
asserts the paymentId on the second (replay) response; capture and parse the
first response to extract its collectionId and then assert the second response
returns the exact same collectionId to prove idempotency. After performing the
first mockMvc.post to "/v1/collections" with the request, store the response
body and extract the collectionId (e.g., from the JSON), then perform the second
post and assert both jsonPath("$.paymentId", is(PAYMENT_ID.toString())) and
jsonPath("$.collectionId", is(firstCollectionId)) where firstCollectionId is the
value extracted from the first response. Ensure you reference the existing
variables/methods request, mockMvc, objectMapper, PAYMENT_ID and the endpoint
"/v1/collections".
Comment on lines
+37
to
+71
| @Test | ||
| @DisplayName("should return 200 OK for valid succeeded webhook") | ||
| void shouldReturn200ForSucceededWebhook() throws Exception { | ||
| // given — save an awaiting-confirmation order with matching pspReference | ||
| var order = anAwaitingConfirmationOrder(); | ||
| collectionOrderRepository.save(order); | ||
|
|
||
| var rawBody = aSucceededEventJson(PSP_REFERENCE, 100000L, "usd", order.collectionId()); | ||
|
|
||
| // when/then — fallback validator always accepts signature | ||
| mockMvc.perform(post("/internal/webhooks/psp/stripe") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .header("Stripe-Signature", "t=1234567890,v1=dummy") | ||
| .content(rawBody)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.status", is("received"))); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should return 200 OK for valid failed webhook") | ||
| void shouldReturn200ForFailedWebhook() throws Exception { | ||
| // given | ||
| var order = anAwaitingConfirmationOrder(); | ||
| collectionOrderRepository.save(order); | ||
|
|
||
| var rawBody = aFailedEventJson(PSP_REFERENCE, order.collectionId()); | ||
|
|
||
| // when/then | ||
| mockMvc.perform(post("/internal/webhooks/psp/stripe") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .header("Stripe-Signature", "t=1234567890,v1=dummy") | ||
| .content(rawBody)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.status", is("received"))); | ||
| } |
There was a problem hiding this comment.
Assert the persisted state transition, not just the 200 ACK.
Both success-path tests pass even if the controller accepts the webhook and never mutates the collection. Reload the saved order after the POST and assert the expected terminal status for succeeded vs failed events.
🤖 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/integration-test/java/com/stablecoin/payments/onramp/application/controller/StripeWebhookControllerIT.java`
around lines 37 - 71, Update the two tests in StripeWebhookControllerIT
(shouldReturn200ForSucceededWebhook and shouldReturn200ForFailedWebhook) to
assert the persisted state transition: after performing the POST (using
anAwaitingConfirmationOrder(), PSP_REFERENCE and
collectionOrderRepository.save(...)), reload the saved order from
collectionOrderRepository (e.g., via findById or findByCollectionId using
order.collectionId()) and assert its terminal status changed to the expected
value for each case (succeeded → expected terminal success status, failed →
expected terminal failure status) in addition to the existing 200/"received"
assertions.
Comment on lines
+32
to
+33
| psp: | ||
| provider: dev |
There was a problem hiding this comment.
This profile bypasses real Stripe signature verification.
With app.psp.provider=dev, the Stripe webhook ITs only prove header presence, not Stripe HMAC validation. That leaves the authenticated webhook path untested; use a Stripe-specific test profile for StripeWebhookControllerIT or override this property in that suite and sign payloads via WebhookFixtures.computeSignature(...).
🤖 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/integration-test/resources/application-integration-test.yml`
around lines 32 - 33, The integration test profile currently sets psp provider
to dev which disables real Stripe HMAC verification; for
StripeWebhookControllerIT either switch the test to a Stripe-specific profile
instead of using app.psp.provider=dev or override that property in the test
setup to use the Stripe provider and ensure requests are signed by computing and
adding valid signatures using WebhookFixtures.computeSignature(...); update
StripeWebhookControllerIT to toggle the profile or set the property and sign
payloads so the webhook path performs real HMAC validation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/testFixtures/java/com/stablecoin/payments/onramp/fixtures/CollectionOrderFixtures.java`:
- Around line 110-126: The aCollectionRequest() method contains duplicated
literal values (e.g., "1000.00", "SEPA", "IBAN", "DE", "EUR", "Stripe", sha256
string) that are already used elsewhere; refactor by replacing these magic
literals with shared constants or reusing existing fixture/value-object
factories so all tests draw from a single source of truth—update
CollectionOrderFixtures.aCollectionRequest() to reference the existing fixture
constants (or a shared Factory) for PAYMENT_ID, CORRELATION_ID, AMOUNT,
CURRENCY, PAYMENT_TYPE, COUNTRY, RECEIVING_CURRENCY, PROVIDER_ID, PROVIDER_NAME,
HASH, BIC, ACCOUNT_TYPE, and ACCOUNT_COUNTRY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1dca934-0a02-481d-9c27-ea42ecf56260
📒 Files selected for processing (1)
fiat-on-ramp/fiat-on-ramp/src/testFixtures/java/com/stablecoin/payments/onramp/fixtures/CollectionOrderFixtures.java
Comment on lines
+110
to
+126
| public static CollectionRequest aCollectionRequest() { | ||
| return new CollectionRequest( | ||
| PAYMENT_ID, | ||
| CORRELATION_ID, | ||
| new BigDecimal("1000.00"), | ||
| "USD", | ||
| "SEPA", | ||
| "DE", | ||
| "EUR", | ||
| "stripe_001", | ||
| "Stripe", | ||
| "sha256_abc123def456", | ||
| "DEUTDEFF", | ||
| "IBAN", | ||
| "DE" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting magic values to constants for consistency.
The method duplicates literal values ("1000.00", "SEPA", "IBAN", etc.) that are already embedded in other fixture methods. If these ever drift, tests may pass with inconsistent data.
Optional: leverage existing value-object factories for derivation, or extract shared constants.
♻️ Proposed refactor (optional)
+ // Add constants section
+ public static final BigDecimal DEFAULT_AMOUNT = new BigDecimal("1000.00");
+ public static final String DEFAULT_CURRENCY = "USD";
+ public static final String DEFAULT_RAIL_TYPE = PaymentRailType.SEPA.name();
+ public static final String DEFAULT_ACCOUNT_TYPE = AccountType.IBAN.name();
public static CollectionRequest aCollectionRequest() {
return new CollectionRequest(
PAYMENT_ID,
CORRELATION_ID,
- new BigDecimal("1000.00"),
- "USD",
- "SEPA",
+ DEFAULT_AMOUNT,
+ DEFAULT_CURRENCY,
+ DEFAULT_RAIL_TYPE,
"DE",
"EUR",
"stripe_001",
"Stripe",
"sha256_abc123def456",
"DEUTDEFF",
- "IBAN",
+ DEFAULT_ACCOUNT_TYPE,
"DE"
);
}🤖 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/testFixtures/java/com/stablecoin/payments/onramp/fixtures/CollectionOrderFixtures.java`
around lines 110 - 126, The aCollectionRequest() method contains duplicated
literal values (e.g., "1000.00", "SEPA", "IBAN", "DE", "EUR", "Stripe", sha256
string) that are already used elsewhere; refactor by replacing these magic
literals with shared constants or reusing existing fixture/value-object
factories so all tests draw from a single source of truth—update
CollectionOrderFixtures.aCollectionRequest() to reference the existing fixture
constants (or a shared Factory) for PAYMENT_ID, CORRELATION_ID, AMOUNT,
CURRENCY, PAYMENT_TYPE, COUNTRY, RECEIVING_CURRENCY, PROVIDER_ID, PROVIDER_NAME,
HASH, BIC, ACCOUNT_TYPE, and ACCOUNT_COUNTRY.
…rollback Pass collectionId as Stripe Idempotency-Key header so that if the transaction rolls back after a successful PSP call, retries are safe. Addresses CodeRabbit critical review on CollectionCommandHandler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterTest.java (1)
170-177: 🧹 Nitpick | 🔵 TrivialGood verification of Idempotency-Key header.
Consider adding a test case for when
idempotencyKeyis null to verify the header is not sent. This would cover the conditional branch inStripePspAdapter.initiatePayment().Suggested additional test case
`@Test` `@DisplayName`("should not send Idempotency-Key header when idempotencyKey is null") void initiatePayment_noIdempotencyKeyHeader_whenNull() { wireMock.stubFor(post(urlEqualTo("/v1/payment_intents")) .willReturn(aResponse() .withStatus(200) .withHeader("Content-Type", "application/json") .withBody(""" { "id": "pi_no_idem", "status": "succeeded", "amount": 25000, "currency": "usd", "client_secret": "pi_no_idem_secret" } """))); var requestWithoutIdempotencyKey = new PspPaymentRequest( COLLECTION_ID, new Money(new BigDecimal("250.00"), "USD"), new PaymentRail(PaymentRailType.ACH, "US", "USD"), new BankAccount("hash123", "021000021", AccountType.ACH_ROUTING, "US"), "stripe", null // no idempotency key ); adapter.initiatePayment(requestWithoutIdempotencyKey); wireMock.verify(postRequestedFor(urlEqualTo("/v1/payment_intents")) .withoutHeader("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/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterTest.java` around lines 170 - 177, Add a new unit test in StripePspAdapterTest that calls StripePspAdapter.initiatePayment with a PspPaymentRequest whose idempotencyKey is null and stubs a successful /v1/payment_intents response; after invoking adapter.initiatePayment, verify via wireMock.verify that the request to "/v1/payment_intents" does not include the "Idempotency-Key" header (use wireMock's withoutHeader matcher). This covers the conditional branch in StripePspAdapter.initiatePayment that should omit the header when idempotencyKey is null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterTest.java`:
- Around line 170-177: Add a new unit test in StripePspAdapterTest that calls
StripePspAdapter.initiatePayment with a PspPaymentRequest whose idempotencyKey
is null and stubs a successful /v1/payment_intents response; after invoking
adapter.initiatePayment, verify via wireMock.verify that the request to
"/v1/payment_intents" does not include the "Idempotency-Key" header (use
wireMock's withoutHeader matcher). This covers the conditional branch in
StripePspAdapter.initiatePayment that should omit the header when idempotencyKey
is null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c92b377-326f-4127-9d2b-401c965d1636
📒 Files selected for processing (5)
fiat-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/service/CollectionCommandHandler.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.javafiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/domain/service/CollectionCommandHandlerTest.javafiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterTest.java
3 tasks
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
CollectionCommandHandler— save collection order before PspTransactionrawResponsehandling inPspTransactionPersistenceMapperaCollectionRequest()andaRefundRequest()test fixturesTest Coverage
S3 totals: 245 unit + 40 IT = 285 tests
Bug Fix
CollectionCommandHandler.initiateCollection()savedPspTransactionbeforeCollectionOrder, violating the FK constraint (psp_transactions.collection_id → collection_orders.id). Reordered to save the order first.Test plan
Closes STA-128
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores