It does seem like a somewhat arbitrary historical restriction. Collin's PoC change is surprisingly small and simple.
Seems like it would be fine if Django allowed add() and let any errors > about missing data bubble-up. > Agree, this is also a precedent from get_or_create. > I personally think passing in a defaults dict (just like get_or_create > does) would also be fine, but a callback seems like overkill. > This is a more consistent approach to the callback. One could always use custom logic in the through model's save method, or a signal, to achieve things that can't be done with through_defaults. On 21 March 2017 at 00:46, Collin Anderson <[email protected]> wrote: > Hi, > > Check out https://code.djangoproject.com/ticket/9475 > > Seems like it would be fine if Django allowed add() and let any errors > about missing data bubble-up. I personally think passing in a defaults dict > (just like get_or_create does) would also be fine, but a callback seems > like overkill. > > Here's a proof of concept: https://github.com/ > django/django/compare/master...collinanderson:ticket9475 > > Collin > > > On Mon, Mar 20, 2017 at 5:46 PM, Luis Masuelli <[email protected]> > wrote: > >> I was reading this link in the official history >> <https://code.djangoproject.com/ticket/6095#comment:31> and this other >> link in this group >> <https://groups.google.com/forum/#!topic/django-developers/J-ycVFfgb4E>, >> but still do not understand why the issue against a way to call .add() to >> add a through model. I thought several approaches (they are all backward >> compatible for the end-user) that could work: >> >> 1. Avoid the restriction to call add() if the model has only the two >> FK fields. >> 2. An additional way to call .add() specifying additional fields to >> fill (this one is not mine; was discussed in the linked thread). You risk >> getting a (wrapped?) exception if you do not populate other fields >> appropriately. >> 3. (This one was the first I thought and perhaps the easiest to >> implement) Allow the ManyToManyField to specify a kwarg like >> `through_factory=` which expects a callable which would populate more >> data. >> The restriction to call .add() would remain if no `through_factory=` is >> specified. >> 4. Avoid the restriction to call delete() if the model has only the >> two FK fields. >> >> I considered these cases: >> >> - It is quite trivial a model with only two fields, but perhaps the >> intermediate models could have additional useful methods. I see no trouble >> having such model and allowing .add() or .delete() calls there. >> - Having a special factory method would allow calls to .add() since >> we'd be providing a way to make .add() actually know how to create an >> instance of the intermediate model. >> - You can combine these points, implement one, none, or all of them. >> - (I did not consider extended use cases for delete() intentionally, >> since perhaps a strange model (with different field pairs) could fit in >> different relationships, although I cannot think in a case with multiple >> relationships not incurrin in, perhaps, duplicate data when tried to be >> used as through= model, but anyway I prefer to keep silence for other >> cases >> involving delete(), but the case I stated). >> - It is up to the user to be careful regarding migrating an >> intermediate model regarding adding, changing, or deleting fields. But >> anyway, this applies to any modification in the database models right now. >> - Points 1, 2, 3 and/or 4 could be implemented with no clash. A >> combined approach of 1, 2, 3 would look like this (this flow only applies >> for the case when the through= is present - such scenario right now only >> consists of raising an exception; the case with no through= model would >> not >> be affected at all): >> - Instantiate `instance = ThroughModel(fka=a, fkb=b, >> **kwargs_from_add)` with the respective model instances a and b, from >> classes A and B which hold the desired M2M relationship. In this case, >> the >> point 2 just adds the **kwargs_from_add. If point 2 is not >> implemented, no >> **kwargs_from_add would be present. >> - (*If point 3 is implemented*) Call the callable in >> `through_factory=` invoking it `like_this(instance)`, if the callable >> is >> present. It is expected to save the instance. >> - (*If either point 1 is implemented and the model has only two >> fields, or point 2 is implemented*) Manually save the instance (if >> point 3 was not implemented or it was but the factory callable was not >> specified). (*Otherwise - point 2 not implemented AND (point 1 not >> implemented or model with more than two fields*)) Raise the >> currently implemented exception for the .add() method with through= >> specified (with a different string message) because the >> through_factory was >> not present, and so the framework does not know how to populate >> additional >> fields. >> - Catch-and-reraise (or don't catch at all and let them be) the >> error for missing data in the tried-to-save model. >> - An example of the callable in point 3 would be like this (just an >> example for, say, a game!): >> - def on_added(through_model_instance): >> through_model_instance.balance = 1000.0 #although this one >> could be a default value at db level. >> through_model_instance.save() >> through_model_instance.achievements.create(tag='joined-this- >> relation') >> >> I understand these approaches, combined or not, could not be perfect or >> bug-free. But I'd like to discuss them instead of plainly discard them. >> AFAIK right now the calls to .add() are disallowed when through is >> specified. >> >> Could we work or at least discuss these use cases now? The history I >> quoted seems to be pretty.... historic, and we're in 2017. The first thing >> I'd like to know right not (with django 1.10 alive) is the feasability of >> this feature in a future django version. >> >> I await your comments and/or criticism ^^. >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Django developers (Contributions to Django itself)" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To post to this group, send email to [email protected]. >> Visit this group at https://groups.google.com/group/django-developers. >> To view this discussion on the web visit https://groups.google.com/d/ms >> gid/django-developers/ec5ffdfc-ef4b-4c30-a53d-d427cbe053e2% >> 40googlegroups.com >> <https://groups.google.com/d/msgid/django-developers/ec5ffdfc-ef4b-4c30-a53d-d427cbe053e2%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> For more options, visit https://groups.google.com/d/optout. >> > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/django-developers. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/django-developers/CAFO84S73z%2BXW4xj9YKmHNXu4KQ% > 2Be%3DEKqFfXJeK5JyCSXGANGfQ%40mail.gmail.com > <https://groups.google.com/d/msgid/django-developers/CAFO84S73z%2BXW4xj9YKmHNXu4KQ%2Be%3DEKqFfXJeK5JyCSXGANGfQ%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- Adam -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM2O-hDvy3gnJTx02gpqZz%2BWJ6uGS0cK98n9492Pijyp9w%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
