From patchwork Thu Apr 14 21:19:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guilherme Salgado X-Patchwork-Id: 91297 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (localhost [IPv6:::1]) by ozlabs.org (Postfix) with ESMTP id 18A05B6FE2 for ; Fri, 15 Apr 2011 07:19:53 +1000 (EST) Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by ozlabs.org (Postfix) with ESMTP id EA124B6FDA for ; Fri, 15 Apr 2011 07:19:48 +1000 (EST) Received: from youngberry.canonical.com ([91.189.89.112]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1QATx5-00004q-25; Thu, 14 Apr 2011 21:19:47 +0000 Received: from [187.126.166.24] (helo=feioso) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1QATx3-0008C2-QZ; Thu, 14 Apr 2011 21:19:47 +0000 Received: from localhost6.localdomain6 (localhost.localdomain [127.0.0.1]) by feioso (Postfix) with ESMTP id 6D6C2419D9; Thu, 14 Apr 2011 18:19:42 -0300 (BRT) Subject: [PATCH] Show the bulk edit form when the user can edit all patches in the list To: patchwork@lists.ozlabs.org From: Guilherme Salgado Date: Thu, 14 Apr 2011 18:19:42 -0300 Message-ID: <20110414211324.830.99901.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 We used to show that form only when the user is a maintainer of the project to which the patches in the list apply, but now the user just needs to have edit rights on all patches of the paginator's current page to see the form, which makes sense and as a bonus will make it possible to do bulk changes on the bundle view. Signed-off-by: Guilherme Salgado --- This is one of the changes I discussed with Jeremy earlier today (on IRC) to be able to use generic_list() for the 'my patches' view while keeping the ability to do bulk changes. I'm submitting this alone because it allows people to do bulk changes on the bundle view, which is a nice addition. Oh, and it uses the ObjectFactory I submitted earlier for its tests. apps/patchwork/tests/updates.py | 40 ++++++++++++++++++++++++++++++++++++++ apps/patchwork/views/__init__.py | 9 +++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/apps/patchwork/tests/updates.py b/apps/patchwork/tests/updates.py index ba1cb6d..e1b861e 100644 --- a/apps/patchwork/tests/updates.py +++ b/apps/patchwork/tests/updates.py @@ -20,10 +20,12 @@ from django.test import TestCase from django.core.urlresolvers import reverse from patchwork.models import Patch, Person, State +from patchwork.tests.factory import ObjectFactory from patchwork.tests.utils import defaults, create_maintainer class MultipleUpdateTest(TestCase): def setUp(self): + self.factory = ObjectFactory() defaults.project.save() self.user = create_maintainer(defaults.project) self.client.login(username = self.user.username, @@ -116,3 +118,41 @@ class MultipleUpdateTest(TestCase): response = self._testDelegateChange('') for p in self.patches: self.assertEquals(Patch.objects.get(pk = p.pk).delegate, None) + + def testFormShownWhenUserHasEditRightsOnPatches(self): + project = self.factory.makeProject() + user_person = Person.objects.get(user=self.user) + patch1 = self.factory.makePatch( + project=project, submitter=user_person) + patch2 = self.factory.makePatch( + project=project, submitter=user_person) + + # Our user has edit rights on both patches, but not on the project. + self.assertTrue(patch1.is_editable(self.user)) + self.assertTrue(patch2.is_editable(self.user)) + self.assertFalse(project.is_editable(self.user)) + + # Even though our user has no edit rights on the project, they can + # edit all patches in the list, so the form is shown. + url = reverse('patchwork.views.patch.list', args=[project.linkname]) + response = self.client.get(url) + self.assertContains( + response, self.properties_form_id, status_code=200) + + def testFormNotShownWhenUserHasNoEditRightsOnPatches(self): + project = self.factory.makeProject() + user_person = Person.objects.get(user=self.user) + patch1 = self.factory.makePatch( + project=project, submitter=user_person) + patch2 = self.factory.makePatch(project=project) + + # Here our user has edit rights on only one patch. + self.assertTrue(patch1.is_editable(self.user)) + self.assertFalse(patch2.is_editable(self.user)) + self.assertFalse(project.is_editable(self.user)) + + # Since the user can't edit one of the patches, the form is not shown. + url = reverse('patchwork.views.patch.list', args=[project.linkname]) + response = self.client.get(url) + self.assertNotContains( + response, self.properties_form_id, status_code=200) diff --git a/apps/patchwork/views/__init__.py b/apps/patchwork/views/__init__.py index bae40c6..aaf9ae8 100644 --- a/apps/patchwork/views/__init__.py +++ b/apps/patchwork/views/__init__.py @@ -42,9 +42,7 @@ def generic_list(request, project, view, if request.method == 'POST': data = request.POST user = request.user - properties_form = None - if project.is_editable(user): - properties_form = MultiplePatchForm(project, data = data) + properties_form = MultiplePatchForm(project, data = data) if request.method == 'POST' and data.get('form') == 'patchlistform': action = data.get('action', '').lower() @@ -59,7 +57,7 @@ def generic_list(request, project, view, if action in bundle_actions: errors = set_bundle(user, project, action, data, ps, context) - elif properties_form and action == properties_form.action: + elif action == properties_form.action: errors = process_multiplepatch_form(properties_form, user, action, ps, context) else: @@ -84,6 +82,9 @@ def generic_list(request, project, view, patches = patches.order_by(order.query()) paginator = Paginator(request, patches) + for patch in paginator.current_page.object_list: + if not patch.is_editable(user): + properties_form = None context.update({ 'page': paginator.current_page,