From c24e22bb55d5bf6cf523fa9ad3f9b02e07e87566 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Fri, 3 Sep 2010 11:29:28 +0800 Subject: [PATCH] forms: change MultiplePatchForm from a ModelForm to a Form ModelForm was causing validation issues, especially with no-change fields on required model fields. Add a test for updating a required field (state) through MultiplePatchForm. Change it to a normal form, and call instance.setattr manually. This makes the new test pass. Signed-off-by: Jeremy Kerr --- apps/patchwork/forms.py | 31 +++++++++---------------------- apps/patchwork/tests/updates.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py index 2c137e3..1c5aeef 100644 --- a/apps/patchwork/forms.py +++ b/apps/patchwork/forms.py @@ -176,38 +176,22 @@ class MultipleBooleanField(forms.ChoiceField): def is_no_change(self, value): return value == self.no_change_choice[0] -class MultiplePatchForm(PatchForm): +class MultiplePatchForm(forms.Form): state = OptionalModelChoiceField(queryset = State.objects.all()) archived = MultipleBooleanField() def __init__(self, project, *args, **kwargs): - super(MultiplePatchForm, self).__init__(project = project, - *args, **kwargs) + super(MultiplePatchForm, self).__init__(*args, **kwargs) self.fields['delegate'] = OptionalDelegateField(project = project, required = False) - def _clean_fields(self): - super(MultiplePatchForm, self)._clean_fields() - # remove optional fields - opts = self.instance._meta - for f in opts.fields: - if not f.name in self.cleaned_data: - continue - - field = self.fields.get(f.name, None) - if field is None: - continue - - if field.is_no_change(self.cleaned_data[f.name]): - del self.cleaned_data[f.name] - def save(self, instance, commit = True): opts = instance.__class__._meta if self.errors: raise ValueError("The %s could not be changed because the data " "didn't validate." % opts.object_name) data = self.cleaned_data - # remove 'no change fields' from the data + # Update the instance for f in opts.fields: if not f.name in data: continue @@ -217,10 +201,13 @@ class MultiplePatchForm(PatchForm): continue if field.is_no_change(data[f.name]): - del data[f.name] + continue + + setattr(instance, f.name, data[f.name]) - return forms.save_instance(self, instance, - self._meta.fields, 'changed', commit) + if commit: + instance.save() + return instance class UserPersonLinkForm(forms.Form): email = forms.EmailField(max_length = 200) diff --git a/apps/patchwork/tests/updates.py b/apps/patchwork/tests/updates.py index 1a04c91..9a92ba0 100644 --- a/apps/patchwork/tests/updates.py +++ b/apps/patchwork/tests/updates.py @@ -72,6 +72,25 @@ class MultipleUpdateTest(TestCase): self.assertEquals(response.context['errors'], ['The submitted form data was invalid']) + def testDelegateChange(self): + delegate = create_maintainer(defaults.project) + data = {'action': 'Update', + 'project': str(defaults.project.id), + 'form': 'patchlistform', + 'archived': '*', + 'state': '*', + 'delegate': str(delegate.pk), + } + for patch in self.patches: + data['patch_id:%d' % patch.id] = 'checked' + + url = reverse('patchwork.views.patch.list', + args = [defaults.project.linkname]) + response = self.client.post(url, data) + self.failUnlessEqual(response.status_code, 200) + for patch in [Patch.objects.get(pk = p.pk) for p in self.patches]: + self.assertEquals(patch.delegate, delegate) + def tearDown(self): for p in self.patches: p.delete() -- 2.39.2