#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 , 16 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 , 16 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.