diff --git a/CHANGELOG b/CHANGELOG index 5ba0f2c3d..6a68e5eaa 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,8 +4,16 @@ http://openslides.org -Version 1.6.2 (unreleased) -========================== +Version 1.7 (unreleased) +======================== +[https://github.com/OpenSlides/OpenSlides/milestones/1.7] + +motion: +- New Feature to create amendments, which are related to a parent motion. + + +Version 1.6.2 +============= [https://github.com/OpenSlides/OpenSlides/milestones/1.6.2] Motions: - Added possibility to hide motions from non staff users in some states. diff --git a/openslides/motion/forms.py b/openslides/motion/forms.py index 9ecae40a8..e55a9e261 100644 --- a/openslides/motion/forms.py +++ b/openslides/motion/forms.py @@ -3,7 +3,6 @@ from django import forms from django.utils.translation import ugettext_lazy -from openslides.config.api import config from openslides.mediafile.models import Mediafile from openslides.utils.forms import (CleanHtmlFormMixin, CssClassMixin, CSVImportForm, LocalizedModelChoiceField) @@ -66,8 +65,6 @@ class BaseMotionForm(CleanHtmlFormMixin, CssClassMixin, forms.ModelForm): self.initial['text'] = last_version.text self.initial['reason'] = last_version.reason self.initial['attachments'] = self.motion.attachments.all() - else: - self.initial['text'] = config['motion_preamble'] super(BaseMotionForm, self).__init__(*args, **kwargs) diff --git a/openslides/motion/models.py b/openslides/motion/models.py index 54a5b3e08..093815fd6 100644 --- a/openslides/motion/models.py +++ b/openslides/motion/models.py @@ -71,8 +71,12 @@ class Motion(SlideMixin, AbsoluteUrlMixin, models.Model): Many to many relation to mediafile objects. """ - # TODO: proposal - # master = models.ForeignKey('self', null=True, blank=True) + parent = models.ForeignKey('self', null=True, blank=True, related_name='amendments') + """ + Field for amendments to reference to the motion that should be altered. + + Null if the motion is not an amendment. + """ class Meta: permissions = ( @@ -206,19 +210,31 @@ class Motion(SlideMixin, AbsoluteUrlMixin, models.Model): def set_identifier(self): """ - Sets the motion identifier automaticly according to the config - value if it is not set yet. + Sets the motion identifier automaticly according to the config value if + it is not set yet. """ + # The identifier is already set or should be set manually if config['motion_identifier'] == 'manually' or self.identifier: # Do not set an identifier. return + + # The motion is an amendment + elif self.is_amendment(): + motions = self.parent.amendments.all() + + # The motions should be counted per category elif config['motion_identifier'] == 'per_category': motions = Motion.objects.filter(category=self.category) - else: # That means: config['motion_identifier'] == 'serially_numbered' + + # The motions should be counted over all. + else: motions = Motion.objects.all() number = motions.aggregate(Max('identifier_number'))['identifier_number__max'] or 0 - if self.category is None or not self.category.prefix: + if self.is_amendment(): + parent_identifier = self.parent.identifier or '' + prefix = '%s %s ' % (parent_identifier, config['motion_amendments_prefix']) + elif self.category is None or not self.category.prefix: prefix = '' else: prefix = '%s ' % self.category.prefix @@ -524,6 +540,15 @@ class Motion(SlideMixin, AbsoluteUrlMixin, models.Model): """ MotionLog.objects.create(motion=self, message_list=message_list, person=person) + def is_amendment(self): + """ + Returns True if the motion is an amendment. + + A motion is a amendment if amendments are activated in the config and + the motion has a parent. + """ + return config['motion_amendments_enabled'] and self.parent is not None + class MotionVersion(AbsoluteUrlMixin, models.Model): """ @@ -819,10 +844,12 @@ class State(models.Model): """If true, new versions are not automaticly set active.""" dont_set_identifier = models.BooleanField(default=False) - """Decides if the motion gets an identifier. + """ + Decides if the motion gets an identifier. If true, the motion does not get an identifier if the state change to - this one, else it does.""" + this one, else it does. + """ def __unicode__(self): """Returns the name of the state.""" diff --git a/openslides/motion/signals.py b/openslides/motion/signals.py index aba69d1bb..f6b9a4778 100644 --- a/openslides/motion/signals.py +++ b/openslides/motion/signals.py @@ -3,7 +3,7 @@ from django import forms from django.dispatch import receiver from django.utils.translation import ugettext as _ -from django.utils.translation import ugettext_lazy, ugettext_noop +from django.utils.translation import ugettext_lazy, ugettext_noop, pgettext from openslides.config.api import ConfigGroup, ConfigGroupedCollection, ConfigVariable from openslides.config.signals import config_signal @@ -67,6 +67,25 @@ def setup_motion_config(sender, **kwargs): motion_stop_submitting, motion_allow_disable_versioning)) + # Amendments + motion_amendments_enabled = ConfigVariable( + name='motion_amendments_enabled', + default_value=False, + form_field=forms.BooleanField( + label=ugettext_lazy('Activate amendments'), + required=False)) + + motion_amendments_prefix = ConfigVariable( + name='motion_amendments_prefix', + default_value=pgettext('Prefix for amendment', 'A'), + form_field=forms.CharField( + required=False, + label=ugettext_lazy('Prefix for the identifier for amendments'))) + + group_amendments = ConfigGroup( + title=ugettext_lazy('Amendments'), + variables=(motion_amendments_enabled, motion_amendments_prefix)) + # Supporters motion_min_supporters = ConfigVariable( name='motion_min_supporters', @@ -148,7 +167,8 @@ def setup_motion_config(sender, **kwargs): title=ugettext_noop('Motion'), url='motion', weight=30, - groups=(group_general, group_supporters, group_ballot_papers, group_pdf)) + groups=(group_general, group_amendments, group_supporters, + group_ballot_papers, group_pdf)) @receiver(post_database_setup, dispatch_uid='motion_create_builtin_workflows') diff --git a/openslides/motion/templates/motion/motion_detail.html b/openslides/motion/templates/motion/motion_detail.html index 9eaa029fd..99929a5fa 100644 --- a/openslides/motion/templates/motion/motion_detail.html +++ b/openslides/motion/templates/motion/motion_detail.html @@ -29,6 +29,9 @@ {% endif %} {% endif %} + {% if motion.is_amendment %} + (Amendment of {{ motion.parent.identifier|default:motion.parent }}) + {% endif %} @@ -280,6 +283,23 @@ {{ version.creation_time }} + {% if 'motion_amendments_enabled'|get_config %} +
{% trans 'Amendments' %}:
+ {% with amendments=motion.amendments.all %} + {% if amendments %} + + {% endif %} + {% endwith %} + + + {% trans 'New amendment' %} + + {% endif %} + {% if perms.motion.can_support_motion and 'motion_min_supporters'|get_config > 0 %} {% if allowed_actions.unsupport %} diff --git a/openslides/motion/templates/motion/motion_list.html b/openslides/motion/templates/motion/motion_list.html index 53be1324c..0b5313387 100644 --- a/openslides/motion/templates/motion/motion_list.html +++ b/openslides/motion/templates/motion/motion_list.html @@ -33,7 +33,7 @@ {% if perms.motion.can_create_motion %} {% if not 'motion_stop_submitting'|get_config or perms.motion.can_manage_motion %} - {% trans 'New' %} + {% trans 'New' %} {% endif %} {% endif %} {% if perms.motion.can_manage_motion %} @@ -64,7 +64,14 @@ {% for motion in motion_list %} {{ motion.identifier|default:'' }} - {{ motion.title }} + + {{ motion.title }} + {% if motion.is_amendment %} + + {{ 'motion_amendments_prefix'|get_config }} + + {% endif %} + {% if motion.category %}{{ motion.category }}{% else %}–{% endif %} {% trans motion.state.name %} diff --git a/openslides/motion/templates/motion/slide.html b/openslides/motion/templates/motion/slide.html index ae64d6696..ab7ca9190 100644 --- a/openslides/motion/templates/motion/slide.html +++ b/openslides/motion/templates/motion/slide.html @@ -77,6 +77,9 @@ {{ motion.active_version.title }} {% trans "Motion" %} {{ motion.identifier|default:'' }} + {% if motion.is_amendment %} + (Amendment of {{ motion.parent.identifier|default:motion.parent }}) + {% endif %} {% if motion.get_active_version.version_number > 1 %} | {% trans 'Version' %} {{ motion.active_version.version_number }}{% endif %} diff --git a/openslides/motion/urls.py b/openslides/motion/urls.py index af8085b03..c9d73e2ce 100644 --- a/openslides/motion/urls.py +++ b/openslides/motion/urls.py @@ -2,107 +2,112 @@ from django.conf.urls import patterns, url +from . import views + # TODO: define the Views inhere urlpatterns = patterns( 'openslides.motion.views', url(r'^$', - 'motion_list', + views.MotionListView.as_view(), name='motion_list'), url(r'^new/$', - 'motion_create', - # TODO: rename to motion_create - name='motion_new'), + views.MotionCreateView.as_view(), + name='motion_create'), url(r'^(?P\d+)/$', - 'motion_detail', + views.MotionDetailView.as_view(), name='motion_detail'), url(r'^(?P\d+)/edit/$', - 'motion_update', + views.MotionUpdateView.as_view(), name='motion_update'), url(r'^(?P\d+)/del/$', - 'motion_delete', + views.MotionDeleteView.as_view(), name='motion_delete'), + url(r'^(?P\d+)/new_amendment/$', + views.MotionCreateAmendmentView.as_view(), + name='motion_create_amendment'), + url(r'^(?P\d+)/version/(?P\d+)/$', - 'motion_detail', + views.MotionDetailView.as_view(), name='motion_version_detail'), url(r'^(?P\d+)/version/(?P\d+)/permit/$', - 'version_permit', + views.VersionPermitView.as_view(), name='motion_version_permit'), url(r'^(?P\d+)/version/(?P\d+)/del/$', - 'version_delete', + views.VersionDeleteView.as_view(), name='motion_version_delete'), url(r'^(?P\d+)/diff/$', - 'version_diff', + views.VersionDiffView.as_view(), name='motion_version_diff'), url(r'^(?P\d+)/support/$', - 'motion_support', + views.SupportView.as_view(support=True), name='motion_support'), url(r'^(?P\d+)/unsupport/$', - 'motion_unsupport', + views.SupportView.as_view(support=False), name='motion_unsupport'), url(r'^(?P\d+)/create_poll/$', - 'poll_create', + views.PollCreateView.as_view(), name='motionpoll_create'), url(r'^(?P\d+)/poll/(?P\d+)/edit/$', - 'poll_update', + views.PollUpdateView.as_view(), name='motionpoll_update'), url(r'^(?P\d+)/poll/(?P\d+)/del/$', - 'poll_delete', + views.PollDeleteView.as_view(), name='motionpoll_delete'), url(r'^(?P\d+)/poll/(?P\d+)/pdf/$', - 'poll_pdf', + views.PollPDFView.as_view(), name='motionpoll_pdf'), url(r'^(?P\d+)/set_state/(?P\d+)/$', - 'set_state', + views.MotionSetStateView.as_view(), name='motion_set_state'), url(r'^(?P\d+)/reset_state/$', - 'reset_state', + views.MotionSetStateView.as_view(reset=True), name='motion_reset_state'), url(r'^(?P\d+)/agenda/$', - 'create_agenda_item', + views.CreateRelatedAgendaItemView.as_view(), name='motion_create_agenda'), url(r'^pdf/$', - 'motion_list_pdf', + views.MotionPDFView.as_view(print_all_motions=True), name='motion_list_pdf'), url(r'^(?P\d+)/pdf/$', - 'motion_detail_pdf', + views.MotionPDFView.as_view(print_all_motions=False), name='motion_detail_pdf'), url(r'^category/$', - 'category_list', + views.CategoryListView.as_view(), name='motion_category_list'), url(r'^category/new/$', - 'category_create', + views.CategoryCreateView.as_view(), name='motion_category_create'), url(r'^category/(?P\d+)/edit/$', - 'category_update', + views.CategoryUpdateView.as_view(), name='motion_category_update'), url(r'^category/(?P\d+)/del/$', - 'category_delete', + views.CategoryDeleteView.as_view(), name='motion_category_delete'), url(r'^csv_import/$', - 'motion_csv_import', + views.MotionCSVImportView.as_view(), name='motion_csv_import'), ) diff --git a/openslides/motion/views.py b/openslides/motion/views.py index cbd1cbbdf..168929064 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -57,8 +57,6 @@ class MotionListView(ListView): motions.append(motion) return motions -motion_list = MotionListView.as_view() - class MotionDetailView(DetailView): """ @@ -96,8 +94,6 @@ class MotionDetailView(DetailView): 'reason': version.reason}) return super(MotionDetailView, self).get_context_data(**kwargs) -motion_detail = MotionDetailView.as_view() - class MotionEditMixin(object): """ @@ -205,17 +201,26 @@ class MotionCreateView(MotionEditMixin, CreateView): def form_valid(self, form): """ - Write a log message if the form is valid. + Write a log message and set the submitter if necessary. """ + # First, validate and process the form and create the motion response = super(MotionCreateView, self).form_valid(form) + + # Write the log message self.object.write_log([ugettext_noop('Motion created')], self.request.user) + + # Set submitter to request.user if no submitter is set yet if ('submitter' not in form.cleaned_data or not form.cleaned_data['submitter']): self.object.add_submitter(self.request.user) return response def get_initial(self): + """ + Sets the initial data for the MotionCreateForm. + """ initial = super(MotionCreateView, self).get_initial() + initial['text'] = config['motion_preamble'] if self.request.user.has_perm('motion.can_manage_motion'): initial['workflow'] = config['motion_workflow'] return initial @@ -229,7 +234,53 @@ class MotionCreateView(MotionEditMixin, CreateView): self.object.reset_state(workflow) self.version = self.object.get_new_version() -motion_create = MotionCreateView.as_view() + +class MotionCreateAmendmentView(MotionCreateView): + """ + Create an amendment. + """ + + def dispatch(self, *args, **kwargs): + if not config['motion_amendments_enabled']: + raise Http404('Amendments are disabled in the config.') + return super(MotionCreateAmendmentView, self).dispatch(*args, **kwargs) + + def get_parent_motion(self): + """ + Gets the parent motion from the url. + + Caches the value. + """ + try: + parent = self._object_parent + except AttributeError: + # self.get_object() is the django method, which does not cache the + # object. For now this is not a problem, because get_object() is only + # called once. + parent = self._object_parent = self.get_object() + return parent + + def manipulate_object(self, form): + """ + Sets the parent to the motion to which this amendment refers. + """ + self.object.parent = self.get_parent_motion() + super(MotionCreateAmendmentView, self).manipulate_object(form) + + def get_initial(self): + """ + Sets the initial values to the form. + + This are the values for title, text and reason which are set to the + values from the parent motion. + """ + initial = super(MotionCreateAmendmentView, self).get_initial() + parent = self.get_parent_motion() + initial['title'] = parent.title + initial['text'] = parent.text + initial['reason'] = parent.reason + initial['category'] = parent.category + return initial class MotionUpdateView(MotionEditMixin, UpdateView): @@ -297,8 +348,6 @@ class MotionUpdateView(MotionEditMixin, UpdateView): self.version = self.object.get_last_version() self.used_new_version = False -motion_update = MotionUpdateView.as_view() - class MotionDeleteView(DeleteView): """ @@ -316,8 +365,6 @@ class MotionDeleteView(DeleteView): def get_final_message(self): return _('%s was successfully deleted.') % _('Motion') -motion_delete = MotionDeleteView.as_view() - class VersionDeleteView(DeleteView): """ @@ -349,8 +396,6 @@ class VersionDeleteView(DeleteView): def get_url_name_args(self): return (self.get_object().motion_id, ) -version_delete = VersionDeleteView.as_view() - class VersionPermitView(SingleObjectMixin, QuestionView): """ @@ -396,8 +441,6 @@ class VersionPermitView(SingleObjectMixin, QuestionView): ugettext_noop('permitted')], person=self.request.user) -version_permit = VersionPermitView.as_view() - class VersionDiffView(DetailView): """ @@ -438,8 +481,6 @@ class VersionDiffView(DetailView): }) return context -version_diff = VersionDiffView.as_view() - class SupportView(SingleObjectMixin, QuestionView): """ @@ -502,9 +543,6 @@ class SupportView(SingleObjectMixin, QuestionView): else: return _("You have unsupported this motion successfully.") -motion_support = SupportView.as_view(support=True) -motion_unsupport = SupportView.as_view(support=False) - class PollCreateView(SingleObjectMixin, RedirectView): """ @@ -528,8 +566,6 @@ class PollCreateView(SingleObjectMixin, RedirectView): """ return reverse('motionpoll_update', args=[self.get_object().pk, self.poll.poll_number]) -poll_create = PollCreateView.as_view() - class PollMixin(object): """ @@ -595,8 +631,6 @@ class PollUpdateView(PollMixin, PollFormView): self.get_object().write_log([ugettext_noop('Poll updated')], self.request.user) return value -poll_update = PollUpdateView.as_view() - class PollDeleteView(PollMixin, DeleteView): """ @@ -618,8 +652,6 @@ class PollDeleteView(PollMixin, DeleteView): """ return reverse('motion_detail', args=[self.get_object().motion.pk]) -poll_delete = PollDeleteView.as_view() - class PollPDFView(PollMixin, PDFView): """ @@ -649,8 +681,6 @@ class PollPDFView(PollMixin, PDFView): """ motion_poll_to_pdf(pdf, self.get_object()) -poll_pdf = PollPDFView.as_view() - class MotionSetStateView(SingleObjectMixin, RedirectView): """ @@ -688,9 +718,6 @@ class MotionSetStateView(SingleObjectMixin, RedirectView): _('The state of the motion was set to %s.') % html_strong(_(self.get_object().state.name))) -set_state = MotionSetStateView.as_view() -reset_state = MotionSetStateView.as_view(reset=True) - class CreateRelatedAgendaItemView(_CreateRelatedAgendaItemView): """ @@ -705,8 +732,6 @@ class CreateRelatedAgendaItemView(_CreateRelatedAgendaItemView): super(CreateRelatedAgendaItemView, self).pre_redirect(request, *args, **kwargs) self.get_object().write_log([ugettext_noop('Agenda item created')], self.request.user) -create_agenda_item = CreateRelatedAgendaItemView.as_view() - class MotionPDFView(SingleObjectMixin, PDFView): """ @@ -767,16 +792,11 @@ class MotionPDFView(SingleObjectMixin, PDFView): else: motion_to_pdf(pdf, self.get_object()) -motion_list_pdf = MotionPDFView.as_view(print_all_motions=True) -motion_detail_pdf = MotionPDFView.as_view(print_all_motions=False) - class CategoryListView(ListView): required_permission = 'motion.can_manage_motion' model = Category -category_list = CategoryListView.as_view() - class CategoryCreateView(CreateView): required_permission = 'motion.can_manage_motion' @@ -784,8 +804,6 @@ class CategoryCreateView(CreateView): success_url_name = 'motion_category_list' url_name_args = [] -category_create = CategoryCreateView.as_view() - class CategoryUpdateView(UpdateView): required_permission = 'motion.can_manage_motion' @@ -793,8 +811,6 @@ class CategoryUpdateView(UpdateView): success_url_name = 'motion_category_list' url_name_args = [] -category_update = CategoryUpdateView.as_view() - class CategoryDeleteView(DeleteView): required_permission = 'motion.can_manage_motion' @@ -803,8 +819,6 @@ class CategoryDeleteView(DeleteView): url_name_args = [] success_url_name = 'motion_category_list' -category_delete = CategoryDeleteView.as_view() - class MotionCSVImportView(CSVImportView): """ @@ -830,5 +844,3 @@ class MotionCSVImportView(CSVImportView): messages.error(self.request, error) # Overleap method of CSVImportView return super(CSVImportView, self).form_valid(form) - -motion_csv_import = MotionCSVImportView.as_view() diff --git a/tests/motion/test_models.py b/tests/motion/test_models.py index 1e6d30ec1..65dba507a 100644 --- a/tests/motion/test_models.py +++ b/tests/motion/test_models.py @@ -147,6 +147,64 @@ class ModelTest(TestCase): # motion.__unicode__() raised an AttributeError self.assertEqual(str(motion), 'test_identifier_VohT1hu9uhiSh6ooVBFS | test_title_Koowoh1ISheemeey1air') + def test_is_amendment(self): + config['motion_amendments_enabled'] = True + amendment = Motion.objects.create(title='amendment', parent=self.motion) + + self.assertTrue(amendment.is_amendment()) + self.assertFalse(self.motion.is_amendment()) + + def test_set_identifier_allready_set(self): + """ + If the motion already has a identifier, the method does nothing. + """ + motion = Motion(identifier='My test identifier') + + motion.set_identifier() + + self.assertEqual(motion.identifier, 'My test identifier') + + def test_set_identifier_manually(self): + """ + If the config is set to manually, the method does nothing. + """ + config['motion_identifier'] = 'manually' + motion = Motion() + + motion.set_identifier() + + # If the identifier should be set manually, the method does nothing + self.assertIsNone(motion.identifier) + + def test_set_identifier_amendment(self): + """ + If the motion is an amendment, the identifier is the identifier from the + parent + a suffix. + """ + config['motion_amendments_enabled'] = True + self.motion.identifier = 'Parent identifier' + self.motion.save() + motion = Motion(parent=self.motion) + + motion.set_identifier() + + self.assertEqual(motion.identifier, 'Parent identifier A 1') + + def test_set_identifier_second_amendment(self): + """ + If a motion has already an amendment, the second motion gets another + identifier. + """ + config['motion_amendments_enabled'] = True + self.motion.identifier = 'Parent identifier' + self.motion.save() + Motion.objects.create(title='Amendment1', parent=self.motion) + motion = Motion(parent=self.motion) + + motion.set_identifier() + + self.assertEqual(motion.identifier, 'Parent identifier A 2') + class ConfigTest(TestCase): def test_stop_submitting(self): diff --git a/tests/motion/test_views.py b/tests/motion/test_views.py index 273215c4b..c68c48735 100644 --- a/tests/motion/test_views.py +++ b/tests/motion/test_views.py @@ -2,12 +2,15 @@ import os import tempfile +from mock import MagicMock from django.conf import settings +from django.test import RequestFactory from django.test.client import Client from openslides.config.api import config from openslides.mediafile.models import Mediafile +from openslides.motion import views from openslides.motion.models import Category, Motion, MotionLog, State from openslides.participant.models import Group, User from openslides.utils.test import TestCase @@ -244,6 +247,67 @@ class TestMotionCreateView(MotionViewTestCase): self.assertEqual(MotionLog.objects.get(pk=1).message_list, ['Motion created']) +class TestMotionCreateAmendmentView(MotionViewTestCase): + url = '/motion/1/new_amendment/' + + def test_get_amendment_active(self): + config['motion_amendments_enabled'] = True + self.check_url(self.url, self.admin_client, 200) + + def test_get_amendment_inactive(self): + config['motion_amendments_enabled'] = False + self.check_url(self.url, self.admin_client, 404) + + def test_get_parent_motion(self): + motion = Motion.objects.create(title='Test Motion') + view = views.MotionCreateAmendmentView() + view.request = RequestFactory().get(self.url) + view.kwargs = {'pk': motion.pk} + + self.assertEqual(view.get_parent_motion(), motion) + + def test_manipulate_object(self): + motion = Motion.objects.create(title='Test Motion') + view = views.MotionCreateAmendmentView() + view.request = RequestFactory().get(self.url) + view.kwargs = {'pk': motion.pk} + view.object = MagicMock() + + view.manipulate_object(MagicMock()) + + self.assertEqual(view.object.parent, motion) + + def test_get_initial(self): + motion = Motion.objects.create( + title='Test Motion', text='Parent Motion text', reason='test reason') + view = views.MotionCreateAmendmentView() + view.request = MagicMock() + view.kwargs = {'pk': motion.pk} + + self.assertEqual(view.get_initial(), { + 'reason': u'test reason', + 'text': u'Parent Motion text', + 'title': u'Test Motion', + 'category': None, + 'workflow': '1'}) + + def test_get_initial_with_category(self): + category = Category.objects.create(name='test category') + motion = Motion.objects.create( + title='Test Motion', text='Parent Motion text', reason='test reason', + category=category) + view = views.MotionCreateAmendmentView() + view.request = MagicMock() + view.kwargs = {'pk': motion.pk} + + self.assertEqual(view.get_initial(), { + 'reason': u'test reason', + 'text': u'Parent Motion text', + 'title': u'Test Motion', + 'category': category, + 'workflow': '1'}) + + class TestMotionUpdateView(MotionViewTestCase): url = '/motion/1/edit/'