#289 closed defect (wontfix)
[patch] more details with "Please correct the errors below."
Reported by: | 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)
Change History (22)
comment:1 by , 19 years ago
comment:2 by , 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 , 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 , 19 years ago
Attachment: | precise-errors.diff added |
---|
Patch to provide more detailed error reporting.
by , 19 years ago
Attachment: | precise-errors.2.diff added |
---|
The first patch had some unrelated diffing going on.
comment:4 by , 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:6 by , 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 , 18 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
Closing. See previous comment.
comment:8 by , 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 , 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 , 18 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
Type: | → defect |
workin with 0.95 no chance to find the error without some logdata or something else.
please fix it.
comment:11 by , 18 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
comment:12 by , 18 years ago
Resolution: | wontfix |
---|---|
Status: | closed → 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 by , 18 years ago
Summary: | Patch: more details with "Please correct the errors below." → [Patch] more details with "Please correct the errors below." |
---|
comment:14 by , 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 , 18 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Marked as accepted, as this is a nice idea, but the patch needs improvement as per Adrian and Ubernostrum's suggestions.
by , 18 years ago
Attachment: | 289_error_list.2.diff added |
---|
error_list template tag (with template file)
comment:16 by , 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 , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → 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 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
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.