Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#13126 closed (fixed)

admin list_editable with unique_together error indication

Reported by: slafs Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: list_editable unique_together custom validation error
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

this is somehow related to #13091

When using list_editable in case when all fields from unique_together are in list_editable, the error message does show. However it isn't very informative (only "Please correct the errors below." but nothing below is indicated).

Done this on sqlite3 and PostgreSQL.

Attachments (2)

13126_changelist_non_field_errors.diff (5.0 KB) - added by julien 4 years ago.
13126_changelist_non_field_errors.2.diff (6.3 KB) - added by julien 4 years ago.
Slightly more robust tests

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

To clarify - the issue here is that an object level (as opposed to field level) validation error isn't rendered. If you have a model with list_editable in admin, and you try to edit an object to violate a unique_together clause, you get the generic top level error, but no error on the object row itself.

comment:2 Changed 5 years ago by slafs

  • Keywords custom validation added

I noticed that this issue is related to other (custom) validation rules also. Not only the uniqueness of a model.
When I have some custom validation logic in model's clean method the behaviour is the same. Maybe we should reconsider changing the title of this ticket. Isn't this a candidate for a 1.2 milestone?

comment:3 Changed 5 years ago by russellm

  • milestone set to 1.3

No need to change the ticket title - the title isn't the be-all and end-all of the reporting process, it's just a helpful reminder.

This isn't 1.2 critical - the validation is still occurring, so there's no risk of data loss. I will fully admit that not getting validation error messages is a really annoying bug, but we can live without it. However, I will put it onto the 1.3 milestone so it gets priority next time around.

Changed 4 years ago by julien

comment:4 Changed 4 years ago by julien

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

The issue here is that currently the non_field_errors are never displayed for each form in the changelist's formset. The attached patch should fix this. I'm promoting to RFC hoping this bug gets a core dev's attention before 1.3 lands (which is pretty soon) ;-)

Changed 4 years ago by julien

Slightly more robust tests

comment:5 Changed 4 years ago by julien

If you're interested in this ticket, I recommend you to also check #13091, which I think should also be RFC.

comment:6 Changed 4 years ago by russellm

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

In [15580]:

Fixed #13126 -- Ensured that individual form errors are displayed when errors occur on a list-editable changelist. Thanks to slafs for the report, and to Julien Phalip for the patch.

comment:7 Changed 4 years ago by russellm

In [15581]:

[1.2.X] Fixed #13126 -- Ensured that individual form errors are displayed when errors occur on a list-editable changelist. Thanks to slafs for the report, and to Julien Phalip for the patch.

Backport of r15580 from trunk.

comment:8 Changed 4 years ago by russellm

In [15593]:

Tweaked the changes from changeset r15580 so as to avoid introducing a backwards incompatible context change to the change_list_results template. Refs #13126. Thanks to Sean Brant for the report.

comment:9 Changed 4 years ago by russellm

In [15594]:

[1.2.X] Tweaked the changes from changeset r15580 so as to avoid introducing a backwards incompatible context change to the change_list_results template. Refs #13126. Thanks to Sean Brant for the report.

Backport of r15593 from trunk.

comment:10 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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