[Django] #25461: Model _meta API Migration Guide
Django
-------------------------------+--------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The _meta API migration guide for Django 1.8 shows a replacement for
_meta.get_all_related_objects() that isn't quite right when used on a
multi-table inheritance child model.
{{{#!python
class Party():
is_active = models.BooleanField(default=True)
class Contact(Party):
first_name = models.CharField(max_length=128)
last_name = models.CharField(max_length=128)
}}}
{{{#!python
[f for f in Contact._meta.get_all_related_objects()]
...
[]
[f for f in Contact._meta.get_fields() if (f.one_to_many or f.one_to_one)
and f.auto_created]
...
[<django.db.models.fields.related.OneToOneField: party_ptr>]
}}}
The _meta.get_all_related_objects() method would return nothing assuming
no other external relations but the replacement method would return the
automatically created OneToOneField pointing to the parent. Maybe changing
the docs to use is_concrete would work, unless this causes other problems?
{{{#!python
[f for f in Contact._meta.get_all_related_objects()]
...
[]
[f for f in Contact._meta.get_fields() if (f.one_to_many or f.one_to_one)
and f.auto_created and not f.concrete]
...
[]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25461>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
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 timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:1>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by romgar):
We just encountered that issue in `django-deep-collector` project.
The proposed change seems to fix it.
What do you need to resolve this ticket?
Updating the documentation, adding some tests?
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:2>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by romgar):
* cc: romain.garrigues.cs@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:3>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Yes, unfortunately I don't think we have any tests for the upgrade guide
(they would likely be in `tests/model_meta`), so I'm hesitant to make the
change for fear that it might break another case.
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:4>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by romgar):
I wanted to add the compatibility functions described in the upgrade guide
in one of my libraries to avoid copy/pasting this code in different
packages, and cover them with unit tests.
Do you think it makes sense to write a unit test for each situation (each
field "type", normal and reverse situations, inheritance, ...), or a big
one that will contains all field types / all situations in one go?
If I can write them in a way that can be helpful for Django, it could be
nice.
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:5>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I don't think we'll need to add the tests in Django since the old APIs are
removed in master at this point. I guess we can just update the docs and
see if any other reports come in.
About replicating the old APIs in a compatibility library, I understand
the rationale, but I'm not highly enthusiastic about it for the reason
pointed out in the docs:
Although it’s possible to make strictly equivalent replacements of the
old methods, that might not be the best approach. Taking the time to
refactor any field loops to make better use of the new API - and possibly
include fields that were previously excluded - will almost certainly
result in better code
The compatibility library would only be needed for Django 1.7 which is
unsupported.
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:6>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by romgar):
Thanks for the explanation, it makes totally sense to not add these tests
in Django.
Just to give you a bit of context why I wanted to create this backward-
compatible library, I had to handle a Django upgrade from 1.4 to 1.8 on a
big project, and I decided to make this project compatible between 1.4 and
1.X.
It made the whole process way easier by merging small chunks of upgrades
(if interested, I described my strategy there:
http://romgar.github.io/presentations/django_upgrade/).
And I thought it could have been helpful for other people in this kind of
situation.
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:7>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by romgar):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:8>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8be84e2ac42b2556fd6fa07794b3708b143ef341" 8be84e2a]:
{{{
#!CommitTicketReference repository=""
revision="8be84e2ac42b2556fd6fa07794b3708b143ef341"
Fixed #25461 -- Corrected meta API code examples to account for MTI.
In the case of multiple-table inheritance models,
get_all_related_objects() and
get_all_related_objects_with_model() don't return the auto-created
OneToOneField, but the new examples didn't account for this.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:9>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"ca7b926c8558f6e9bbd33a748bc53235b909e29a" ca7b926]:
{{{
#!CommitTicketReference repository=""
revision="ca7b926c8558f6e9bbd33a748bc53235b909e29a"
[1.9.x] Fixed #25461 -- Corrected meta API code examples to account for
MTI.
In the case of multiple-table inheritance models,
get_all_related_objects() and
get_all_related_objects_with_model() don't return the auto-created
OneToOneField, but the new examples didn't account for this.
Backport of 8be84e2ac42b2556fd6fa07794b3708b143ef341 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:10>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8976d08edbf34289b1d1dc5be02d15634b96c266" 8976d08]:
{{{
#!CommitTicketReference repository=""
revision="8976d08edbf34289b1d1dc5be02d15634b96c266"
[1.10.x] Fixed #25461 -- Corrected meta API code examples to account for
MTI.
In the case of multiple-table inheritance models,
get_all_related_objects() and
get_all_related_objects_with_model() don't return the auto-created
OneToOneField, but the new examples didn't account for this.
Backport of 8be84e2ac42b2556fd6fa07794b3708b143ef341 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:11>
Django
-------------------------------+------------------------------------
Reporter: ad-65 | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"2a49d8e9b25581dc601520023861a772e829c14b" 2a49d8e9]:
{{{
#!CommitTicketReference repository=""
revision="2a49d8e9b25581dc601520023861a772e829c14b"
[1.8.x] Fixed #25461 -- Corrected meta API code examples to account for
MTI.
In the case of multiple-table inheritance models,
get_all_related_objects() and
get_all_related_objects_with_model() don't return the auto-created
OneToOneField, but the new examples didn't account for this.
Backport of 8be84e2ac42b2556fd6fa07794b3708b143ef341 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25461#comment:12>