From patchwork Fri Feb 25 19:31:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guilherme Salgado X-Patchwork-Id: 84565 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 80D34B70FF for ; Sat, 26 Feb 2011 06:31:45 +1100 (EST) Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by ozlabs.org (Postfix) with ESMTP id 26E5DB70F5 for ; Sat, 26 Feb 2011 06:31:42 +1100 (EST) Received: from youngberry.canonical.com ([91.189.89.112]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1Pt3O7-0003iN-Mj; Fri, 25 Feb 2011 19:31:39 +0000 Received: from [187.126.147.231] (helo=feioso) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1Pt3O7-0002aP-1W; Fri, 25 Feb 2011 19:31:39 +0000 Received: from localhost6.localdomain6 (localhost.localdomain [127.0.0.1]) by feioso (Postfix) with ESMTP id 4D90A41DD8; Fri, 25 Feb 2011 16:31:35 -0300 (BRT) Subject: [PATCH] Refactor the generic_list() view to make it less complicated. To: patchwork@lists.ozlabs.org From: Guilherme Salgado Date: Fri, 25 Feb 2011 16:31:35 -0300 Message-ID: <20110225193135.9524.64622.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 When a form is submitted it now delegates to separate processing functions according to the action. Apart from being more readable it's now a lot easier to add extra forms for processing lists of patches. Signed-off-by: Guilherme Salgado --- 0 files changed, 0 insertions(+), 0 deletions(-) diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py index caa1949..cb7a94e 100644 --- a/apps/patchwork/forms.py +++ b/apps/patchwork/forms.py @@ -197,6 +197,7 @@ class MultipleBooleanField(forms.ChoiceField): raise ValueError('Unknown value: %s' % value) class MultiplePatchForm(forms.Form): + actions = ['update'] state = OptionalModelChoiceField(queryset = State.objects.all()) archived = MultipleBooleanField() @@ -205,7 +206,30 @@ class MultiplePatchForm(forms.Form): self.fields['delegate'] = OptionalDelegateField(project = project, required = False) - def save(self, instance, commit = True): + def process(self, user, action, patches, context): + errors = [] + if not self.is_valid() or action not in self.actions: + return ['The submitted form data was invalid'] + + for patch in patches: + if not patch.is_editable(user): + errors.append( + "You don't have permissions to edit patch '%s'" + % patch.name) + continue + + self.save(patch) + + if len(patches) == 1: + context.add_message("1 patch updated") + elif len(patches) > 1: + context.add_message("%d patches updated" % len(patches)) + else: + context.add_message("No patches selected; nothing updated") + + return errors + + def save(self, instance): opts = instance.__class__._meta if self.errors: raise ValueError("The %s could not be changed because the data " @@ -225,8 +249,7 @@ class MultiplePatchForm(forms.Form): setattr(instance, f.name, data[f.name]) - if commit: - instance.save() + instance.save() return instance class UserPersonLinkForm(forms.Form): diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py index 5df6404..e41ffb6 100644 --- a/apps/patchwork/utils.py +++ b/apps/patchwork/utils.py @@ -18,8 +18,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -from patchwork.forms import MultiplePatchForm -from patchwork.models import Bundle, Project, BundlePatch, UserProfile +from patchwork.models import Bundle, Project, BundlePatch from django.shortcuts import get_object_or_404 def get_patch_ids(d, prefix = 'patch_id'): @@ -138,47 +137,3 @@ def set_bundle(user, project, action, data, patches, context): bundle.save() return [] - - -def set_patches(user, project, action, data, patches, context): - errors = [] - form = MultiplePatchForm(project = project, data = data) - - try: - project = Project.objects.get(id = data['project']) - except: - errors = ['No such project'] - return (errors, form) - - str = '' - - # this may be a bundle action, which doesn't modify a patch. in this - # case, don't require a valid form, or patch editing permissions - if action in bundle_actions: - errors = set_bundle(user, project, action, data, patches, context) - return (errors, form) - - if not form.is_valid(): - errors = ['The submitted form data was invalid'] - return (errors, form) - - for patch in patches: - if not patch.is_editable(user): - errors.append('You don\'t have permissions to edit the ' + \ - 'patch "%s"' \ - % patch.name) - continue - - if action == 'update': - form.save(patch) - str = 'updated' - - - if len(patches) > 0: - if len(patches) == 1: - str = 'patch ' + str - else: - str = 'patches ' + str - context.add_message(str) - - return (errors, form) diff --git a/apps/patchwork/views/__init__.py b/apps/patchwork/views/__init__.py index 3f50380..c5ffeab 100644 --- a/apps/patchwork/views/__init__.py +++ b/apps/patchwork/views/__init__.py @@ -19,7 +19,8 @@ from base import * -from patchwork.utils import Order, get_patch_ids, set_patches +from patchwork.utils import ( + Order, get_patch_ids, bundle_actions, set_bundle) from patchwork.paginator import Paginator from patchwork.forms import MultiplePatchForm @@ -34,36 +35,45 @@ def generic_list(request, project, view, context.project = project order = Order(request.REQUEST.get('order'), editable = editable_order) - form = MultiplePatchForm(project) - - if request.method == 'POST' and \ - request.POST.get('form') == 'patchlistform': - action = request.POST.get('action', None) - if action: - action = action.lower() + # Explicitly set data to None because request.POST will be an empty dict + # when the form is not submitted, but passing a non-None data argument to + # a forms.Form will make it bound and we don't want that to happen unless + # there's been a form submission. + data = None + if request.method == 'POST': + data = request.POST + user = request.user + properties_form = None + if (user.is_authenticated() + and project in user.get_profile().maintainer_projects.all()): + properties_form = MultiplePatchForm(project, data = data) + + if request.method == 'POST' and data.get('form') == 'patchlistform': + action = data.get('action', '').lower() # special case: the user may have hit enter in the 'create bundle' # text field, so if non-empty, assume the create action: - if request.POST.get('bundle_name', False): + if data.get('bundle_name', False): action = 'create' ps = [] - for patch_id in get_patch_ids(request.POST): + for patch_id in get_patch_ids(data): try: patch = Patch.objects.get(id = patch_id) except Patch.DoesNotExist: pass ps.append(patch) - (errors, form) = set_patches(request.user, project, action, \ - request.POST, ps, context) + if action in bundle_actions: + errors = set_bundle(user, project, action, data, ps, context) + elif properties_form and action in properties_form.actions: + errors = properties_form.process(user, action, ps, context) + else: + errors = [] + if errors: context['errors'] = errors - if not (request.user.is_authenticated() and \ - project in request.user.get_profile().maintainer_projects.all()): - form = None - for (filterclass, setting) in filter_settings: if isinstance(setting, dict): context.filters.set_status(filterclass, **setting) @@ -83,7 +93,7 @@ def generic_list(request, project, view, context.update({ 'page': paginator.current_page, - 'patchform': form, + 'patchform': properties_form, 'project': project, 'order': order, })