Problem/Motivation

#2968972: Cannot PATCH an entity with dangling references in an ER field only limited validation of fields in JSON API PATCH requests to fields received.

JSON API should do what core REST introduced in #2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors: limit validation of fields in PATCH requests to fields changed.

Proposed resolution

  1. Wait for #2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors to land so that JSON API can reuse core's rest_test test helper module.
  2. Port all test coverage.
  3. Port all changes from core REST's EntityResource & friends.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Port #2821077 to JSON API: limit validation of fields in PATCH requests to fields *changed* » [PP-1] Port #2821077 to JSON API: limit validation of fields in PATCH requests to fields *changed*
gabesullice’s picture

gabesullice
Primary language English
commented

TBH, I'd like to wait for an actual bug report about this. I'm not sure this is an actual feature people need.

If you're sending a field, you should expect it to be validated. If you don't intend to change it and you don't want it validated, don't send it.

This could end up being buggy behavior because a valid value for one user could be an invalid value for another, without validated unchanged, sent fields, a server can't ever communicate that to the client.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Port #2821077 to JSON API: limit validation of fields in PATCH requests to fields *changed* » Port #2821077 to JSON API: limit validation of fields in PATCH requests to fields *changed*
Issue summary: View changes
Status: Postponed » Active

#2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors landed at last.

I'm not sure this is an actual feature people need.

I strongly disagree with this. I don't think this is an artificial use case.

It's perfectly reasonable for people to GET a resource, modify some data, then resubmit it using PATCH. Any data that is unmodified should not be validated again, per the robustness principle.

The irony is that we actually already implemented this by now because people reported this to be an actual problem! See #2968972: Cannot PATCH an entity with dangling references in an ER field.

AFAICT this is now merely a matter of porting #2979945: Port #2821077 to JSON API: limit validation of fields in PATCH requests to fields *changed*'s test coverage.

gabesullice’s picture

gabesullice
Primary language English
commented

The irony is that we actually already implemented this by now because people reported this to be an actual problem! See #2968972: Cannot PATCH an entity with dangling references in an ER field.

I don't believe that we did. As the IS says, what we did is only validate the entity fields for those fields included in the request document. We do not currently check if the field has been modified.

I think I still feel like I did in #3. I think that because JSON API allows one to omit fields which one does not wish to modify (and this behavior is defined in the spec) that it's okay if our handling of this is different.

Out of curiosity, I decided to test the behavior of a non-decoupled form.

  1. I created a test node type with an integer field.
  2. I created a node of that type with the integer value set to -10.
  3. I updated the field requirements to accept a minimum value of 0.
  4. I attempted to edit and re-save the node without changing anything.
  5. HTML5 input validation prevented me from submitting the form.
  6. I edited the HTML source to remove the input attribute.
  7. I submitted the form.
  8. The submission was rejected and a validation error appeared on the form.

With the proposed change of behavior. It becomes impossible to implement that same behavior.

Our current solution allows the frontend developer to choose which UX they prefer based on whether they decide to send an unmodified field or not.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Closed (works as designed)

As the IS says, what we did is only validate the entity fields for those fields included in the request document. We do not currently check if the field has been modified.

You're right. #2968972: Cannot PATCH an entity with dangling references in an ER field modified JSON API to only validate received fields, not only modified fields.

I think that because JSON API allows one to omit fields which one does not wish to modify

rest.module does this too.

Our current solution allows the frontend developer to choose which UX they prefer based on whether they decide to send an unmodified field or not.

Fair.

Considering this working as designed!