Skip to content

Conversation

mfn
Copy link
Collaborator

@mfn mfn commented Apr 21, 2021

Summary

I don't think the intended output was ever to have the MessageBag
being an intermediary for serializing the response, when directly
returning the messages yields the same result.

I noticed this already when I added support for Laravels ValidationException directly in #748 where I already used getMessages().

The gracious changes in the tests result from not having the MessageBag
available anymore and making the shortcut just comparing the whole
response.

IMHO it also gives a clearer indication how such a thing looks like
(the shape) and find the individual probes into the result harder to
grasp in the big picture.

This lead to funny things to properly assess the error payload like in adea531#diff-3843ca02a9a09b621b2ae517b99bf9f84452d016556d6d433be47d628754e392R32-R33


Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@mfn mfn self-assigned this Apr 21, 2021
I don't think the intended output was ever to have the MessageBag
being an intermediary for serializing the response, when directly
returning the messages yields the same result.

The gracious changes in the tests result from not having the MessageBag
available anymore and making the shortcut just comparing the whole
response.

IMHO it also gives a clearer indication how such a thing looks like
(the shape) and find the individual probes into the result harder to
grasp in the big picture.
@mfn mfn force-pushed the mfn-validationerror-getmessages branch from 99edad3 to 8f189d5 Compare April 21, 2021 20:22
@mfn mfn merged commit c9df956 into master Apr 21, 2021
@mfn mfn deleted the mfn-validationerror-getmessages branch April 21, 2021 20:24
@mfn mfn added this to the 8.0.0 milestone May 11, 2021
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.

1 participant