[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