fixed #558

openslides/motion/models.py

@@ -102,7 +102,7 @@ class Motion(SlideMixin, models.Model):
         return self.get_title()
 
     # TODO: Use transaction
-    def save(self, *args, **kwargs):
+    def save(self, no_new_version=False, *args, **kwargs):
         """
         Save the motion.
 
@@ -122,6 +122,8 @@ class Motion(SlideMixin, models.Model):
 
         the config 'motion_create_new_version' is set to
         'ALWAYS_CREATE_NEW_VERSION'.
+
+        If no_new_version is True, a new version will never be used.
         """
         if not self.state:
             self.reset_state()
@@ -131,6 +133,9 @@ class Motion(SlideMixin, models.Model):
 
         super(Motion, self).save(*args, **kwargs)
 
+        if no_new_version:
+            return
+
         # Find out if the version data has changed
         for attr in ['title', 'text', 'reason']:
             if not self.versions.exists():
@@ -142,7 +147,8 @@ class Motion(SlideMixin, models.Model):
         else:
             new_data = False
 
-        # TODO: Check everything here. The decision whether to create a new version has to be done in the view. Update docstings too.
+        # TODO: Check everything here. The decision whether to create a new
+        #       version has to be done in the view. Update docstings too.
         need_new_version = self.state.versioning
         if hasattr(self, '_new_version') or (new_data and need_new_version):
             version = self.new_version
@@ -512,9 +518,9 @@ class Motion(SlideMixin, models.Model):
         """
         MotionLog.objects.create(motion=self, message=message, person=person)
 
-    def activate_version(self, version):
+    def set_active_version(self, version):
         """
-        Set the active state of a version to True.
+        Set the active state of a version to 'version'.
 
         'version' can be a version object, or the version_number of a version.
         """

openslides/motion/views.py

@@ -83,21 +83,22 @@ motion_detail = MotionDetailView.as_view()
 
 
 class MotionMixin(object):
-    """Mixin for MotionViewsClasses to save the version data."""
+    """
+    Mixin for MotionViewsClasses to save the version data.
+    """
 
     def manipulate_object(self, form):
-        """Save the version data into the motion object before it is saved in
-        the Database."""
-
+        """
+        Save the version data into the motion object before it is saved in
+        the Database.
+        """
         super(MotionMixin, self).manipulate_object(form)
         for attr in ['title', 'text', 'reason']:
             setattr(self.object, attr, form.cleaned_data[attr])
 
-        try:
-            if form.cleaned_data['new_version']:
+        if type(self) != MotionCreateView:
+            if self.object.state.versioning and form.cleaned_data.get('new_version', True):
                 self.object.new_version
-        except KeyError:
-            pass
 
         try:
             self.object.category = form.cleaned_data['category']
@@ -210,29 +211,40 @@ motion_delete = MotionDeleteView.as_view()
 
 
 class VersionPermitView(GetVersionMixin, SingleObjectMixin, QuestionMixin, RedirectView):
-    """View to permit a version of a motion."""
+    """
+    View to permit a version of a motion.
+    """
 
     model = Motion
     question_url_name = 'motion_version_detail'
     success_url_name = 'motion_version_detail'
+    success_message = ugettext_lazy('Version successfully permitted.')
 
     def get(self, *args, **kwargs):
-        """Set self.object to a motion."""
+        """
+        Set self.object to a motion.
+        """
         self.object = self.get_object()
         return super(VersionPermitView, self).get(*args, **kwargs)
 
     def get_url_name_args(self):
-        """Return a list with arguments to create the success- and question_url."""
+        """
+        Return a list with arguments to create the success- and question_url.
+        """
         return [self.object.pk, self.object.version.version_number]
 
     def get_question(self):
-        """Return a string, shown to the user as question to permit the version."""
+        """
+        Return a string, shown to the user as question to permit the version.
+        """
         return _('Are you sure you want permit Version %s?') % self.object.version.version_number
 
     def case_yes(self):
-        """Activate the version, if the user chooses 'yes'."""
-        self.object.activate_version(self.object.version)  # TODO: Write log message
-        self.object.save()
+        """
+        Activate the version, if the user chooses 'yes'.
+        """
+        self.object.set_active_version(self.object.version)  # TODO: Write log message
+        self.object.save(no_new_version=True)
 
 version_permit = VersionPermitView.as_view()
 

tests/motion/test_models.py

@@ -141,6 +141,32 @@ class ModelTest(TestCase):
         motion1 = Motion.objects.create(title='foo', text='bar', identifier='')
         motion2 = Motion.objects.create(title='foo2', text='bar2', identifier='')
 
+    def test_do_not_create_new_version_when_permit_old_version(self):
+        motion = Motion()
+        motion.title = 'foo'
+        motion.text = 'bar'
+        first_version = motion.version
+        my_state = State.objects.create(name='automatic_versioning', workflow=self.workflow,
+                                        versioning=True, dont_set_new_version_active=True)
+        motion.state = my_state
+        motion.save()
+
+        motion = Motion.objects.get(pk=motion.pk)
+        motion.new_version
+        motion.title = 'New Title'
+        motion.save()
+        new_version = motion.last_version
+        self.assertEqual(motion.versions.count(), 2)
+
+        motion.set_active_version(new_version)
+        motion.save()
+        self.assertEqual(motion.versions.count(), 2)
+
+        motion.set_active_version(first_version)
+        motion.version = first_version
+        motion.save(no_new_version=True)
+        self.assertEqual(motion.versions.count(), 2)
+
 
 class ConfigTest(TestCase):
     def test_stop_submitting(self):

tests/motion/test_views.py

@@ -165,3 +165,31 @@ class TestMotionDeleteView(MotionViewTestCase):
         motion = Motion.objects.get(pk=2).add_submitter(self.delegate)
         response = self.delegate_client.post('/motion/2/del/', {})
         self.assertRedirects(response, '/motion/')
+
+
+class TestVersionPermitView(MotionViewTestCase):
+    def setUp(self):
+        super(TestVersionPermitView, self).setUp()
+        self.motion1.new_version
+        self.motion1.save()
+
+    def test_get(self):
+        response = self.check_url('/motion/1/version/2/permit/', self.admin_client, 302)
+        self.assertRedirects(response, '/motion/1/version/2/')
+
+    def test_post(self):
+        new_version = self.motion1.last_version
+        response = self.admin_client.post('/motion/1/version/2/permit/', {'yes': 1})
+        self.assertRedirects(response, '/motion/1/version/2/')
+        self.assertEqual(self.motion1.active_version, new_version)
+
+    def test_activate_old_version(self):
+        new_version = self.motion1.last_version
+        first_version = self.motion1.versions.order_by('version_number')[0]
+
+        self.motion1.set_active_version(new_version)
+        self.assertEqual(self.motion1.versions.count(), 2)
+        response = self.admin_client.post('/motion/1/version/1/permit/', {'yes': 1})
+        self.motion1 = Motion.objects.get(pk=1)
+        self.assertEqual(self.motion1.active_version, first_version)
+        self.assertEqual(self.motion1.versions.count(), 2)