diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index d3c24d144d5e1136b2f149fe2daa09130169a697..e5e77f64e3d6c13ee7c1914795be104c0685b29d 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model from django.template.loader import render_to_string +from django.utils import timezone from .models import INTERNAL, PUBLIC from .options import MESSAGES @@ -332,6 +333,7 @@ class ActivityAdapter(AdapterBase): Activity.actions.create( user=user, submission=submission, + timestamp=timezone.now(), message=message, visibility=visibility, related_object=related_object, 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 0000000000000000000000000000000000000000..10034b8d281f0e15ab24d0a15c7c44ce5499551d --- /dev/null +++ b/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py @@ -0,0 +1,34 @@ +# 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.AlterField( + model_name='activity', + name='timestamp', + field=models.DateTimeField(), + ), + 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 159bd219cf8270dd8e977cd087e633050363973d..089686efeeb4264c43c9e802eae6d69e720e480e 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): @@ -79,13 +82,18 @@ class ActionManager(ActivityBaseManager): class Activity(models.Model): - timestamp = models.DateTimeField(auto_now_add=True) + timestamp = models.DateTimeField() type = models.CharField(choices=ACTIVITY_TYPES.items(), max_length=30) user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.PROTECT) submission = models.ForeignKey('funds.ApplicationSubmission', related_name='activities', on_delete=models.CASCADE) 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/templates/activity/include/listing_base.html b/opentech/apply/activity/templates/activity/include/listing_base.html index f999f30d48506338aea55ecea8cafb49b56a566a..95352d4d922dce15b32b2ffbb69523a802752db1 100644 --- a/opentech/apply/activity/templates/activity/include/listing_base.html +++ b/opentech/apply/activity/templates/activity/include/listing_base.html @@ -3,10 +3,25 @@ <div class="feed__pre-content"> <p class="feed__label feed__label--{{ activity.type }}">{{ activity.type|capfirst }}</p> </div> - <div class="feed__content"> - <div class="feed__meta"> + <div class="feed__content js-feed-content"> + <div class="feed__meta js-feed-meta"> <p class="feed__label feed__label--{{ activity.type }} feed__label--mobile">{{ activity.type|capfirst }}</p> <p class="feed__meta-item"><span>{{ activity|display_author:request.user }}</span> – {{ activity.timestamp|date:"Y-m-d H:i" }}</p> + + {% if editable %} + {% if activity.user == request.user %} + <p class="feed__meta-item feed__meta-item--edit-button"> + <a class="link link--edit-submission is-active js-edit-comment" href="#"> + Edit + <svg class="icon icon--pen"><use xlink:href="#pen"></use></svg> + </a> + </p> + {% endif %} + <p class="feed__meta-item feed__meta-item--last-edited" {% if not activity.edited %} hidden {% endif %}> + (Last edited: <span class="js-last-edited">{{ activity.edited|date:"Y-m-d H:i" }}</span>) + </p> + {% endif %} + {% if activity.private %} <p class="feed__meta-item feed__meta-item--right"> <svg class="icon icon--eye"><use xlink:href="#eye"></use></svg> @@ -14,12 +29,21 @@ </p> {% endif %} </div> + <p class="feed__heading"> {% if submission_title %} updated <a href="{{ activity.submission.get_absolute_url }}">{{ activity.submission.title }}</a> {% endif %} - {{ activity|display_for:request.user|submission_links|markdown|bleach }} + {% if editable %} + <div class="feed__comment js-comment" data-id="{{activity.id}}" data-comment="{{activity|display_for:request.user|to_markdown}}" data-edit-url="{% url 'funds:api:comments:edit' pk=activity.pk %}"> + {{ activity|display_for:request.user|submission_links|markdown|bleach }} + </div> + + <div class="js-edit-block" aria-live="polite"></div> + {% else %} + {{ activity|display_for:request.user|submission_links|markdown|bleach }} + {% endif %} {% if not submission_title and activity|user_can_see_related:request.user %} {% with url=activity.related_object.get_absolute_url %} diff --git a/opentech/apply/activity/tests/factories.py b/opentech/apply/activity/tests/factories.py index 30c09ab31a5971a7300269b9af699093afeca13d..fb312efc0b9cd7bb634709535b9b320b45b09adb 100644 --- a/opentech/apply/activity/tests/factories.py +++ b/opentech/apply/activity/tests/factories.py @@ -1,6 +1,7 @@ import uuid import factory +from django.utils import timezone from opentech.apply.activity.models import Activity, Event, INTERNAL, Message, MESSAGES, REVIEWER from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory @@ -18,6 +19,7 @@ class CommentFactory(factory.DjangoModelFactory): submission = factory.SubFactory(ApplicationSubmissionFactory) user = factory.SubFactory(UserFactory) message = factory.Faker('sentence') + timestamp = factory.LazyFunction(timezone.now) @classmethod def _get_manager(cls, model_class): diff --git a/opentech/apply/activity/tests/test_models.py b/opentech/apply/activity/tests/test_models.py new file mode 100644 index 0000000000000000000000000000000000000000..2230036f34e6a4796565d09f05f6e7cc52515d80 --- /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/activity/views.py b/opentech/apply/activity/views.py index 07ccbd32c0358a21384d9ac4eab48519af0808e9..cba63a229760a14bc68c2d3907b140cd876ecf89 100644 --- a/opentech/apply/activity/views.py +++ b/opentech/apply/activity/views.py @@ -1,4 +1,5 @@ from django.views.generic import CreateView +from django.utils import timezone from opentech.apply.utils.views import DelegatedViewMixin @@ -57,6 +58,7 @@ class CommentFormView(DelegatedViewMixin, CreateView): form.instance.user = self.request.user form.instance.submission = self.kwargs['submission'] form.instance.type = COMMENT + form.instance.timestamp = timezone.now() response = super().form_valid(form) messenger( MESSAGES.COMMENT, diff --git a/opentech/apply/determinations/tests/test_views.py b/opentech/apply/determinations/tests/test_views.py index 0f7f418d9d1a1c1c4ff1336656ced8c685d4947b..f3e71f9993958c43c092ae6c0cf592eea53411fd 100644 --- a/opentech/apply/determinations/tests/test_views.py +++ b/opentech/apply/determinations/tests/test_views.py @@ -1,6 +1,15 @@ +import urllib + +from django.contrib.messages.storage.fallback import FallbackStorage +from django.contrib.sessions.middleware import SessionMiddleware +from django.test import RequestFactory +from django.urls import reverse_lazy + from opentech.apply.activity.models import Activity from opentech.apply.determinations.models import ACCEPTED, REJECTED +from opentech.apply.determinations.views import BatchDeterminationCreateView from opentech.apply.users.tests.factories import StaffFactory, UserFactory +from opentech.apply.funds.models import ApplicationSubmission from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory from opentech.apply.utils.testing import BaseViewTestCase @@ -123,6 +132,15 @@ class BatchDeterminationTestCase(BaseViewTestCase): url_name = 'funds:submissions:determinations:{}' base_view_name = 'batch' + def dummy_request(self, path): + request = RequestFactory().get(path) + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + request.user = StaffFactory() + request._messages = FallbackStorage(request) + return request + def test_cant_access_without_submissions(self): url = self.url(None) + '?action=rejected' response = self.client.get(url, follow=True, secure=True) @@ -157,6 +175,32 @@ class BatchDeterminationTestCase(BaseViewTestCase): self.assertRedirects(response, self.url_from_pattern('apply:submissions:list')) + def test_sets_next_on_redirect(self): + test_path = '/a/path/?with=query&a=sting' + request = RequestFactory().get('', PATH_INFO=test_path) + redirect = BatchDeterminationCreateView.should_redirect( + request, + ApplicationSubmission.objects.none(), + ['rejected'], + ) + url = urllib.parse.urlparse(redirect.url) + query = urllib.parse.parse_qs(url.query) + next_path = urllib.parse.unquote_plus(query['next'][0]) + self.assertEqual(next_path, test_path) + + def test_success_redirects_if_exists(self): + test_path = '/a/path/?with=query&a=sting' + view = BatchDeterminationCreateView() + view.request = self.dummy_request('?next=' + urllib.parse.quote_plus(test_path)) + redirect_url = view.get_success_url() + self.assertEqual(redirect_url, test_path) + + def test_success_if_no_next(self): + view = BatchDeterminationCreateView() + view.request = self.dummy_request('') + redirect_url = view.get_success_url() + self.assertEqual(redirect_url, reverse_lazy('apply:submissions:list')) + def test_message_created_if_determination_exists(self): submissions = ApplicationSubmissionFactory.create_batch(2) diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py index ea1e37af51ba01a29cde3fc82da498f0015dfbdf..a5e65f32d0d16335708f4c0cd52d278dfaf6280a 100644 --- a/opentech/apply/determinations/views.py +++ b/opentech/apply/determinations/views.py @@ -1,9 +1,12 @@ +from urllib import parse + from django.contrib import messages from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy +from django.utils import timezone from django.utils.decorators import method_decorator from django.utils.translation import ugettext_lazy as _ from django.views.generic import DetailView, CreateView @@ -61,6 +64,7 @@ class BatchDeterminationCreateView(CreateView): if not self.get_action() or not self.get_submissions(): messages.warning(self.request, 'Improperly configured request, please try again.') return HttpResponseRedirect(self.get_success_url()) + return super().dispatch(*args, **kwargs) def get_action(self): @@ -132,6 +136,7 @@ class BatchDeterminationCreateView(CreateView): # We keep a record of the message sent to the user in the comment Activity.comments.create( message=determination.stripped_message, + timestamp=timezone.now(), user=self.request.user, submission=submission, related_object=determination, @@ -162,13 +167,17 @@ class BatchDeterminationCreateView(CreateView): return HttpResponseRedirect( reverse_lazy('apply:submissions:determinations:batch') + "?action=" + action + - "&submissions=" + ','.join([str(submission.id) for submission in submissions]) + "&submissions=" + ','.join([str(submission.id) for submission in submissions]) + + "&next=" + parse.quote_plus(request.get_full_path()), ) elif set(actions) != non_determine_states: raise ValueError('Inconsistent states provided - please talk to an admin') def get_success_url(self): - return reverse_lazy('apply:submissions:list') + try: + return self.request.GET['next'] + except KeyError: + return reverse_lazy('apply:submissions:list') @method_decorator(staff_required, name='dispatch') @@ -245,6 +254,7 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): # We keep a record of the message sent to the user in the comment Activity.comments.create( message=self.object.stripped_message, + timestamp=timezone.now(), user=self.request.user, submission=self.submission, related_object=self.object, diff --git a/opentech/apply/funds/api_views.py b/opentech/apply/funds/api_views.py index f4017549d73e23fa0304316b877b793ac9df1402..26ffec25506d942b6783f3da1db5200d02c55fe3 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 @@ -53,7 +56,7 @@ class SubmissionsFilter(filters.FilterSet): class SubmissionList(generics.ListAPIView): - queryset = ApplicationSubmission.objects.current() + queryset = ApplicationSubmission.objects.current().with_latest_update() serializer_class = SubmissionListSerializer permission_classes = ( permissions.IsAuthenticated, IsApplyStaffUser, @@ -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/models/submissions.py b/opentech/apply/funds/models/submissions.py index 5f2a9027306d1e7b2e89eb612be34e6d64d8dd50..93442fa1e2ab35518ff80bed336238899962f96e 100644 --- a/opentech/apply/funds/models/submissions.py +++ b/opentech/apply/funds/models/submissions.py @@ -115,18 +115,23 @@ class ApplicationSubmissionQueryset(JSONOrderable): # Applications which have the current stage active (have not been progressed) return self.exclude(next__isnull=False) - def for_table(self, user): + def with_latest_update(self): activities = self.model.activities.field.model latest_activity = activities.objects.filter(submission=OuterRef('id')).select_related('user') + return self.annotate( + last_user_update=Subquery(latest_activity[:1].values('user__full_name')), + last_update=Subquery(latest_activity.values('timestamp')[:1]), + ) + + def for_table(self, user): + activities = self.model.activities.field.model comments = activities.comments.filter(submission=OuterRef('id')).visible_to(user) roles_for_review = self.model.assigned.field.model.objects.with_roles().filter( submission=OuterRef('id'), reviewer=user) reviews = self.model.reviews.field.model.objects.filter(submission=OuterRef('id')) - return self.annotate( - last_user_update=Subquery(latest_activity[:1].values('user__full_name')), - last_update=Subquery(latest_activity.values('timestamp')[:1]), + return self.with_latest_update().annotate( comment_count=Coalesce( Subquery( comments.values('submission').order_by().annotate(count=Count('pk')).values('count'), diff --git a/opentech/apply/funds/permissions.py b/opentech/apply/funds/permissions.py index ec6f22f83b78b3476cf267333a68149c7c32e0df..8d7438c54d7673c851d8d6d99288d2696884c602 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 4f3640364745f041ecd37c2ef90a98d2f1375c3b..bf933e22d34771eb49056adeef5fcd7d3889ffd2 100644 --- a/opentech/apply/funds/serializers.py +++ b/opentech/apply/funds/serializers.py @@ -122,13 +122,19 @@ class ReviewSummarySerializer(serializers.Serializer): return response +class TimestampField(serializers.Field): + def to_representation(self, value): + return value.timestamp() + + class SubmissionListSerializer(serializers.ModelSerializer): url = serializers.HyperlinkedIdentityField(view_name='funds:api:submissions:detail') round = serializers.SerializerMethodField() + last_update = TimestampField() class Meta: model = ApplicationSubmission - fields = ('id', 'title', 'status', 'url', 'round') + fields = ('id', 'title', 'status', 'url', 'round', 'last_update') def get_round(self, obj): """ @@ -211,10 +217,11 @@ class RoundLabSerializer(serializers.ModelSerializer): class CommentSerializer(serializers.ModelSerializer): user = serializers.StringRelatedField() message = serializers.SerializerMethodField() + edit_url = serializers.HyperlinkedIdentityField(view_name='funds:api:comments:edit') class Meta: model = Activity - fields = ('id', 'timestamp', 'user', 'submission', 'message', 'visibility') + fields = ('id', 'timestamp', 'user', 'submission', 'message', 'visibility', 'edited', 'edit_url') def get_message(self, obj): return bleach_value(markdown(obj.message)) @@ -222,7 +229,14 @@ class CommentSerializer(serializers.ModelSerializer): class CommentCreateSerializer(serializers.ModelSerializer): user = serializers.StringRelatedField() + edit_url = serializers.HyperlinkedIdentityField(view_name='funds:api:comments:edit') class Meta: model = Activity - fields = ('id', 'timestamp', 'user', 'message', 'visibility') + fields = ('id', 'timestamp', 'user', 'message', 'visibility', 'edited', 'edit_url') + read_only_fields = ('timestamp', 'edited',) + + +class CommentEditSerializer(CommentCreateSerializer): + class Meta(CommentCreateSerializer.Meta): + read_only_fields = ('timestamp', 'visibility', 'edited',) diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html index a81322aeaeb9fc2aba792ba3ef39b0ba8f0f6250..71bcd9abd08346e88a7168220a028c3d5408827d 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html @@ -47,11 +47,13 @@ {{ comment_form.media.js }} {{ partner_form.media.js }} <script src="//cdnjs.cloudflare.com/ajax/libs/fancybox/3.4.1/jquery.fancybox.min.js"></script> + <script src="//cdnjs.cloudflare.com/ajax/libs/jquery-cookie/1.4.1/jquery.cookie.min.js"></script> <script src="{% static 'js/apply/fancybox-global.js' %}"></script> <script src="{% static 'js/apply/tabs.js' %}"></script> <script src="{% static 'js/apply/toggle-actions-panel.js' %}"></script> <script src="{% static 'js/apply/toggle-reviewers.js' %}"></script> <script src="{% static 'js/apply/toggle-sidebar.js' %}"></script> <script src="{% static 'js/apply/submission-text-cleanup.js' %}"></script> + <script src="{% static 'js/apply/edit-comment.js' %}"></script> <script src="{% static 'js/apply/toggle-related.js' %}"></script> {% endblock %} diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html index b41916ddd77c87e053f51834173a7f16b15fe72c..0326bfbdfdcf20dceb7f9319c19c33d666d53e12 100644 --- a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html +++ b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html @@ -141,7 +141,7 @@ <div class="tabs__content" id="tab-2"> <div class="feed"> {% include "activity/include/comment_form.html" %} - {% include "activity/include/comment_list.html" %} + {% include "activity/include/comment_list.html" with editable=True %} </div> </div> diff --git a/opentech/apply/funds/templates/funds/includes/activity-feed.html b/opentech/apply/funds/templates/funds/includes/activity-feed.html index 0a3595a6d1765fd7ab028c1bc83200a646f45054..37117315ccd58c1fb4b4da12cd87404d99020698 100644 --- a/opentech/apply/funds/templates/funds/includes/activity-feed.html +++ b/opentech/apply/funds/templates/funds/includes/activity-feed.html @@ -15,7 +15,7 @@ <div class="wrapper wrapper--medium wrapper--activity-feed js-tabs-content"> <div class="tabs__content tabs__content--current" id="tab-1"> - {% include "activity/include/comment_list.html" with submission_title=True %} + {% include "activity/include/comment_list.html" with submission_title=True editable=False %} </div> <div class="tabs__content" id="tab-2"> diff --git a/opentech/apply/funds/templatetags/markdown_tags.py b/opentech/apply/funds/templatetags/markdown_tags.py index 9ba5ff19dc0828092dcfab40fdcd9ab6876f7b58..bddae4d8a14e7e8c522967c64fd4eafa261e78a7 100644 --- a/opentech/apply/funds/templatetags/markdown_tags.py +++ b/opentech/apply/funds/templatetags/markdown_tags.py @@ -1,11 +1,21 @@ import mistune +import tomd from django import template register = template.Library() +mistune_markdown = mistune.Markdown() + @register.filter def markdown(value): - markdown = mistune.Markdown() - return markdown(value) + return mistune_markdown(value) + + +@register.filter +def to_markdown(value): + # pass through markdown to ensure comment is a + # fully formed HTML block + value = markdown(value) + return tomd.convert(value) 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 0000000000000000000000000000000000000000..df9cb2aab1c150399bdf0d9118102b2f8de98026 --- /dev/null +++ b/opentech/apply/funds/tests/test_api_views.py @@ -0,0 +1,90 @@ +from django.test import TestCase, override_settings +from django.urls import reverse_lazy +from django.utils import timezone + +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, response.json()) + self.assertEqual(Activity.objects.count(), 2) + + comment.refresh_from_db() + + # Match the behaviour of DRF + time = comment.timestamp.astimezone(timezone.get_current_timezone()).isoformat() + if time.endswith('+00:00'): + time = time[:-6] + 'Z' + + self.assertEqual(time, response.json()['timestamp']) + 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, response.json()) + + 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, response.json()) + self.assertEqual(response.json()['visibility'], PRIVATE) + + def test_out_of_order_does_nothing(self): + user = UserFactory() + comment = CommentFactory(user=user) + self.client.force_login(user) + + new_message = 'hi there' + newer_message = 'hello there' + + response_one = self.post_to_edit(comment.pk, new_message) + response_two = self.post_to_edit(comment.pk, newer_message) + + self.assertEqual(response_one.status_code, 200, response_one.json()) + self.assertEqual(response_two.status_code, 404, response_two.json()) + self.assertEqual(Activity.objects.count(), 2) diff --git a/opentech/apply/funds/urls.py b/opentech/apply/funds/urls.py index 72b29d3b792b1860a57f56c853a6e9e896f0c0dc..88990380d67ad35c9ec99e2c36042d1cd6aa9e39 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') diff --git a/opentech/static_src/src/app/src/components/GroupedListing/index.js b/opentech/static_src/src/app/src/components/GroupedListing/index.js index 88bd5dc9e075c8f18a9e4757de8136ac7190c607..43c72b22d8682ff2491d1e3f293949e65e9f6a11 100644 --- a/opentech/static_src/src/app/src/components/GroupedListing/index.js +++ b/opentech/static_src/src/app/src/components/GroupedListing/index.js @@ -89,6 +89,10 @@ export default class GroupedListing extends React.Component { items: values.reduce((acc, value) => acc.concat(groupedItems[value] || []), []) })).filter(({items}) => items.length !== 0) + orderedItems.map(value => { + value.items.sort((a,b) => a.lastUpdate > b.lastUpdate ? -1 : 1) + }) + this.setState({orderedItems}); } diff --git a/opentech/static_src/src/app/src/containers/ReviewInformation.js b/opentech/static_src/src/app/src/containers/ReviewInformation.js index 02ddf9d854ebe090bd081ae1a99f99ef13b8b973..2f0bfad65296aae2fe16aadaf1b17641b16aa74a 100644 --- a/opentech/static_src/src/app/src/containers/ReviewInformation.js +++ b/opentech/static_src/src/app/src/containers/ReviewInformation.js @@ -34,6 +34,8 @@ const ReviewInformation = ({ submission }) => { people.sort((a,b) => { if (a.role.order === null) { return 100; + } else if (b.role.order === null) { + return -1; } return a.role.order - b.role.order; }) diff --git a/opentech/static_src/src/javascript/apply/edit-comment.js b/opentech/static_src/src/javascript/apply/edit-comment.js new file mode 100644 index 0000000000000000000000000000000000000000..3b7c3d88d3283c98bfad623468bb89a495e61d0d --- /dev/null +++ b/opentech/static_src/src/javascript/apply/edit-comment.js @@ -0,0 +1,152 @@ +(function ($) { + + 'use strict'; + + const comment = '.js-comment'; + const pageDown = '.js-pagedown'; + const feedMeta = '.js-feed-meta'; + const editBlock = '.js-edit-block'; + const lastEdited = '.js-last-edited'; + const editButton = '.js-edit-comment'; + const feedContent = '.js-feed-content'; + const commentError = '.js-comment-error'; + const cancelEditButton = '.js-cancel-edit'; + const submitEditButton = '.js-submit-edit'; + + // handle edit + $(editButton).click(function (e) { + e.preventDefault(); + + closeAllEditors(); + + const editBlockWrapper = $(this).closest(feedContent).find(editBlock); + const commentWrapper = $(this).closest(feedContent).find(comment); + const commentContents = $(commentWrapper).attr('data-comment'); + + // hide the edit link and original comment + $(this).parent().hide(); + $(commentWrapper).hide(); + + const markup = ` + <div class="js-pagedown form"> + <div id="wmd-button-bar-edit-comment" class="wmd-button-bar"></div> + <textarea id="wmd-input-edit-comment" class="wmd-input" rows="10">${commentContents}</textarea> + <div id="wmd-preview-edit-comment" class="wmd-preview"></div> + <div class="wrapper--outer-space-medium"> + <button class="button button--primary js-submit-edit" type="submit">Update</button> + <button class="button button--white js-cancel-edit">Cancel</button> + </div> + </div> + `; + + // add the comment to the editor + $(editBlockWrapper).append(markup); + + // run the editor + initEditor(); + }); + + // handle cancel + $(document).on('click', cancelEditButton, function () { + showComment(this); + showEditButton(this); + hidePageDownEditor(this); + if ($(commentError).length) { + hideError(); + } + }); + + // handle submit + $(document).on('click', submitEditButton, function () { + const commentContainer = $(this).closest(editBlock).siblings(comment); + const editedComment = $(this).closest(pageDown).find('.wmd-preview').html(); + const commentMD = $(this).closest(editBlock).find('textarea').val(); + const editUrl = $(commentContainer).attr('data-edit-url'); + + fetch(editUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': $.cookie('csrftoken') + }, + body: JSON.stringify({ + message: editedComment + }) + }).then(response => { + if (!response.ok) { + const error = Object.assign({}, response, { + status: response.status, + statusText: response.statusText + }); + return Promise.reject(error); + } + return response.json(); + }).then(data => { + updateComment(commentContainer, data.id, data.message, data.edit_url, commentMD); + updateLastEdited(this, data.edited); + showComment(this); + showEditButton(this); + hidePageDownEditor(this); + }).catch((error) => { + if (error.status === 404) { + handleError(this, 'Update unsuccessful. This comment has been edited elsewhere. To get the latest updates please refresh the page, but note any unsaved changes will be lost by doing so.'); + } + else { + handleError(this, 'An error has occured. Please try again later.'); + } + }); + }); + + const handleError = (el, message) => { + $(el).closest(editBlock).append(`<p class="wrapper--error js-comment-error">${message}</p>`); + $(el).attr('disabled', true); + }; + + const initEditor = () => { + const converterOne = window.Markdown.getSanitizingConverter(); + const commentEditor = new window.Markdown.Editor(converterOne, '-edit-comment'); + commentEditor.run(); + }; + + const showEditButton = (el) => { + $(el).closest(editBlock).siblings(feedMeta).find(editButton).parent().show(); + }; + + const hidePageDownEditor = (el) => { + $(el).closest(pageDown).remove(); + }; + + const showComment = (el) => { + $(el).closest(editBlock).siblings(comment).show(); + }; + + const updateLastEdited = (el, date) => { + const parsedDate = new Date(date).toISOString().split('T')[0]; + const time = new Date(date).toLocaleTimeString([], {hour: '2-digit', minute: '2-digit'}); + $(el).closest(feedContent).find(lastEdited).parent().attr('hidden', false); + $(el).closest(feedContent).find(lastEdited).html(`${parsedDate} ${time}`); + }; + + const updateComment = (el, id, comment, editUrl, commentMarkdown) => { + $(el).html(comment); + $(el).attr('data-id', id); + $(el).attr('data-edit-url', editUrl); + $(el).attr('data-comment', commentMarkdown); + }; + + const closeAllEditors = () => { + $(comment).show(); + $(pageDown).remove(); + $(editButton).parent().show(); + }; + + const hideError = () => $(commentError).remove(); + + window.addEventListener('beforeunload', (e) => { + if ($(submitEditButton).length) { + e.preventDefault(); + e.returnValue = 'It looks like you\'re still editing a comment. Are you sure you want to leave?'; + } + }); + +})(jQuery); diff --git a/opentech/static_src/src/sass/apply/components/_feed.scss b/opentech/static_src/src/sass/apply/components/_feed.scss index 2ab489a4bf4b1029ce4e6df43d4de6063046599f..d3afc66ee7327436224057045a9fcf735e18661c 100644 --- a/opentech/static_src/src/sass/apply/components/_feed.scss +++ b/opentech/static_src/src/sass/apply/components/_feed.scss @@ -101,6 +101,20 @@ margin: 0 15px 0 0; } + &--edit-button { + border-left: 2px solid $color--mid-grey; + padding-left: 15px; + } + + &--last-edited { + margin-right: 5px; + color: $color--mid-dark-grey; + + span { + font-weight: $weight--normal; + } + } + &--right { margin-left: auto; } diff --git a/opentech/static_src/src/sass/apply/components/_wrapper.scss b/opentech/static_src/src/sass/apply/components/_wrapper.scss index b80aa2b154d104af6634779ee87cbfa784cadd25..e7165e01b4cd95706618d388763e6ce4601e6f5f 100644 --- a/opentech/static_src/src/sass/apply/components/_wrapper.scss +++ b/opentech/static_src/src/sass/apply/components/_wrapper.scss @@ -89,6 +89,10 @@ margin: 0 auto 2rem; background: $color--light-pink; border: 1px solid $color--tomato; + + .feed & { + margin: 0 0 1rem; + } } &--bottom-space { diff --git a/requirements.txt b/requirements.txt index 192f6e09ab86362a00dc1f4f7af179e3984463c3..ec3bc53376c6f238efb19d9ffea89edd0134b4e9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -43,6 +43,7 @@ mistune==0.8.4 Pillow==4.3.0 psycopg2==2.7.3.1 social_auth_app_django==3.1.0 +tomd==0.1.3 wagtail~=2.2.0 wagtail-cache==0.5.1 whitenoise==4.0