Skip to content

Conversation

@bckohan
Copy link
Contributor

@bckohan bckohan commented Dec 4, 2025

Fixes #376 #390

I need to rebase and didn't have permissions to push to the original branch for #544, so this replaces that PR.

When polymorphic support was added to django-polymorphic circa 2013-2014 the design decision was to treat proxy models as concrete models wrt to polymorphism and to restrict the managers to those instances. This works because for_concrete_model=False is passed when the the ContentTypes are tagged onto the models - so if a row is created from the proxy model the proxy model's ContentType will be used to tag it. This is how you get situations described by #376:

class MyModel(PolymorphicModel):
    ...

class MyProxy(MyModel):
    class Meta:
        proxy = True

MyModel.objects.create()
MyProxy.objects.create()

MyModel.objects.count() == 2
MyProxy.objects.count() == 1
MyProxy.objects.non_polymorphic().count() == 2

I think this is pretty confusing default behavior because the main point of a proxy model is to provide a different python interface to an existing db row and this by default both treats the proxies as polymorphic extensions and restricts the querysets to rows created from the proxy instance. There is certainly a case for it, and Django allows the ContentType system to opt into proxy distinctions (thought not by default) - I just think it would have made more sense for the default behavior to be what people naturally should expect.

Where to go from here:

  1. I'm going to create a discussion question to continue debate about what to do for version 5 and if any breaking changes are warranted to the default behavior.
  2. I'm accepting this PR with some edits. It will allow users to toggle on more canonical behavior.

The new interface - which is now documented - will be:

class MyModel(PolymorphicModel):
    ...

class MyProxy(MyModel):
    class Meta:
        # this removes the instance_of from the default polymorphic queryset and
        # also toggles proxy on
        polymorphic_proxy = True  

MyModel.objects.create()
MyProxy.objects.create()

MyModel.objects.count() == 2
MyProxy.objects.count() == 2
MyProxy.objects.instance_of(MyProxy).count() == 1
MyProxy.objects.non_polymorphic().count() == 2

The other change that I'm going to add to @pgammans PR is for proxy models to behave like proxies in querysets as well. For example, from above:

for proxy in MyProxy.objects.all():
    assert isinstance(proxy, MyProxy)

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.57%. Comparing base (b934093) to head (88f30f9).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   75.39%   75.57%   +0.18%     
==========================================
  Files          21       21              
  Lines        1345     1355      +10     
  Branches      212      216       +4     
==========================================
+ Hits         1014     1024      +10     
  Misses        259      259              
  Partials       72       72              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new polymorphic__proxy meta option to control whether proxy models behave as traditional Django proxies (aliases) or as polymorphic proxies (distinct types). This addresses issues #376 and #390 by allowing developers to opt-in to traditional Django proxy behavior when needed.

Key Changes

  • Introduced polymorphic__proxy meta attribute that can be set to True (traditional Django proxy/alias) or False (polymorphic proxy with distinct type)
  • Modified the manager's queryset logic to respect the new attribute when applying instance_of filters
  • Added comprehensive test coverage with multiple proxy model variants to validate the new behavior

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/polymorphic/base.py Added metaclass logic to extract and store polymorphic__proxy from model Meta, with default based on proxy setting
src/polymorphic/managers.py Changed queryset logic from checking _meta.proxy to checking _meta.polymorphic__proxy to control instance filtering
src/polymorphic/tests/models.py Added 6 new test model classes demonstrating different proxy configurations and inheritance hierarchies
src/polymorphic/tests/test_orm.py Added 6 new test cases to validate polymorphic proxy behavior, traditional Django proxy behavior, and type conversion scenarios
src/polymorphic/tests/migrations/0001_initial.py Generated migration file for the new test models

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bckohan bckohan force-pushed the django-proxy-support branch 3 times, most recently from 27c6473 to a83683d Compare December 8, 2025 22:06
@bckohan bckohan force-pushed the django-proxy-support branch from a83683d to c626431 Compare December 9, 2025 18:33
@bckohan bckohan changed the title Add option to model meta class to prevent proxy models being polymorphic Add polymorphic_proxy flag to enable polymorphic queries from proxy models. Dec 9, 2025
@bckohan
Copy link
Contributor Author

bckohan commented Dec 10, 2025

On further consideration I'm bumping this PR to version 5 because I'm unsatisfied with any backwards compatible interface.

I do not believe by default concrete models should return proxy instances and that this behavior should be toggleable by using polymorphic_proxy flag.

@bckohan bckohan changed the title Add polymorphic_proxy flag to enable polymorphic queries from proxy models. Fix proxy model support. Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proxy model needs manager definition

2 participants