#10784 closed (duplicate)
list_editable will not work for the default ordering field.
Reported by: | 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 )
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)
Change History (13)
comment:1 by , 16 years ago
comment:3 by , 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 , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:5 by , 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 , 16 years ago
Resolution: | wontfix |
---|---|
Status: | closed → 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).
by , 16 years ago
Attachment: | 10784-admin-list_editable-ordering.diff added |
---|
Regression test for a problem with list_editable on fields that order.
comment:8 by , 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 , 16 years ago
Attachment: | 10784-admin-list_editable-ordering-validation.diff added |
---|
Alternative to fixing problem: Admin validation to prevent the situation arising.
comment:9 by , 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 , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | reopened → 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.
sorry, the code didn't come out as I had hoped, but I'm sure the problem is clear.