[PATCH V2, 1/2] Refactor the generic_list() view to make it less complicated.
Guilherme Salgado
guilherme.salgado at linaro.org
Wed Apr 13 07:34:57 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>
---
This is a new version of the patch I've submitted previously. It contains all
the changes suggested by Jeremy but there's also a second patch in the series
with a small refactoring of permission checks.
apps/patchwork/forms.py | 1 +
apps/patchwork/utils.py | 47 +-----------------------
apps/patchwork/views/__init__.py | 74 +++++++++++++++++++++++++++++---------
3 files changed, 59 insertions(+), 63 deletions(-)
diff --git a/apps/patchwork/forms.py b/apps/patchwork/forms.py
index caa1949..2c57c08 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):
+ 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..f66a2bf 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,30 +35,40 @@ 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,39 @@ 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..12c5d0c 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>
More information about the Patchwork
mailing list