#4305 closed (duplicate)
[Patch] duplicated settings in the admin site seem to cause confusion
Reported by: | Owned by: | Julien Phalip | |
---|---|---|---|
Component: | contrib.admin | Version: | newforms-admin |
Severity: | Keywords: | nfa-someday | |
Cc: | django@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
from the tutorial, if you duplicate settings
fields = (
(None, {'fields': ('pub_date', 'question',)}),
('Date information', {'fields': ('pub_date',)}),
)
Then set only one of the 'duplicated' fields, it responds with an error that they must all be filled in. Odd, but I suppose when you have dupliated fields the error is really yours, anyway, and I'm not sure what it really should do, in that case.
Attachments (2)
Change History (15)
comment:1 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 17 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
Somebody might want to write a patch for newforms-admin. It should be easy to detect and worth pointing out to the developer. This is worth leaving open (there are lots of low priority enhancement tickets like this open; it's not really a burden).
comment:4 by , 17 years ago
as a side note also if you duplicate fields within a model specification django replies with odd errors.. (http://betterlogic.com/roger/?p=41) -- you may want to check and throw cleaner errors :) thanks!
comment:5 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
Version: | SVN → newforms-admin |
This feels like a combination of "you shouldn't be doing that" and "validation of ModelAdmin
in newforms-admin should fix this". Changing component and reassigning to jkocherhans.
comment:6 by , 17 years ago
Component: | django-admin.py → Admin interface |
---|---|
Owner: | changed from | to
Not working on this right now, if someone wants to take it up, #3222 is related.
comment:7 by , 17 years ago
Keywords: | nfa-someday added |
---|
comment:8 by , 16 years ago
milestone: | → 1.0 |
---|
comment:9 by , 16 years ago
Owner: | changed from | to
---|
comment:10 by , 16 years ago
Cc: | added |
---|---|
Has patch: | set |
Summary: | duplicated settings in the admin site seem to cause confusion → [Patch+Doc] duplicated settings in the admin site seem to cause confusion |
I added a patch with documentation. I also added my name in the AUTHORS as it would be my first contribution, hope that's fine ;)
The patch is I believe the fastest solution for detecting the presence of duplicates. It will raise an exception at compilation time if a duplicate is found. This would force the developer to remove the duplicates, which is probably best practice.
The patch does not identify the actual duplicates to display them in the exception message, but doing so would be overkill and be costly in performance. The developer could easily spot the duplicates himself anyway.
comment:11 by , 16 years ago
Summary: | [Patch+Doc] duplicated settings in the admin site seem to cause confusion → [Patch] duplicated settings in the admin site seem to cause confusion |
---|
Just added tests to the patch.
E:\Software\workspace\django-newforms-admin\tests>runtests.py --settings=settings modeladmin
Ran 1 test in 0.031s
OK
by , 16 years ago
Attachment: | 4305.duplicate_fieldnames.diff added |
---|
Updated patch so it uses the ModelAdmin's validation introduced in [7929]
comment:12 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Closing in favour of #7885 which goes beyond by taking care of both 'fields' and 'fieldsets' attributes.
Perhaps we should show an error, but I'm not convinced it's worth the development time. If anyone wants to provide a patch, feel free to reopen.