Mistake in model _meta API migration guide for multi-table inheritance

[ad-65]

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.

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)

[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?

[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]
...
[]

[Romain Garrigues]

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?

[Tim Graham]

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.

[Romain Garrigues]

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.

[Tim Graham]

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.

[Romain Garrigues]

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.

[Tim Graham <timograham@…>]

In 8be84e2a:

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.

[Tim Graham <timograham@…>]

In ca7b926:

[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

[Tim Graham <timograham@…>]

In 8976d08:

[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

[Tim Graham <timograham@…>]

In 2a49d8e9:

[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