Opened 16 years ago

Closed 16 years ago

Last modified 13 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: dev
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Alex Gaynor)

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 16 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 16 years ago.
Alternative to fixing problem: Admin validation to prevent the situation arising.

Download all attachments as: .zip

Change History (13)

comment:1 by andrepleblanc@…, 16 years ago

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

comment:2 by Alex Gaynor, 16 years ago

Description: modified (diff)

Please use preview

comment:3 by Alex Gaynor, 16 years ago

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 by Alex Gaynor, 16 years ago

Resolution: wontfix
Status: newclosed

comment:5 by django@…, 16 years ago

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 by django@…, 16 years ago

Resolution: wontfix
Status: closedreopened

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).

by django, 16 years ago

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

comment:7 by Alex Gaynor, 16 years ago

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

comment:8 by Will Hardy, 16 years ago

milestone: 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.

by Will Hardy, 16 years ago

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

comment:9 by anonymous, 16 years ago

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 by Karen Tracey, 16 years ago

Resolution: duplicate
Status: reopenedclosed

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

milestone: 1.1

Milestone 1.1 deleted

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