add() and remove() on ManyToManyField with through model
George Sakkis
remove() and add() for ManyToManyFields with 'through' model is
necessary.
For remove, the docs say "The remove method is disabled for similar
reasons (to add)" but it's not clear why. All it is required for
"beatles.members.remove(john)" to work are the two foreign keys; the
extra data of the relationship are not needed.
But even for add() the check is restrictive. For one thing, a through
model might be defined just to add or override some methods, without
any other fields than the two foreign keys. Or, more likely, all
additional fields may not require initialisation (nullable, default,
auto_now, etc.). These cases would just work without any change in the
API. A backwards compatible change to add() would allow even extra
required fields: "add(*objs, **relationship_attrs)". Thus the example
in the docs:
Membership.objects.create(person=paul, group=beatles,
date_joined=date(1960, 8,
1),
invite_reason= "Wanted to
form a band.")
could be written as:
beatles.members.add(paul, date_joined=date(1960, 8, 1), reason=
"Wanted to form a band.")
Thoughts ?
George
Russell Keith-Magee
This was discussed at the time that m2m-intermediate models were
added. If you want some of the history, see the ticket discussion on
#6095 (in particular, around here [1]).
[1] http://code.djangoproject.com/ticket/6095#comment:31
There are are a couple of issues with the approach you describe.
They're not necessarily insurmountable issues, but they were enough of
a problem for us to make the decision to punt them until later in the
interests of delivering basic m2m-intermediate functionality.
Now that we've been living with m2m-intermediates for a while (almost
2 years), the time may be ripe to revisit these dangling issues. If
you want to make a proposal in this area, feel free to do so -- just
make sure you address the history when you do. It's not necessarily as
simple as just adding kwargs to the add() method.
For the record, the same approach was also taken for admin handling of
m2m intermediates; however, with the refactoring of m2m that occurred
in order to support multi-db, those admin issues were resolved.
Yours,
Russ Magee %-)
George Sakkis
wrote:
sure I spotted any particularly unsolvable problems. Just to clarify,
I am interested in the following three issues, more or less
independently from each other:
1) remove(): The only related comment I read in the ticket is "There
is an analogous problem with remove - since you can now have multiple
relations between objects, a simple remove method is no longer an
option.". But remove() is a method of a descriptor referring to a
specific m2m relation, so it is clear to which one it refers to. E.g.
in your example:
a_group.club_membership.remove(a_person) # remove person from
club_members
a_group.team_membership.remove(a_person) # remove person from
team_members.
If the issue had to do specifically with the fact that that the same
through model (Membership) was used between Group and Person more than
once, I am not interested in allowing this; the current validation
approach is fine.
2) add() for m2m-intermediates with only null/default/derived extra
fields. If I'm reading your comments correctly, you were against
adding null=True to fields that are not semantically nullable, just as
a workaround for add(). I agree and I'm not suggesting adding nulls or
defaults just for the sake of making add() work, but for fields that
should accept null or a default regardless, why prevent them ? As for
the implementation, I'm not suggesting any 'compile time' validation
and introspection; just let any errors happen when add() eventually
calls through.create(...).
3) add() for m2m-intermediates with required extra fields. The
suggestion in the ticket is to pass an 'extra' dict argument; I'm fine
with that instead of passing them as **extra; it has the added benefit
it can be used in create() as well (although I'm less interested in
allowing create). Fully agree with your comments about not adding more
fine-grained, per-object extra arguments (you can always use multiple
calls to add()) or an additional "add_extra" method.
If there are any other issues I ignored, let me know and I'll be happy
to consider them.
George