fix(crdtagy): emit CommSuccess=Y / CommFailCode=0 on the success path#41
Conversation
Closes #31. The CRDTAGY endpoint previously echoed CommSuccess and CommFailCode from the incoming commarea, so callers could receive a populated CommCreditScore alongside whatever placeholder they sent. Mirror the convention used by DbcrfunController and DelaccController: on the success path emit terminal Y / 0 so the HTTP response indicators line up with the populated score. CRDTAGY1 itself never touches these flags in COBOL — the parent CRECUST orchestrator does — but at the Java endpoint level they are the natural per-agency success/fail flag. Tests: tighten the parameterized success test to assert Y/0 across all five agencies and add overridesPlaceholderSuccessFlagsFromRequest.
|
✅ Test coverage looks good. The new/changed behavior in this PR has adequate test coverage. No additional tests needed. The behavioral change (setting
|
![augment-app-staging[bot]](https://avatars.githubusercontent.com/in/1008152?s=60&v=4){kind=small}
| commarea.commDateOfBirth() | ||
| ), agencyNumber); | ||
|
|
||
| // Mirror the convention used by the other Java endpoint controllers |
There was a problem hiding this comment.
The comment cites DELACC as an example, but DelaccController doesn’t use CommSuccess/CommFailCode (it uses DelAccSuccess/DelAccFailCd, etc.), so this may be misleading context for future maintainers.
Severity: low
🤖 Was this useful? React with 👍 or 👎
| defaultInt(commarea.commCsReviewDate()), | ||
| defaultString(commarea.commSuccess()), | ||
| defaultString(commarea.commFailCode()) | ||
| "Y", |
There was a problem hiding this comment.
awaitCreditScore() falls back to 0 when the future completes with Optional.empty(), but this success path now always emits CommSuccess="Y" / CommFailCode="0"; can you confirm Optional.empty() is impossible here (or represents success), so we never return a 0 score with a success indicator?
Severity: medium
🤖 Was this useful? React with 👍 or 👎
1. Comment cited DELACC inaccurately (DelaccController uses DelAccSuccess / DelAccFailCd field names, not CommSuccess / CommFailCode). Replace with a DBCRFUN-only reference, which is the actual precedent. 2. awaitCreditScore previously fell back to 0 on Optional.empty(). Now that the success path always emits CommSuccess=Y / CommFailCode=0, that fallback would mean we could return a 0 score with terminal success indicators. CreditAgencyService always returns Optional.of(score), so refuse Optional.empty() with a PLOP abend instead. Add emptyOptionalScoreSurfacesAsPlopAbend regression test.
|
Round-1 review addressed:
Local ./mvnw verify green: 198/198. Pushed as 13ba7e8. augment review |
1 participant
Closes #31.
What
On the success path,
CrdtagyControllerpreviously echoedCommSuccessandCommFailCodestraight from the request, so callers could receive a populatedCommCreditScorealongside whatever placeholder they originally sent (often blank or"N"/"X").Now the controller mirrors the convention used by the other Java endpoint controllers (e.g.
DbcrfunController,DelaccController): on the success path it setsCommSuccess = "Y"andCommFailCode = "0"unconditionally, so the HTTP response carries terminal indicators that line up with the populated score. CRDTAGY1 itself never touches these flags in COBOL — the parent CRECUST orchestrator does — but at the Java endpoint level they are the natural success/fail flag for the per-agency call.Failure paths are unchanged: timeouts and async failures still surface as
CbsaAbendException(PLOP), validation failures as 400 ProblemDetail.Tests
CrdtagyControllerWebMvcTest(9 tests, all green):returnsSuccessfulResponseForEveryAgencyRoute(5 cases) is tightened to also assertCommSuccess == "Y"andCommFailCode == "0".overridesPlaceholderSuccessFlagsFromRequest: caller sendsCommSuccess: "N",CommFailCode: "X"; response still carriesY/0.Local
./mvnw verifygreen: 197/197.augment review