From 3bde0a8af92187462327142190b14d8f5909ed07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Wed, 19 Sep 2012 03:10:15 +0200 Subject: [PATCH 1/5] Use Class-based View to support or unsupport an application. Use QuestionMixin for a confirm message --- openslides/application/models.py | 2 ++ openslides/application/urls.py | 6 ++-- openslides/application/views.py | 55 +++++++++++++++++++------------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/openslides/application/models.py b/openslides/application/models.py index df1d8195f..e1c6c3555 100644 --- a/openslides/application/models.py +++ b/openslides/application/models.py @@ -263,6 +263,7 @@ class Application(models.Model, SlideMixin): if not self.is_supporter(person): ApplicationSupporter(application=self, person=person).save() self.writelog(_("Supporter: +%s") % (person)) + # TODO: Raise a precise exception for the view in else-clause def unsupport(self, person): """ @@ -274,6 +275,7 @@ class Application(models.Model, SlideMixin): try: object = self.applicationsupporter_set.get(person=person).delete() except ApplicationSupporter.DoesNotExist: + # TODO: Don't do nothing but raise a precise exception for the view pass else: self.writelog(_("Supporter: -%s") % (person)) diff --git a/openslides/application/urls.py b/openslides/application/urls.py index 3b3e83e9e..a6eaec924 100644 --- a/openslides/application/urls.py +++ b/openslides/application/urls.py @@ -13,7 +13,7 @@ from django.conf.urls.defaults import url, patterns from openslides.application.views import (ApplicationDelete, ViewPoll, - ApplicationPDF, ApplicationPollPDF, CreateAgendaItem) + ApplicationPDF, ApplicationPollPDF, CreateAgendaItem, SupportView) urlpatterns = patterns('openslides.application.views', url(r'^$', @@ -99,12 +99,12 @@ urlpatterns = patterns('openslides.application.views', ), url(r'^(?P\d+)/support/$', - 'support', + SupportView.as_view(unsupport=False, answer_url='support/'), name='application_support', ), url(r'^(?P\d+)/unsupport/$', - 'unsupport', + SupportView.as_view(unsupport=True, answer_url='unsupport/'), name='application_unsupport', ), diff --git a/openslides/application/views.py b/openslides/application/views.py index 87eca0f1a..310d8c0d3 100644 --- a/openslides/application/views.py +++ b/openslides/application/views.py @@ -41,7 +41,8 @@ from openslides.utils.pdf import stylesheet from openslides.utils.template import Tab from openslides.utils.utils import (template, permission_required, del_confirm_form, gen_confirm_form) -from openslides.utils.views import PDFView, RedirectView, DeleteView, FormView +from openslides.utils.views import (PDFView, RedirectView, DeleteView, + FormView, SingleObjectMixin, QuestionMixin) from openslides.utils.person import get_person from openslides.config.models import config @@ -363,32 +364,40 @@ def reset(request, application_id): return redirect(reverse('application_view', args=[application_id])) -@permission_required('application.can_support_application') -@template('application/view.html') -def support(request, application_id): +class SupportView(RedirectView, SingleObjectMixin, QuestionMixin): """ - support an application. + Support or unsupport an application """ - try: - Application.objects.get(pk=application_id).support(person=request.user) - messages.success(request, _("You have support the motion successfully.") ) - except Application.DoesNotExist: - pass - return redirect(reverse('application_view', args=[application_id])) + permission_required = 'application.can_support_application' + model = Application + pk_url_kwarg = 'application_id' # TODO: Is this line neccessary? + unsupport = False # Must be given in SupportView.as_view() + answer_url = None # Must be given in SupportView.as_view() + def get_question(self): + if not self.unsupport: + return _('Do you really want to support this motion?') + else: + return _('Do you really want to unsupport this motion?') -@permission_required('application.can_support_application') -@template('application/view.html') -def unsupport(request, application_id): - """ - unsupport an application. - """ - try: - Application.objects.get(pk=application_id).unsupport(person=request.user) - messages.success(request, _("You have unsupport the motion successfully.") ) - except Application.DoesNotExist: - pass - return redirect(reverse('application_view', args=[application_id])) + # TODO: Why do we have to overwrite this method? + def pre_redirect(self, request, *args, **kwargs): + self.confirm_form() + + def pre_post_redirect(self, request, *args, **kwargs): + if self.get_answer().lower() == 'yes': + if not self.unsupport: + Application.objects.get(pk=kwargs['application_id']).support(person=request.user) + # Should the Exception Application.DoesNotExist be kept or not? + self.success_message = _("You have supported this motion successfully.") + else: + Application.objects.get(pk=kwargs['application_id']).unsupport(person=request.user) + # Should the Exception Application.DoesNotExist be kept or not? + self.success_message = _("You have unsupported this motion successfully.") + messages.success(request, self.success_message) + + def get_redirect_url(self, **kwargs): + return reverse('application_view', args=[kwargs['application_id']]) @permission_required('application.can_manage_application') From c036be05bfaef40a3f5618fc361895e9a6396e1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Tue, 23 Oct 2012 11:54:18 +0200 Subject: [PATCH 2/5] Fix new class based view for support/unsupport motions. Fix strange error in QuestionMixin. --- openslides/application/views.py | 16 +++++----------- openslides/utils/views.py | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/openslides/application/views.py b/openslides/application/views.py index 310d8c0d3..8afb73921 100644 --- a/openslides/application/views.py +++ b/openslides/application/views.py @@ -364,13 +364,13 @@ def reset(request, application_id): return redirect(reverse('application_view', args=[application_id])) -class SupportView(RedirectView, SingleObjectMixin, QuestionMixin): +class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): """ Support or unsupport an application """ permission_required = 'application.can_support_application' model = Application - pk_url_kwarg = 'application_id' # TODO: Is this line neccessary? + pk_url_kwarg = 'application_id' unsupport = False # Must be given in SupportView.as_view() answer_url = None # Must be given in SupportView.as_view() @@ -380,24 +380,18 @@ class SupportView(RedirectView, SingleObjectMixin, QuestionMixin): else: return _('Do you really want to unsupport this motion?') - # TODO: Why do we have to overwrite this method? - def pre_redirect(self, request, *args, **kwargs): - self.confirm_form() - def pre_post_redirect(self, request, *args, **kwargs): if self.get_answer().lower() == 'yes': if not self.unsupport: - Application.objects.get(pk=kwargs['application_id']).support(person=request.user) - # Should the Exception Application.DoesNotExist be kept or not? + self.get_object().support(person=request.user) self.success_message = _("You have supported this motion successfully.") else: - Application.objects.get(pk=kwargs['application_id']).unsupport(person=request.user) - # Should the Exception Application.DoesNotExist be kept or not? + self.get_object().unsupport(person=request.user) self.success_message = _("You have unsupported this motion successfully.") messages.success(request, self.success_message) def get_redirect_url(self, **kwargs): - return reverse('application_view', args=[kwargs['application_id']]) + return reverse('application_view', args=[kwargs[self.pk_url_kwarg]]) @permission_required('application.can_manage_application') diff --git a/openslides/utils/views.py b/openslides/utils/views.py index becdbfc8b..363bb2d90 100644 --- a/openslides/utils/views.py +++ b/openslides/utils/views.py @@ -143,7 +143,7 @@ class QuestionMixin(object): 'option_fields': option_fields}) def pre_redirect(self, request, *args, **kwargs): - self.confirm_form(request, self.object) + self.confirm_form() def pre_post_redirect(self, request, *args, **kwargs): messages.success(request) From 2e41dcdefea4c765542c4123ef95a0a33d242bae Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Thu, 25 Oct 2012 10:15:41 +0200 Subject: [PATCH 3/5] Fixt problems with the permission of supporting and unsupporting a motion Ticket #377 --- openslides/motion/models.py | 8 +------- openslides/motion/urls.py | 6 +++--- openslides/motion/views.py | 30 ++++++++++++++++++++++++------ 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/openslides/motion/models.py b/openslides/motion/models.py index 3f5635a19..71a6aa80f 100644 --- a/openslides/motion/models.py +++ b/openslides/motion/models.py @@ -258,11 +258,8 @@ class Motion(models.Model, SlideMixin): """ if person == self.submitter: # TODO: Use own Exception - raise NameError('Supporter can not be the submitter of a ' \ + raise NameError('Supporter can not be the submitter of a ' 'motion.') - if self.permitted is not None: - # TODO: Use own Exception - raise NameError('This motion is already permitted.') if not self.is_supporter(person): MotionSupporter(motion=self, person=person).save() self.writelog(_("Supporter: +%s") % (person)) @@ -272,9 +269,6 @@ class Motion(models.Model, SlideMixin): """ remove a supporter from the list of supporters of the motion """ - if self.permitted is not None: - # TODO: Use own Exception - raise NameError('This motion is already permitted.') try: object = self.motionsupporter_set.get(person=person).delete() except MotionSupporter.DoesNotExist: diff --git a/openslides/motion/urls.py b/openslides/motion/urls.py index 55543d615..6c7956c60 100644 --- a/openslides/motion/urls.py +++ b/openslides/motion/urls.py @@ -13,7 +13,7 @@ from django.conf.urls.defaults import url, patterns from openslides.motion.views import (MotionDelete, ViewPoll, - MotionPDF, MotionPollPDF, CreateAgendaItem) + MotionPDF, MotionPollPDF, CreateAgendaItem, SupportView) urlpatterns = patterns('openslides.motion.views', url(r'^$', @@ -99,12 +99,12 @@ urlpatterns = patterns('openslides.motion.views', ), url(r'^(?P\d+)/support/$', - 'support', + SupportView.as_view(unsupport=False, answer_url='support/'), name='motion_support', ), url(r'^(?P\d+)/unsupport/$', - 'unsupport', + SupportView.as_view(unsupport=True, answer_url='unsupport/'), name='motion_unsupport', ), diff --git a/openslides/motion/views.py b/openslides/motion/views.py index 1b4166870..0ef1f140f 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -381,14 +381,32 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): return _('Do you really want to unsupport this motion?') def pre_post_redirect(self, request, *args, **kwargs): - if self.get_answer().lower() == 'yes': - if not self.unsupport: - self.get_object().support(person=request.user) - self.success_message = _("You have supported this motion successfully.") + motion = self.get_object() + allowed_actions = motion.get_allowed_actions(request.user) + if not self.get_answer().lower() == 'yes': + return + if not self.unsupport: + if 'support' in allowed_actions: + motion.support(person=request.user) + messages.success( + request, + _("You have supported this motion successfully.")) + else: + messages.error( + request, + _('You can not support this motion.')) + else: + if 'unsupport' in allowed_actions: self.get_object().unsupport(person=request.user) - self.success_message = _("You have unsupported this motion successfully.") - messages.success(request, self.success_message) + messages.success( + request, + _("You have unsupported this motion successfully.")) + else: + messages.error( + request, + _('You can not support this motion.')) + def get_redirect_url(self, **kwargs): return reverse('motion_view', args=[kwargs[self.pk_url_kwarg]]) From 1002ecc6e60d77968e7b2d060a6969e0b3057191 Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Thu, 25 Oct 2012 10:25:59 +0200 Subject: [PATCH 4/5] Remove redundant keyword for the SupportView class --- openslides/motion/urls.py | 4 ++-- openslides/motion/views.py | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/openslides/motion/urls.py b/openslides/motion/urls.py index 6c7956c60..177a5b04f 100644 --- a/openslides/motion/urls.py +++ b/openslides/motion/urls.py @@ -99,12 +99,12 @@ urlpatterns = patterns('openslides.motion.views', ), url(r'^(?P\d+)/support/$', - SupportView.as_view(unsupport=False, answer_url='support/'), + SupportView.as_view(support=True), name='motion_support', ), url(r'^(?P\d+)/unsupport/$', - SupportView.as_view(unsupport=True, answer_url='unsupport/'), + SupportView.as_view(support=False), name='motion_unsupport', ), diff --git a/openslides/motion/views.py b/openslides/motion/views.py index 0ef1f140f..c6d902602 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -371,11 +371,10 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): permission_required = 'motion.can_support_motion' model = Motion pk_url_kwarg = 'motion_id' - unsupport = False # Must be given in SupportView.as_view() - answer_url = None # Must be given in SupportView.as_view() + support = True def get_question(self): - if not self.unsupport: + if self.support: return _('Do you really want to support this motion?') else: return _('Do you really want to unsupport this motion?') @@ -385,7 +384,7 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): allowed_actions = motion.get_allowed_actions(request.user) if not self.get_answer().lower() == 'yes': return - if not self.unsupport: + if self.support: if 'support' in allowed_actions: motion.support(person=request.user) messages.success( @@ -405,12 +404,19 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): else: messages.error( request, - _('You can not support this motion.')) - + _('You can not unsupport this motion.')) def get_redirect_url(self, **kwargs): return reverse('motion_view', args=[kwargs[self.pk_url_kwarg]]) + def get_answer_url(self): + if self.support: + answer_url = 'motion_support' + else: + answer_url = 'motion_unsupport' + return reverse(answer_url, args=[self.kwargs[self.pk_url_kwarg]]) + + @permission_required('motion.can_manage_motion') @template('motion/view.html') def gen_poll(request, motion_id): From 9d3d0951629a04c2374348127af12ba4d23e3a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Sun, 28 Oct 2012 02:09:47 +0200 Subject: [PATCH 5/5] Clean up SupportView. Raise error message at GET request instead of POST. --- openslides/motion/views.py | 40 ++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/openslides/motion/views.py b/openslides/motion/views.py index c6d902602..5a675dd22 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -366,7 +366,8 @@ def reset(request, motion_id): class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): """ - Support or unsupport an motion. + Classed based view to support or unsupport a motion. Use + support=True or support=False in urls.py """ permission_required = 'motion.can_support_motion' model = Motion @@ -379,32 +380,25 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): else: return _('Do you really want to unsupport this motion?') + def pre_redirect(self, request, *args, **kwargs): + allowed_actions = self.get_object().get_allowed_actions(request.user) + if self.support and not 'support' in allowed_actions: + messages.error(request, _('You can not support this motion.')) + elif not self.support and not 'unsupport' in allowed_actions: + messages.error(request, _('You can not unsupport this motion.')) + else: + super(SupportView, self).pre_redirect(request, *args, **kwargs) + def pre_post_redirect(self, request, *args, **kwargs): motion = self.get_object() - allowed_actions = motion.get_allowed_actions(request.user) - if not self.get_answer().lower() == 'yes': - return - if self.support: - if 'support' in allowed_actions: + if self.get_answer().lower() == 'yes': + if self.support: motion.support(person=request.user) - messages.success( - request, - _("You have supported this motion successfully.")) - + self.success_message = _("You have supported this motion successfully.") else: - messages.error( - request, - _('You can not support this motion.')) - else: - if 'unsupport' in allowed_actions: - self.get_object().unsupport(person=request.user) - messages.success( - request, - _("You have unsupported this motion successfully.")) - else: - messages.error( - request, - _('You can not unsupport this motion.')) + motion.unsupport(person=request.user) + self.success_message = _("You have unsupported this motion successfully.") + messages.success(request, self.success_message) def get_redirect_url(self, **kwargs): return reverse('motion_view', args=[kwargs[self.pk_url_kwarg]])