save last change in changeset and lock changesets before changing stuff

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,
+        ),
+    ]

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.')

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:

src/c3nav/editor/templates/editor/changeset.html

@@ -22,9 +22,10 @@
             {% blocktrans %}created at {{ datetime }}{% endblocktrans %}
         {% endif %}
     {% endwith %}
-    {% if changeset.proposed %}
-        <br>{% with datetime=changeset.proposed|date:"DATETIME_FORMAT" %}{% blocktrans %}proposed at {{ datetime }}{% endblocktrans %}{% endwith %}
-    {% endif %}
+    <br>
+    {% with datetime=changeset.last_change|date:"DATETIME_FORMAT" %}
+        {% blocktrans %}last change at {{ datetime }}{% endblocktrans %}
+    {% endwith %}
 </p>
 {% bootstrap_messages %}
 {% if changeset.description %}

src/c3nav/editor/templates/editor/edit.html

@@ -15,21 +15,28 @@
         {% endwith %}
     {% endif %}
 </h3>
+{% bootstrap_messages %}
 <form action="{{ path }}" method="post" {% if nozoom %}data-nozoom {% endif %}data-onbeforeunload {% if new %}data-new="{{ model_name }}" data-geomtype="{{ geomtype }}"{% else %}data-editing="{{ model_name }}-{{ pk }}"{% endif %}>
     {% csrf_token %}
     {% bootstrap_form form %}
     {% buttons %}
     <button class="invisiblesubmit" type="submit"></button>
-    {% if not new %}
-        <button type="submit" name="delete" value="1" class="btn btn-danger">
-            {% trans 'Delete' %}
+    {% if can_edit %}
+        {% if not new %}
+            <button type="submit" name="delete" value="1" class="btn btn-danger">
+                {% trans 'Delete' %}
+            </button>
+        {% endif %}
+        <button type="submit" accesskey="s" class="btn btn-primary pull-right">
+            {% trans 'Save' %}
         </button>
     {% endif %}
-    <button type="submit" accesskey="s" class="btn btn-primary pull-right">
-        {% trans 'Save' %}
-    </button>
-    <a class="btn {% if new %}btn-danger{% else %}btn-default pull-right{% endif %} cancel-btn" href="{{ back_url }}">
-        {% trans 'Cancel' %}
+    <a class="btn {% if new %}btn-danger{% else %}btn-default {% if can_edit %}pull-right{% endif %}{% endif %} cancel-btn" href="{{ back_url }}">
+        {% if can_edit %}
+            {% trans 'Cancel' %}
+        {% else %}
+            {% trans 'Back' %}
+        {% endif %}
     </a>
     {% endbuttons %}
 </form>

src/c3nav/editor/templates/editor/index.html

@@ -1,11 +1,14 @@
 {% load bootstrap3 %}
 {% load i18n %}
 
-<a class="btn btn-default btn-sm pull-right" accesskey="n" href="{% url 'editor.levels.create' %}">
-    {% trans 'Level' as model_title %}
-    {% blocktrans %}New {{ model_title }}{% endblocktrans %}
-</a>
+{% if can_edit %}
+    <a class="btn btn-default btn-sm pull-right" accesskey="n" href="{% url 'editor.levels.create' %}">
+        {% trans 'Level' as model_title %}
+        {% blocktrans %}New {{ model_title }}{% endblocktrans %}
+    </a>
+{% endif %}
 <h3>{% trans 'Level' %}</h3>
+{% bootstrap_messages %}
 <div class="list-group">
     {% for l in levels %}
         <a href="{% url 'editor.levels.detail' pk=l.pk %}" class="list-group-item">

src/c3nav/editor/templates/editor/level.html

@@ -21,21 +21,24 @@
         <a href="{% url 'editor.levels.detail' pk=level.on_top_of.pk %}">&laquo; {% trans 'back to parent level' %}</a>
     {% endif %}
 </p>
+{% bootstrap_messages %}
 
 {% include 'editor/fragment_child_models.html' %}
 
 <div class="clearfix"></div>
 
-{% if level.on_top_of == None %}
-<a class="btn btn-default btn-sm pull-right" accesskey="n" href="{% url 'editor.levels_on_top.create' on_top_of=level.pk %}">
-    {% blocktrans %}New {{ model_title }}{% endblocktrans %}
-</a>
-<h3>{% trans 'Levels on top' %}</h3>
-<div class="list-group">
-{% for l in levels_on_top %}
-    <a href="{% url 'editor.levels.detail' pk=l.pk %}" class="list-group-item">
-        {{ l.title }}
-    </a>
-{% endfor %}
-</div>
+{% if level.on_top_of is None %}
+    {% if can_edit %}
+        <a class="btn btn-default btn-sm pull-right" accesskey="n" href="{% url 'editor.levels_on_top.create' on_top_of=level.pk %}">
+            {% blocktrans %}New {{ model_title }}{% endblocktrans %}
+        </a>
+    {% endif %}
+    <h3>{% trans 'Levels on top' %}</h3>
+    <div class="list-group">
+    {% for l in levels_on_top %}
+        <a href="{% url 'editor.levels.detail' pk=l.pk %}" class="list-group-item">
+            {{ l.title }}
+        </a>
+    {% endfor %}
+    </div>
 {% endif %}

src/c3nav/editor/templates/editor/list.html

@@ -3,9 +3,11 @@
 
 {% include 'editor/fragment_levels.html' %}
 
-<a class="btn btn-default btn-sm pull-right" accesskey="n" href="{{ create_url }}">
-    {% blocktrans %}New {{ model_title }}{% endblocktrans %}
-</a>
+{% if can_edit %}
+    <a class="btn btn-default btn-sm pull-right" accesskey="n" href="{{ create_url }}">
+        {% blocktrans %}New {{ model_title }}{% endblocktrans %}
+    </a>
+{% endif %}
 <h3>
     {% blocktrans %}{{ model_title_plural }}{% endblocktrans %}
     {% if level %}
@@ -19,6 +21,7 @@
         {% endwith %}
     {% endif %}
 </h3>
+{% bootstrap_messages %}
 
 {% if explicit_edit %}
     {% trans 'Details' as edit_caption %}

src/c3nav/editor/templates/editor/space.html

@@ -10,5 +10,6 @@
 <p>
     <a href="{% url 'editor.spaces.list' level=space.level.pk %}">&laquo; {% trans 'back to overview' %}</a>
 </p>
+{% bootstrap_messages %}
 
 {% include 'editor/fragment_child_models.html' %}

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}))
 

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)