Code

Opened 9 years ago

Closed 7 years ago

Last modified 3 years ago

#289 closed defect (wontfix)

[patch] more details with "Please correct the errors below."

Reported by: brantley (deadwisdom@… Owned by: nobody
Component: contrib.admin Version:
Severity: normal Keywords: error please correct
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Sometimes this error appears in the admin side of a site, without actually showing any errors below it. This is most assuredly an error on the part of the developer, but with the multitude of things that can go wrong, this obfuscated error reporting surely doesn't help.

Attachments (4)

precise-errors.diff (1.8 KB) - added by Manuzhai 9 years ago.
Patch to provide more detailed error reporting.
precise-errors.2.diff (964 bytes) - added by Manuzhai 9 years ago.
The first patch had some unrelated diffing going on.
289_error_list.diff (2.0 KB) - added by paulsmith@… 7 years ago.
error_list template tag
289_error_list.2.diff (2.6 KB) - added by paulsmith@… 7 years ago.
error_list template tag (with template file)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 9 years ago by Manuzhai

Just some explanation: I'm fairly sure this is a complaint about an unsatisfied validation requirement. As was noted in IRC, this might very well be due to inline editing of another object, or it could be that there is no admin interface for a required element (I'm not sure how smart Django is about these things). Anyway, maybe Django could somehow figure out which errors there are, and extend the message to say so.

comment:2 Changed 9 years ago by anonymous

I also ran into this problem. In my case, it was because I was customizing my admin interface to show a certain ordering and grouping of fields, and forgot to include a required field. As such, the field was not included in the form, and so failed validation.

Of course, since the field didn't exist on the form, no error message showed up next to a field, and the "please correct below errors" message was quite misleading. :)

comment:3 Changed 9 years ago by Manuzhai

  • Summary changed from "Please correct the errors below." to Patch: more details with "Please correct the errors below."

I'm attaching a patch that states the fields that have errors in the error message. This makes it more obvious if there are any errors for fields not in the admin interface, although it may be overkill if a field is in the admin interface, but an extra notification can't hurt?

Changed 9 years ago by Manuzhai

Patch to provide more detailed error reporting.

Changed 9 years ago by Manuzhai

The first patch had some unrelated diffing going on.

comment:4 Changed 9 years ago by Manuzhai

And yes, my patch makes the line way too long, so it might be better to split stuff up, but I couldn't think of a good way to split it up and I just wanted to get the idea out here.

comment:5 Changed 9 years ago by adrian

#442 is a duplicate.

comment:6 Changed 9 years ago by adrian

The reason I've hesitated to include this patch is that the displayed output isn't as user-friendly as I'd like. It displays the "machine" names for the fields rather than the human-readable fields, and, if there are many errors, the "Please correct the errors below" line is quite long and unwieldy. Any suggestions on solving those two problems?

comment:7 Changed 8 years ago by adrian

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

Closing. See previous comment.

comment:8 Changed 8 years ago by nirvdrum

I'd rather have an overly verbose line than nothing at all. At least with the former there's something to work with.

Having said that, it seems like the wrong fix. I eventually figured out that whenever I saw the problem is was due to me not including a required field in the admin interface. Given that I only used the admin interface to manipulate existing entries, it seemed like a good idea to just not include the fields I didn't want manipulated in the Admin class. As it turns out, that doesn't really work well with django. So, an error message for this (common?) problem would be having an error message saying "required fields missing in 'fields' option" or something. Additionally, if a required field is not added, it could just be saved with its existing value (i.e., skip validation if it's not in the 'fields' listing).

comment:9 Changed 8 years ago by Go

  • Summary changed from Patch: more details with "Please correct the errors below." to Patch: more details with "Please correct the errors below."
  • Type defect deleted

comment:10 Changed 8 years ago by chris

  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Type set to defect

workin with 0.95 no chance to find the error without some logdata or something else.

please fix it.

comment:11 Changed 8 years ago by adrian

  • Resolution set to wontfix
  • Status changed from reopened to closed

comment:12 Changed 8 years ago by ubernostrum

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Maybe the best option here would be to have the following setup:

  • At the top of the page, "Please correct the errors below", with a control underneath that can be clicked to expand, in place, a full list of the errors.
  • Down in the fieldsets themselves, continue matching error messages with fields as before.

This way the full error list doesn't take up a huge amount of space unless a user asks to see it, but is still there for confused developers who don't realize they've left a field out of the admin. Getting the human-readable fields for the error names in the full list up top wouldn't be hard, considering we already do it for the fieldsets.

I think it'd also be a good idea to have the model validator print something to the console -- but not error out -- if a model has a required field that won't be listed in the admin.

comment:13 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Summary changed from Patch: more details with &#34;Please correct the errors below.&#34; to [Patch] more details with "Please correct the errors below."

comment:14 Changed 7 years ago by adrian

  • Summary changed from [Patch] more details with "Please correct the errors below." to [patch] more details with "Please correct the errors below."

comment:15 Changed 7 years ago by Simon G. <dev@…>

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Marked as accepted, as this is a nice idea, but the patch needs improvement as per Adrian and Ubernostrum's suggestions.

Changed 7 years ago by paulsmith@…

error_list template tag

Changed 7 years ago by paulsmith@…

error_list template tag (with template file)

comment:16 Changed 7 years ago by paulsmith@…

Don't know if this is useful with newforms-admin on the horizon, but here's a stab at an improved patch. Issues: ordering of fields is undefined; could use accompanying JavaScript to toggle hide/show as ubernostrum described; prettify with CSS.

comment:17 Changed 7 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from reopened to closed

newforms-admin will deal with this, I believe, and makes this ticket obsolete (IIRC ModelAdmin will raise an error if you attempt to set up an interface where a required field isn't shown).

comment:18 Changed 3 years ago by anonymous

  • Easy pickings unset
  • UI/UX unset

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.