It was broken because MultipleBooleanField() was leaking string values instead of boolens as expected by MultiplePatchForm.
Signed-off-by: Guilherme Salgado <[email protected]> --- (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() - _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
