Opened 8 years ago

Closed 7 years ago

Last modified 4 years ago

#4305 closed (duplicate)

[Patch] duplicated settings in the admin site seem to cause confusion

Reported by: rogerpack2005@… Owned by: julien
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: UI/UX:

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)

duplicate_fieldnames.diff (3.5 KB) - added by julien 7 years ago.
patch + doc + tests
4305.duplicate_fieldnames.diff (2.9 KB) - added by julien 7 years ago.
Updated patch so it uses the ModelAdmin's validation introduced in [7929]

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

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.

comment:2 Changed 8 years ago by mtredinnick

  • Resolution wontfix deleted
  • Status changed from closed to 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:3 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Accepted

Fair enough.

comment:4 Changed 8 years ago by roger

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 Changed 8 years ago by ubernostrum

  • Owner changed from nobody to jkocherhans
  • Status changed from reopened to new
  • Version changed from SVN to 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 Changed 8 years ago by jkocherhans

  • Component changed from django-admin.py to Admin interface
  • Owner changed from jkocherhans to nobody

Not working on this right now, if someone wants to take it up, #3222 is related.

comment:7 Changed 8 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-someday added

comment:8 Changed 7 years ago by programmerq

  • milestone set to 1.0

comment:9 Changed 7 years ago by julien

  • Owner changed from nobody to julien

comment:10 Changed 7 years ago by julien

  • Cc django@… added
  • Has patch set
  • Summary changed from duplicated settings in the admin site seem to cause confusion to [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.

Changed 7 years ago by julien

patch + doc + tests

comment:11 Changed 7 years ago by julien

  • Summary changed from [Patch+Doc] duplicated settings in the admin site seem to cause confusion to [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

Changed 7 years ago by julien

Updated patch so it uses the ModelAdmin's validation introduced in [7929]

comment:12 Changed 7 years ago by julien

  • Resolution set to duplicate
  • Status changed from new to closed

Closing in favour of #7885 which goes beyond by taking care of both 'fields' and 'fieldsets' attributes.

comment:13 Changed 4 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top