Conversation
adamdickmeiss
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the patron-request “execute action” response to return a richer structured result (status + outcome + from/to state), wiring that data through the action service/event result and updating the OpenAPI contract and tests accordingly.
Changes:
- Expand action task
ResultDatato carryoutcomeandtoState, and populate them during action finalization. - Update
POST /patron-requests/{id}/actionto return the newActionResultshape (includingfromState/toState). - Adjust OpenAPI schema and update tests to unmarshal and assert the structured response.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| broker/test/patron_request/api/api-handler_test.go | Updates API tests to validate the expanded ActionResult JSON response. |
| broker/patron_request/service/action_test.go | Updates service tests to expect non-nil action result data. |
| broker/patron_request/service/action.go | Adds outcome/toState propagation and refactors outcome handling into EventResult. |
| broker/patron_request/api/api-handler.go | Returns expanded ActionResult fields (result/message/outcome/fromState/toState). |
| broker/oapi/open-api.yaml | Updates the ActionResult schema (renames field, adds required fields). |
| broker/events/eventmodels.go | Adds Outcome and ToState to shared event data/result payloads. |
| ActionResult: | ||
| type: object | ||
| properties: | ||
| actionResult: | ||
| result: | ||
| type: string | ||
| description: Action result | ||
| message: | ||
| type: string | ||
| description: Action message | ||
| outcome: | ||
| type: string | ||
| description: Action outcome ("success", "failure") | ||
| fromState: | ||
| type: string | ||
| description: State before action execution | ||
| toState: | ||
| type: string | ||
| description: State after action execution | ||
| required: | ||
| - actionResult | ||
| - result | ||
| - outcome | ||
| - fromState |
There was a problem hiding this comment.
Renaming the response field from actionResult to result is a breaking API change for any existing clients. If backward compatibility is required, consider supporting both fields during a deprecation window (or bumping an API version) rather than changing the property name in-place.
There was a problem hiding this comment.
@adamdickmeiss how is bruno test not failing here?
https://index-data.atlassian.net/browse/CROSSLINK-232