diff --git a/src/c3nav/editor/migrations/0010_auto_20170704_1431.py b/src/c3nav/editor/migrations/0010_auto_20170704_1431.py new file mode 100644 index 00000000..803210d4 --- /dev/null +++ b/src/c3nav/editor/migrations/0010_auto_20170704_1431.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.2 on 2017-07-04 14:31 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('editor', '0009_auto_20170701_1218'), + ] + + operations = [ + migrations.RemoveField( + model_name='changedobject', + name='last_update', + ), + migrations.RemoveField( + model_name='changedobject', + name='stale', + ), + migrations.AddField( + model_name='changeset', + name='last_change', + field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now, verbose_name='last change'), + preserve_default=False, + ), + ] diff --git a/src/c3nav/editor/models/changedobject.py b/src/c3nav/editor/models/changedobject.py index 37429c8d..9c52ddc0 100644 --- a/src/c3nav/editor/models/changedobject.py +++ b/src/c3nav/editor/models/changedobject.py @@ -2,7 +2,6 @@ import typing from itertools import chain from django.contrib.contenttypes.models import ContentType -from django.core.cache import cache from django.db import models from django.db.models import Field from django.utils.translation import ugettext_lazy as _ @@ -20,14 +19,12 @@ class ChangedObjectManager(models.Manager): class ChangedObject(models.Model): changeset = models.ForeignKey('editor.ChangeSet', on_delete=models.CASCADE, verbose_name=_('Change Set')) created = models.DateTimeField(auto_now_add=True, verbose_name=_('created')) - last_update = models.DateTimeField(auto_now=True, verbose_name=_('last update')) content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) existing_object_pk = models.PositiveIntegerField(null=True, verbose_name=_('id of existing object')) updated_fields = JSONField(default={}, verbose_name=_('updated fields')) m2m_added = JSONField(default={}, verbose_name=_('added m2m values')) m2m_removed = JSONField(default={}, verbose_name=_('removed m2m values')) deleted = models.BooleanField(default=False, verbose_name=_('object was deleted')) - stale = models.BooleanField(default=False, verbose_name=_('stale')) objects = ChangedObjectManager() @@ -109,10 +106,7 @@ class ChangedObject(models.Model): model = self.model_class pk = self.obj_pk - if not self.stale: - self.changeset.changed_objects.setdefault(model, {})[pk] = self - else: - self.changeset.changed_objects.get(model, {}).pop(pk, None) + self.changeset.changed_objects.setdefault(model, {})[pk] = self if self.is_created: if not self.deleted: @@ -268,18 +262,16 @@ class ChangedObject(models.Model): self.m2m_added = {name: tuple(values) for name, values in self._m2m_added_cache.items()} self.m2m_removed = {name: tuple(values) for name, values in self._m2m_removed_cache.items()} if not self.does_something: - self.stale = True - if not self.stale: + if self.pk: + self.delete() + else: if not standalone and self.changeset.pk is None: self.changeset.save() self.changeset = self.changeset - else: - self.existing_object_pk = None if not standalone and not self.changeset.fill_changes_cache(): self.update_changeset_cache() - if not self.stale or self.pk is not None: + if self.does_something: super().save(*args, **kwargs) - cache.set('changeset:%s:last_change' % self.changeset_id, self.last_update, 900) def delete(self, **kwargs): raise TypeError('changed objects can not be deleted directly.') diff --git a/src/c3nav/editor/models/changeset.py b/src/c3nav/editor/models/changeset.py index 12c9ba4f..34ae9f06 100644 --- a/src/c3nav/editor/models/changeset.py +++ b/src/c3nav/editor/models/changeset.py @@ -1,12 +1,13 @@ from collections import OrderedDict +from contextlib import contextmanager from itertools import chain from django.apps import apps from django.conf import settings -from django.core.cache import cache -from django.db import models -from django.db.models import Max, Q +from django.db import models, transaction +from django.db.models import Q from django.urls import reverse +from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy @@ -28,6 +29,7 @@ class ChangeSet(models.Model): ('applied', _('accepted')), ) created = models.DateTimeField(auto_now_add=True, verbose_name=_('created')) + last_change = models.DateTimeField(auto_now_add=True, verbose_name=_('last change')) state = models.CharField(max_length=20, choices=STATES, default='unproposed') author = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete=models.PROTECT, verbose_name=_('Author')) title = models.CharField(max_length=100, default='', verbose_name=_('Title')) @@ -139,7 +141,7 @@ class ChangeSet(models.Model): return self.wrap_model(instance.__class__).create_wrapped_model_class()(self, instance) def relevant_changed_objects(self): - return self.changed_objects_set.exclude(stale=True).exclude(existing_object_pk__isnull=True, deleted=True) + return self.changed_objects_set.exclude(existing_object_pk__isnull=True, deleted=True) def fill_changes_cache(self, include_deleted_created=False): """ @@ -158,7 +160,7 @@ class ChangeSet(models.Model): return False if include_deleted_created: - qs = self.changed_objects_set.exclude(stale=True) + qs = self.changed_objects_set.all() else: qs = self.relevant_changed_objects() @@ -166,8 +168,6 @@ class ChangeSet(models.Model): for change in qs: change.update_changeset_cache() - self._last_change_cache = max(change.last_update for change in qs) - return True """ @@ -269,13 +269,33 @@ class ChangeSet(models.Model): return self.state in ('unproposed', 'review') def can_see(self, request): - return self.session_id == request.session.session_key or self.author_id is request.user.pk + return self.author == request.user or (not request.user.is_authenticated and self.author is None) - def can_edit(self, request): - return (self.session_id == request.session.session_key and self.state in ('unproposed', 'review')) + @contextmanager + def lock_to_edit_changes(self, request): + with transaction.atomic(): + if self.pk is not None: + changeset = ChangeSet.objects.select_for_update().get(pk=self.pk) + if not changeset.can_edit_changes(request): + raise PermissionError + yield + changeset.last_change = timezone.now() + changeset.save() + else: + yield + + def could_edit_changes(self, request): + if self.state == 'unproposed': + return self.author == request.user or (self.author is None and not request.user.is_authenticated) + elif self.state == 'review': + return self.assigned_to == request.user + return False + + def can_edit_changes(self, request): + return self.session_id == request.session.session_key and self.could_edit_changes(request) def can_delete(self, request): - return self.can_edit(request) and self.state == 'unproposed' + return self.can_edit_changes(request) and self.state == 'unproposed' def can_propose(self, request): return self.author_id == request.user.pk and self.state == 'unproposed' @@ -315,26 +335,6 @@ class ChangeSet(models.Model): return (ungettext_lazy('%(num)d changed object', '%(num)d changed objects', 'num') % {'num': self.changed_objects_count}) - @property - def last_change(self): - last_change = cache.get('changeset:%s:last_change' % self.pk) - if last_change is None: - # was not in cache, calculate it - try: - last_change = self.changed_objects_set.aggregate(Max('last_update'))['last_update__max'] - except ChangedObject.DoesNotExist: - last_change = self.created - elif self.last_change_cache is None or self.last_change_cache <= last_change: - # was in cache and our local value (if we had one) is not newer - return last_change - else: - # our local value is newer - last_change = self.last_change_cache - - # update cache - cache.set('changeset:%s:last_change' % self.pk, last_change, 900) - return last_change - @property def cache_key(self): if self.pk is None: diff --git a/src/c3nav/editor/templates/editor/changeset.html b/src/c3nav/editor/templates/editor/changeset.html index 72f0cd1a..6ba3d4c3 100644 --- a/src/c3nav/editor/templates/editor/changeset.html +++ b/src/c3nav/editor/templates/editor/changeset.html @@ -22,9 +22,10 @@ {% blocktrans %}created at {{ datetime }}{% endblocktrans %} {% endif %} {% endwith %} - {% if changeset.proposed %} -
{% with datetime=changeset.proposed|date:"DATETIME_FORMAT" %}{% blocktrans %}proposed at {{ datetime }}{% endblocktrans %}{% endwith %} - {% endif %} +
+ {% with datetime=changeset.last_change|date:"DATETIME_FORMAT" %} + {% blocktrans %}last change at {{ datetime }}{% endblocktrans %} + {% endwith %}

{% bootstrap_messages %} {% if changeset.description %} diff --git a/src/c3nav/editor/templates/editor/edit.html b/src/c3nav/editor/templates/editor/edit.html index 69d8b6c9..4f9d54c7 100644 --- a/src/c3nav/editor/templates/editor/edit.html +++ b/src/c3nav/editor/templates/editor/edit.html @@ -15,21 +15,28 @@ {% endwith %} {% endif %} +{% bootstrap_messages %}
{% csrf_token %} {% bootstrap_form form %} {% buttons %} - {% if not new %} - + {% endif %} + {% endif %} - - - {% trans 'Cancel' %} + + {% if can_edit %} + {% trans 'Cancel' %} + {% else %} + {% trans 'Back' %} + {% endif %} {% endbuttons %}
diff --git a/src/c3nav/editor/templates/editor/index.html b/src/c3nav/editor/templates/editor/index.html index 693047ba..b24d8fe8 100644 --- a/src/c3nav/editor/templates/editor/index.html +++ b/src/c3nav/editor/templates/editor/index.html @@ -1,11 +1,14 @@ {% load bootstrap3 %} {% load i18n %} - - {% trans 'Level' as model_title %} - {% blocktrans %}New {{ model_title }}{% endblocktrans %} - +{% if can_edit %} + + {% trans 'Level' as model_title %} + {% blocktrans %}New {{ model_title }}{% endblocktrans %} + +{% endif %}

{% trans 'Level' %}

+{% bootstrap_messages %}
{% for l in levels %} diff --git a/src/c3nav/editor/templates/editor/level.html b/src/c3nav/editor/templates/editor/level.html index fb04d44f..d7873faf 100644 --- a/src/c3nav/editor/templates/editor/level.html +++ b/src/c3nav/editor/templates/editor/level.html @@ -21,21 +21,24 @@ « {% trans 'back to parent level' %} {% endif %}

+{% bootstrap_messages %} {% include 'editor/fragment_child_models.html' %}
-{% if level.on_top_of == None %} - - {% blocktrans %}New {{ model_title }}{% endblocktrans %} - -

{% trans 'Levels on top' %}

-
-{% for l in levels_on_top %} - - {{ l.title }} - -{% endfor %} -
+{% if level.on_top_of is None %} + {% if can_edit %} + + {% blocktrans %}New {{ model_title }}{% endblocktrans %} + + {% endif %} +

{% trans 'Levels on top' %}

+
+ {% for l in levels_on_top %} + + {{ l.title }} + + {% endfor %} +
{% endif %} diff --git a/src/c3nav/editor/templates/editor/list.html b/src/c3nav/editor/templates/editor/list.html index d85bdda0..68c7312f 100644 --- a/src/c3nav/editor/templates/editor/list.html +++ b/src/c3nav/editor/templates/editor/list.html @@ -3,9 +3,11 @@ {% include 'editor/fragment_levels.html' %} - - {% blocktrans %}New {{ model_title }}{% endblocktrans %} - +{% if can_edit %} + + {% blocktrans %}New {{ model_title }}{% endblocktrans %} + +{% endif %}

{% blocktrans %}{{ model_title_plural }}{% endblocktrans %} {% if level %} @@ -19,6 +21,7 @@ {% endwith %} {% endif %}

+{% bootstrap_messages %} {% if explicit_edit %} {% trans 'Details' as edit_caption %} diff --git a/src/c3nav/editor/templates/editor/space.html b/src/c3nav/editor/templates/editor/space.html index 7581e948..2b04b5fd 100644 --- a/src/c3nav/editor/templates/editor/space.html +++ b/src/c3nav/editor/templates/editor/space.html @@ -10,5 +10,6 @@

« {% trans 'back to overview' %}

+{% bootstrap_messages %} {% include 'editor/fragment_child_models.html' %} diff --git a/src/c3nav/editor/views/changes.py b/src/c3nav/editor/views/changes.py index 97b8fe23..59b2b94a 100644 --- a/src/c3nav/editor/views/changes.py +++ b/src/c3nav/editor/views/changes.py @@ -26,7 +26,7 @@ def changeset_detail(request, pk): if not changeset.can_see(request): raise Http404 - can_edit = changeset.can_edit(request) + can_edit = changeset.can_edit_changes(request) can_delete = changeset.can_delete(request) if request.method == 'POST': @@ -36,14 +36,18 @@ def changeset_detail(request, pk): raise PermissionDenied try: - changed_object = changeset.changed_objects_set.get(pk=restore) - except: - pass - else: - if changed_object.deleted: - changed_object.deleted = False - changed_object.save(standalone=True) - messages.success(request, _('Object has been successfully restored.')) + with changeset.lock_to_edit_changes(request): + try: + changed_object = changeset.changed_objects_set.get(pk=restore) + except: + pass + else: + if changed_object.deleted: + changed_object.deleted = False + changed_object.save(standalone=True) + messages.success(request, _('Object has been successfully restored.')) + except PermissionDenied: + messages.error(request, _('You can not edit changes on this changeset.')) return redirect(reverse('editor.changesets.detail', kwargs={'pk': changeset.pk})) diff --git a/src/c3nav/editor/views/edit.py b/src/c3nav/editor/views/edit.py index b669753d..64af1d11 100644 --- a/src/c3nav/editor/views/edit.py +++ b/src/c3nav/editor/views/edit.py @@ -1,5 +1,6 @@ from contextlib import suppress +from django.contrib import messages from django.core.exceptions import FieldDoesNotExist, PermissionDenied from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse @@ -70,6 +71,8 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e Level = request.changeset.wrap_model('Level') Space = request.changeset.wrap_model('Space') + can_edit = request.changeset.can_edit_changes(request) + obj = None if pk is not None: # Edit existing map item @@ -98,6 +101,7 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e 'pk': pk, 'model_name': model.__name__.lower(), 'model_title': model._meta.verbose_name, + 'can_edit': can_edit, 'new': new, 'title': obj.title if obj else None, } @@ -170,7 +174,13 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e if not new and request.POST.get('delete') == '1': # Delete this mapitem! if request.POST.get('delete_confirm') == '1': - obj.delete() + try: + with request.changeset.lock_to_edit_changes(request): + obj.delete() + except PermissionDenied: + messages.error(request, _('You can not edit changes on this changeset.')) + return redirect(request.path) + messages.success(request, _('Object was successfully deleted.')) if model == Level: if obj.on_top_of_id is not None: return redirect(reverse('editor.levels.detail', kwargs={'pk': obj.on_top_of_id})) @@ -201,19 +211,23 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e if on_top_of is not None: obj.on_top_of = on_top_of - obj.save() + try: + with request.changeset.lock_to_edit_changes(request): + obj.save() - if form.redirect_slugs is not None: - for slug in form.add_redirect_slugs: - obj.redirects.create(slug=slug) + if form.redirect_slugs is not None: + for slug in form.add_redirect_slugs: + obj.redirects.create(slug=slug) - for slug in form.remove_redirect_slugs: - obj.redirects.filter(slug=slug).delete() + for slug in form.remove_redirect_slugs: + obj.redirects.filter(slug=slug).delete() - form.save_m2m() - # request.changeset.changes.all().delete() - - return redirect(ctx['back_url']) + form.save_m2m() + except PermissionDenied: + messages.error(request, _('You can not edit changes on this changeset.')) + else: + messages.success(request, _('Object was successfully saved.')) + return redirect(ctx['back_url']) else: form = model.EditorForm(instance=obj, request=request)