From e3f9cdfdfa96f5074a31fe2de0caf0ac52b1aff9 Mon Sep 17 00:00:00 2001 From: Guilherme Salgado Date: Tue, 12 Apr 2011 11:34:57 +0000 Subject: [PATCH] views: Refactor generic_list() to make it less complicated 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 Signed-off-by: Jeremy Kerr --- apps/patchwork/forms.py | 1 + apps/patchwork/utils.py | 47 +------------------ apps/patchwork/views/__init__.py | 73 ++++++++++++++++++++++------- templates/patchwork/patch-list.html | 2 +- 4 files changed, 59 insertions(+), 64 deletions(-) diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py index dc7a464..3d3abb3 100644 --- a/apps/patchwork/forms.py +++ b/apps/patchwork/forms.py @@ -195,6 +195,7 @@ class MultipleBooleanField(forms.ChoiceField): raise ValueError('Unknown value: %s' % value) class MultiplePatchForm(forms.Form): + action = 'update' state = OptionalModelChoiceField(queryset = State.objects.all()) archived = MultipleBooleanField() 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 fbe44f5..e4043bb 100644 --- a/apps/patchwork/views/__init__.py +++ b/apps/patchwork/views/__init__.py @@ -19,7 +19,7 @@ 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,30 +34,41 @@ 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 = Patch.objects.filter(id__in = get_patch_ids(request.POST)) + ps = Patch.objects.filter(id__in = get_patch_ids(data)) + + if action in bundle_actions: + errors = set_bundle(user, project, action, data, ps, context) + + elif properties_form and action == properties_form.action: + errors = process_multiplepatch_form(properties_form, user, + action, ps, context) + else: + errors = [] - (errors, form) = set_patches(request.user, project, action, \ - request.POST, ps, context) 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) @@ -77,10 +88,38 @@ def generic_list(request, project, view, context.update({ 'page': paginator.current_page, - 'patchform': form, + 'patchform': properties_form, 'project': project, 'order': order, }) return context + +def process_multiplepatch_form(form, user, action, patches, context): + errors = [] + if not form.is_valid() or action != form.action: + return ['The submitted form data was invalid'] + + if len(patches) == 0: + context.add_message("No patches selected; nothing updated") + return errors + + changed_patches = 0 + for patch in patches: + if not patch.is_editable(user): + errors.append("You don't have permissions to edit patch '%s'" + % patch.name) + continue + + changed_patches += 1 + form.save(patch) + + if changed_patches == 1: + context.add_message("1 patch updated") + elif changed_patches > 1: + context.add_message("%d patches updated" % changed_patches) + else: + context.add_message("No patches updated") + + return errors diff --git a/templates/patchwork/patch-list.html b/templates/patchwork/patch-list.html index 43e0550..770f005 100644 --- a/templates/patchwork/patch-list.html +++ b/templates/patchwork/patch-list.html @@ -186,7 +186,7 @@ - + -- 2.39.2