Opened 18 months ago

Closed 3 months ago

#27654 closed Cleanup/optimization (wontfix)

Propogate alters_data value to subclasses

Reported by: vinay karanam Owned by: nobody
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Current implementation to prevent data changes within templates relies on the attribute alters_data = True.

By default this is set for all Model, QuerySet and ModelForm methods that can alter data.

But if someone overrides these methods, they need to explicitly set this attribute again.
It is common to override save method and not set this attribute.

Even a lot of popular third-party packages end up making this mistake.

These classes need to make sure that the attribute value is set for their subclasses methods if not explicitly overwritten.

Change History (3)

comment:1 Changed 18 months ago by Tim Graham

Component: UncategorizedCore (Other)
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

WIP PR

This ticket is a result of discussion in #27638 which suggested to solve the problem by issuing a rollback after rendering templates (closed as wontfix).

Last edited 18 months ago by Tim Graham (previous) (diff)

comment:2 Changed 8 months ago by Asif Saifuddin Auvi

Patch needs improvement: unset
Version: 1.10master

comment:3 Changed 3 months ago by Carlton Gibson

Resolution: wontfix
Status: newclosed

After looking at the implementation on the PR, the proposed metaclass solution is too much to given the relatively small improvement over the current alters_data solution.

The remaining suggestion from #27638 was to use the system checks framework to warn users of a problem. That might be worth a go. It would be able to capture the most common cases on Model classes certainly. (Whether we want to add that isn't clear: in lots of cases save is overridden without setting alters_data — to make that an error might be more annoying than a benefit, but it could at least be silenced.)

Note: See TracTickets for help on using tickets.
Back to Top