Opened 8 years ago
Last modified 7 years ago
#29346 closed New feature
Add "intermediary" kwarg to ModelForm._save_m2m — at Initial Version
| Reported by: | Douglas Denhartog | Owned by: | nobody |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | yes |
| Needs tests: | yes | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
This is my first ticket, so while I attempted to follow the applicable guidelines, I apologize in advance if this ticket is not a model guideline-following example.
Django's documentation clearly explain Intermediary Model limitations: "[u]nlike normal many-to-many fields, you can’t use add(), create(), or set() to create relationships"
Thus, when ModelForm.save() is called and an intermediary model field is not in self._meta.exclude, users will get an error like so:
Cannot set values on a ManyToManyField which specifies an intermediary model. Use app.IntermediaryModelName's Manager instead.
This presents complexity when the intermediary field is included in the form.
My proposal is to add a kwarg intermediary to ModelForm._save_m2m. This kwarg behaves exactly like self._meta.exclude within ModelForm._save_m2m.
I have already created a github branch with my solution, but have not created a pull request per contribution guidelines (but will do so if/when this ticket is approved). My solution adds three/four (3/4) lines of code, denoted in my solution below with EOL comment # ADDED. I believe my simple solution is more appropriate than a complex introspective analysis of every potential ManyToManyField's 'through' attribute.
The final code of my proposal is (in django.forms.models):
def _save_m2m(self, intermediary=None): # ADDED
"""
Save the many-to-many fields and generic relations for this form.
"""
# could assert/convert intermediary == str, list, tuple # ADDED
cleaned_data = self.cleaned_data
exclude = self._meta.exclude
fields = self._meta.fields
opts = self.instance._meta
# Note that for historical reasons we want to include also
# private_fields here. (GenericRelation was previously a fake
# m2m field).
for f in chain(opts.many_to_many, opts.private_fields):
if not hasattr(f, 'save_form_data'):
continue
if fields and f.name not in fields:
continue
if exclude and f.name in exclude:
continue
if intermediary and f.name in intermediary: # ADDED
continue # ADDED
if f.name in cleaned_data:
f.save_form_data(self.instance, cleaned_data[f.name])
And yes, this means adding the intermediary kwarg to ModelForm.save() so that intermediary can be passed to ModelForm._save_m2m.