Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10784 closed (duplicate)

list_editable will not work for the default ordering field.

Reported by: andrepleblanc@… Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by Alex)

specifying a DateField as list_editable AND in the ordering of a model's Meta causes an error message stating 'Please correct the errors below' but doesn't list any errors.

eg.

class TestModel(models.Model):
    name = models.CharField(max_length=40)
    birthdate = models.DateField(null=True, blank=True)
    class Meta:
        ordering = ('birthdate',)

class TestModelAdmin(admin.ModelAdmin):
    list_display = ('name', 'birthdate')
    list_editable = ('birthdate', )

the birthdate field cannot actually be edited on the change list, it just shows that unhelpful message. Not sure if this applies to all Field types or not, but definitely DateFields.

Attachments (2)

10784-admin-list_editable-ordering.diff (2.7 KB) - added by django 5 years ago.
Regression test for a problem with list_editable on fields that order.
10784-admin-list_editable-ordering-validation.diff (872 bytes) - added by Will Hardy 5 years ago.
Alternative to fixing problem: Admin validation to prevent the situation arising.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 5 years ago by andrepleblanc@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

sorry, the code didn't come out as I had hoped, but I'm sure the problem is clear.

comment:2 Changed 5 years ago by Alex

  • Description modified (diff)

Please use preview

comment:3 Changed 5 years ago by Alex

  • Description modified (diff)

I'm unable to reproduce with:

class Article(models.Model):
    title = models.CharField(max_length=100)
    pubdate = models.DateField()

    class Meta:
        ordering = ('pubdate',)

class ArticleAdmin(admin.ModelAdmin):
    list_display = ('title', 'pubdate',)
    list_editable = ('pubdate',)

I'm going to close as worksforme now, if somehow can provide additional information to reproduce it or a testcase for Django's tests feel free to reopen.

comment:4 Changed 5 years ago by Alex

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

comment:5 Changed 5 years ago by django@…

I'm seeing this as well (with all editable field types), but I can't write a regression test that fails! Testing with my browser uses the same models definition, admin defintion and database as the regression test, but only the browser test fails. Using django r10838.

The error messages that we can't see are complaining about the values for the 'id' fields already existing (of course they are, it's an edit). This also explains why we don't see the error message (the 'id' fields are hidden).

I'll keep looking for the reason and a way to replicate this, but I thought I would say something to let people know there still might be an issue.

@andrepleblanc were you using the geodjango models/admin?

comment:6 Changed 5 years ago by django@…

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Now reproduceable!

The problem is when you edit a field that dictates the ordering. It could be because the fields are reordered, or because objects share the same value for the sorting field. I'll attach a regression test, which does both.

No time to look at a fix now (this doesn't have anything to do with my current projects), but I did notice that a number of model instances that the formsets were dealing with all had the same id. The error appears to be the unique_error_message in django/forms/models.py, called on line 326 (r10838) after line 320 fails to exclude the correct values (because the instance has the wrong pk, or it's the wrong instance).

Changed 5 years ago by django

Regression test for a problem with list_editable on fields that order.

comment:7 Changed 5 years ago by Alex

This looks like it may actually be a symptom of #10922.

comment:8 Changed 5 years ago by Will Hardy

  • milestone set to 1.1

Putting this under the 1.1 milestone... sorry core developers :-(

It will come up anytime someone uses list_editable to edit a non-unique field which determines the ordering of objects. Despite what Karen wrote in the django-developers thread for #10922, I have the impression that this is a pretty common way to implement manual object ordering, and list_editable would be a great way to control it from the admin.

If it can't be fixed, we should at least add an admin validation, which throws an error when any non-unique field is listed in list_editable and determines the default ordering. Failing that, a little bit of documentation on the issue.

Changed 5 years ago by Will Hardy

Alternative to fixing problem: Admin validation to prevent the situation arising.

comment:9 Changed 5 years ago by anonymous

BTW the attached validation approach wont stop users from ordering the result list through a GET variable and getting at the bug that way.

comment:10 Changed 5 years ago by kmtracey

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

The most recent patch on #10922 addresses this, I believe. I'd rather pursue getting that change ready for commit than adding validation or documentation of a restriction, etc. I'm going to close this as a dup of that; if it turns out we can't get #10922 fixed for 1.1 this can be re-opened for investigation of what to do to mitigate this problem.

comment:11 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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.