Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #16869: BaseGenericInlineFormSet.save_new should use form's save() method #1599

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 6 additions & 7 deletions django/contrib/contenttypes/generic.py
Expand Up @@ -415,13 +415,12 @@ def get_default_prefix(cls):
)) ))


def save_new(self, form, commit=True): def save_new(self, form, commit=True):
kwargs = { setattr(form.instance, self.ct_field.get_attname(),
self.ct_field.get_attname(): ContentType.objects.get_for_model( ContentType.objects.get_for_model(self.instance).pk)
self.instance, for_concrete_model=self.for_concrete_model).pk, setattr(form.instance, self.ct_fk_field.get_attname(),
self.ct_fk_field.get_attname(): self.instance.pk, self.instance.pk)
} return form.save(commit=commit)
new_obj = self.model(**kwargs)
return save_instance(form, new_obj, commit=commit)


def generic_inlineformset_factory(model, form=ModelForm, def generic_inlineformset_factory(model, form=ModelForm,
formset=BaseGenericInlineFormSet, formset=BaseGenericInlineFormSet,
Expand Down
32 changes: 31 additions & 1 deletion tests/generic_relations/tests.py
Expand Up @@ -306,6 +306,37 @@ def test_generic_inlineformset_factory(self):
form = Formset().forms[0] form = Formset().forms[0]
self.assertIsInstance(form['tag'].field.widget, CustomWidget) self.assertIsInstance(form['tag'].field.widget, CustomWidget)


def test_save_new_uses_form_save(self):
"""
Regression for #16260: save_new should call form's save
method.
"""

# Custom form used to check save was called
class SaveTestForm(forms.ModelForm):
def save(self, *args, **kwargs):
self.instance.saved_by = "custom method"
return super(SaveTestForm, self).save(*args, **kwargs)

Formset = generic_inlineformset_factory(
ForProxyModelModel, fields='__all__', form=SaveTestForm)

instance = ProxyRelatedModel.objects.create()

data = {
'form-TOTAL_FORMS': '1',
'form-INITIAL_FORMS': '0',
'form-MAX_NUM_FORMS': '',
'form-0-title': 'foo',
}

formset = Formset(data, instance=instance, prefix='form')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think prefix needs to be specified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it fails if I remove the prefix ManagementForm data is missing or has been tampered with

self.assertTrue(formset.is_valid())

new_obj = formset.save()[0]

self.assertEqual(new_obj.saved_by, "custom method")

def test_save_new_for_proxy(self): def test_save_new_for_proxy(self):
Formset = generic_inlineformset_factory(ForProxyModelModel, Formset = generic_inlineformset_factory(ForProxyModelModel,
fields='__all__', for_concrete_model=False) fields='__all__', for_concrete_model=False)
Expand Down Expand Up @@ -344,7 +375,6 @@ def test_save_new_for_concrete(self):
new_obj, = formset.save() new_obj, = formset.save()
self.assertNotIsInstance(new_obj.obj, ProxyRelatedModel) self.assertNotIsInstance(new_obj.obj, ProxyRelatedModel)



class ProxyRelatedModelTest(TestCase): class ProxyRelatedModelTest(TestCase):
def test_default_behavior(self): def test_default_behavior(self):
""" """
Expand Down