From cd19920223364197ffa87063be3628f6f92b756a Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Sat, 1 Jun 2013 12:36:42 +0200 Subject: [PATCH] Rework of the motion model, to make it more explcit. --- openslides/motion/forms.py | 3 +- openslides/motion/models.py | 234 ++++++++-------- .../templates/motion/motion_detail.html | 46 ++-- openslides/motion/views.py | 255 ++++++++++-------- tests/motion/test_models.py | 64 ++--- tests/motion/test_views.py | 26 +- 6 files changed, 329 insertions(+), 299 deletions(-) diff --git a/openslides/motion/forms.py b/openslides/motion/forms.py index 061d2ba43..c4df70ec6 100644 --- a/openslides/motion/forms.py +++ b/openslides/motion/forms.py @@ -67,7 +67,8 @@ class BaseMotionForm(CleanHtmlFormMixin, CssClassMixin, forms.ModelForm): class MotionSubmitterMixin(forms.ModelForm): """Mixin to append the submitter field to a MotionForm.""" - submitter = MultiplePersonFormField(label=ugettext_lazy("Submitter")) + submitter = MultiplePersonFormField(label=ugettext_lazy("Submitter"), + required=False) """Submitter of the motion. Can be one or more persons.""" def __init__(self, *args, **kwargs): diff --git a/openslides/motion/models.py b/openslides/motion/models.py index 517ac238d..fcd9c9548 100644 --- a/openslides/motion/models.py +++ b/openslides/motion/models.py @@ -93,8 +93,7 @@ class Motion(SlideMixin, models.Model): ('can_support_motion', ugettext_noop('Can support motions')), ('can_manage_motion', ugettext_noop('Can manage motions')), ) - # TODO: order per default by category and identifier - # ordering = ('number',) + ordering = ('identifier', ) def __unicode__(self): """ @@ -103,67 +102,90 @@ class Motion(SlideMixin, models.Model): return self.active_version.title # TODO: Use transaction - def save(self, ignore_version_data=False, *args, **kwargs): + def save(self, use_version=None, *args, **kwargs): """ Save the motion. 1. Set the state of a new motion to the default state. 2. Ensure that the identifier is not an empty string. 3. Save the motion object. - 4. Save the version data, if ignore_version_data == False. - 5. Set the active version for the motion, if ignore_version_data == False. + 4. Save the version data + 5. Set the active version for the motion, if a new version object was saved. + + The version data is *not* saved, if + 1. The django-feature 'update_fields' is used or + 2. The argument use_version is False (differ to None). + + The version object into which the data is saved is picked in this order: + 1. The argument use_version. + 2. The attribute use_version. As default, use_version is the + active_version. If the active_version is not set, it is the + last_version. If the last_version is not set, it is a + new_version. See use_version property. + + use_version is the version object, in which the version data is saved. + * If use_version is False, no version data ist saved. + * If use_version is None, the last version is used. + + To use a new version object, you have to set it via use_version. You have + to set the title, text and reason into this version object. motion.title + etc will be ignored. """ if not self.state: self.reset_state() - # TODO: Bad hack here to make Motion.objects.create() work - # again. We have to remove the flag to force an INSERT given - # by Django's create() method without knowing its advantages - # because of our misuse of the save() method in the - # set_identifier() method. - kwargs.pop('force_insert', None) - if not self.identifier and self.identifier is not None: # TODO: Why not >if self.identifier is '':< + # Solves the problem, that there can only be one motion with an empty + # string as identifier + if self.identifier is '': self.identifier = None super(Motion, self).save(*args, **kwargs) - if not ignore_version_data: - # Select version object - version = self.last_version - if hasattr(self, '_new_version'): - version = self.new_version - del self._new_version - version.motion = self # TODO: Test if this line is really neccessary. + if 'update_fields' in kwargs: + # Do not save the version-data, if only some motion fields are updated + return - # Save title, text and reason in the version object + if use_version is False: + # We do not need to save the version + return + elif use_version is None: + use_version = self.get_last_version() + + # Save title, text and reason in the version object. for attr in ['title', 'text', 'reason']: _attr = '_%s' % attr - try: - setattr(version, attr, getattr(self, _attr)) + data = getattr(self, _attr, None) + if data is not None: + setattr(use_version, attr, data) delattr(self, _attr) - except AttributeError: - if self.versions.exists(): - # If the _attr was not set, use the value from last_version - setattr(version, attr, getattr(self.last_version, attr)) - # Set version_number of the new Version (if neccessary) and save it into the DB - if version.id is None: - # TODO: auto increment the version_number in the Database - version_number = self.versions.aggregate(Max('version_number'))['version_number__max'] or 0 - version.version_number = version_number + 1 - version.save() + # If version is not in the database, test if it has new data and set + # the version_number + if use_version.id is None: + if not self.version_data_changed(use_version): + # We do not need to save the version + return + version_number = self.versions.aggregate(Max('version_number'))['version_number__max'] or 0 + use_version.version_number = version_number + 1 - # Set the active version of this motion. This has to be done after the - # version is saved to the database - if self.active_version is None or not self.state.leave_old_version_active: - self.active_version = version - self.save(ignore_version_data=True) + # Necessary line, if the version was set before the motion had an id. + # propably a django bug. + use_version.motion = use_version.motion + + use_version.save() + + # Set the active version of this motion. This has to be done after the + # version is saved to the database + if self.active_version is None or not self.state.leave_old_version_active: + self.active_version = use_version + self.save(update_fields=['active_version']) def get_absolute_url(self, link='detail'): """ Return an URL for this version. - The keyword argument 'link' can be 'detail', 'view', 'edit', 'update' or 'delete'. + The keyword argument 'link' can be 'detail', 'view', 'edit', + 'update' or 'delete'. """ if link == 'view' or link == 'detail': return reverse('motion_detail', args=[str(self.id)]) @@ -172,6 +194,23 @@ class Motion(SlideMixin, models.Model): if link == 'delete': return reverse('motion_delete', args=[str(self.id)]) + def version_data_changed(self, version): + """ + Compare the version with the last version of the motion. + + Returns True if the version data (title, text, reason) is different. + Else, returns False. + """ + if not self.versions.exists(): + # if there is no version in the database, the data has always changed + return True + + last_version = self.get_last_version() + for attr in ['title', 'text', 'reason']: + if getattr(last_version, attr) != getattr(version, attr): + return True + return False + def set_identifier(self): """ Sets the motion identifier automaticly according to the config @@ -189,20 +228,16 @@ class Motion(SlideMixin, models.Model): if self.category is None or not self.category.prefix: prefix = '' else: - prefix = self.category.prefix + ' ' + prefix = '%s ' % self.category.prefix - # TODO: Do not use the save() method in this method, see note in - # the save() method above. - while True: + number += 1 + identifier = '%s%d' % (prefix, number) + while Motion.objects.filter(identifier=identifier).exists(): number += 1 - self.identifier = '%s%d' % (prefix, number) - self.identifier_number = number - try: - self.save(ignore_version_data=True) - except IntegrityError: - continue - else: - break + identifier = '%s%d' % (prefix, number) + + self.identifier = identifier + self.identifier_number = number def get_title(self): """ @@ -213,7 +248,7 @@ class Motion(SlideMixin, models.Model): try: return self._title except AttributeError: - return self.version.title + return self.get_active_version().title def set_title(self, title): """ @@ -240,7 +275,7 @@ class Motion(SlideMixin, models.Model): try: return self._text except AttributeError: - return self.version.text + return self.get_active_version().text def set_text(self, text): """ @@ -266,7 +301,7 @@ class Motion(SlideMixin, models.Model): try: return self._reason except AttributeError: - return self.version.reason + return self.get_active_version().reason def set_reason(self, reason): """ @@ -283,72 +318,47 @@ class Motion(SlideMixin, models.Model): Is saved in a MotionVersion object. """ - @property - def new_version(self): + def get_new_version(self): """ Return a version object, not saved in the database. - On the first call, it creates a new version. On any later call, it - use the existing new version. - - The new_version object will be deleted when it is saved into the db. + The version data of the new version object is populated with the data + set via motion.title, motion.text, motion.reason. If the data is not set, + it is population with the data from the last version object. """ - try: - return self._new_version - except AttributeError: - self._new_version = MotionVersion(motion=self) - return self._new_version - - def get_version(self): - """ - Get the 'active' version object. - - This version will be used to get the data for this motion. - """ - try: - return self._version - except AttributeError: - return self.last_version - - def set_version(self, version): - """ - Set the 'active' version object. - - The keyword argument 'version' can be a MotionVersion object or the - version_number of a version object or None. - - If the argument is None, the newest version will be used. - """ - if version is None: - try: - del self._version - except AttributeError: - pass + new_version = MotionVersion(motion=self) + if self.versions.exists(): + last_version = self.get_last_version() else: - if type(version) is int: - version = self.versions.get(version_number=version) - elif type(version) is not MotionVersion: - raise ValueError('The argument \'version\' has to be int or ' - 'MotionVersion, not %s' % type(version)) - # TODO: Test, that the version is one of this motion - self._version = version + last_version = None + for attr in ['title', 'text', 'reason']: + _attr = '_%s' % attr + data = getattr(self, _attr, None) + if data is None and not last_version is None: + data = getattr(last_version, attr) + if data is not None: + setattr(new_version, attr, data) + return new_version - version = property(get_version, set_version) - """ - The active version of this motion. - """ + def get_active_version(self): + """ + Returns the active version of the motion. - @property - def last_version(self): + If no active version is set by now, the last_version is used + """ + if self.active_version: + return self.active_version + else: + return self.get_last_version() + + def get_last_version(self): """ Return the newest version of the motion. """ - # TODO: Fix the case, that the motion has no version. - # Check whether the case, that a motion has not any version, can still appear. try: return self.versions.order_by('-version_number')[0] except IndexError: - return self.new_version + return self.get_new_version() @property def submitters(self): @@ -459,7 +469,7 @@ class Motion(SlideMixin, models.Model): """ Return a title for the Agenda. """ - return self.active_version.title + return self.title def get_agenda_title_supplement(self): """ @@ -518,16 +528,6 @@ class Motion(SlideMixin, models.Model): """ MotionLog.objects.create(motion=self, message_list=message_list, person=person) - def set_active_version(self, version): - """ - Set the active version of a motion to 'version'. - - 'version' can be a version object, or the version_number of a version. - """ - if type(version) is int: - version = self.versions.get(version_number=version) - self.active_version = version - class MotionVersion(models.Model): """ diff --git a/openslides/motion/templates/motion/motion_detail.html b/openslides/motion/templates/motion/motion_detail.html index cf1b1ad24..eda356bfb 100644 --- a/openslides/motion/templates/motion/motion_detail.html +++ b/openslides/motion/templates/motion/motion_detail.html @@ -8,7 +8,7 @@ {% block content %}

- {{ motion.title }} {{ motion.category|default:'' }} + {{ title }} {{ motion.category|default:'' }}
{% if motion.identifier %} @@ -17,7 +17,7 @@ [{% trans "no number" %}], {% endif %} {# TODO: show only for workflow with versioning #} - {% trans "Version" %} {{ motion.version.version_number }} + {% trans "Version" %} {{ version.version_number }} {% trans "Back to overview" %} @@ -57,33 +57,31 @@
{# TODO: show only for workflow with versioning #} - {% if motion.version.version_number != motion.last_version.version_number %} - - {% trans "This is not the newest version." %} - - {% trans "Go to the newest version" %} - (# {{ motion.last_version.version_number }}) - {% endif %} - {% if motion.version.version_number != motion.active_version.version_number %} - - {% trans "This version is not authorized." %} - - {% trans "Go to the authorized version" %} - (# {{ motion.active_version.version_number }}) - {% endif %} + {% with last_version=motion.get_last_version active_version=motion.get_active_version %} + {% if version.version_number != last_version.version_number %} + + {% trans "This is not the newest version." %} + + {% trans "Go to the newest version" %} + (# {{ last_version.version_number }}) + {% endif %} + {% if version.version_number != active_version.version_number %} + + {% trans "This version is not authorized." %} + + {% trans "Go to the authorized version" %} + (# {{ active_version.version_number }}) + {% endif %} + {% endwith %}

{% trans "Motion text" %}:

- {{ motion.version.text|safe }} + {{ text|safe }}

{% trans "Reason" %}:

- {% if motion.version.reason %} - {{ motion.version.reason|safe }} - {% else %} - – - {% endif %} + {{ reason|safe|default:'–' }}
@@ -102,7 +100,7 @@ {% trans "Actions" %} {% endif %} - + {% if version == motion.active_version %} @@ -239,7 +237,7 @@ {% endif %} {% endwith %}

- {{ motion.version.creation_time }} + {{ motion.use_version.creation_time }} {# TODO: Check this button #} diff --git a/openslides/motion/views.py b/openslides/motion/views.py index f76ab2945..c93c8128a 100644 --- a/openslides/motion/views.py +++ b/openslides/motion/views.py @@ -18,7 +18,7 @@ from django.db import transaction from django.db.models import Model from django.utils.translation import ugettext as _, ugettext_lazy, ugettext_noop from django.views.generic.detail import SingleObjectMixin -from django.http import Http404 +from django.http import Http404, HttpResponseRedirect from reportlab.platypus import SimpleDocTemplate @@ -44,74 +44,71 @@ from .csv_import import import_motions class MotionListView(ListView): - """View, to list all motions.""" + """ + View, to list all motions. + """ permission_required = 'motion.can_see_motion' model = Motion motion_list = MotionListView.as_view() -class GetVersionMixin(object): - """Mixin to set a specific version to a motion.""" - - def get_object(self): - """Return a Motion object. The id is taken from the url and the version - is set to the version with the 'version_number' from the URL.""" - object = super(GetVersionMixin, self).get_object() - version_number = self.kwargs.get('version_number', None) - if version_number is not None: - try: - object.version = int(version_number) - except MotionVersion.DoesNotExist: - raise Http404('Version %s not found' % version_number) - else: - object.version = object.active_version - return object - - -class MotionDetailView(GetVersionMixin, DetailView): - """Show one motion.""" +class MotionDetailView(DetailView): + """ + Show one motion. + """ permission_required = 'motion.can_see_motion' model = Motion def get_context_data(self, **kwargs): - """Return the template context. + """ + Return the template context. Append the allowed actions for the motion to the context. """ - context = super(MotionDetailView, self).get_context_data(**kwargs) - context['allowed_actions'] = self.object.get_allowed_actions(self.request.user) - return context + version_number = self.kwargs.get('version_number', None) + if version_number is not None: + try: + version = self.object.versions.get(version_number=int(version_number)) + except MotionVersion.DoesNotExist: + raise Http404('Version %s not found' % version_number) + else: + version = self.object.get_active_version() + + kwargs.update({ + 'allowed_actions': self.object.get_allowed_actions(self.request.user), + 'version': version, + 'title': version.title, + 'text': version.text, + 'reason': version.reason}) + return super(MotionDetailView, self).get_context_data(**kwargs) motion_detail = MotionDetailView.as_view() -class MotionMixin(object): +class MotionEditMixin(object): """ Mixin for MotionViewsClasses to save the version data. """ - def manipulate_object(self, form): + def form_valid(self, form): """ - Saves the version data into the motion object before it is saved in - the Database. Does also set category, identifier and new workflow - if given. + Saves the Create or UpdateForm into a motion object. """ - super(MotionMixin, self).manipulate_object(form) - for attr in ['title', 'text', 'reason']: - setattr(self.object, attr, form.cleaned_data[attr]) + self.object = form.save(commit=False) - if type(self) == MotionCreateView: - self.object.new_version - else: - for attr in ['title', 'text', 'reason']: - if getattr(self.object, attr) != getattr(self.object.last_version, attr): - new_data = True - break + if type(self) == MotionUpdateView: + # Decide if a new version is saved to the database + if (self.object.state.versioning and + not form.cleaned_data.get('disable_versioning', False)): + version = self.object.get_new_version() else: - new_data = False - if new_data and self.object.state.versioning and not form.cleaned_data.get('disable_versioning', False): - self.object.new_version + version = self.object.get_last_version() + else: + version = self.object.get_new_version() + + for attr in ['title', 'text', 'reason']: + setattr(version, attr, form.cleaned_data[attr]) try: self.object.category = form.cleaned_data['category'] @@ -127,11 +124,9 @@ class MotionMixin(object): if workflow: self.object.reset_state(workflow) - def post_save(self, form): - """ - Save the submitter an the supporter so the motion. - """ - super(MotionMixin, self).post_save(form) + self.object.save(use_version=version) + + # Save the submitter an the supporter so the motion. # TODO: only delete and save neccessary submitters and supporter if 'submitter' in form.cleaned_data: self.object.submitter.all().delete() @@ -143,6 +138,7 @@ class MotionMixin(object): MotionSupporter.objects.bulk_create( [MotionSupporter(motion=self.object, person=person) for person in form.cleaned_data['supporter']]) + return HttpResponseRedirect(self.get_success_url()) def get_form_class(self): """ @@ -173,8 +169,10 @@ class MotionMixin(object): return type('MotionForm', tuple(form_classes), {}) -class MotionCreateView(MotionMixin, CreateView): - """View to create a motion.""" +class MotionCreateView(MotionEditMixin, CreateView): + """ + View to create a motion. + """ model = Motion def has_permission(self, request, *args, **kwargs): @@ -190,21 +188,22 @@ class MotionCreateView(MotionMixin, CreateView): return False def form_valid(self, form): - """Write a log message, if the form is valid.""" - value = super(MotionCreateView, self).form_valid(form) + """ + Write a log message, if the form is valid. + """ + response = super(MotionCreateView, self).form_valid(form) self.object.write_log([ugettext_noop('Motion created')], self.request.user) - return value - - def post_save(self, form): - super(MotionCreateView, self).post_save(form) if not 'submitter' in form.cleaned_data: self.object.add_submitter(self.request.user) + return response motion_create = MotionCreateView.as_view() -class MotionUpdateView(MotionMixin, UpdateView): - """View to update a motion.""" +class MotionUpdateView(MotionEditMixin, UpdateView): + """ + View to update a motion. + """ model = Motion def has_permission(self, request, *args, **kwargs): @@ -213,32 +212,28 @@ class MotionUpdateView(MotionMixin, UpdateView): def form_valid(self, form): """Write a log message, if the form is valid.""" - value = super(MotionUpdateView, self).form_valid(form) - self.object.write_log([ugettext_noop('Motion updated')], self.request.user) - return value - - def manipulate_object(self, *args, **kwargs): - """ - Removes the supporters if config option is True and supporting is still - available in the state. - """ - return_value = super(MotionUpdateView, self).manipulate_object(*args, **kwargs) + response = super(MotionUpdateView, self).form_valid(form) 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 return_value + self.object.write_log([ugettext_noop('Motion updated')], self.request.user) + return response motion_edit = MotionUpdateView.as_view() class MotionDeleteView(DeleteView): - """View to delete a motion.""" + """ + View to delete a motion. + """ model = Motion success_url_name = 'motion_list' def has_permission(self, request, *args, **kwargs): - """Check if the request.user has the permission to delete the motion.""" + """ + Check if the request.user has the permission to delete the motion. + """ return self.get_object().get_allowed_actions(request.user)['delete'] def get_success_message(self): @@ -247,7 +242,7 @@ class MotionDeleteView(DeleteView): motion_delete = MotionDeleteView.as_view() -class VersionPermitView(GetVersionMixin, SingleObjectMixin, QuestionMixin, RedirectView): +class VersionPermitView(SingleObjectMixin, QuestionMixin, RedirectView): """ View to permit a version of a motion. """ @@ -262,46 +257,56 @@ class VersionPermitView(GetVersionMixin, SingleObjectMixin, QuestionMixin, Redir Set self.object to a motion. """ self.object = self.get_object() + version_number = self.kwargs.get('version_number', None) + try: + self.version = self.object.versions.get(version_number=int(version_number)) + except MotionVersion.DoesNotExist: + raise Http404('Version %s not found' % version_number) 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. + Returns a list with arguments to create the success- and question_url. """ - return [self.object.pk, self.object.version.version_number] + return [self.object.pk, self.version.version_number] def get_question(self): """ 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 + return _('Are you sure you want permit version %s?') % self.version.version_number def case_yes(self): """ Activate the version, if the user chooses 'yes'. """ - self.object.set_active_version(self.object.version) - self.object.save(ignore_version_data=True) + self.object.active_version = self.version + self.object.save(update_fields=['active_version']) self.object.write_log( - message_list=[ugettext_noop('Version %d permitted') % self.object.version.version_number], + message_list=[ugettext_noop('Version %d permitted') + % self.version.version_number], person=self.request.user) version_permit = VersionPermitView.as_view() class VersionDiffView(DetailView): - """Show diff between two versions of a motion.""" + """ + Show diff between two versions of a motion. + """ permission_required = 'motion.can_see_motion' model = Motion template_name = 'motion/motion_diff.html' def get_context_data(self, **kwargs): - """Return the template context with versions and html diff strings.""" + """ + Return the template context with versions and html diff strings. + """ try: rev1 = int(self.request.GET['rev1']) rev2 = int(self.request.GET['rev2']) - version_rev1 = self.object.versions.get(version_number=self.request.GET['rev1']) - version_rev2 = self.object.versions.get(version_number=self.request.GET['rev2']) + version_rev1 = self.object.versions.get(version_number=rev1) + version_rev2 = self.object.versions.get(version_number=rev2) diff_text = htmldiff(version_rev1.text, version_rev2.text) diff_reason = htmldiff(version_rev1.reason, version_rev2.reason) except (KeyError, ValueError, MotionVersion.DoesNotExist): @@ -323,7 +328,8 @@ version_diff = VersionDiffView.as_view() class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): - """View to support or unsupport a motion. + """ + View to support or unsupport a motion. If self.support is True, the view will append a request.user to the supporter list. @@ -335,12 +341,16 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): support = True def get(self, request, *args, **kwargs): - """Set self.object to a motion.""" + """ + Set self.object to a motion. + """ self.object = self.get_object() return super(SupportView, self).get(request, *args, **kwargs) def check_permission(self, request): - """Return True if the user can support or unsupport the motion. Else: False.""" + """ + Return True if the user can support or unsupport the motion. Else: False. + """ allowed_actions = self.object.get_allowed_actions(request.user) if self.support and not allowed_actions['support']: messages.error(request, _('You can not support this motion.')) @@ -352,14 +362,17 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): return True def get_question(self): - """Return the question string.""" + """ + Return the question string. + """ if self.support: return _('Do you really want to support this motion?') else: return _('Do you really want to unsupport this motion?') def case_yes(self): - """Append or remove the request.user from the motion. + """ + Append or remove the request.user from the motion. First the method checks the permissions, and writes a log message after appending or removing the user. @@ -374,14 +387,18 @@ class SupportView(SingleObjectMixin, QuestionMixin, RedirectView): self.object.write_log([ugettext_noop("Supporter: -%s") % user], user) def get_success_message(self): - """Return the success message.""" + """ + Return the success message. + """ if self.support: return _("You have supported this motion successfully.") else: return _("You have unsupported this motion successfully.") def get_redirect_url(self, **kwargs): - """Return the url, the view should redirect to.""" + """ + Return the url, the view should redirect to. + """ return self.object.get_absolute_url() motion_support = SupportView.as_view(support=True) @@ -389,23 +406,31 @@ motion_unsupport = SupportView.as_view(support=False) class PollCreateView(SingleObjectMixin, RedirectView): - """View to create a poll for a motion.""" + """ + View to create a poll for a motion. + """ permission_required = 'motion.can_manage_motion' model = Motion def get(self, request, *args, **kwargs): - """Set self.object to a motion.""" + """ + Set self.object to a motion. + """ self.object = self.get_object() return super(PollCreateView, self).get(request, *args, **kwargs) def pre_redirect(self, request, *args, **kwargs): - """Create the poll for the motion.""" + """ + Create the poll for the motion. + """ self.poll = self.object.create_poll() self.object.write_log([ugettext_noop("Poll created")], request.user) messages.success(request, _("New vote was successfully created.")) def get_redirect_url(self, **kwargs): - """Return the URL to the EditView of the poll.""" + """ + Return the URL to the EditView of the poll. + """ return reverse('motion_poll_edit', args=[self.object.pk, self.poll.poll_number]) poll_create = PollCreateView.as_view() @@ -555,31 +580,38 @@ class MotionSetStateView(SingleObjectMixin, RedirectView): except WorkflowError, e: # TODO: Is a WorkflowError still possible here? messages.error(request, e) else: - self.object.save(ignore_version_data=True) + self.object.save(update_fields=['state']) self.object.write_log( message_list=[ugettext_noop('State changed to '), self.object.state.name], person=self.request.user) messages.success(request, - _('The state of the motion was set to %s.') % html_strong(_(self.object.state.name))) + _('The state of the motion was set to %s.') + % html_strong(_(self.object.state.name))) set_state = MotionSetStateView.as_view() reset_state = MotionSetStateView.as_view(reset=True) class CreateAgendaItemView(SingleObjectMixin, RedirectView): - """View to create and agenda item for a motion.""" + """ + 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.""" + """ + 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.""" + """ + Create the agenda item. + """ self.item = Item.objects.create(related_sid=self.object.sid) self.object.write_log([ugettext_noop('Agenda item created')], self.request.user) @@ -587,32 +619,40 @@ create_agenda_item = CreateAgendaItemView.as_view() class MotionPDFView(SingleObjectMixin, PDFView): - """Create the PDF for one, or all motions. + """ + Create the PDF for one, or all motions. If self.print_all_motions is True, the view returns a PDF with all motions. If self.print_all_motions is False, the view returns a PDF with only one - motion.""" + motion. + """ permission_required = 'motion.can_see_motion' model = Motion top_space = 0 print_all_motions = False def get(self, request, *args, **kwargs): - """Set self.object to a motion.""" + """ + Set self.object to a motion. + """ if not self.print_all_motions: self.object = self.get_object() return super(MotionPDFView, self).get(request, *args, **kwargs) def get_filename(self): - """Return the filename for the PDF.""" + """ + Return the filename for the PDF. + """ if self.print_all_motions: return _("Motions") else: return _("Motion: %s") % unicode(self.object) def append_to_pdf(self, pdf): - """Append PDF objects.""" + """ + Append PDF objects. + """ if self.print_all_motions: motions_to_pdf(pdf) else: @@ -694,7 +734,9 @@ motion_csv_import = MotionCSVImportView.as_view() def register_tab(request): - """Return the motion tab.""" + """ + Return the motion tab. + """ # TODO: Find a better way to set the selected var. return Tab( title=_('Motions'), @@ -705,7 +747,8 @@ def register_tab(request): def get_widgets(request): - """Return the motion widgets for the dashboard. + """ + Return the motion widgets for the dashboard. There is only one widget. It shows all motions. """ diff --git a/tests/motion/test_models.py b/tests/motion/test_models.py index 6458a0a67..66f5ad13f 100644 --- a/tests/motion/test_models.py +++ b/tests/motion/test_models.py @@ -19,30 +19,24 @@ class ModelTest(TestCase): def setUp(self): self.motion = Motion.objects.create(title='v1') self.test_user = User.objects.create(username='blub') + # Use the simple workflow self.workflow = Workflow.objects.get(pk=1) def test_create_new_version(self): - motion = Motion.objects.create(title='m1') + motion = self.motion self.assertEqual(motion.versions.count(), 1) - motion.new_version - motion.save() - self.assertEqual(motion.versions.count(), 2) - + # new data, but no new version motion.title = 'new title' motion.save() - self.assertEqual(motion.versions.count(), 2) - - motion.save() - self.assertEqual(motion.versions.count(), 2) + self.assertEqual(motion.versions.count(), 1) + # new data and new version motion.text = 'new text' - motion.new_version - motion.save() - self.assertEqual(motion.versions.count(), 3) - - motion.save() - self.assertEqual(motion.versions.count(), 3) + motion.save(use_version=motion.get_new_version()) + self.assertEqual(motion.versions.count(), 2) + self.assertEqual(motion.title, 'new title') + self.assertEqual(motion.text, 'new text') def test_version_data(self): motion = Motion() @@ -53,36 +47,24 @@ class ModelTest(TestCase): motion.title = 'title' self.assertEqual(motion._title, 'title') + motion.text = 'text' + self.assertEqual(motion._text, 'text') + motion.reason = 'reason' self.assertEqual(motion._reason, 'reason') def test_version(self): - motion = Motion.objects.create(title='v1') + motion = self.motion motion.title = 'v2' - motion.new_version - motion.save() - v2_version = motion.version + motion.save(use_version=motion.get_new_version()) + v2_version = motion.get_last_version() motion.title = 'v3' - motion.new_version - motion.save() + motion.save(use_version=motion.get_new_version()) with self.assertRaises(AttributeError): self._title self.assertEqual(motion.title, 'v3') - motion.version = 1 - self.assertEqual(motion.title, 'v1') - - motion.version = v2_version - self.assertEqual(motion.title, 'v2') - - motion.version = None - motion.version = None # Test to set a version to None, which is already None - self.assertEqual(motion.title, 'v3') - - with self.assertRaises(ValueError): - motion.version = 'wrong' - def test_absolute_url(self): motion_id = self.motion.id @@ -147,23 +129,21 @@ class ModelTest(TestCase): motion = Motion() motion.title = 'foo' motion.text = 'bar' - first_version = motion.version motion.save() + first_version = motion.get_last_version() motion = Motion.objects.get(pk=motion.pk) - motion.new_version motion.title = 'New Title' - motion.save() - new_version = motion.last_version + motion.save(use_version=motion.get_new_version()) + new_version = motion.get_last_version() self.assertEqual(motion.versions.count(), 2) - motion.set_active_version(new_version) + motion.active_version = new_version motion.save() self.assertEqual(motion.versions.count(), 2) - motion.set_active_version(first_version) - motion.version = first_version - motion.save(ignore_version_data=True) + motion.active_version = first_version + motion.save(use_version=False) self.assertEqual(motion.versions.count(), 2) diff --git a/tests/motion/test_views.py b/tests/motion/test_views.py index d77dd1775..c9f3e39d0 100644 --- a/tests/motion/test_views.py +++ b/tests/motion/test_views.py @@ -65,6 +65,16 @@ class TestMotionDetailView(MotionViewTestCase): self.check_url('/motion/500/', self.admin_client, 404) +class TestMotionDetailVersionView(MotionViewTestCase): + def test_get(self): + self.motion1.title = 'AFWEROBjwerGwer' + self.motion1.save(use_version=self.motion1.get_new_version()) + self.check_url('/motion/1/version/1/', self.admin_client, 200) + response = self.check_url('/motion/1/version/2/', self.admin_client, 200) + self.assertContains(response, 'AFWEROBjwerGwer') + self.check_url('/motion/1/version/500/', self.admin_client, 404) + + class TestMotionCreateView(MotionViewTestCase): url = '/motion/new/' @@ -72,7 +82,6 @@ class TestMotionCreateView(MotionViewTestCase): self.check_url(self.url, self.admin_client, 200) def test_admin(self): - self.assertFalse(Motion.objects.filter(versions__title='new motion').exists()) response = self.admin_client.post(self.url, {'title': 'new motion', 'text': 'motion text', 'reason': 'motion reason', @@ -81,7 +90,6 @@ class TestMotionCreateView(MotionViewTestCase): self.assertTrue(Motion.objects.filter(versions__title='new motion').exists()) def test_delegate(self): - self.assertFalse(Motion.objects.filter(versions__title='delegate motion').exists()) response = self.delegate_client.post(self.url, {'title': 'delegate motion', 'text': 'motion text', 'reason': 'motion reason', @@ -91,7 +99,6 @@ class TestMotionCreateView(MotionViewTestCase): self.assertTrue(motion.is_submitter(self.delegate)) def test_registered(self): - self.assertFalse(Motion.objects.filter(versions__title='registered motion').exists()) response = self.registered_client.post(self.url, {'title': 'registered motion', 'text': 'motion text', 'reason': 'motion reason', @@ -297,24 +304,25 @@ class TestMotionDeleteView(MotionViewTestCase): class TestVersionPermitView(MotionViewTestCase): def setUp(self): super(TestVersionPermitView, self).setUp() - self.motion1.new_version - self.motion1.save() + self.motion1.title = 'new' + self.motion1.save(use_version=self.motion1.get_new_version()) 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 + new_version = self.motion1.get_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) + self.assertEqual(self.motion1.get_active_version(), new_version) def test_activate_old_version(self): - new_version = self.motion1.last_version + new_version = self.motion1.get_last_version() first_version = self.motion1.versions.order_by('version_number')[0] - self.motion1.set_active_version(new_version) + self.motion1.active_version = new_version + self.motion1.save() 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)