From e630d600edbf9edae7048f73651790fcbb141250 Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Thu, 13 Jun 2013 15:43:17 +0200 Subject: [PATCH] Fixes #648 delete function for the motion versions --- openslides/motion/models.py | 25 +++++++++++++------ .../templates/motion/motion_detail.html | 1 - openslides/motion/urls.py | 5 ++++ openslides/motion/views.py | 21 ++++++++++++++++ openslides/utils/views.py | 1 + tests/motion/test_views.py | 16 ++++++++++++ 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/openslides/motion/models.py b/openslides/motion/models.py index 0fdeee171..3dd2683aa 100644 --- a/openslides/motion/models.py +++ b/openslides/motion/models.py @@ -49,7 +49,8 @@ class Motion(SlideMixin, models.Model): """ active_version = models.ForeignKey('MotionVersion', null=True, - related_name="active_version") + related_name="active_version", + on_delete=models.SET_NULL) """ Points to a specific version. @@ -315,20 +316,24 @@ class Motion(SlideMixin, models.Model): Is saved in a MotionVersion object. """ - def get_new_version(self): + def get_new_version(self, **kwargs): """ Return a version object, not saved in the database. The version data of the new version object is populated with the data - set via motion.title, motion.text, motion.reason. If the data is not set, - it is populated with the data from the last version object. + set via motion.title, motion.text, motion.reason if these data are + not given as keyword arguments. If the data is not set in the motion + attributes, it is populated with the data from the last version + object if such object exists. """ - new_version = MotionVersion(motion=self) + new_version = MotionVersion(motion=self, **kwargs) if self.versions.exists(): last_version = self.get_last_version() else: last_version = None for attr in ['title', 'text', 'reason']: + if attr in kwargs: + continue _attr = '_%s' % attr data = getattr(self, _attr, None) if data is None and last_version is not None: @@ -561,16 +566,20 @@ class MotionVersion(models.Model): def __unicode__(self): """Return a string, representing this object.""" counter = self.version_number or ugettext_lazy('new') - return "%s Version %s" % (self.motion, counter) # TODO: Should this really be self.motion or the title of the specific version? + return "Motion %s, Version %s" % (self.motion_id, counter) def get_absolute_url(self, link='detail'): - """Return the URL of this Version. + """ + Return the URL of this Version. - The keyargument link can be 'view' or 'detail'. + The keyargument link can be 'detail' or 'delete'. """ if link == 'view' or link == 'detail': return reverse('motion_version_detail', args=[str(self.motion.id), str(self.version_number)]) + if link == 'delete': + return reverse('motion_version_delete', args=[str(self.motion.id), + str(self.version_number)]) @property def active(self): diff --git a/openslides/motion/templates/motion/motion_detail.html b/openslides/motion/templates/motion/motion_detail.html index bb594f21f..c89a61e9c 100644 --- a/openslides/motion/templates/motion/motion_detail.html +++ b/openslides/motion/templates/motion/motion_detail.html @@ -130,7 +130,6 @@ - {# TODO: add delete version function #} diff --git a/openslides/motion/urls.py b/openslides/motion/urls.py index 59ac4eec7..dbe919f99 100644 --- a/openslides/motion/urls.py +++ b/openslides/motion/urls.py @@ -50,6 +50,11 @@ urlpatterns = patterns('openslides.motion.views', name='motion_version_permit', ), + url(r'^(?P\d+)/version/(?P\d+)/del/$', + 'version_delete', + name='motion_version_delete', + ), + url(r'^(?P\d+)/diff/$', 'version_diff', name='motion_version_diff', diff --git a/openslides/motion/views.py b/openslides/motion/views.py index c82e6024f..769fc97d7 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -247,6 +247,26 @@ class MotionDeleteView(DeleteView): motion_delete = MotionDeleteView.as_view() +class VersionDeleteView(DeleteView): + """ + View to delete a motion version. + """ + model = MotionVersion + permission_required = 'motion.can_manage_motion' + success_url_name = 'motion_detail' + + def get_object(self): + motion_id = int(self.kwargs.get('pk')) + version_number = int(self.kwargs.get('version_number')) + return MotionVersion.objects.get(motion=motion_id, + version_number=version_number) + + def get_success_url_name_args(self): + return (self.object.motion_id, ) + +version_delete = VersionDeleteView.as_view() + + class VersionPermitView(SingleObjectMixin, QuestionMixin, RedirectView): """ View to permit a version of a motion. @@ -255,6 +275,7 @@ class VersionPermitView(SingleObjectMixin, QuestionMixin, RedirectView): question_url_name = 'motion_version_detail' success_url_name = 'motion_version_detail' success_message = ugettext_lazy('Version successfully permitted.') + permission_required = 'motion.can_manage_motion' def get(self, *args, **kwargs): """ diff --git a/openslides/utils/views.py b/openslides/utils/views.py index d8bb38a86..2229bda2c 100644 --- a/openslides/utils/views.py +++ b/openslides/utils/views.py @@ -172,6 +172,7 @@ class QuestionMixin(object): url_name_args = None def get_redirect_url(self, **kwargs): + # TODO: raise error when question_url_name/success_url_name is not present if self.request.method == 'GET': return reverse(self.question_url_name, args=self.get_question_url_name_args()) else: diff --git a/tests/motion/test_views.py b/tests/motion/test_views.py index 54e120d24..6cda947c9 100644 --- a/tests/motion/test_views.py +++ b/tests/motion/test_views.py @@ -344,3 +344,19 @@ class TestVersionPermitView(MotionViewTestCase): self.motion1 = Motion.objects.get(pk=1) self.assertEqual(self.motion1.active_version, first_version) self.assertEqual(self.motion1.versions.count(), 2) + + +class TestVersionDeleteView(MotionViewTestCase): + def test_get(self): + response = self.check_url('/motion/1/version/1/del/', self.admin_client, 302) + self.assertRedirects(response, '/motion/1/version/1/') + + def test_post(self): + new_version = self.motion1.get_new_version + self.motion1.save(use_version=new_version(title='new', text='new')) + self.motion1.save(use_version=new_version(title='new2', text='new')) + self.assertEqual(self.motion1.versions.count(), 3) + + response = self.admin_client.post('/motion/1/version/2/del/', {'yes': 1}) + self.assertRedirects(response, '/motion/1/') + self.assertEqual(self.motion1.versions.count(), 2)