Use `update_fields` on PATCH requests.

[lovelydinosaur]

See #2622.

[lovelydinosaur]

Be interesting to see what code exactly would need to change to support this.
That'd help us make the assessment of if it's a worthwhile change.

[w0rp]

I just found this thread through a random Google search, wondering if DRF supports such a thing. I'll contribute some thoughts I had on this.

update_fields based on the fields specified via a PATCH request will work, except for cases where .save is overridden, or a pre_save signal is used, etc., which could cause some field not among the list of fields specified for PATCH to not be saved to a database, when they should. I think it would be possible to add update_fields support like this, but also add support for specifying something like a list of fields which should always be updated for PATCH, or maybe a method for returning the list of fields to use for update_fields.

So in my imagined codebase, it would use update_fields by default, but mention in the documentation "override get_update_fields(...) if you want to change which fields will be saved."

Just some thoughts I had on this.

[lovelydinosaur]

Thanks @w0rp.

except for cases where .save is overridden, or a pre_save signal is used, etc.

Right. Those are good enough reasons for use to not do this in core. If anyone wants this then it ought to be as an optional third party package, that eg provides custom serializer classes.

[askholme]

As i wrote on #2622 i believe that decision is right as i did not consider the full complexity of messing with the save functionality when i made the original PR. Using a case specific serializer is clearly the best approach here.

I would however suggest that this is documented more explicit. For certain scenarios the likelihood of hitting a race condition here (if a custom serializer is not used) is quite high.

Writing something on the serialializers api-guide or alike would be recommendable.

[lovelydinosaur]

Welcome to consider submissions. Otherwise probably not high enough on our todo list right now to treat as an active issue.

[patrys]

@tomchristie I did not yet have a chance to try this myself but would it be possible for DRF to use transaction.atomic() and .select_for_update() to serialize PATCH requests touching the same record? I'm not sure how viable that is given that most of the implementation is method-agnostic.

[lovelydinosaur]

@patrys - Possibly, yes. In discussion at #4675.

[lovelydinosaur]

select_for_update seems like a better tack here than update_fields. If nothing else it's a nice simple approach. I think we'd want to rely on the user deciding if they want to set ATOMIC_REQUESTS themselves, rather than introducing an explicit transaction ourself. Possibly make sure to document that both in the docs, and as a comment in the get_queryset method on the generic view.