Add create_from_super method and test #684
Conversation
bckohan
force-pushed
the
create_from_super
branch
from
53e1602 to
9542b55
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #684 +/- ##
==========================================
+ Coverage 75.39% 75.47% +0.08%
==========================================
Files 21 21
Lines 1345 1366 +21
Branches 212 216 +4
==========================================
+ Hits 1014 1031 +17
- Misses 259 260 +1
- Partials 72 75 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bckohan
force-pushed
the
create_from_super
branch
from
9542b55 to
f6c14ff
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a create_from_super method to the PolymorphicManager class that enables "recasting" an existing polymorphic model instance to a more specific subclass type. The method takes an instance of a parent class and creates a child class instance with the same database ID and inherited data, effectively converting the object down the inheritance hierarchy.
Key Changes:
- Adds
create_from_super()method toPolymorphicManagerfor model recasting functionality - Implements validation to ensure objects can only be cast to direct child classes
- Creates new test suite to verify recasting behavior across multiple inheritance levels
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| src/polymorphic/managers.py | Adds create_from_super manager method with validation, database operations, and content type updates to enable polymorphic recasting |
| src/polymorphic/tests/test_recasting.py | Adds test suite covering successful recasting from Model2C→Model2D and error cases for non-adjacent inheritance levels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_create_from_super(self): | ||
| # run create test 3 times because initial implementation | ||
| # would fail after first success. | ||
| for i in range(3): | ||
| mc = Model2C.objects.create( | ||
| field1="C1{}".format(i), field2="C2{}".format(i), field3="C3{}".format(i) | ||
| ) | ||
| mc.save() | ||
| field4 = "D4{}".format(i) | ||
| md = Model2D.objects.create_from_super(mc, field4=field4) | ||
| self.assertEqual(mc.id, md.id) | ||
| self.assertEqual(mc.field1, md.field1) | ||
| self.assertEqual(mc.field2, md.field2) | ||
| self.assertEqual(mc.field3, md.field3) | ||
| self.assertEqual(md.field4, field4) |
There was a problem hiding this comment.
The test doesn't verify that the returned object is actually an instance of Model2D (the target class). Consider adding self.assertIsInstance(md, Model2D) or self.assertEqual(type(md), Model2D) to ensure the recasting actually produces the correct type.
| def test_create_from_super(self): | ||
| # run create test 3 times because initial implementation | ||
| # would fail after first success. | ||
| for i in range(3): | ||
| mc = Model2C.objects.create( | ||
| field1="C1{}".format(i), field2="C2{}".format(i), field3="C3{}".format(i) | ||
| ) | ||
| mc.save() | ||
| field4 = "D4{}".format(i) | ||
| md = Model2D.objects.create_from_super(mc, field4=field4) | ||
| self.assertEqual(mc.id, md.id) | ||
| self.assertEqual(mc.field1, md.field1) | ||
| self.assertEqual(mc.field2, md.field2) | ||
| self.assertEqual(mc.field3, md.field3) | ||
| self.assertEqual(md.field4, field4) |
There was a problem hiding this comment.
The test doesn't verify that the polymorphic_ctype field is correctly updated to point to Model2D after the recasting operation. Consider adding a check like self.assertEqual(md.polymorphic_ctype, ContentType.objects.get_for_model(Model2D)) to ensure the content type is properly updated.
| mc = Model2C.objects.create( | ||
| field1="C1{}".format(i), field2="C2{}".format(i), field3="C3{}".format(i) | ||
| ) | ||
| mc.save() |
There was a problem hiding this comment.
The explicit mc.save() call after .create() is redundant since .create() already saves the object to the database. This line can be removed.
| mc.save() |
bckohan
force-pushed
the
create_from_super
branch
from
fa2d026 to
97c663f
Compare
|
It might be best to solve this upstream in Django - a solution to this would also work for polymorphic models because they are just multi-table models with a fancy manager. |
bckohan
force-pushed
the
create_from_super
branch
from
97c663f to
10bdd97
Compare
Co-authored-by: Joshua <jcoxwell@gmail.com> Co-authored-by: Brian Kohan <bckohan@gmail.com>
bckohan
force-pushed
the
create_from_super
branch
from
10bdd97 to
a5554d6
Compare
2 participants
This PR replaces #65
It needs some more work. I would like it to support casting across multiple levels of hierarchy.
The discussion in #65 and the details of this long standing Django ticket should be considered.
Fixes #296