From b8be4a810564791e215ef4824184f3e4458fea4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Kl=C3=BCnder?= Date: Tue, 4 Jul 2017 20:11:26 +0200 Subject: [PATCH] reimplement propose and unpropose with lock_for_update --- src/c3nav/editor/models/changeset.py | 61 ++++++++++++++++------------ src/c3nav/editor/views/changes.py | 35 +++++++--------- src/c3nav/editor/views/edit.py | 6 +-- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/src/c3nav/editor/models/changeset.py b/src/c3nav/editor/models/changeset.py index c23e77b0..5661acd3 100644 --- a/src/c3nav/editor/models/changeset.py +++ b/src/c3nav/editor/models/changeset.py @@ -78,19 +78,6 @@ class ChangeSet(models.Model): qs = qs.filter(author__isnull=True, session_id=request.session.session_key) return qs - @classmethod - def qs_for_request_editable(cls, request): - """ - Returns a base QuerySet to get only changesets the current user is allowed to edit - """ - qs = cls.qs_for_request(request) - if request.user.is_authenticated: - qs = qs.filter(Q(state='review', assigned_to=request.user) | - Q(state='unproposed', author=request.user)) - else: - qs = qs.filter(state='unproposed') - return qs - @classmethod def get_for_request(cls, request): """ @@ -103,7 +90,7 @@ class ChangeSet(models.Model): In any case, the default autor for changes added to the queryset during this request will be set to the current user. """ - qs = cls.qs_for_request_editable(request) + qs = cls.qs_for_request(request) if request.session.session_key is not None: changeset = qs.filter(session_id=request.session.session_key).first() @@ -266,22 +253,27 @@ class ChangeSet(models.Model): Permissions """ @property - def editable(self): - return self.state in ('unproposed', 'review') + def changes_editable(self): + return self.state in ('unproposed', 'rejected', 'review') + + @property + def proposed(self): + return self.state not in ('unproposed', 'rejected') def can_see(self, request): return self.author == request.user or (not request.user.is_authenticated and self.author is None) @contextmanager - def lock_to_edit_changes(self, request): + def lock_to_edit(self, request=None): 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): + if request is not None and not changeset.can_edit(request): raise PermissionError + self._object_changed = False - yield - if self._object_changed: + yield changeset + if self._object_changed and request is not None: update = changeset.updates.create(user=request.user if request.user.is_authenticated else None, objects_changed=True) changeset.last_update = update.datetime @@ -290,24 +282,41 @@ class ChangeSet(models.Model): else: yield - def could_edit_changes(self, request): + def could_edit(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_edit(self, request): + return self.session_id == request.session.session_key and self.could_edit(request) def can_delete(self, request): - return self.can_edit_changes(request) and self.state == 'unproposed' + return self.can_edit(request) and self.state == 'unproposed' def can_propose(self, request): - return self.author_id == request.user.pk and self.state == 'unproposed' + return self.can_edit(request) and not self.proposed def can_unpropose(self, request): - return self.author_id == request.user.pk and self.state in ('proposed', 'rejected') + return self.author_id == request.user.pk and self.state in ('proposed', 'reproposed') + + """ + Update methods + """ + def propose(self, user): + new_state = {'unproposed': 'proposed', 'rejected': 'reproposed'}[self.state] + update = self.updates.create(user=user, state=new_state) + self.state = new_state + self.last_update = update.datetime + self.save() + + def unpropose(self, user): + new_state = {'proposed': 'unproposed', 'reproposed': 'rejected'}[self.state] + update = self.updates.create(user=user, state=new_state) + self.state = new_state + self.last_update = update.datetime + self.save() """ Methods for display diff --git a/src/c3nav/editor/views/changes.py b/src/c3nav/editor/views/changes.py index 59b2b94a..b861b85b 100644 --- a/src/c3nav/editor/views/changes.py +++ b/src/c3nav/editor/views/changes.py @@ -7,7 +7,6 @@ from django.core.exceptions import PermissionDenied from django.http import Http404 from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse -from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from c3nav.editor.forms import ChangeSetForm @@ -26,17 +25,14 @@ def changeset_detail(request, pk): if not changeset.can_see(request): raise Http404 - can_edit = changeset.can_edit_changes(request) + can_edit = changeset.can_edit(request) can_delete = changeset.can_delete(request) if request.method == 'POST': restore = request.POST.get('restore') if restore and restore.isdigit(): - if not can_edit: - raise PermissionDenied - try: - with changeset.lock_to_edit_changes(request): + with changeset.lock_to_edit(request): try: changed_object = changeset.changed_objects_set.get(pk=restore) except: @@ -56,25 +52,22 @@ def changeset_detail(request, pk): messages.info(request, _('You need to log in to propose changes.')) return redirect(reverse('editor.login')+'?redirect='+request.path) - if changeset.can_propose(request): - changeset.proposed = timezone.now() - changeset.session_id = None - changeset.save() - messages.success(request, _('You proposed your changes.')) - else: - messages.error(request, _('You cannot propose this changeset.')) + with changeset.lock_to_edit() as changeset: + if changeset.can_propose(request): + changeset.propose(request.user) + messages.success(request, _('You proposed your changes.')) + else: + messages.error(request, _('You cannot propose this changeset.')) return redirect(reverse('editor.changesets.detail', kwargs={'pk': changeset.pk})) elif request.POST.get('unpropose') == '1': - if changeset.can_unpropose(request): - changeset.proposed = None - changeset.assigned_to = None - changeset.session_id = None - changeset.save() - messages.success(request, _('You unproposed your changes.')) - else: - messages.error(request, _('You cannot unpropose this changeset.')) + with changeset.lock_to_edit() as changeset: + if changeset.can_unpropose(request): + changeset.unpropose(request.user) + messages.success(request, _('You unproposed your changes.')) + else: + messages.error(request, _('You cannot unpropose 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 64af1d11..baa12948 100644 --- a/src/c3nav/editor/views/edit.py +++ b/src/c3nav/editor/views/edit.py @@ -71,7 +71,7 @@ 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) + can_edit = request.changeset.can_edit(request) obj = None if pk is not None: @@ -175,7 +175,7 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e # Delete this mapitem! if request.POST.get('delete_confirm') == '1': try: - with request.changeset.lock_to_edit_changes(request): + with request.changeset.lock_to_edit(request): obj.delete() except PermissionDenied: messages.error(request, _('You can not edit changes on this changeset.')) @@ -212,7 +212,7 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e obj.on_top_of = on_top_of try: - with request.changeset.lock_to_edit_changes(request): + with request.changeset.lock_to_edit(request): obj.save() if form.redirect_slugs is not None: