[PATCH] Refactor the generic_list() view to make it less complicated.

Guilherme Salgado guilherme.salgado at linaro.org
Sat Feb 26 06:31:35 EST 2011


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 <guilherme.salgado at linaro.org>
---
 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,
             })



More information about the Patchwork mailing list