Skip to content

fix: honor accessor status and add 204 path in ProductController | MC-16536 - #400

Merged
SandhyaBhatia merged 1 commit into
masterfrom
sb/product-controller-rev6-status-fix
Oct 2, 2026
Merged

SandhyaBhatia merged 1 commit into
masterfrom
sb/product-controller-rev6-status-fix

Conversation

@SandhyaBhatia

@SandhyaBhatia SandhyaBhatia commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary of Changes

ProductController.buildResponse() derived the HTTP status purely from whether
result.getChallenges() was non-empty, completely ignoring any explicit
AccessorResponse.getStatus() the accessor had set. This made an accessor's
.withStatus(PathResponseStatus.OK) dead code whenever the result still carried a
trailing challenge — which is exactly what path-accessor-central-pacific's CPB
account-opening terminal response does intentionally (it keeps the
account_opening_success challenge alongside status: COMPLETE so moneymobilex can
render the success screen from it). The practical effect: that terminal call likely
returns 202 where 200 is spec-correct, and no one has noticed because
moneymobilex doesn't check the HTTP status for that step — it keys off
challenges[0].id == "account_opening_success" as a string match instead.

There was also no path to 204 No Content at all. Per the Gutenberg spec
(mdx/product/_spec.md.erb, Update a product), the PUT endpoint must return 202 while
challenged, 200 while the product remains available, and 204 if it's no longer
available. buildResponse() could only ever produce 200, 202, or 404.

PayeesController.addPayee() already has the correct pattern elsewhere in this same
codebase:

HttpStatus status = accessorResponse.getStatus() != null
    ? HttpStatus.valueOf(accessorResponse.getStatus().value())
    : HttpStatus.OK;

ProductController.buildResponse() now follows that same pattern, plus the new
NO_CONTENT path:

PathResponseStatus accessorStatus = response.getStatus();

if (accessorStatus == PathResponseStatus.NO_CONTENT) {
  return new ResponseEntity<>(createMultiMapForResponse(response.getHeaders()), HttpStatus.NO_CONTENT);
}

Product result = response.getResult();
if (result == null) {
  return new ResponseEntity<>(createMultiMapForResponse(response.getHeaders()), HttpStatus.NOT_FOUND);
}

HttpStatus status;
if (accessorStatus != null) {
  status = HttpStatus.valueOf(accessorStatus.value());
} else if (result.getChallenges() != null && result.getChallenges().size() > 0) {
  status = HttpStatus.ACCEPTED;
} else {
  status = HttpStatus.OK;
}

The NO_CONTENT check runs before the null-result check so "product existed and is
now gone" (204) stays distinguishable from "product id never existed" (404) — those
are different accessor outcomes and shouldn't collapse to the same response.

This is the companion fix to MC-16523
(path-accessor-central-pacific Rev 6 change) — that ticket's terminal response relies
on this fix to actually return 200.

Fixes # (issue)
https://mxcom.atlassian.net/browse/MC-16536

Public API Additions/Changes

ProductController (mdx-web/src/main/java/com/mx/path/model/mdx/web/controller/ProductController.java):

  • buildResponse() (private) now reads AccessorResponse.getStatus() first and uses
    it directly when set, instead of always recomputing the status from
    result.getChallenges().
  • Added a PathResponseStatus.NO_CONTENT → HTTP 204 No Content branch, checked
    before the existing null-result → 404 branch.
  • No change to the controller's public method signatures (getProduct,
    updateProduct) — this is internal to buildResponse().

Downstream Consumer Impact

Non-breaking for any accessor that doesn't set AccessorResponse.getStatus() —
buildResponse() falls back to the exact same challenge-derived 200/202 logic as
before for those, unchanged.

For accessors that do set an explicit status (currently just
path-accessor-central-pacific's ProductAccessor, once MC-16523 ships), the HTTP
status returned to the client will change to match what the accessor intended:

  • CPB's terminal account-opening response will start returning 200 OK instead of
    202 Accepted. Needs a QA capture against the live/preview CPB connector to
    confirm this is the only consumer affected and that moneymobilex's behavior in
    production is unaffected
    (per the ticket, moneymobilex doesn't branch on this
    status code today, only on the challenge id, so this should be safe — but verifying
    against a real capture before this reaches production is called out explicitly in
    both MC-16523 and MC-16536).
  • Any accessor returning PathResponseStatus.NO_CONTENT will now correctly produce
    204 instead of falling through to whatever buildResponse() did with a null
    result before (previously 404, which was itself not spec-correct for this case).

No migration steps required for existing consumers that don't set an explicit status.

How Has This Been Tested?

Extended ProductControllerTest.groovy (Spock) with 7 new cases alongside the 7
existing ones, covering every branch combination:

  • Accessor sets OK with non-empty challenges → controller returns 200 (not 202) — the Rev 6 terminal-response case
  • No accessor status, non-empty challenges → 202 (unchanged default, regression guard)
  • No accessor status, empty challenges → 200 (unchanged default, regression guard)
  • Accessor sets NO_CONTENT → controller returns 204 with a null body
  • Null result (product id not found) still returns 404, even though NO_CONTENT handling now exists — confirms the two don't collapse
  • Same OK-wins-over-challenges case repeated for getProduct (GET), not just updateProduct (PUT)
  • Same NO_CONTENT case repeated for getProduct

./gradlew :mdx-web:build is fully green: 388 tests pass (14 in
ProductControllerTest), checkstyle/spotless/spotbugs all clean.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

…-16536

ProductController.buildResponse() derived the HTTP status purely from
result.getChallenges() presence, ignoring any explicit
AccessorResponse.getStatus() the accessor set. This made
ProductAccessor's .withStatus(OK) dead code whenever a terminal
response still carries a trailing challenge (e.g. CPB's
account_opening_success), so it silently returned 202 instead of 200.

There was also no path to 204 at all, which Rev 6 requires when a
product is no longer available.

Mirrors PayeesController.addPayee()'s existing, correct pattern:
accessor status wins when set; challenge-derived 200/202 remains the
default when it isn't. NO_CONTENT is checked before the null-result
check so 'no longer available' (204) and 'never existed' (404) stay
distinguishable.
@SandhyaBhatia
SandhyaBhatia merged commit c274703 into master Oct 2, 2026
7 checks passed
@SandhyaBhatia
SandhyaBhatia deleted the sb/product-controller-rev6-status-fix branch October 2, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants