From c800884a435a59b0f856c6ec69905a409bccadd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Sat, 7 Sep 2013 00:18:13 +0200 Subject: [PATCH] Use GenericForeignKey for agenda related items, fix #865 --- openslides/agenda/forms.py | 4 +- openslides/agenda/models.py | 66 ++++++++------------ openslides/agenda/signals.py | 15 +++++ openslides/agenda/templates/agenda/edit.html | 6 +- openslides/agenda/templates/agenda/view.html | 4 +- openslides/agenda/views.py | 32 +++++++++- openslides/assignment/models.py | 4 ++ openslides/assignment/urls.py | 6 +- openslides/assignment/views.py | 17 ++--- openslides/motion/models.py | 3 +- openslides/motion/views.py | 19 ++---- tests/agenda/models.py | 10 +++ tests/agenda/tests.py | 32 +++++++--- 13 files changed, 127 insertions(+), 91 deletions(-) diff --git a/openslides/agenda/forms.py b/openslides/agenda/forms.py index 00a48281e..5808eb441 100644 --- a/openslides/agenda/forms.py +++ b/openslides/agenda/forms.py @@ -37,7 +37,7 @@ class ItemForm(CssClassMixin, forms.ModelForm): class Meta: model = Item - exclude = ('closed', 'weight', 'related_sid') + exclude = ('closed', 'weight', 'content_type', 'object_id') class RelatedItemForm(ItemForm): @@ -46,7 +46,7 @@ class RelatedItemForm(ItemForm): """ class Meta: model = Item - exclude = ('closed', 'type', 'weight', 'related_sid', 'title', 'text') + exclude = ('closed', 'type', 'weight', 'content_type', 'object_id', 'title', 'text') class ItemOrderForm(CssClassMixin, forms.Form): diff --git a/openslides/agenda/models.py b/openslides/agenda/models.py index 41392603f..776a5721c 100644 --- a/openslides/agenda/models.py +++ b/openslides/agenda/models.py @@ -14,6 +14,8 @@ from datetime import datetime from django.db import models from django.contrib.auth.models import AnonymousUser +from django.contrib.contenttypes.models import ContentType +from django.contrib.contenttypes import generic from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy, ugettext_noop, ugettext as _ @@ -87,11 +89,19 @@ class Item(MPTTModel, SlideMixin): Weight to sort the item in the agenda. """ - related_sid = models.CharField(null=True, blank=True, max_length=63) + content_type = models.ForeignKey(ContentType, null=True, blank=True) + """ + Field for generic relation to a related object. Type of the object. """ - Slide-ID to another object to show it in the agenda. - For example a motion or assignment. + object_id = models.PositiveIntegerField(null=True, blank=True) + """ + Field for generic relation to a related object. Id of the object. + """ + + content_object = generic.GenericForeignKey() + """ + Field for generic relation to a related object. General field to the related object. """ speaker_list_closed = models.BooleanField( @@ -125,53 +135,27 @@ class Item(MPTTModel, SlideMixin): if link == 'delete': return reverse('item_delete', args=[str(self.id)]) - def get_related_slide(self): - """ - Return the object at which the item points. - """ - # TODO: Rename it to 'get_related_object' - object = get_slide_from_sid(self.related_sid, element=True) - if object is None: - self.title = _('< Item for deleted slide (%s) >') % self.related_sid - self.related_sid = None - self.save() - return self - else: - return object - - def get_related_type(self): - """ - Return the type of the releated slide. - """ - return self.get_related_slide().prefix - - def print_related_type(self): - """ - Print the type of the related item. - - For use in Template - ??Why does {% trans item.print_related_type|capfirst %} not work?? - """ - return _(self.get_related_type().capitalize()) - def get_title(self): """ Return the title of this item. """ - if self.related_sid is None: + if not self.content_object: return self.title - return self.get_related_slide().get_agenda_title() + try: + return self.content_object.get_agenda_title() + except AttributeError: + raise NotImplementedError('You have to provide a get_agenda_title method on your related model.') def get_title_supplement(self): """ Return a supplement for the title. """ - if self.related_sid is None: + if not self.content_object: return '' try: - return self.get_related_slide().get_agenda_title_supplement() + return self.content_object.get_agenda_title_supplement() except AttributeError: - return '(%s)' % self.print_related_type() + raise NotImplementedError('You have to provide a get_agenda_title_supplement method on your related model.') def slide(self): """ @@ -180,11 +164,11 @@ class Item(MPTTModel, SlideMixin): There are four cases: * summary slide * list of speakers - * related slide, i. e. the slide of the related object + * related item, i. e. the slide of the related object * normal slide of the item The method returns only one of them according to the config value - 'presentation_argument' and the attribute 'related_sid'. + 'presentation_argument' and the attribute 'content_object'. """ if config['presentation_argument'] == 'summary': data = {'title': self.get_title(), @@ -198,8 +182,8 @@ class Item(MPTTModel, SlideMixin): 'item': self, 'template': 'projector/agenda_list_of_speaker.html', 'list_of_speakers': list_of_speakers} - elif self.related_sid: - data = self.get_related_slide().slide() + elif self.content_object: + data = self.content_object.slide() else: data = {'item': self, diff --git a/openslides/agenda/signals.py b/openslides/agenda/signals.py index 843bb239e..c454da57f 100644 --- a/openslides/agenda/signals.py +++ b/openslides/agenda/signals.py @@ -12,6 +12,8 @@ from datetime import datetime +from django.contrib.contenttypes.models import ContentType +from django.db.models.signals import pre_delete from django.dispatch import receiver from django import forms from django.utils.translation import ugettext_lazy, ugettext_noop, ugettext as _ @@ -111,3 +113,16 @@ def agenda_list_of_speakers(sender, **kwargs): return render_to_string('agenda/overlay_speaker_projector.html', context) return Overlay(name, get_widget_html, get_projector_html) + + +@receiver(pre_delete) +def listen_to_related_object_delete_signal(sender, instance, **kwargs): + """ + Receiver to listen whether a related item has been deleted. + """ + if hasattr(instance, 'get_agenda_title'): + for item in Item.objects.filter(content_type=ContentType.objects.get_for_model(sender), object_id=instance.pk): + item.title = '< Item for deleted slide (%s) >' % instance.get_agenda_title() + item.content_type = None + item.object_id = None + item.save() diff --git a/openslides/agenda/templates/agenda/edit.html b/openslides/agenda/templates/agenda/edit.html index f62090c3b..74886d454 100644 --- a/openslides/agenda/templates/agenda/edit.html +++ b/openslides/agenda/templates/agenda/edit.html @@ -24,9 +24,9 @@

- {% if item.related_sid %} - - {% blocktrans with type=item.get_related_type|trans name=item.get_related_slide %}Edit {{ type }} {{ name }}{% endblocktrans %} + {% if item.content_object %} + + {% blocktrans with type=item.content_type.name|trans name=item.content_object %}Edit {{ type }} {{ name }}{% endblocktrans %} {% endif %}

diff --git a/openslides/agenda/templates/agenda/view.html b/openslides/agenda/templates/agenda/view.html index 50848af14..0d7bc6087 100644 --- a/openslides/agenda/templates/agenda/view.html +++ b/openslides/agenda/templates/agenda/view.html @@ -43,10 +43,10 @@

- {% if not item.related_sid %} + {% if not item.content_object %} {{ item.text|safe|linebreaks }} {% else %} - {% trans item.get_related_type %} {{ item.get_related_slide }} + {% trans item.content_type.name %} {{ item.content_object }} {% endif %}

diff --git a/openslides/agenda/views.py b/openslides/agenda/views.py index 9333de7c3..cf636a922 100644 --- a/openslides/agenda/views.py +++ b/openslides/agenda/views.py @@ -197,10 +197,11 @@ class ItemUpdate(UpdateView): success_url_name = 'item_overview' def get_form_class(self): - if self.object.related_sid is None: - return ItemForm + if self.object.content_object: + form = RelatedItemForm else: - return RelatedItemForm + form = ItemForm + return form class ItemCreate(CreateView): @@ -245,6 +246,31 @@ class ItemDelete(DeleteView): % html_strong(self.object)) +class CreateRelatedAgendaItemView(SingleObjectMixin, RedirectView): + """ + View to create and agenda item for a related object. + + This view is only for subclassing in views of related apps. You + have to define 'model = ....' + """ + permission_required = 'agenda.can_manage_agenda' + url_name = 'item_overview' + url_name_args = [] + + def get(self, request, *args, **kwargs): + """ + Set self.object to the relevant object. + """ + self.object = self.get_object() + return super(CreateRelatedAgendaItemView, self).get(request, *args, **kwargs) + + def pre_redirect(self, request, *args, **kwargs): + """ + Create the agenda item. + """ + self.item = Item.objects.create(content_object=self.object) + + class AgendaPDF(PDFView): """ Create a full agenda-PDF. diff --git a/openslides/assignment/models.py b/openslides/assignment/models.py index e4f6fa570..265843a3d 100644 --- a/openslides/assignment/models.py +++ b/openslides/assignment/models.py @@ -207,6 +207,9 @@ class Assignment(models.Model, SlideMixin): def get_agenda_title(self): return self.name + def get_agenda_title_supplement(self): + return '(%s)' % _('Assignment') + def delete(self): # Remove any Agenda-Item, which is related to this assignment. for item in Item.objects.filter(related_sid=self.sid): @@ -248,6 +251,7 @@ class Assignment(models.Model, SlideMixin): ('can_manage_assignment', ugettext_noop('Can manage assignments')), # TODO: Add plural s also to the codestring ) ordering = ('name',) + verbose_name = ugettext_noop('Assignment') register_slidemodel(Assignment) diff --git a/openslides/assignment/urls.py b/openslides/assignment/urls.py index 55ef169cb..714ddc6df 100644 --- a/openslides/assignment/urls.py +++ b/openslides/assignment/urls.py @@ -13,7 +13,7 @@ from django.conf.urls import url, patterns from openslides.assignment.views import (ViewPoll, AssignmentPDF, - AssignmentPollPDF, AssignmentPollDelete, CreateAgendaItem) + AssignmentPollPDF, AssignmentPollDelete, CreateRelatedAgendaItemView) urlpatterns = patterns('openslides.assignment.views', url(r'^$', @@ -70,8 +70,8 @@ urlpatterns = patterns('openslides.assignment.views', name='print_assignment_poll', ), - url(r'^(?P\d+)/agenda/$', - CreateAgendaItem.as_view(), + url(r'^(?P\d+)/agenda/$', + CreateRelatedAgendaItemView.as_view(), name='assignment_create_agenda', ), diff --git a/openslides/assignment/views.py b/openslides/assignment/views.py index d2660ebc2..482749684 100644 --- a/openslides/assignment/views.py +++ b/openslides/assignment/views.py @@ -34,7 +34,7 @@ from openslides.config.api import config from openslides.participant.models import User, Group from openslides.projector.projector import Widget from openslides.poll.views import PollFormView -from openslides.agenda.models import Item +from openslides.agenda.views import CreateRelatedAgendaItemView as _CreateRelatedAgendaItemView from openslides.assignment.models import Assignment, AssignmentPoll from openslides.assignment.forms import AssignmentForm, AssignmentRunForm @@ -487,16 +487,11 @@ class AssignmentPDF(PDFView): '
'), stylesheet['Paragraph'])) -class CreateAgendaItem(RedirectView): - permission_required = 'agenda.can_manage_agenda' - - def pre_redirect(self, request, *args, **kwargs): - self.assignment = Assignment.objects.get(pk=kwargs['assignment_id']) - self.item = Item(related_sid=self.assignment.sid) - self.item.save() - - def get_redirect_url(self, **kwargs): - return reverse('item_overview') +class CreateRelatedAgendaItemView(_CreateRelatedAgendaItemView): + """ + View to create and agenda item for an assignment. + """ + model = Assignment class AssignmentPollPDF(PDFView): diff --git a/openslides/motion/models.py b/openslides/motion/models.py index ea2485020..62f5491ec 100644 --- a/openslides/motion/models.py +++ b/openslides/motion/models.py @@ -95,6 +95,7 @@ class Motion(SlideMixin, models.Model): ('can_manage_motion', ugettext_noop('Can manage motions')), ) ordering = ('identifier', ) + verbose_name = ugettext_noop('Motion') def __unicode__(self): """ @@ -475,7 +476,7 @@ class Motion(SlideMixin, models.Model): def get_agenda_title(self): """ - Return a title for the Agenda. + Return a title for the agenda. """ return self.title diff --git a/openslides/motion/views.py b/openslides/motion/views.py index bbbcc31ab..55b840e6d 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -18,7 +18,6 @@ from django.db import transaction from django.db.models import Model from django.utils.text import slugify from django.utils.translation import ugettext as _, ugettext_lazy, ugettext_noop -from django.views.generic.detail import SingleObjectMixin from django.http import Http404, HttpResponseRedirect from reportlab.platypus import SimpleDocTemplate @@ -33,7 +32,7 @@ from openslides.poll.views import PollFormView from openslides.projector.api import get_active_slide from openslides.projector.projector import Widget, SLIDE from openslides.config.api import config -from openslides.agenda.models import Item +from openslides.agenda.views import CreateRelatedAgendaItemView as _CreateRelatedAgendaItemView from .models import (Motion, MotionSubmitter, MotionSupporter, MotionPoll, MotionVersion, State, WorkflowError, Category) @@ -674,30 +673,20 @@ set_state = MotionSetStateView.as_view() reset_state = MotionSetStateView.as_view(reset=True) -class CreateAgendaItemView(SingleObjectMixin, RedirectView): +class CreateRelatedAgendaItemView(_CreateRelatedAgendaItemView): """ View to create and agenda item for a motion. """ - permission_required = 'agenda.can_manage_agenda' model = Motion - url_name = 'item_overview' - url_name_args = [] - - def get(self, request, *args, **kwargs): - """ - Set self.object to a motion. - """ - self.object = self.get_object() - return super(CreateAgendaItemView, self).get(request, *args, **kwargs) def pre_redirect(self, request, *args, **kwargs): """ Create the agenda item. """ - self.item = Item.objects.create(related_sid=self.object.sid) + super(CreateRelatedAgendaItemView, self).pre_redirect(request, *args, **kwargs) self.object.write_log([ugettext_noop('Agenda item created')], self.request.user) -create_agenda_item = CreateAgendaItemView.as_view() +create_agenda_item = CreateRelatedAgendaItemView.as_view() class MotionPDFView(SingleObjectMixin, PDFView): diff --git a/tests/agenda/models.py b/tests/agenda/models.py index bee6a250f..1aa8eee7a 100644 --- a/tests/agenda/models.py +++ b/tests/agenda/models.py @@ -9,6 +9,9 @@ class ReleatedItem(SlideMixin, models.Model): name = models.CharField(max_length='255') + class Meta: + verbose_name = 'Releated Item CHFNGEJ5634DJ34F' + def get_agenda_title(self): return self.name @@ -19,4 +22,11 @@ class ReleatedItem(SlideMixin, models.Model): return '/absolute-url-here/' +class BadReleatedItem(SlideMixin, models.Model): + prefix = 'badreleateditem' + + name = models.CharField(max_length='255') + + register_slidemodel(ReleatedItem) +register_slidemodel(BadReleatedItem) diff --git a/tests/agenda/tests.py b/tests/agenda/tests.py index 0a9d2bb7b..b4438f9c9 100644 --- a/tests/agenda/tests.py +++ b/tests/agenda/tests.py @@ -6,7 +6,7 @@ Unit test for the agenda app. - :copyright: 2011, 2012 by OpenSlides team, see AUTHORS. + :copyright: 2011-2013 by OpenSlides team, see AUTHORS. :license: GNU GPL, see LICENSE for more details. """ @@ -19,7 +19,7 @@ from openslides.participant.models import User from openslides.agenda.models import Item from openslides.agenda.slides import agenda_show -from .models import ReleatedItem +from .models import ReleatedItem, BadReleatedItem # TODO: Rename releated to related class ItemTest(TestCase): @@ -28,8 +28,8 @@ class ItemTest(TestCase): 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.releated = ReleatedItem.objects.create(name='foo') - self.item5 = Item.objects.create(title='item5', related_sid=self.releated.sid) + self.releated = ReleatedItem.objects.create(name='ekdfjen458gj1siek45nv') + self.item5 = Item.objects.create(title='item5', content_object=self.releated) def testClosed(self): self.assertFalse(self.item1.closed) @@ -61,10 +61,6 @@ class ItemTest(TestCase): self.assertEqual(initial['parent'], 0) self.assertEqual(initial['weight'], item.weight) - def testRelated_sid(self): - self.item1.related_sid = 'foobar' - self.assertFalse(self.item1.get_related_slide() is None) - def test_title_supplement(self): self.assertEqual(self.item1.get_title_supplement(), '') @@ -91,8 +87,24 @@ class ItemTest(TestCase): def test_releated_item(self): self.assertEqual(self.item5.get_title(), self.releated.name) self.assertEqual(self.item5.get_title_supplement(), 'test item') - self.assertEqual(self.item5.get_related_type(), 'releateditem') - self.assertEqual(self.item5.print_related_type(), 'Releateditem') + self.assertEqual(self.item5.content_type.name, 'Releated Item CHFNGEJ5634DJ34F') + + def test_deleted_releated_item(self): + self.releated.delete() + self.assertFalse(ReleatedItem.objects.all().exists()) + self.assertEqual(Item.objects.get(pk=self.item5.pk).title, '< Item for deleted slide (ekdfjen458gj1siek45nv) >') + + def test_bad_releated_item(self): + bad = BadReleatedItem.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):