Small agenda improvments

Updated the tests and changed only small internal parts of method of the agenda model. No API changes

openslides/agenda/models.py

@@ -162,14 +162,15 @@ class Item(RESTModelMixin, SlideMixin, AbsoluteUrlMixin, MPTTModel):
         """
         Return the title of this item.
         """
-        item_no = self.item_no
         if not self.content_object:
-            return '%s %s' % (item_no, self.title) if item_no else self.title
+            agenda_title = self.title
-        try:
+        else:
-            agenda_title = self.content_object.get_agenda_title()
+            try:
-            return '%s %s' % (item_no, agenda_title) if item_no else agenda_title
+                agenda_title = self.content_object.get_agenda_title()
-        except AttributeError:
+            except AttributeError:
-            raise NotImplementedError('You have to provide a get_agenda_title method on your related model.')
+                raise NotImplementedError('You have to provide a get_agenda_title '
+                                          'method on your related model.')
+        return '%s %s' % (self.item_no, agenda_title) if self.item_no else agenda_title
 
     def get_title_supplement(self):
         """
@@ -182,13 +183,6 @@ class Item(RESTModelMixin, SlideMixin, AbsoluteUrlMixin, MPTTModel):
         except AttributeError:
             raise NotImplementedError('You have to provide a get_agenda_title_supplement method on your related model.')
 
-    def set_closed(self, closed=True):
-        """
-        Changes the closed-status of the item.
-        """
-        self.closed = closed
-        self.save()
-
     @property
     def weight_form(self):
         """
@@ -209,12 +203,17 @@ class Item(RESTModelMixin, SlideMixin, AbsoluteUrlMixin, MPTTModel):
     def delete(self, with_children=False):
         """
         Delete the Item.
+
+        If with_children is True, all children of the item will be deleted as
+        well. If with_children is False, all children will be children of the
+        parent of the item.
         """
         if not with_children:
             for child in self.get_children():
                 child.move_to(self.parent)
                 child.save()
-        super(Item, self).delete()
+        super().delete()
+        # TODO: Try to remove the rebuild call
         Item.objects.rebuild()
 
     def get_list_of_speakers(self, old_speakers_count=None, coming_speakers_count=None):

openslides/agenda/views.py

@@ -224,7 +224,9 @@ class SetClosed(SingleObjectMixin, RedirectView):
 
     def pre_redirect(self, request, *args, **kwargs):
         closed = kwargs['closed']
-        self.get_object().set_closed(closed)
+        # TODO: use update here
+        self.get_object().closed = closed
+        self.get_object().save()
         return super(SetClosed, self).pre_redirect(request, *args, **kwargs)
 
     def get_url_name_args(self):

openslides/urls.py

@@ -2,6 +2,7 @@ from django.conf.urls import include, patterns, url
 
 from openslides.core.views import IndexView, ErrorView
 from openslides.utils.rest_api import router
+from openslides.users.views import UserSettingsView, UserPasswordSettingsView
 
 handler403 = ErrorView.as_view(status_code=403)
 handler404 = ErrorView.as_view(status_code=404)
@@ -34,7 +35,6 @@ urlpatterns += patterns(
 )
 
 # TODO: move this patterns into core or the participant app
-from openslides.users.views import UserSettingsView, UserPasswordSettingsView
 urlpatterns += patterns(
     '',
     (r'^jsi18n/$', 'django.views.i18n.javascript_catalog', js_info_dict),

tests/old/agenda/tests.py

@@ -9,117 +9,8 @@ from django.test.client import Client
 from openslides.agenda.models import Item
 from openslides.agenda.slides import agenda_slide
 from openslides.users.models import User
-from openslides.projector.api import set_active_slide
 from openslides.utils.test import TestCase
 
-from .models import BadRelatedItem, RelatedItem
-
-
-class ItemTest(TestCase):
-    def setUp(self):
-        self.item1 = Item.objects.create(title='item1')
-        self.item2 = Item.objects.create(title='item2')
-        self.item3 = Item.objects.create(title='item1A', parent=self.item1)
-        self.item4 = Item.objects.create(title='item1Aa', parent=self.item3)
-        self.related = RelatedItem.objects.create(name='ekdfjen458gj1siek45nv')
-        self.item5 = Item.objects.create(title='item5', content_object=self.related)
-
-    def testClosed(self):
-        self.assertFalse(self.item1.closed)
-
-        self.item1.set_closed()
-        self.assertTrue(self.item1.closed)
-
-        self.item1.set_closed(closed=False)
-        self.assertFalse(self.item1.closed)
-
-    def testParents(self):
-        self.assertFalse(self.item1.get_ancestors())
-        self.assertTrue(self.item1 in self.item3.get_ancestors())
-        self.assertTrue(self.item1 in self.item4.get_ancestors())
-        self.assertFalse(self.item2 in self.item4.get_ancestors())
-
-    def testChildren(self):
-        self.assertEqual(list(self.item2.get_children()), [])
-        self.assertTrue(self.item3 in self.item1.get_children())
-        self.assertFalse(self.item4 in self.item1.get_children())
-
-    def testForms(self):
-        for item in Item.objects.all():
-            initial = item.weight_form.initial
-            self.assertEqual(initial['self'], item.id)
-            if item.parent:
-                self.assertEqual(initial['parent'], item.parent.id)
-            else:
-                self.assertEqual(initial['parent'], 0)
-            self.assertEqual(initial['weight'], item.weight)
-
-    def test_title_supplement(self):
-        self.assertEqual(self.item1.get_title_supplement(), '')
-
-    def test_delete_item(self):
-        new_item1 = Item.objects.create()
-        new_item2 = Item.objects.create(parent=new_item1)
-        new_item3 = Item.objects.create(parent=new_item2)
-        new_item1.delete()
-        self.assertTrue(new_item3 in Item.objects.all())
-        new_item2.delete(with_children=True)
-        self.assertFalse(new_item3 in Item.objects.all())
-
-    def test_absolute_url(self):
-        self.assertEqual(self.item1.get_absolute_url(), '/agenda/1/')
-        self.assertEqual(self.item1.get_absolute_url('update'), '/agenda/1/edit/')
-        self.assertEqual(self.item1.get_absolute_url('delete'), '/agenda/1/del/')
-
-    def test_related_item(self):
-        self.assertEqual(self.item5.get_title(), self.related.name)
-        self.assertEqual(self.item5.get_title_supplement(), 'test item')
-        self.assertEqual(self.item5.content_type.name, 'Related Item CHFNGEJ5634DJ34F')
-
-    def test_deleted_related_item(self):
-        self.related.delete()
-        self.assertFalse(RelatedItem.objects.all().exists())
-        self.assertEqual(Item.objects.get(pk=self.item5.pk).title,
-                         '< Item for deleted slide (ekdfjen458gj1siek45nv) >')
-
-    def test_related_item_get_absolute_url(self):
-        """
-        Tests that the get_absolute_url method with the link 'projector'
-        and 'projector_preview' returns the absolute_url for the related
-        item.
-        """
-        self.assertEqual(self.item5.get_absolute_url('projector'),
-                         '/projector/activate/test_related_item/?pk=1')
-        self.assertEqual(self.item5.get_absolute_url('projector_preview'),
-                         '/projector/preview/test_related_item/?pk=1')
-
-    def test_activate_related_item(self):
-        """
-        The agenda item has to be active, if its related item is.
-        """
-        set_active_slide('test_related_item', pk=1)
-        self.assertTrue(self.item5.is_active_slide())
-
-    def test_is_active_related_item_list_of_speakers(self):
-        """
-        Test the method 'is_active_slide' if the item is related but the list
-        of speakers is shown on the projector.
-        """
-        set_active_slide('agenda', type='list_of_speakers', pk=5)
-        self.assertTrue(self.item5.is_active_slide())
-
-    def test_bad_related_item(self):
-        bad = BadRelatedItem.objects.create(name='dhfne94irkgl2047fzvb')
-        item = Item.objects.create(title='item_jghfndzrh46w738kdmc', content_object=bad)
-        self.assertRaisesMessage(
-            NotImplementedError,
-            'You have to provide a get_agenda_title method on your related model.',
-            item.get_title)
-        self.assertRaisesMessage(
-            NotImplementedError,
-            'You have to provide a get_agenda_title_supplement method on your related model.',
-            item.get_title_supplement)
-
 
 class ViewTest(TestCase):
     def setUp(self):

tests/unit/agenda/test_models.py

@@ -0,0 +1,89 @@
+from unittest import TestCase
+from unittest.mock import MagicMock, patch
+
+from openslides.agenda.models import Item
+
+
+class ItemTitle(TestCase):
+    def test_get_title_without_item_no(self):
+        item = Item(title='test_title')
+        self.assertEqual(
+            item.get_title(),
+            'test_title')
+
+    @patch('openslides.agenda.models.Item.item_no', '5')
+    def test_get_title_with_item_no(self):
+        item = Item(title='test_title')
+        self.assertEqual(
+            item.get_title(),
+            '5 test_title')
+
+    @patch('openslides.agenda.models.Item.content_object')
+    def test_get_title_from_related(self, content_object):
+        item = Item(title='test_title')
+        content_object.get_agenda_title.return_value = 'related_title'
+
+        self.assertEqual(
+            item.get_title(),
+            'related_title')
+
+    @patch('openslides.agenda.models.Item.content_object')
+    def test_get_title_invalid_related(self, content_object):
+        item = Item(title='test_title')
+        content_object.get_agenda_title.return_value = 'related_title'
+        del content_object.get_agenda_title
+
+        with self.assertRaises(NotImplementedError):
+            item.get_title()
+
+    def test_title_supplement_without_related(self):
+        item = Item()
+        self.assertEqual(
+            item.get_title_supplement(),
+            '')
+
+    @patch('openslides.agenda.models.Item.content_object')
+    def test_title_supplement_with_related(self, content_object):
+        item = Item()
+        content_object.get_agenda_title_supplement.return_value = 'related_title_supplement'
+
+        self.assertEqual(
+            item.get_title_supplement(),
+            'related_title_supplement')
+
+    @patch('openslides.agenda.models.Item.content_object')
+    def test_title_supplement_invalid_related(self, content_object):
+        item = Item()
+        del content_object.get_agenda_title_supplement
+
+        with self.assertRaises(NotImplementedError):
+            item.get_title_supplement()
+
+
+@patch('openslides.agenda.models.Item.objects.rebuild')
+@patch('openslides.agenda.models.Item.get_children')
+class ItemDelete(TestCase):
+    def test_delete_with_children_is_true(self, get_children, rebuild):
+        item = Item()
+
+        with patch('builtins.super') as mock_super:
+            item.delete(with_children=True)
+
+        self.assertFalse(get_children.called)
+        rebuild.assert_called_once_with()
+        mock_super().delete.assert_called_once_with()
+
+    def test_delete_with_children_is_false(self, get_children, rebuild):
+        parent = Item()
+        item = Item()
+        item.parent = parent
+        child_item = MagicMock()
+        get_children.return_value = [child_item]
+
+        with patch('builtins.super') as mock_super:
+            item.delete(with_children=False)
+
+        child_item.move_to.assert_called_once_with(item.parent)
+        child_item.save_assert_called_once_with()
+        rebuild.assert_called_once_with()
+        mock_super().delete.assert_called_once_with()