subject:"Re\: \[Django\] #17522\: ModelAdmin.ordering validation too strict"

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2024-03-12 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  Sebastian Goll   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Ülgen Sarıkavak):

 * cc: Ülgen Sarıkavak (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e316ee2e1-8025fd88-e5b1-43a2-a217-41c39cb48fbd-00%40eu-central-1.amazonses.com.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2023-06-04 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  Sebastian Goll   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Sarah Boyce):

 * cc: Sarah Boyce (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018887289ae8-0a7cad0c-dca2-4599-9199-5f3c5204c5f4-00%40eu-central-1.amazonses.com.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2022-04-27 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  Sebastian Goll   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Sylvain Fankhauser):

 For those like me wondering why the following fails with an error `Cannot
 resolve keyword 'bar_count' into field` on Django 3.2:


 {{{
 #!python
 class FooAdmin(admin.ModelAdmin):
 list_display = ('bar_count',)

 def get_queryset(self, request):
 qs = super().get_queryset(request)
 return qs.annotate(
 bar_count=models.Count('bar_set')
 )

 def get_ordering(self, request):
 return ('bar_count',)

 def bar_count(self, foo):
 return foo.bar_count
 bar_count.admin_order_field = 'bar_count'
 }}}

 That’s because `super().get_queryset(request)` tries to apply the ordering
 on the `bar_count` field that’s not yet been added to the queryset. This
 can be fixed by changing the `get_queryset` method to:

 {{{
 #!python
 def get_queryset(self, request):
 qs = self.model._default_manager.get_queryset().annotate(
 bar_count=models.Count('bar_set')
 )

 ordering = self.get_ordering(request)
 if ordering:
 qs = qs.order_by(*ordering)

 return qs
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701806b7daf6d-d0be33e4-16ef-41eb-a28d-7f2537aacbb7-00%40eu-central-1.amazonses.com.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2019-01-01 Thread Django

Comment (by Javier Abadia Miranda):

Replying to [comment:7 Simon Charette]:
>
> I have to say I'm not convinced this needs to be fixed anyhow. Why
wouldn't the following be acceptable?
>
Because the `ordering` tuple purpose is to specify the **default**
ordering that the user can change afterwards by clicking on column
headers. If I'm forced to embed the ordering in the `get_queryset()`
method, then:
1) I am forced to implement a get_queryset in situations where I don't
need to do it, because defining additional columns via properties with
`get_` methods works fine and it's convenient and consistent with
`list_display`
2) The user can't change the ordering afterwards, or I need to put logic
inside `get_queryset()` to include the sorting or not depending on what
the user does.
The funny thing is that everything works as expected, if you can work
around the too strict (in my opinion) validation.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.a65ba8daffd08d45a0eaa163c5f33b41%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2018-12-27 Thread Django

Comment (by Kevin Christopher Henry):

> If you are dynamically defining an annotation then why are you not
dynamically ordering by this annotation and expecting static options to
work with it?

I'm expecting it because that's how `list_display` works. The purpose of
`admin_order_field` is to allow you to ''statically declare'' how to order
a ''dynamic'' piece of code. Given that this exists, it seems wrong not to
respect it when it comes to the `ordering` attribute.

To me, the bug isn't about how much you can or can't declare statically,
it's about making the API more consistent. I haven't looked at the
proposed patch, but I think this general idea - considering
`admin_order_field` when looking at `ordering` - will be an improvement.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.dffcc7598f44ee60391472691a31de30%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2018-12-25 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  Sebastian Goll   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 #29669 is duplicate with a patch that implements the third option of
 comment:5

 I have to say I'm not convinced this needs to be fixed anyhow. Why
 wouldn't the following be acceptable?

 {{{#!python
 class FooAdmin(admin.ModelAdmin):
 list_display = ('bar_count',)

 def get_queryset(self, request):
 qs = super().get_queryset(request)
 return qs.annotate(
 bar_count=models.Count('bar_set')
 ).order_by('bar_count')

 def bar_count(self, foo):
 return foo.bar_count
 bar_count.admin_order_field = 'bar_count'
 }}}

 or

 {{{#!python
 class FooAdmin(admin.ModelAdmin):
 list_display = ('bar_count',)

 def get_queryset(self, request):
 qs = super().get_queryset(request)
 return qs.annotate(
 bar_count=models.Count('bar_set')
 )

 def get_ordering(self, request):
 return ('bar_count',)

 def bar_count(self, foo):
 return foo.bar_count
 bar_count.admin_order_field = 'bar_count'
 }}}

 If you are dynamically defining an annotation then why are you not
 dynamically ordering by this annotation and expecting static options to
 work with it? The fact `ordering` is not attempted to be applied by
 `super().queryset()` is really just an implementation detail and would
 crash otherwise.

 Given `ModelAdmin.ordering` now accepts expressions `ordering =
 models.Count('bar_set'),` would work as well if there isn't need to refer
 to it using an alias for callable field ordering references.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.1b72a4f68a9b29519b656bb6418368dd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2017-07-26 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  Sebastian Goll   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Andreas Backx):

 Could this ticket be revisited? This still applies to Django 1.11.3. It's
 something minor, but a patch seems to already have been made a long time
 ago, but I guess the ticket was forgotten.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.144c2213abd8cd481d69135203062f78%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2014-09-25 Thread Django

Comment (by marfire):

The issue still exists. To briefly recap: the question is how to allow
ordering by annotations (e.g. the `Count` of a related model) while still
preserving static checks on the field names.

I can think of a few approaches:

1. Leave things the way they are. The obvious downside is that it really
does seem like a bug that you can view and order the change list a certain
way, but you can’t specify that as the default order. (Some other
downsides will become apparent below.)

2. Get rid of the checks on ordering. This has some justification: after
all, there is an inherent conflict between trying to do static, start-up-
time checks on a model's fields when the design allows them to be created
dynamically (via `get_queryset()` and `annotate()`). Moreover, these
checks don't seem especially critical in practice—if your ordering is bad,
you'll immediately see a 500 with an error message approximately as
informative as the checks framework message. Still, it seems a shame to
lose checks that could be helpful in the majority of cases for the sake of
an edge case.

3. Allow callables and methods marked with `admin_order_field` to be used.
(This is the proposal made above by sebastian.) The main problem here is
that it's pretty convoluted to have to add:
{{{
def my_method(self, foo):
return foo.bar__count
my_method.admin_order_field = 'bar__count'
}}}
when all you really want to say is `ordering = ('bar__count,')`. It seems
like a pretty hacky way to opt out of the checks. This also has the
downside (shared with `list_display`) that `admin_order_field` isn't
subject to any checks.

4. Figure out whether or not it's possible for the model instance to have
dynamic field names, and either perform or ignore the checks based on
that. For example, we could ignore checks if the `ModelAdmin` has a
`get_queryset()` method. This could be complicated, though, since you also
have look at the default `Manager`. Another advantage, if this can be done
reliably, is that we could start checking the value of
`admin_order_field`.

5. Since the purpose of all this is to allow annotations to show up on the
change list page, we could just add support for that directly. For
example, `ordering = ('Count(bar_set)',)`. And similarly with
`list_display` and `admin_order_field`. This would allow keeping the
checks (and adding them to `admin_order_field`) while avoiding the
pointless method declarations of 3 and skipping the need for
`get_queryset()` when doing annotations. This is potentially the nicest
design, but is probably also the most complicated solution.

--
Ticket URL:
Django
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.6cff7fcde6683491952f93d1f948d4e7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2014-09-18 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  sebastian|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by marfire):

 * cc: k@… (added)


--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.7e1f5124fe22164fcb5be8987eb2cdab%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2014-06-13 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  sebastian|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by timo):

 * needs_better_patch:  0 => 1


Comment:

 Patch no longer applies cleanly.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.1663e45e4a307f69fcdd4a67a184f535%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2012-01-28 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  sebastian|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  SVN
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:  Accepted
  ordering, strict   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by julien):

 * stage:  Unreviewed => Accepted


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

2012-01-14 Thread Django

#17522: ModelAdmin.ordering validation too strict
-+-
 Reporter:  sebastian|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  contrib.admin|  Version:  SVN
 Severity:  Normal   |   Resolution:
 Keywords:  admin, validation,   | Triage Stage:
  ordering, strict   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by sebastian):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 Let me add some thoughts as to why I think the current validation should
 be changed.

 It is quite possible to add annotations to the change list queryset with
 `queryset()`. When used with `admin_order_field`, such columns are
 perfectly sortable in the change list view by clicking the corresponding
 column. What is not possible, however, as of r17372, is to specify these
 columns as ''default'' sorting for the corresponding change list view.

 {{{#!python
 # models.py

 class Foo(models.Model):
 pass

 class Bar(models.Model):
 foo = models.ForeignKey(Foo)

 # admin.py

 class FooAdmin(admin.ModelAdmin):
 list_display = ('bar_count',)

 def queryset(self, request):
 return super(FooAdmin,
 self).queryset(request).annotate(bar_count=models.Count('bar_set'))

 def bar_count(self, foo):
 return foo.bar_count
 bar_count.admin_order_field = 'bar_count'

 # This does not work:
 #ordering = ('bar_count',)
 }}}

 I understand the motivation for doing some basic sanity checks on the
 model admin's attributes. In this case, however, I think these are too
 strict, i.e. checking only if a matching field exists on the model is not
 what should be done.

 On the other hand, instead of entirely removing the checks so that we
 simply assume that the corresponding attribute will exist eventually at
 the time the query is executed, I propose this compromise: check if there
 is (a) an attribute, or (b) a method on either the model or admin class
 with `admin_order_field` set.

 This way, we can still specify arbitrary fields, such as produced by
 annotations, while retaining the basic sanity check that there must be at
 least ''something'' declared manually, either a field or a method. This
 should prevent most typos while allowing the necessary freedom to add
 default-sortable columns based on annotation fields.

 BTW, this is exactly how `admin_order_field` works in regular
 `list_display` in the first place. No checks for the existence of the
 given field are done in the validation code, for the same reason: to be
 able to include annotations as sortable columns.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

Re: [Django] #17522: ModelAdmin.ordering validation too strict

12 matches

Site Navigation

Mail list logo

Footer information