Calling save_m2m() in UserCreationForm.save()

2022-11-23 Thread Mark Gensler
Hello all!

I noticed that when calling contrib.auth.forms.UserCreationForm.save(), 
self.save_m2m() is not called when commit=True.

This caused an issue in one of my projects which uses a custom User model 
with ManyToMany fields. Is this something that should be changed? I saw in 
the docs [1] that it is advised to extend UserCreationForm and 
UserChangeForm if using a custom User model.

A few reasons I see to add this step to the UserCreationForm.save() method:

   - UserCreationForm is a subclass of ModelForm, which does call 
   save_m2m() when commit=True.
   - UserChangeForm *does* call save_m2m() as part of save(), because the 
   save() method is not overloaded. This seems inconsistent!

The solution I'd propose is:

class UserCreationForm(forms.ModelForm):
...
def save(self, commit=True):
user = super().save(commit=False)
user.set_password(self.cleaned_data["password1"])
if commit:
user.save()
*if hasattr(self, "save_m2m"):*
*self.save_m2m()*
return user

I'd be happy to raise a ticket and work on a patch if this change would be 
useful.

Thanks
Mark

[1] 
https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c8dac66b-4a0f-4afa-b548-260fffb06e9fn%40googlegroups.com.


Re: Calling save_m2m() in UserCreationForm.save()

2022-11-27 Thread Mark Gensler
Hi Adam, thanks for the reply. I'll open a ticket and start work on a PR. 
Mark

On Sunday, November 27, 2022 at 10:47:01 AM UTC Adam Johnson wrote:

> Your proposal seems reasonable - if actually saving, we should save the 
> m2m fields too.
>
> I think the best next step would be to file a ticket and work on a PR. The 
> first step would be to add a test case reproducing the issue.
>
> On Wed, Nov 23, 2022 at 4:59 PM Mark Gensler  wrote:
>
>> Hello all!
>>
>> I noticed that when calling contrib.auth.forms.UserCreationForm.save(), 
>> self.save_m2m() is not called when commit=True.
>>
>> This caused an issue in one of my projects which uses a custom User model 
>> with ManyToMany fields. Is this something that should be changed? I saw in 
>> the docs [1] that it is advised to extend UserCreationForm and 
>> UserChangeForm if using a custom User model.
>>
>> A few reasons I see to add this step to the UserCreationForm.save() 
>> method:
>>
>>- UserCreationForm is a subclass of ModelForm, which does call 
>>save_m2m() when commit=True.
>>- UserChangeForm *does* call save_m2m() as part of save(), because 
>>the save() method is not overloaded. This seems inconsistent!
>>
>> The solution I'd propose is:
>>
>> class UserCreationForm(forms.ModelForm):
>> ...
>> def save(self, commit=True):
>> user = super().save(commit=False)
>> user.set_password(self.cleaned_data["password1"])
>> if commit:
>> user.save()
>> *if hasattr(self, "save_m2m"):*
>> *self.save_m2m()*
>> return user
>>
>> I'd be happy to raise a ticket and work on a patch if this change would 
>> be useful.
>>
>> Thanks
>> Mark
>>
>> [1] 
>> https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms
>>
>> -- 
>> 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 view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/c8dac66b-4a0f-4afa-b548-260fffb06e9fn%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/django-developers/c8dac66b-4a0f-4afa-b548-260fffb06e9fn%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e96fff49-becc-476c-a7fc-5b4c00c7b8d7n%40googlegroups.com.


Passing initial data to model formsets - a suggested improvement

2018-09-06 Thread Mark Gensler
Initial data for modelformsets is treated in an odd way, which was outlined 
and discussed in the issues below

https://code.djangoproject.com/ticket/16995#comment:1
https://code.djangoproject.com/ticket/29739 (submitted by me)

In summary, if the initial data provided to BaseModelFormSet.__init__ is 
longer than the number of extra forms, the excess data won't be rendered in 
forms.

This causes problems when 
a) writing abstract code which constructs the class separately from 
initialising it, or
b) the code reuses the same class with differing initial data. 

In these cases the length of the initial data may not be known at the time 
of class construction. 

In order to minimise regressions I'd suggest that model formsets use 
max(self.extra, len(self.initial_extra)) in place of self.extra when 
calculating total_form_count().

I can't see this causing major regressions as if a user would like to limit 
the length of the forms they should already be doing so with the `max_num` 
parameter to the modelformset_factory. Othewrise, why not use the 
additional data in self.initial_extra?

Mark

-- 
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+unsubscr...@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/a0b827c5-b06f-43bb-83d1-bb89e1a6e78a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.