[Django] #29408: ordering by field from related model does not validate if field exists

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29408: ordering by field from related model does not validate if field exists

Django
#29408: ordering by field from related model does not validate if field exists
-----------------------------------------+------------------------
               Reporter:  Shadi Akiki    |          Owner:  nobody
                   Type:  Uncategorized  |         Status:  new
              Component:  Uncategorized  |        Version:  2.0
               Severity:  Normal         |       Keywords:
           Triage Stage:  Unreviewed     |      Has patch:  0
    Needs documentation:  0              |    Needs tests:  0
Patch needs improvement:  0              |  Easy pickings:  0
                  UI/UX:  0              |
-----------------------------------------+------------------------
 When the `ordering` class member in `Meta` of a model contains a field
 from a related model, and that field does not exist, django's
 `makemigrations` does not throw an error. However, if it is a direct field
 member of the same class, `makemigrations` does throw an error.
 Example below tested on Django 2.0.5

 {{{
 from django.db import models

 # Create your models here.

 class Agreement(models.Model):
     agreement_id = models.AutoField(verbose_name='ID', serialize=False,
 auto_created=True, primary_key=True)
     #class Meta:
       # generates error in makemigrations
       # app.Agreement: (models.E015) 'ordering' refers to the nonexistent
 field 'id'.
       # ordering = ['id']

 class Order(models.Model):
     agreement = models.ForeignKey(Agreement, models.DO_NOTHING)
     class Meta:
       # does not generate error in makemigrations
       # but does so during runtime
       # e.g. [x for x in Order.objects.all()]
       ordering = ['agreement__id']

 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29408>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/057.6f64193eb09411898d970a3a306364d7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of ordering by a field from related model. (was: ordering by field from related model does not validate if field exists)

Django
#29408: Add validation of ordering by a field from related model.
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * component:  Uncategorized => Database layer (models, ORM)
 * version:  2.0 => master
 * type:  Uncategorized => Cleanup/optimization
 * stage:  Unreviewed => Accepted


Comment:

 I'm going to accept this provisionally. There's
 [https://github.com/django/django/blob/2dcc5d629a6439b5547cdd6e67815cabf608fcd4/django/db/models/base.py#L1598-L1600
 a `FIXME` in `models/base.py`] specifically about this:


 {{{
         # Skip ordering in the format field1__field2 (FIXME: checking
         # this format would be nice, but it's a little fiddly).
         fields = (f for f in fields if LOOKUP_SEP not in f)
 }}}

 Added in
 [https://github.com/django/django/commit/d818e0c9b2b88276cc499974f9eee893170bf0a8
 d818e0c9b2b88276cc499974f9eee893170bf0a8].

 Either we should address this, or remove the comment and close as
 `wontfix` if "fiddly" turns out to be more effort than it's worth.

 A test case and a patch showing what "fiddly" actually entails would be
 great.

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:1>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.9e88489961d56291259f958546935a1b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of ordering by a field from related model.

Django
In reply to this post by Django
#29408: Add validation of ordering by a field from related model.
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Windson yang):

 I think we can just address this in the document and don't fix it.

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:2>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.2ed8826d14dc60aee882b6620db2d128%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of ordering by a field from related model.

Django
In reply to this post by Django
#29408: Add validation of ordering by a field from related model.
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Jeff):

 * cc: Jeff (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:3>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.84450db76e9c3df0cf698ef3138c8b24%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of ordering by a field from related model.

Django
In reply to this post by Django
#29408: Add validation of ordering by a field from related model.
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

 * status:  new => assigned
 * owner:  nobody => Hasan Ramezani
 * has_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:4>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.aedd40d06e2620e26a2da2dd327da377%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering (was: Add validation of ordering by a field from related model.)

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * needs_better_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:5>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.908c0fe7daf3b446eee468b747cc4fb3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Hasan Ramezani):

 patch updated and new method added.

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:6>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.eba90aa37e20a73ba5fb85d109d27ca4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

 * needs_better_patch:  1 => 0


--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:7>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.d376bf6e47e9ef781487fe03a8ea8f60%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Hasan Ramezani):

 Any updates? if there is something to change please inform me. I am ready.

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:8>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.74c4c19408f480463664c270fc59d88c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1


Comment:

 Left comments on PR: patch would need to handle JSON paths, which should
 be valid in `ordering` since #24747. (Similar issue arises in #29622.)

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:9>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.1926aaab67db66605ac3c73b09c48d6e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29408: Add validation of related fields in model Meta.ordering

Django
In reply to this post by Django
#29408: Add validation of related fields in model Meta.ordering
-------------------------------------+-------------------------------------
     Reporter:  Shadi Akiki          |                    Owner:  Hasan
         Type:                       |  Ramezani
  Cleanup/optimization               |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Hasan Ramezani):

 PR updated. `JSON paths` handled and some tests added for it

--
Ticket URL: <https://code.djangoproject.com/ticket/29408#comment:10>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/072.984967adfbbbedcde87bf502f6732ee7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.