Opened 19 years ago

Closed 17 years ago

Last modified 13 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 19 years ago.
Patch to provide more detailed error reporting.
precise-errors.2.diff (964 bytes ) - added by Manuzhai 19 years ago.
The first patch had some unrelated diffing going on.
289_error_list.diff (2.0 KB ) - added by paulsmith@… 18 years ago.
error_list template tag
289_error_list.2.diff (2.6 KB ) - added by paulsmith@… 18 years ago.
error_list template tag (with template file)

Download all attachments as: .zip

Change History (22)

comment:1 by Manuzhai, 19 years ago

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 by anonymous, 19 years ago

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 by Manuzhai, 19 years ago

Summary: "Please correct the errors below."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?

by Manuzhai, 19 years ago

Attachment: precise-errors.diff added

Patch to provide more detailed error reporting.

by Manuzhai, 19 years ago

Attachment: precise-errors.2.diff added

The first patch had some unrelated diffing going on.

comment:4 by Manuzhai, 19 years ago

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 by Adrian Holovaty, 19 years ago

#442 is a duplicate.

comment:6 by Adrian Holovaty, 19 years ago

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 by Adrian Holovaty, 18 years ago

Resolution: worksforme
Status: newclosed

Closing. See previous comment.

comment:8 by nirvdrum, 18 years ago

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 by Go, 18 years ago

Summary: Patch: more details with "Please correct the errors below."Patch: more details with "Please correct the errors below."
Type: defect

comment:10 by chris, 18 years ago

Resolution: worksforme
Status: closedreopened
Type: defect

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

please fix it.

comment:11 by Adrian Holovaty, 18 years ago

Resolution: wontfix
Status: reopenedclosed

comment:12 by James Bennett, 18 years ago

Resolution: wontfix
Status: closedreopened

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 by Gary Wilson <gary.wilson@…>, 18 years ago

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

comment:14 by Adrian Holovaty, 18 years ago

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

comment:15 by Simon G. <dev@…>, 18 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

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

by paulsmith@…, 18 years ago

Attachment: 289_error_list.diff added

error_list template tag

by paulsmith@…, 18 years ago

Attachment: 289_error_list.2.diff added

error_list template tag (with template file)

comment:16 by paulsmith@…, 18 years ago

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 by James Bennett, 17 years ago

Resolution: wontfix
Status: reopenedclosed

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 by anonymous, 13 years ago

Easy pickings: unset
UI/UX: unset
Note: See TracTickets for help on using tickets.
Back to Top