Conversation
…morphicModel __new__. This ensures that related objects will use the polymorphic manager even in inheritance scenarios where non-polymorphic base models are higher in the MRO. Co-authored-by: Matt Covalt <mcovalt@mailbox.org> Co-authored-by: Brian Kohan <bckohan@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #679 +/- ##
==========================================
+ Coverage 75.50% 75.53% +0.03%
==========================================
Files 21 21
Lines 1343 1345 +2
Branches 212 213 +1
==========================================
+ Hits 1014 1016 +2
Misses 257 257
Partials 72 72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #437 where foreign keys to polymorphic models could incorrectly resolve to the parent class in multiple inheritance scenarios. The fix ensures that when a polymorphic model inherits from both an abstract non-polymorphic model and PolymorphicModel, the polymorphic manager is correctly set as the base manager for related field queries.
- Adds automatic setting of
base_manager_nameto "objects" in PolymorphicModelBase metaclass when it's None - Adds a test case using multiple inheritance with an abstract model to verify the fix
- Updates migrations to reflect that
base_manager_nameis now set automatically rather than explicitly
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/polymorphic/base.py | Adds logic to set base_manager_name to "objects" if None during model class creation, preventing issues with multiple inheritance |
| src/polymorphic/tests/models.py | Introduces RelationAbstractModel and modifies RelationBase to inherit from it, creating a test case for the multiple inheritance scenario |
| src/polymorphic/tests/test_orm.py | Adds .order_by("pk") to test query to ensure deterministic ordering |
| src/polymorphic/tests/migrations/0001_initial.py | Removes explicit base_manager_name settings for RelationBase and its subclasses since it's now set automatically |
| docs/changelog.rst | Documents the fix for issue #437 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
This PR sets base_model_manager to "objects" if it is None during PolygmorphicModel class creation. This ensures that even if a non-polymorphic Meta class is higher in the MRO of a polymorphic model it doesn't trigger the use of a vanilla Manager for the polymorphic model when it is referenced from another model.
Fixes #437
Replaces #438 - I had trouble force pushing to that branch.