[Django] #27654: Propogate alters_data value to subclasses
Django
------------------------------------------------+------------------------
Reporter: vinay karanam | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27654>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: vinay karanam | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* component: Uncategorized => Core (Other)
* stage: Unreviewed => Accepted
Comment:
[https://github.com/django/django/pull/7759 WIP PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:1>
Django
Reporter: vinay karanam | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Asif Saifuddin Auvi):
* needs_better_patch: 1 => 0
* version: 1.10 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:2>
Django
Reporter: vinay karanam | Owner: nobody
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
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.)
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:3>
Django
Reporter: vinay karanam | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Shai Berger):
* status: closed => new
* resolution: wontfix =>
Comment:
Reopening the ticket following a discussion on the security mailing list,
which led to https://github.com/django/django/pull/16149
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:4>
Django
Reporter: vinay karanam | Owner: nobody
Cleanup/optimization |
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Shai Berger):
* stage: Accepted => Ready for checkin
Comment:
(in the last review the PR still had a couple of minor issues, but I
believe it is ready for a merger review)
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:5>
Django
Reporter: vinay karanam | Owner: nobody
Cleanup/optimization |
Component: Core (Other) | Version: dev
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e20c9eb60ab9d1c84b19672def918097c943edd8" e20c9eb]:
{{{
#!CommitTicketReference repository=""
revision="e20c9eb60ab9d1c84b19672def918097c943edd8"
Fixed #27654 -- Propagated alters_data attribute to callables overridden
in subclasses.
Thanks Shai Berger and Adam Johnson for reviews and the implementation
idea.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:6>
Django
Reporter: vinay karanam | Owner: nobody
Cleanup/optimization |
Component: Core (Other) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"e20c9eb60ab9d1c84b19672def918097c943edd8" e20c9eb]:
{{{
#!CommitTicketReference repository=""
revision="e20c9eb60ab9d1c84b19672def918097c943edd8"
Fixed #27654 -- Propagated alters_data attribute to callables overridden
in subclasses.
Thanks Shai Berger and Adam Johnson for reviews and the implementation
idea.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27654#comment:7>