From patchwork Wed Feb 23 20:28:11 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guilherme Salgado X-Patchwork-Id: 84239 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bilbo.ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 7EEEDB742D for ; Thu, 24 Feb 2011 07:28:22 +1100 (EST) Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by ozlabs.org (Postfix) with ESMTP id BE6AEB73D5 for ; Thu, 24 Feb 2011 07:28:18 +1100 (EST) Received: from youngberry.canonical.com ([91.189.89.112]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1PsLJo-0001Q6-BY; Wed, 23 Feb 2011 20:28:16 +0000 Received: from [187.126.171.60] (helo=feioso) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1PsLJm-00028U-UO; Wed, 23 Feb 2011 20:28:15 +0000 Received: from localhost6.localdomain6 (localhost.localdomain [127.0.0.1]) by feioso (Postfix) with ESMTP id D880A41960; Wed, 23 Feb 2011 17:28:11 -0300 (BRT) Subject: [PATCH] Fix archiving/unarchiving of patches on patch lists. To: patchwork@lists.ozlabs.org From: Guilherme Salgado Date: Wed, 23 Feb 2011 17:28:11 -0300 Message-ID: <20110223202750.9291.46134.stgit@localhost6.localdomain6> User-Agent: StGit/0.15 MIME-Version: 1.0 Cc: patches@linaro.org X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org It was broken because MultipleBooleanField() was leaking string values instead of boolens as expected by MultiplePatchForm. --- 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() -