ManytoMany Field save_form_data has not support for through table

[Andrew Standley]

Greetings,
    I've been messing around with trying to update some custom forms to
instead be model forms, and I ran into something that struck me as odd.

ModelForm relies on the models fields to select a default form field,
and to save the returned form data. However the ManytoManyField does not
appear to have behavior for dealing with 'through' models. The
save_form_data just calls `set` on the ManyRelatedManager, which errors
out if a through model without auto_created=True is defined.

As far as I can tell I either have to write a lot of boiler plate in my
ModelForm (add a custom field, override the __init__, override save(),
etc), or define a custom ManyToManyField which overrides save_form_data.
This seems odd to me as the through model seems like the sensible place
for any behaviour required by the extra fields it defines.

The ManyToMany field has knowledge of everything needed to access the
intermediate model's default manager. It would make perfect sense to me
that to have to implement custom clear/add/remove methods on either the
model or it's manager for any through models that defined extra fields,
and then be able to plug-and-play with the ModelForm framework. So my
question is:

Is there any particular reason that save_form_data does not have some
default behavior for through models?

Cheers,
    Andrew



--
This message has been scanned by E.F.A. Project and is believed to be clean.

[Collin Anderson]

Hi Andrew,

Would allowing set() for through models help in your case? https://github.com/django/django/pull/8981

Thanks,

Collin

--
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 django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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/e1fe8354-e1d1-05a1-e433-6fcf7e46216a%40linear-systems.com.
For more options, visit https://groups.google.com/d/optout.

[Andrew Standley]

Hi Collin,
    Thanks, that would indeed help in my case.

The save_form_data method of the ManyToMany field would still eventually need to be updated to have the ability to pass the through_defaults kwarg to set(). However none of the extra fields on my intermediate model are required so that patch would definitely help clean up the code in my situation.

I'll patch our test server and see if everything runs smoothly and keep an eye that pull request.

Cheers,
    Andrew

On 1/16/2018 8:39 AM, Collin Anderson wrote:

Hi Andrew,

Would allowing set() for through models help in your case? https://github.com/django/django/pull/8981

Thanks,

Collin

On Mon, Jan 15, 2018 at 5:14 PM, Andrew Standley <asta...@linear-systems.com> wrote:

Greetings,
    I've been messing around with trying to update some custom forms to instead be model forms, and I ran into something that struck me as odd.

ModelForm relies on the models fields to select a default form field, and to save the returned form data. However the ManytoManyField does not appear to have behavior for dealing with 'through' models. The save_form_data just calls `set` on the ManyRelatedManager, which errors out if a through model without auto_created=True is defined.

As far as I can tell I either have to write a lot of boiler plate in my ModelForm (add a custom field, override the __init__, override save(), etc), or define a custom ManyToManyField which overrides save_form_data. This seems odd to me as the through model seems like the sensible place for any behaviour required by the extra fields it defines.

The ManyToMany field has knowledge of everything needed to access the intermediate model's default manager. It would make perfect sense to me that to have to implement custom clear/add/remove methods on either the model or it's manager for any through models that defined extra fields, and then be able to plug-and-play with the ModelForm framework. So my question is:

Is there any particular reason that save_form_data does not have some default behavior for through models?

Cheers,
    Andrew



--
This message has been scanned by E.F.A. Project and is believed to be clean.

--
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 django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/group/django-developers.
To view this discussion on the web visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/d/msgid/django-developers/e1fe8354-e1d1-05a1-e433-6fcf7e46216a%40linear-systems.com.
For more options, visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: 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 django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/group/django-developers.
To view this discussion on the web visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/d/msgid/django-developers/CAFO84S5crVgjpsNMiU8gPbs%3DYjBmDwrmgMVi9fjLqxBDa%2B7-0g%40mail.gmail.com.
For more options, visit MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: MailScanner has detected definite fraud in the website at "groups.google.com". Do not trust this website: https://groups.google.com/d/optout.

--
Andrew Standley
Senior Software Engineer
Linear Systems
(909) 899-4345 *225
asta...@linear-systems.com

[Andrew Standley]

Hi Collin,
  The patch seems to work perfectly with intermediate models without required fields. 

However looking over the pull request I noticed that there does not appears to be success tests for intermediate models with extra required fields.
Is this something I could help out with?

Cheers,
  Andrew