Opened 15 years ago
Closed 9 years ago
#11560 closed New feature (fixed)
let proxy models multiple-inherit from the same concrete base model
Reported by: | Ryan Kelly | Owned by: | Akshesh Doshi |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently proxy models are required to have a single concrete base model class. Unfortunately this prevents me from combining several proxy subclasses of the same model. In my particular use case, I have two different apps that provide two different proxies of the standard User model. To successfully use both apps I need to create another subclass that combines the two, e.g:
class MyUser(App1User,App2User): class Meta: proxy = True
This gives a TypeError: "Proxy model 'MyUser' has more than one non-abstract model base class". But since App1User and App2User proxy the same underlying model, there's no ambiguity introduced by this multiple inheritance and I think it should be permitted.
Attached is a simple patch to make this work, by permitting additional concrete base classes if they are identical to the one that was already found.
Attachments (1)
Change History (18)
comment:1 by , 15 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|
comment:2 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:4 by , 14 years ago
Description: | modified (diff) |
---|---|
Needs documentation: | set |
Patch needs improvement: | set |
by , 14 years ago
Attachment: | multiple_proxy_models.diff added |
---|
comment:5 by , 14 years ago
I've redone the patch against latest trunk and added a brief note in the proxy models documentaion.
comment:8 by , 11 years ago
Hi rfk, thanks for the patch file, it works for me. Hope the feature will work on master branch.
comment:9 by , 10 years ago
This has been lurking around for a long time, any idea when this might merge into master? The patch seems to have unittest information now and the feature itself is working. We would much like to use this feature without having to patch django itself.
follow-up: 12 comment:10 by , 10 years ago
The patch needs to be updated to apply cleanly. If you can update it and send a pull request, that will help. Also a mention in the release notes and a .. versionchanged:: 1.8
annotation would be helpful. See our patch review checklist.
comment:11 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Version: | 1.1-beta → master |
comment:12 by , 9 years ago
Replying to timgraham:
The patch needs to be updated to apply cleanly. If you can update it and send a pull request, that will help. Also a mention in the release notes and a
.. versionchanged:: 1.8
annotation would be helpful. See our patch review checklist.
Hi Tim,
Is this issue already resolved by other works ? or I should work on the patch to implement it cleanly?
comment:14 by , 9 years ago
Owner: | changed from | to
---|
comment:15 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:16 by , 9 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | set |
Left a few comments for improvement.
The tests would need to be rewritten using unittests since this is now Django's preferred way.