Merge pull request #773 from normanjaeckel/LogFix

Fix motion logging

openslides/motion/models.py

@@ -175,6 +175,7 @@ class Motion(SlideMixin, models.Model):
         # TODO: Move parts of these last lines of code outside the save method
         # when other versions than the last ones should be edited later on.
         if self.active_version is None or not self.state.leave_old_version_active:
+            # TODO: Don't call this if it was not a new version
             self.active_version = use_version
             self.save(update_fields=['active_version'])
 
@@ -674,10 +675,11 @@ class MotionLog(models.Model):
         Return a string, representing the log message.
         """
         time = formats.date_format(self.time, 'DATETIME_FORMAT')
-        return_message = '%s ' % time + ''.join(map(_, self.message_list))
+        time_and_messages = '%s ' % time + ''.join(map(_, self.message_list))
         if self.person is not None:
-            return_message += _(' by %s') % self.person
+            return _('%(time_and_messages)s by %(person)s') % {'time_and_messages': time_and_messages,
-        return return_message
+                                                               'person': self.person}
+        return time_and_messages
 
 
 class MotionVote(BaseVote):

openslides/motion/views.py

@@ -221,16 +221,32 @@ class MotionUpdateView(MotionEditMixin, UpdateView):
 
     def form_valid(self, form):
         """
-        Write a log message if the form is valid.
+        Writes a log message and removes supports in some cases if the form is valid.
         """
         response = super(MotionUpdateView, self).form_valid(form)
-        self.object.write_log([ugettext_noop('Motion updated')], self.request.user)
+        self.write_log()
         if (config['motion_remove_supporters'] and self.object.state.allow_support and
                 not self.request.user.has_perm('motion.can_manage_motion')):
             self.object.clear_supporters()
             self.object.write_log([ugettext_noop('All supporters removed')], self.request.user)
         return response
 
+    def write_log(self):
+        """
+        Writes a log message. Distinguishs whether a version was created or updated.
+        """
+        if self.version.id is None:
+            number = self.object.get_last_version().version_number
+            created = False
+        else:
+            number = self.version.version_number
+            created = self.used_new_version
+        self.object.write_log(
+            [ugettext_noop('Motion version '),
+             str(number),
+             ugettext_noop(' created') if created else ugettext_noop(' updated')],
+            self.request.user)
+
     def get_initial(self):
         initial = super(MotionUpdateView, self).get_initial()
         if self.request.user.has_perm('motion.can_manage_motion'):
@@ -251,8 +267,10 @@ class MotionUpdateView(MotionEditMixin, UpdateView):
         if (self.object.state.versioning and
                 not form.cleaned_data.get('disable_versioning', False)):
             self.version = self.object.get_new_version()
+            self.used_new_version = True
         else:
             self.version = self.object.get_last_version()
+            self.used_new_version = False
 
 motion_edit = MotionUpdateView.as_view()
 
@@ -337,8 +355,9 @@ class VersionPermitView(SingleObjectMixin, QuestionMixin, RedirectView):
         self.object.active_version = self.version
         self.object.save(update_fields=['active_version'])
         self.object.write_log(
-            message_list=[ugettext_noop('Version %d permitted')
+            message_list=[ugettext_noop('Version '),
-                          % self.version.version_number],
+                          str(self.version.version_number),
+                          ugettext_noop(' permitted')],
             person=self.request.user)
 
 version_permit = VersionPermitView.as_view()
@@ -435,10 +454,10 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView):
             user = self.request.user
             if self.support:
                 self.object.support(person=user)
-                self.object.write_log([ugettext_noop("Supporter: +%s") % user], user)
+                self.object.write_log([ugettext_noop('Motion supported')], user)
             else:
                 self.object.unsupport(person=user)
-                self.object.write_log([ugettext_noop("Supporter: -%s") % user], user)
+                self.object.write_log([ugettext_noop('Motion unsupported')], user)
 
     def get_success_message(self):
         """

tests/motion/test_views.py

@@ -13,7 +13,7 @@ from django.test.client import Client
 from openslides.config.api import config
 from openslides.utils.test import TestCase
 from openslides.participant.models import User, Group
-from openslides.motion.models import Motion, State, Category
+from openslides.motion.models import Motion, State, Category, MotionLog
 
 
 class MotionViewTestCase(TestCase):
@@ -150,6 +150,13 @@ class TestMotionCreateView(MotionViewTestCase):
         motion = Motion.objects.filter(category=category).get()
         self.assertEqual(motion.identifier, 'prefix_raiLie6keik6Eikeiphi 1')
 
+    def test_log(self):
+        self.assertFalse(MotionLog.objects.all().exists())
+        response = self.admin_client.post(self.url, {'title': 'new motion',
+                                                     'text': 'motion text',
+                                                     'workflow': 1})
+        self.assertEqual(MotionLog.objects.get(pk=1).message_list, ['Motion created'])
+
 
 class TestMotionUpdateView(MotionViewTestCase):
     url = '/motion/1/edit/'
@@ -316,6 +323,46 @@ class TestMotionUpdateView(MotionViewTestCase):
         response = self.admin_client.get('/motion/%s/edit/' % motion.id)
         self.assertEqual(response.context['form'].initial['text'], 'tpdfgojwerldkfgertdfg')
 
+    def test_log(self):
+        self.assertFalse(MotionLog.objects.all().exists())
+
+        # Update motion without versioning
+        self.assertFalse(self.motion1.state.versioning)
+        response = self.admin_client.post(self.url, {'title': 'new motion_title',
+                                                     'text': 'motion text',
+                                                     'workflow': 2})
+        self.assertEqual(MotionLog.objects.get(pk=1).message_list, ['Motion version ', '1', ' updated'])
+
+        # Update motion by creating a new version
+        self.motion1.set_state(6)  # Set to state 'permitted' which has versioning=True
+        self.assertTrue(self.motion1.state.versioning)
+        self.motion1.save(use_version=False)
+        response = self.admin_client.post(self.url, {'title': 'new motion_title',
+                                                     'text': 'new motion text',
+                                                     'workflow': 2})
+        self.assertEqual(MotionLog.objects.get(pk=2).message_list, ['Motion version ', '2', ' created'])
+
+        # Update motion with so called 'trivial changes'
+        config['motion_allow_disable_versioning'] = True
+        response = self.admin_client.post(self.url, {'title': 'new motion_title',
+                                                     'text': 'more new motion text',
+                                                     'disable_versioning': 'on',
+                                                     'workflow': 2})
+        self.assertEqual(MotionLog.objects.get(pk=3).message_list, ['Motion version ', '2', ' updated'])
+
+        # Update motion without changes in the version data
+        response = self.admin_client.post(self.url, {'title': 'new motion_title',
+                                                     'text': 'more new motion text',
+                                                     'workflow': 2})
+        self.assertEqual(MotionLog.objects.get(pk=4).message_list, ['Motion version ', '2', ' updated'])
+
+        # Update motion without changes in the version data but also with the 'trivial changes' flag
+        response = self.admin_client.post(self.url, {'title': 'new motion_title',
+                                                     'text': 'more new motion text',
+                                                     'disable_versioning': 'on',
+                                                     'workflow': 2})
+        self.assertEqual(MotionLog.objects.get(pk=5).message_list, ['Motion version ', '2', ' updated'])
+
 
 class TestMotionDeleteView(MotionViewTestCase):
     def test_get(self):