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 <guilherme.salgado@linaro.org>
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

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 @@
      <tr>
       <td></td>
       <td>
-       <input type="submit" name="action" value="Update"/>
+       <input type="submit" name="action" value="{{patchform.action}}"/>
       </td>
      </tr>
     </table>