Re: [Django] #12952: Models history doesn't use verbose names
Django
-------------------------------------+-------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2-beta | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------
Changes (by vasiliyeah):
* needs_better_patch: 0 => 1
* ui_ux: => 0
* easy: => 0
Comment:
This patch can't be applied.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2-beta | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 1
UI/UX: 1 |
-------------------------------------+-------------------------------
Changes (by vasiliyeah):
* needs_better_patch: 1 => 0
* ui_ux: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:5>
Django
-------------------------------------+-------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2-beta | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 1 |
-------------------------------------+-------------------------------
Changes (by vasiliyeah):
* cc: vvangelovski@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:6>
Django
-------------------------------------+-------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2-beta | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
UI/UX: 1 |
-------------------------------------+-------------------------------
Changes (by vasiliyeah):
* needs_tests: 1 => 0
Comment:
Updated patch to apply to current trunk and added a test for the history
view.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:7>
Django
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.2-beta | Severity: Normal
Resolution: | Keywords:
decision needed | Needs tests: 0
Needs documentation: 0 | Easy pickings: 0
Patch needs improvement: 0 |
UI/UX: 1 |
-------------------------------------+-------------------------------------
Changes (by julien):
* stage: Accepted => Design decision needed
Comment:
Marking as DDN until an answer to ticket:14358#comment:3 is provided.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:8>
Django
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by aaugustin):
* stage: Design decision needed => Accepted
Comment:
The objection was "for debug purposes, it would be more useful to have the
field names, as they are necessarily unique, untranslated, etc."
In my opinion:
- uniqueness is a weak argument: if you have two fields with the same
name, you're just asking for trouble;
- translation isn't an issue at all: in doubt, just switch the website to
the default language of your codebase.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:9>
Django
-------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2-beta
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------+------------------------------------
Comment (by aaugustin):
#14358 is a duplicate and has patches too.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:10>
Django
-------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2-beta
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
Patch will need to be updated to apply cleanly.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:11>
Django
-------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.2-beta
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------+------------------------------------
* needs_better_patch: 1 => 0
Comment:
A Github pull request ([https://github.com/django/django/pull/4496 #4496])
has been opened for this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:12>
Django
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
Changes (by timgraham):
* version: 1.2-beta => master
* type: Bug => Cleanup/optimization
Comment:
I am thinking it might be best to try to address #21113 first (allow
messages to be translated to the current language rather than displaying
the language that was active for editing) since this ticket would require
a new implementation for that one.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:13>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Left comments for improvement on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:14>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by RamezIssac):
I think it's better to work with the form labels then model's verbose
name,
I think It's actually a more natural flow and more consistent if the label
is different then the verbose name.
My Approach would be to gather the changed fields' labels, then send it to
get_text_list
https://github.com/django/django/blob/master/django/contrib/admin/options.py#L925-L936
{{{
translated_changed_fields = [form.fields[f].label for f in
form.changed_data]
change_message.append(_('Changed %s.') %
get_text_list(translated_changed_fields, _('and')))
#again for formset
for changed_object, changed_fields in formset.changed_objects:
translated_changed_fields = [formset.fields[f].label for f in
changed_fields]
change_message.append(_('Changed %(list)s for %(name)s "%(object)s".')
% {'list':
get_text_list(translated_changed_fields, _('and')),
#...
}}}
Created a duplicate [https://code.djangoproject.com/ticket/24990 24990]
Regards;
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:15>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by timgraham):
It seems to me that the verbose name is a safer choice to account for the
fact that you might have several different editing forms each with a
different label.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:16>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by RamezIssac):
IMHO, that depend on the intended audience of the changed message in the
history.
If it's the developer then verbose_name are the way to go.
If it's the site 'content' administrator, this is what i think is the
case, then labels are more of expected behavior; and in that case the
"maybe different" verbose name can be confused as a bug.
"I edited the field "More info", why does it show me that i edited
'description' which is not even present on the form ?"
:)
Kind regards;
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:17>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by RamezIssac):
I see that this pull request didn't get merged.
I can go for it with the form label resolution.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:18>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by timgraham):
After further consideration, I think that approach is okay.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:19>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by RamezIssac):
PR created [https://github.com/django/django/pull/5169 #5169]
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:20>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
Comment (by timgraham):
I left some comments for improvement on the pull request. Don't forget to
uncheck "Patch needs improvement" on this ticket after you update it so it
appears in the review queue, thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:21>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:22>
Django
--------------------------------------+------------------------------------
Reporter: acangiano | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
--------------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
Some [https://groups.google.com/d/topic/django-
developers/nELSu0S6N9M/discussion discussion on the mailing list]
confirmed my feeling that we should fix #21113 first.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:23>
Django
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
Changes (by Sanyam Khurana):
* cc: Sanyam Khurana (added)
* owner: nobody => Sanyam Khurana
* status: new => assigned
Comment:
I'm trying to modify the current patch and apply it cleanly to the current
master branch.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/10531 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Comments left on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
I've addressed the reviews. Can you please take another pass?
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
Review comments have been addressed. Patch looks good to go. (Will just
leave time for Claude to follow up if he wants to.)
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Sanyam Khurana):
Addressed Claude's review on the patch and it is ready for a review again
and possibly check-in.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
There were just a couple more comments on the ticket. Sanyam, please
uncheck Patch needs improvement when you've had a look at those. (Thanks
for the great effort!)
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:30>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Hey Carlton, I've addressed the last nit-picks in the patch. Thank you for
your patience!
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Sanyam Khurana):
Hello,
Can you please have a look at the recent updates on this patch?
Please let me know if any more changes are needed.
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:32>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
(The patch is in the review queue, no need to add a comment asking for a
review. I was on vacation last week and am catching up on things.)
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:33>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
PR looks not far off after several reviews. Once comments are addressed it
would be good if previous reviewers could give one more look to make sure
we don't miss anything.
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:34>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Hi Carlton,
The PR was approved a long ago.
Can you please take a look and let me know if you think there is anything
more to be done here?
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:35>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:36>
Django
-------------------------------------+-------------------------------------
Reporter: Antonio Cangiano | Owner: Sanyam
Type: | Khurana
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"87f5d07eededc86f8ce1797fdfca7d4903ee0edc" 87f5d07e]:
{{{
#!CommitTicketReference repository=""
revision="87f5d07eededc86f8ce1797fdfca7d4903ee0edc"
Fixed #12952 -- Adjusted admin log change messages to use form labels
instead of field names.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/12952#comment:37>