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 <jk@ozlabs.org>

diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py
index 2c137e3c027618e12ff223755272f4451813689e..1c5aeef5394dd4e62de228229eb404de4ce0af31 100644 (file)
--- 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 1a04c91ff0aa87336f3d235357ab6414d7a135f6..9a92ba0f989248205b0ed1f247f748ac61517521 100644 (file)
--- 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()