Add default GenericRelations for RevisionMixin and WorkflowMixin #11750
+67
−34
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not entirely related but fixes #11746.
For models that have multi-table inheritance, accessing a
GenericRelation will not always give you the correct results. This can
occur if the content type used by the GenericRelation does not match the
instance at hand, e.g. accessing a GenericRelation that uses
base_content_type from an instance of the specific class, or vice versa.
This is a known quirk when using GenericRelation together with MTI:
https://code.djangoproject.com/ticket/31269
To work around this, we need to have a method or property that resolves
to the correct GenericRelation (or the related model's QuerySet) to use,
depending on whether the current instance is specific or not. This
allows us to use and publicly document RevisionMixin.revisions as
something that always gives you the correct queryset of Revisions for
the object, without having to deal with the MTI quirks.
This is why the RevisionMixin has a
revisions
property that queriesthe Revision model directly, so that accessing MyModelInstance.revisions
always gives you the correct results, no matter how the GenericRelation
is set up for custom models.
For pages, we have access to the model implementation, so the
Page.revisions property is implemented using a GenericRelation that is
always accessed using a specific version of the instance.
For models that don't use MTI, we advise developers to just override the
revisions property with a GenericRelation definition. However, as of
django/django@faeb92e,
the existence of a @Property and a related field accessor of the same
name triggers a system check error.
It is unknown whether the system check error is the desired result or
not. However, the approach that we documented about shadowing the
mixins' default properties with a real GenericRelation clearly worked
before, so it is probably okay.
To work around this, we can use cached_property instead, which bypasses
the check, as Django only checks for the plain Python
property
.Additionally, we can suggest developers to never override the
revisions
andworkflow_states
withGenericRelation
s directly, andinstead define the
GenericRelation
s with other names, and override theproperty as a new property that returns the
GenericRelation
(with anyadditional logic as necessary, e.g. to handle MTI).Please describe the problem you're fixing here. Include the issue number, if applicable.