[Django] #21792: ModelForm.has_changed is not documented

19 views
Skip to first unread message

Django

unread,
Jan 16, 2014, 3:10:16 PM1/16/14
to django-...@googlegroups.com
#21792: ModelForm.has_changed is not documented
-------------------------------+---------------------------------------
Reporter: bjb@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6
Severity: Normal | Keywords: has_changed api ModelForm
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
I cannot find any reference to "has_changed" in the docs
(https://docs.djangoproject.com/en/dev/topics/forms/formsets/) for 1.4,
1.5, 1.6 and dev (tried on 2014/01/16) for Form.has_changed.

The only reference found by the index is to formset.has_changed, and that
is in an example.

At a minimum, it should be mentioned on the api page:
https://docs.djangoproject.com/en/dev/ref/forms/api/

--
Ticket URL: <https://code.djangoproject.com/ticket/21792>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 17, 2014, 4:15:40 AM1/17/14
to django-...@googlegroups.com
#21792: ModelForm.has_changed is not documented
-------------------------------------+-------------------------------------

Reporter: bjb@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6
Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by mjtamlyn):

* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted


Comment:

Agreed it probably should be. It can be a little unreliable in some ways
though, which is why it probably is not documented. In particular,
`form.changed_fields` (which `form.has_changed` uses) can report false
positives, for example with `ModelChoiceField` when there is a difference
between an integer initial value and a string POST value. At present the
resolution of whether a field has changed is the responsibility of the
widget which is not ideal, it should be the field. So I'll accept this as
I feel it would be a good feature, but it would be better to make it much
more solid before documenting it.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:1>

Django

unread,
Jan 17, 2014, 10:31:16 AM1/17/14
to django-...@googlegroups.com
#21792: ModelForm.has_changed is not documented
-------------------------------------+-------------------------------------

Reporter: bjb@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6
Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by charettes):

Thanks to @claudep the `has_changed` logic was moved to the field level in
[ebb504db69] based on a suggestion of @aaugustin in #16612.

There haven been couple of adjustments ([892bc91cb], [cbfb8ed53b],
[9883551d50], [02b0106d43]) since then and the API is definitely reliable
now.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:2>

Django

unread,
Jul 10, 2014, 7:54:55 AM7/10/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------

Reporter: bjb@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6
Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by anubhav9042):

* cc: anubhav9042@… (added)


Comment:

I think it would be better to add:
In `/ref/forms/api.txt`:
`Check if form data is changed` and document `form.has_changed()` and
mention `form.changed_data` as well with an example.

In `/ref/forms/fields.txt`:
document `Field._has_changed()`

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:3>

Django

unread,
Jul 26, 2014, 4:02:03 AM7/26/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by navi7):

* status: new => assigned
* owner: nobody => navi7


--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:4>

Django

unread,
Jul 26, 2014, 6:21:52 AM7/26/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by navi7):

* has_patch: 0 => 1


Comment:

https://github.com/django/django/pull/2966

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:5>

Django

unread,
Jul 26, 2014, 1:20:13 PM7/26/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by timo):

I'm not sure we should document `Field._has_changed()`. Methods that start
with an underscore are generally considered private and not documented.
Was there any discussion about it besides comment 3 in this ticket?

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:6>

Django

unread,
Jul 27, 2014, 6:06:12 PM7/27/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by slurms):

* needs_better_patch: 0 => 1


Comment:

Should `Field._has_changed()` be public API? It seems weird that
`Form.has_changed()` is, but `Field._has_changed()` isn't. If it is public
API it would be useful to add some documentation about it to the custom
form fields section.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:7>

Django

unread,
Jul 27, 2014, 8:34:44 PM7/27/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by timo):

I don't see a reason it shouldn't be. It got a mention in the 1.6 release
notes: "If you defined your own form widgets and defined the
`_has_changed()` method on a widget, you should now define this method on
the form field itself." and seems useful for custom fields (e.g.
`ReadOnlyPasswordHashField` in `contrib.auth` uses it. I'm not sure it's
worth the code church to rename it to `has_changed()` (removing the
underscore) before making it official. #16612 was the ticket where it
moved from widgets to fields and would have been a good time to make the
change if we were going to do it.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:8>

Django

unread,
Jul 29, 2014, 7:44:17 AM7/29/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by anonymous):

OK, so what would be the resolution?

I can do the renaming stuff if that's what needs to be done.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:9>

Django

unread,
Aug 3, 2014, 7:27:04 AM8/3/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by claudep):

I also think now I should have removed the underscore when I moved the
method from the widget to the field. Sorry. It might be the right thing to
do now.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:10>

Django

unread,
Aug 3, 2014, 8:44:30 PM8/3/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Accepted
ModelForm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):

* needs_better_patch: 1 => 0


Comment:

Created #23162 for renaming `Field._has_changed()`. I'll commit the
`Form.has_changed()` docs from the PR for now.

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:11>

Django

unread,
Aug 4, 2014, 7:02:22 AM8/4/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: assigned
Component: Documentation | Version: 1.6
Severity: Normal | Resolution:
Keywords: has_changed api | Triage Stage: Ready for
ModelForm | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:12>

Django

unread,
Aug 5, 2014, 8:48:41 AM8/5/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: closed
Component: Documentation | Version: 1.6
Severity: Normal | Resolution: fixed

Keywords: has_changed api | Triage Stage: Ready for
ModelForm | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"edcc75e5ac5b9dc2f174580e7adacd3be586f8bd"]:
{{{
#!CommitTicketReference repository=""
revision="edcc75e5ac5b9dc2f174580e7adacd3be586f8bd"
Fixed #21792 -- Documented Form.has_changed()

Thanks bjb at credil.org for the suggestion and
Ivan Mesic for the draft patch.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:13>

Django

unread,
Aug 5, 2014, 8:50:34 AM8/5/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: closed
Component: Documentation | Version: 1.6

Severity: Normal | Resolution: fixed
Keywords: has_changed api | Triage Stage: Ready for
ModelForm | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"5e42e9f0592f987a9caba3b1322ba070927a1195"]:
{{{
#!CommitTicketReference repository=""
revision="5e42e9f0592f987a9caba3b1322ba070927a1195"
[1.7.x] Fixed #21792 -- Documented Form.has_changed()

Thanks bjb at credil.org for the suggestion and
Ivan Mesic for the draft patch.

Backport of edcc75e5ac from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:15>

Django

unread,
Aug 5, 2014, 8:52:23 AM8/5/14
to django-...@googlegroups.com
#21792: Form.has_changed is not documented
-------------------------------------+-------------------------------------
Reporter: bjb@… | Owner: navi7
Type: Bug | Status: closed
Component: Documentation | Version: 1.6

Severity: Normal | Resolution: fixed
Keywords: has_changed api | Triage Stage: Ready for
ModelForm | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"5fdfa2d9b49cfb31298d75ffc51800e730bf876c"]:
{{{
#!CommitTicketReference repository=""
revision="5fdfa2d9b49cfb31298d75ffc51800e730bf876c"
[1.6.x] Fixed #21792 -- Documented Form.has_changed()

Thanks bjb at credil.org for the suggestion and
Ivan Mesic for the draft patch.

Backport of edcc75e5ac from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21792#comment:14>

Reply all
Reply to author
Forward
0 new messages