save_m2m() does not honor save_instance's exclude argument

[Margie Roginski]

save_m2m() does not seem to honor the exclude argument that is passed in to save_instance. I have a form that includes a particular m2m field, however I am trying to avoid saving that field. To do this I am setting self._meta.exclude explicitly, just prior to calling the form's save method. After saving the task, I do the standard save_m2m() to save the m2m fields. I find that non-m2m fields are correctly excluded from the save if specified in this way via self._meta.exclude, but that m2m fields are not excluded from the save if specified this way. This seems inconsistent.

I suppose it could be rejected as a bug based on the fact that exclude is a model meta attribute, and if used exactly as specified in the doc, I wouldn't have the field in my form at all, so this wouldn't come up.

Here is some background on what I am doing to make a case for fixing this. I have an app sort of like the admin app. Say I have a model called 'foo'. User A views it and user B views it. User A then changes field 'status', and saves it. User B then changes field 'priority' and saves it. I need to avoid having user B's save change the 'status' field, since user B didn't modify that field. I am using the hidden initial functionality to detect that user B has not changed the status field, and then I am setting self._meta.exclude to all fields in the form that were not changed by the user. This all works fine the fields are non-m2m fields, but breaks down when an m2m field is placed in self._meta.exclude.

Currently save_m2m() is defined like this:

    def save_m2m():
        opts = instance._meta
        cleaned_data = form.cleaned_data
        for f in opts.many_to_many:
            if fields and f.name not in fields:
                continue
            if f.name in cleaned_data:
                f.save_form_data(instance, cleaned_data[f.name])

I find that if I define it like this instead, the m2m fields correctly are omitted from the save if they are in exclude. This seems like more consistent and useful behavior to me.

    def save_m2m():
        opts = instance._meta
        cleaned_data = form.cleaned_data
        for f in opts.many_to_many:
            if fields and f.name not in fields:
                continue
            if exclude and f.name in exclude:   <=== added this if clause
                continue
            if f.name in cleaned_data:
                f.save_form_data(instance, cleaned_data[f.name]) 

[Russell Keith-Magee]

Your suspicion is correct - you can't dynamically change the Meta class and expect those changes to be reflected in the model.

[Margie Roginski]

Are you sure it even works to specify an m2m field in the exclude list? Since exclude is not even used in save_m2m, it seems it would not honor exclude even if specified statically.

In any case, I don't see why you wouldn't want to honor exclude on an m2m field when specified dynamically as I described in my example above. It seems like very inconsistent behavior to not honor exclude for an m2m field and it seems that a simple and backward compatible fix like the one I suggested to could be added to solve the problem.

I don't agree this should be closed, I hope you will reconsider it.

Thanks,
Margie

[James Bennett]

As Russ pointed out, the problem here is the technique you're trying to use: changing Meta partway through the life cycle of the form isn't, and won't be, supported. If you need to change the behavior of the form dynamically, you'll need to use a different approach (and for help on that try the django-users mailing list rather than the ticket tracker).

[brainrape@…]

I'm not trying to alter a form dynamically, but this bug still bites me, when i'm overriding a ManyToManyField with an intermediary model and adding it to exclude.
I'm attaching a patch.

[brainrape@…]

fix

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Stephen Burrows]

Just ran across this myself. My use case looks like this:

class MyOtherModel(Model):
    pass

class MyThroughModel(Model):
    other = models.ForeignKey(MyOtherModel)
    model = models.ForeignKey('MyModel')
    # Plus some metadata fields that are handled automatically.

class MyModel(Model):
    m2m = ManyToManyField(MyOtherModel, through=MyThroughModel)

class MyForm(ModelForm):
    m2m = ModelMultipleChoiceField(queryset=MyOtherModel.objects.all())

    class Meta:
        model = MyModel
        exclude = ('m2m',)

    def save(self, commit=True):
        # ... special behavior to handle the field.

With this setup, I am trying to exclude the default generated field and its behavior. In this example, the m2m field name is used in both the model definition and the form definition to represent a queryset of MyOtherModel instances.

However, I could theoretically exclude the field and put a CharField in its place. The current save_m2m code would then blindly try to assign the value of that CharField to the m2m field on the model.

This is because rather than explicitly checking whether the field is being excluded, the code assumes that if the field's name is in the cleaned_data, it was not excluded. This is not consistent with how fields and exclude are handled elsewhere in the codebase (for example in construct_instance).

As far as I can tell, this would have fit well in ​64ea5af/r8756, which added exclude support to the then-save_instance function. My guess is that it was left out as an oversight rather than intentionally.

[Stephen Burrows]

I've adjusted the patch slightly to also actually *pass* the exclude value from the form's Meta to the save_instance function.

[anonymous]

I've set this up patch up as a branch on github off master. Apparently I never added tests... so I'm not issuing a pull request yet.

​https://github.com/melinath/django/tree/ticket/12337

[Stephen Burrows]

I've issued a pull request for the changes.

[Stephen Burrows]

Forgot to link to the pull request: ​https://github.com/django/django/pull/413

[Tim Graham <timograham@…>]

In e2518fdf46a687454066b8a75263c9019b1e965d:

Fixed #12337 - Honor ModelForm.Meta.exclude when saving ManyToManyFields.

Thanks margieroginski for the report.