[PATCH] Fix archiving/unarchiving of patches on patch lists.

[Guilherme Salgado]

It was broken because MultipleBooleanField() was leaking string values instead
of boolens as expected by MultiplePatchForm.

Signed-off-by: Guilherme Salgado <guilherme.salgado at linaro.org>
---

(resending with a signed-off-by line)

 apps/patchwork/forms.py         |   18 +++++++++
 apps/patchwork/tests/updates.py |   81 ++++++++++++++++++++++-----------------
 2 files changed, 64 insertions(+), 35 deletions(-)

diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py
index 72c2c42..caa1949 100644
--- a/apps/patchwork/forms.py
+++ b/apps/patchwork/forms.py
@@ -178,6 +178,24 @@ class MultipleBooleanField(forms.ChoiceField):
     def is_no_change(self, value):
         return value == self.no_change_choice[0]
 
+    # TODO: Check whether it'd be worth to use a TypedChoiceField here; I
+    # think that'd allow us to get rid of the custom valid_value() and
+    # to_python() methods.
+    def valid_value(self, value):
+        if value in [v1 for (v1, v2) in self.choices]:
+            return True
+        return False
+
+    def to_python(self, value):
+        if self.is_no_change(value):
+            return value
+        elif value == 'True':
+            return True
+        elif value == 'False':
+            return False
+        else:
+            raise ValueError('Unknown value: %s' % value)
+
 class MultiplePatchForm(forms.Form):
     state = OptionalModelChoiceField(queryset = State.objects.all())
     archived = MultipleBooleanField()
diff --git a/apps/patchwork/tests/updates.py b/apps/patchwork/tests/updates.py
index e5f175c..4352c18 100644
--- a/apps/patchwork/tests/updates.py
+++ b/apps/patchwork/tests/updates.py
@@ -17,12 +17,10 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-import unittest
 from django.test import TestCase
-from django.test.client import Client
 from django.core.urlresolvers import reverse
 from patchwork.models import Patch, Person, State
-from patchwork.tests.utils import defaults, create_maintainer, find_in_context
+from patchwork.tests.utils import defaults, create_maintainer
 
 class MultipleUpdateTest(TestCase):
     def setUp(self):
@@ -30,6 +28,13 @@ class MultipleUpdateTest(TestCase):
         self.user = create_maintainer(defaults.project)
         self.client.login(username = self.user.username,
                 password = self.user.username)
+        self.properties_form_id = 'patchform-properties'
+        self.url = reverse(
+            'patchwork.views.patch.list', args = [defaults.project.linkname])
+        self.base_data = {
+            'action': 'Update', 'project': str(defaults.project.id),
+            'form': 'patchlistform', 'archived': '*', 'delegate': '*',
+            'state': '*'}
         self.patches = []
         for name in ['patch one', 'patch two', 'patch three']:
             patch = Patch(project = defaults.project, msgid = name,
@@ -37,22 +42,41 @@ class MultipleUpdateTest(TestCase):
                             submitter = Person.objects.get(user = self.user))
             patch.save()
             self.patches.append(patch)
-        
-    def _testStateChange(self, state):
-        data = {'action':   'Update',
-                'project':  str(defaults.project.id),
-                'form':     'patchlistform',
-                'archived': '*',
-                'delegate': '*',
-                'state':    str(state),
-        }
+
+    def _selectAllPatches(self, data):
         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)
+    def testArchivingPatches(self):
+        data = self.base_data.copy()
+        data.update({'archived': 'True'})
+        self._selectAllPatches(data)
+        response = self.client.post(self.url, data)
+        self.assertContains(
+            response, self.properties_form_id, status_code = 200)
+        for patch in [Patch.objects.get(pk = p.pk) for p in self.patches]:
+            self.assertTrue(patch.archived)
+
+    def testUnArchivingPatches(self):
+        # Start with one patch archived and the remaining ones unarchived.
+        self.patches[0].archived = True
+        self.patches[0].save()
+        data = self.base_data.copy()
+        data.update({'archived': 'False'})
+        self._selectAllPatches(data)
+        response = self.client.post(self.url, data)
+        self.assertContains(
+            response, self.properties_form_id, status_code = 200)
+        for patch in [Patch.objects.get(pk = p.pk) for p in self.patches]:
+            self.assertFalse(patch.archived)
+
+    def _testStateChange(self, state):
+        data = self.base_data.copy()
+        data.update({'state': str(state)})
+        self._selectAllPatches(data)
+        response = self.client.post(self.url, data)
+        self.assertContains(
+            response, self.properties_form_id, status_code = 200)
         return response
 
     def testStateChangeValid(self):
@@ -74,20 +98,12 @@ class MultipleUpdateTest(TestCase):
                         'of the available choices.')
 
     def _testDelegateChange(self, delegate_str):
-        data = {'action':   'Update',
-                'project':  str(defaults.project.id),
-                'form':     'patchlistform',
-                'archived': '*',
-                'state':    '*',
-                'delegate': delegate_str
-        }
-        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)
+        data = self.base_data.copy()
+        data.update({'delegate': delegate_str})
+        self._selectAllPatches(data)
+        response = self.client.post(self.url, data)
+        self.assertContains(
+            response, self.properties_form_id, status_code=200)
         return response
 
     def testDelegateChangeValid(self):
@@ -100,8 +116,3 @@ class MultipleUpdateTest(TestCase):
         response = self._testDelegateChange('')
         for patch in [Patch.objects.get(pk = p.pk) for p in self.patches]:
             self.assertEquals(patch.delegate, None)
-
-    def tearDown(self):
-        for p in self.patches:
-            p.delete()
-