From 4c785ceb83ace7a3507c05206cfa488efed7b5ca Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Tue, 7 May 2019 13:43:58 +0100
Subject: [PATCH] Make the comment editable through the API
---
.../0022_add_versioning_to_comments.py | 29 ++++++++
opentech/apply/activity/models.py | 10 ++-
opentech/apply/activity/tests/test_models.py | 11 +++
opentech/apply/funds/api_views.py | 42 +++++++++++-
opentech/apply/funds/permissions.py | 5 ++
opentech/apply/funds/serializers.py | 11 ++-
opentech/apply/funds/tests/test_api_views.py | 67 +++++++++++++++++++
opentech/apply/funds/urls.py | 2 +
8 files changed, 172 insertions(+), 5 deletions(-)
create mode 100644 opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
create mode 100644 opentech/apply/activity/tests/test_models.py
create mode 100644 opentech/apply/funds/tests/test_api_views.py
diff --git a/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py b/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
new file mode 100644
index 000000000..e883730f1
--- /dev/null
+++ b/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
@@ -0,0 +1,29 @@
+# Generated by Django 2.0.9 on 2019-02-24 00:03
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('activity', '0021_add_review_delete_event'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='activity',
+ name='current',
+ field=models.BooleanField(default=True),
+ ),
+ migrations.AddField(
+ model_name='activity',
+ name='edited',
+ field=models.DateTimeField(default=None, null=True),
+ ),
+ migrations.AddField(
+ model_name='activity',
+ name='previous',
+ field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='activity.Activity'),
+ ),
+ ]
diff --git a/opentech/apply/activity/models.py b/opentech/apply/activity/models.py
index 159bd219c..fcd0c488f 100644
--- a/opentech/apply/activity/models.py
+++ b/opentech/apply/activity/models.py
@@ -59,7 +59,10 @@ class ActivityBaseManager(models.Manager):
return super().create(**kwargs)
def get_queryset(self):
- return super().get_queryset().filter(type=self.type)
+ return super().get_queryset().filter(
+ type=self.type,
+ current=True,
+ )
class CommentQueryset(BaseActivityQuerySet):
@@ -86,6 +89,11 @@ class Activity(models.Model):
message = models.TextField()
visibility = models.CharField(choices=list(VISIBILITY.items()), default=PUBLIC, max_length=10)
+ # Fields for handling versioning of the comment activity models
+ edited = models.DateTimeField(default=None, null=True)
+ current = models.BooleanField(default=True)
+ previous = models.ForeignKey("self", on_delete=models.CASCADE, null=True)
+
# Fields for generic relations to other objects. related_object should implement `get_absolute_url`
content_type = models.ForeignKey(ContentType, blank=True, null=True, on_delete=models.CASCADE)
object_id = models.PositiveIntegerField(blank=True, null=True)
diff --git a/opentech/apply/activity/tests/test_models.py b/opentech/apply/activity/tests/test_models.py
new file mode 100644
index 000000000..2230036f3
--- /dev/null
+++ b/opentech/apply/activity/tests/test_models.py
@@ -0,0 +1,11 @@
+from django.test import TestCase
+
+from .factories import CommentFactory
+from ..models import Activity
+
+
+class TestActivityOnlyIncludesCurrent(TestCase):
+ def test_doesnt_include_non_current(self):
+ CommentFactory()
+ CommentFactory(current=False)
+ self.assertEqual(Activity.comments.count(), 1)
diff --git a/opentech/apply/funds/api_views.py b/opentech/apply/funds/api_views.py
index f4017549d..bc3862af4 100644
--- a/opentech/apply/funds/api_views.py
+++ b/opentech/apply/funds/api_views.py
@@ -1,6 +1,8 @@
from django.core.exceptions import PermissionDenied as DjangoPermissionDenied
+from django.db import transaction
from django.db.models import Q
-from rest_framework import generics, permissions
+from django.utils import timezone
+from rest_framework import generics, mixins, permissions
from rest_framework.response import Response
from rest_framework.exceptions import (NotFound, PermissionDenied,
ValidationError)
@@ -15,13 +17,14 @@ from .models import ApplicationSubmission, RoundsAndLabs
from .serializers import (
CommentSerializer,
CommentCreateSerializer,
+ CommentEditSerializer,
RoundLabDetailSerializer,
RoundLabSerializer,
SubmissionActionSerializer,
SubmissionListSerializer,
SubmissionDetailSerializer,
)
-from .permissions import IsApplyStaffUser
+from .permissions import IsApplyStaffUser, IsAuthor
from .workflow import PHASES
@@ -188,3 +191,38 @@ class CommentListCreate(generics.ListCreateAPIView):
submission=obj.submission,
related=obj,
)
+
+
+class CommentEdit(
+ mixins.RetrieveModelMixin,
+ mixins.CreateModelMixin,
+ generics.GenericAPIView,
+):
+ queryset = Activity.comments.all()
+ serializer_class = CommentEditSerializer
+ permission_classes = (
+ permissions.IsAuthenticated, IsAuthor
+ )
+
+ def post(self, request, *args, **kwargs):
+ return self.edit(request, *args, **kwargs)
+
+ @transaction.atomic
+ def edit(self, request, *args, **kwargs):
+ comment_to_edit = self.get_object()
+ comment_to_update = self.get_object()
+
+ comment_to_edit.previous = comment_to_update
+ comment_to_edit.pk = None
+ comment_to_edit.edited = timezone.now()
+
+ serializer = self.get_serializer(comment_to_edit, data=request.data)
+ serializer.is_valid(raise_exception=True)
+
+ if serializer.validated_data['message'] != comment_to_update.message:
+ self.perform_create(serializer)
+ comment_to_update.current = False
+ comment_to_update.save()
+ return Response(serializer.data)
+
+ return Response(self.get_serializer(comment_to_update).data)
diff --git a/opentech/apply/funds/permissions.py b/opentech/apply/funds/permissions.py
index ec6f22f83..8d7438c54 100644
--- a/opentech/apply/funds/permissions.py
+++ b/opentech/apply/funds/permissions.py
@@ -1,6 +1,11 @@
from rest_framework import permissions
+class IsAuthor(permissions.BasePermission):
+ def has_object_permission(self, request, view, obj):
+ return obj.user == request.user
+
+
class IsApplyStaffUser(permissions.BasePermission):
"""
Custom permission to only allow OTF Staff or higher
diff --git a/opentech/apply/funds/serializers.py b/opentech/apply/funds/serializers.py
index 4f3640364..4d1ca335e 100644
--- a/opentech/apply/funds/serializers.py
+++ b/opentech/apply/funds/serializers.py
@@ -214,7 +214,7 @@ class CommentSerializer(serializers.ModelSerializer):
class Meta:
model = Activity
- fields = ('id', 'timestamp', 'user', 'submission', 'message', 'visibility')
+ fields = ('id', 'timestamp', 'user', 'submission', 'message', 'visibility', 'edited')
def get_message(self, obj):
return bleach_value(markdown(obj.message))
@@ -225,4 +225,11 @@ class CommentCreateSerializer(serializers.ModelSerializer):
class Meta:
model = Activity
- fields = ('id', 'timestamp', 'user', 'message', 'visibility')
+ fields = ('id', 'timestamp', 'user', 'message', 'visibility', 'edited')
+
+
+class CommentEditSerializer(CommentCreateSerializer):
+ user = serializers.StringRelatedField()
+
+ class Meta(CommentCreateSerializer.Meta):
+ read_only_fields = ('visibility', 'edited',)
diff --git a/opentech/apply/funds/tests/test_api_views.py b/opentech/apply/funds/tests/test_api_views.py
new file mode 100644
index 000000000..68251a09e
--- /dev/null
+++ b/opentech/apply/funds/tests/test_api_views.py
@@ -0,0 +1,67 @@
+from django.test import TestCase, override_settings
+from django.urls import reverse_lazy
+
+from opentech.apply.activity.models import Activity, PUBLIC, PRIVATE
+from opentech.apply.activity.tests.factories import CommentFactory
+
+from opentech.apply.users.tests.factories import UserFactory
+
+
+@override_settings(ROOT_URLCONF='opentech.apply.urls')
+class TestCommentEdit(TestCase):
+ def post_to_edit(self, comment_pk, message='my message'):
+ return self.client.post(
+ reverse_lazy('funds:api:comments:edit', kwargs={'pk': comment_pk}),
+ secure=True,
+ data={'message': message},
+ )
+
+ def test_cant_edit_if_not_author(self):
+ comment = CommentFactory()
+ response = self.post_to_edit(comment.pk)
+ self.assertEqual(response.status_code, 403)
+
+ def test_edit_updates_correctly(self):
+ user = UserFactory()
+ comment = CommentFactory(user=user)
+ self.client.force_login(user)
+
+ new_message = 'hi there'
+
+ response = self.post_to_edit(comment.pk, new_message)
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(Activity.objects.count(), 2)
+
+ comment.refresh_from_db()
+ self.assertFalse(comment.current)
+ self.assertEqual(response.json()['message'], new_message)
+
+ def test_incorrect_id_denied(self):
+ response = self.post_to_edit(10000)
+ self.assertEqual(response.status_code, 403)
+
+ def test_does_nothing_if_same_message(self):
+ user = UserFactory()
+ comment = CommentFactory(user=user)
+ self.client.force_login(user)
+
+ self.post_to_edit(comment.pk, comment.message)
+ self.assertEqual(Activity.objects.count(), 1)
+
+ def test_cant_change_visibility(self):
+ user = UserFactory()
+ comment = CommentFactory(user=user, visibility=PRIVATE)
+ self.client.force_login(user)
+
+ response = self.client.post(
+ reverse_lazy('funds:api:comments:edit', kwargs={'pk': comment.pk}),
+ secure=True,
+ data={
+ 'message': 'the new message',
+ 'visibility': PUBLIC,
+ },
+ )
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.json()['visibility'], PRIVATE)
diff --git a/opentech/apply/funds/urls.py b/opentech/apply/funds/urls.py
index 72b29d3b7..88990380d 100644
--- a/opentech/apply/funds/urls.py
+++ b/opentech/apply/funds/urls.py
@@ -14,6 +14,7 @@ from .views import (
SubmissionDeleteView,
)
from .api_views import (
+ CommentEdit,
CommentList,
CommentListCreate,
RoundLabDetail,
@@ -62,6 +63,7 @@ api_urls = ([
], 'rounds'))),
path('comments/', include(([
path('', CommentList.as_view(), name='list'),
+ path('<int:pk>/edit/', CommentEdit.as_view(), name='edit'),
], 'comments')))
], 'api')
--
GitLab