Merge pull request #731 from ostcar/motion_version_delete

Fixes #648

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):

openslides/motion/templates/motion/motion_detail.html

@@ -130,7 +130,6 @@
                         <a href="{% model_url version %}" title="{% trans 'Show' %}" class="btn btn-mini">
                             <i class="icon-search"></i>
                         </a>
-                        {# TODO: add delete version function #}
                         <a href="{% model_url version 'delete' %}" title="{% trans 'Delete' %}" class="btn btn-mini">
                             <i class="icon-remove"></i>
                         </a>

openslides/motion/urls.py

@@ -50,6 +50,11 @@ urlpatterns = patterns('openslides.motion.views',
         name='motion_version_permit',
     ),
 
+    url(r'^(?P<pk>\d+)/version/(?P<version_number>\d+)/del/$',
+        'version_delete',
+        name='motion_version_delete',
+    ),
+
     url(r'^(?P<pk>\d+)/diff/$',
         'version_diff',
         name='motion_version_diff',

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):
         """

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:

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)