#8018 closed (wontfix)
m2m intermediary models (both generic and custom)
| Reported by: | remo | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Keywords: | m2m intermediary | |
| Cc: | remo | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Ticket #6095 introduced the idea of custom intermediary models. Refractoring the m2m code to always use (generic) intermediary models would have benefits:
- a simplier implementation of m2m
- appropriate signals are always sent when a relation is created, modified or deleted (eg. #6778)
- extending the generic relations (using 'through=') would be easier
Of course, we must maintain compatibility to the m2m api, which consists (as far as I see it) of the ManyToManyField and the ManyRelatedManager. See the attachment for usage examples.
Attachments (1)
Change History (4)
by , 16 years ago
| Attachment: | m2m-example.py added |
|---|
comment:1 by , 16 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
The add() syntax you use for m2m-intermediate tables is problematic, for reasons discussed at length on #6095.
Other than that, I can't see anything nothing novel here that isn't already implemented by #6095, or proposed in #6778.
#6095 allows you to define the table used for m2m relations. If you want to do this explicitly rather than using the Django generated default - go right ahead. #6095 will let you do that.
#6778 doesn't require a wholesale refactoring of m2m - it requires a way to listen to signals.
comment:2 by , 16 years ago
russellm: It is not the intention of this proposal to add features over #6095 and #6778. The idea is to clean up the mess that m2m would be with 'through' and 'signals' patches.
In the current patch, 'throught' is handled as a separate case, althought it really is a custom intermediary model. The only way to fix #6778 right now is to do a quick and dirty hack on the ManyRelatedManager.
This is not to say that we shouldn't commit #6095 now or release a quick and dirty fix for #6778. But on the long run, and be it post 1.0, we should seriously consider to clean up this mess. It can be done by implementing what we really mean: a m2m relation has an intermediary model, which sends signals and can be customised.
So I suggest we reopen this ticket so that we can fix this eventually.
comment:3 by , 16 years ago
I'm not sure what you mean when you say "clean up this mess". What mess are you referring to? We have an implementation of m2m. That implementation works. It doesn't have any significant bugs that I'm aware of. It's not particularly difficult to understand the implementation. As of [8136], it allows you to specify intermediate models manually. The entry points for implementing #6778 are relatively straight forward, and don't require wholesale refactoring of the existing m2m code.
If you can show me a refactoring patch that demonstrates how the existing code can be made significantly cleaner, I'm happy to look at it. However, absent of that patch, you haven't provided any detail that convinces me that your proposal will significantly improve the implementation. I'm not even convinced that your proposal is even implementable - creating a mock model is a fine idea in principle, but it's still going to require all the ManyRelatedManagers etc to handle field lookup, and a model that doesn't have a concrete class will open a nest of vipers that I don't think you've fully explored.
usage examples