migrate(DBCRFUN): translate payment debit/credit flow#24
Conversation
|
augment review |
|
✅ Test coverage looks good. The new/changed behavior in this PR has adequate test coverage. No additional tests needed. |
| this.dbcrfunService = Objects.requireNonNull(dbcrfunService, "dbcrfunService must not be null"); | ||
| } | ||
|
|
||
| @PutMapping("/dbcr") |
There was a problem hiding this comment.
docs/translation-rules.md specifies DBCRFUN should be exposed as a POST under /api/v1/dbcrfun, but this controller is currently a PUT under /api/v1/makepayment. If clients/tests are already aligning to the rulebook, this mismatch could cause route/method breakage.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
| } | ||
|
|
||
| private DbcrfunOrigin toOrigin(DbcrfunOriginDto originDto) { | ||
| if (originDto == null) { |
There was a problem hiding this comment.
Defaulting a missing CommOrigin to DbcrfunOrigin.blank() (facilityType=0) means DbcrfunService will treat the request as non-payment traffic. If this endpoint should always enforce payment rules, a client could bypass the insufficient-funds / account-type restrictions by omitting or changing CommFaciltype.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
|
|
||
| @RestController | ||
| @Validated | ||
| @RequestMapping("/api/v1/makepayment") |
There was a problem hiding this comment.
docs/translation-rules.md currently says DBCRFUN maps to POST under /api/v1/<program-lowercase>, but this controller introduces PUT /api/v1/makepayment/dbcr; consider updating the rules doc so future migrations don’t follow the wrong convention.
Severity: low
🤖 Was this useful? React with 👍 or 👎
… and CommFaciltype
|
augment review |
| assertThat(auditRow.value1()).isEqualTo("PDR"); | ||
| assertThat(auditRow.value2()).isEqualTo("ABCDEFGH123456"); | ||
| assertThat(auditRow.value3()).isEqualTo(new BigDecimal("-25.00")); | ||
| assertThat(auditRow.value4()).isEqualTo(LocalDate.now(clock)); |
There was a problem hiding this comment.
This assertion reads LocalDate.now(clock) after the request, but the service persists TRAN_DATE based on a captured Instant; this can become flaky if the test runs across midnight (UTC).
Severity: low
🤖 Was this useful? React with 👍 or 👎
|
|
||
| @RestController | ||
| @Validated | ||
| @RequestMapping("/api/v1/dbcrfun") |
There was a problem hiding this comment.
The PR description lists the mapped endpoint as PUT /api/v1/makepayment/dbcr, but the implementation here is POST /api/v1/dbcrfun; this mismatch could confuse clients/reviewers relying on the PR notes.
Severity: low
🤖 Was this useful? React with 👍 or 👎
…fore-or-after request date
|
augment review |
Source program
Mapped REST endpoints
Notable decisions
Test coverage