From 8431713624a80e95c9ee35c6c6052605bec55835 Mon Sep 17 00:00:00 2001 From: krmax44 Date: Mon, 18 Nov 2024 15:13:45 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20split=20up=20FoiMessage=20and=20Foi?= =?UTF-8?q?MessageDraft?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- froide/foirequest/admin.py | 22 +++++- froide/foirequest/api_views/message.py | 54 ++++++++++++-- froide/foirequest/apps.py | 8 ++- froide/foirequest/models/__init__.py | 9 ++- froide/foirequest/models/message.py | 23 +++++- froide/foirequest/serializers.py | 44 ++++++++++-- froide/foirequest/tasks.py | 6 +- froide/foirequest/tests/test_api_message.py | 80 ++++++++------------- 8 files changed, 174 insertions(+), 72 deletions(-) diff --git a/froide/foirequest/admin.py b/froide/foirequest/admin.py index f7e5da6ef..d13175369 100644 --- a/froide/foirequest/admin.py +++ b/froide/foirequest/admin.py @@ -22,6 +22,7 @@ from froide.account.models import UserTag from froide.comments.models import FroideComment +from froide.foirequest.models.message import FoiMessageDraft from froide.guide.models import Action from froide.guide.utils import assign_guidance_action from froide.helper.admin_utils import ( @@ -521,8 +522,7 @@ class MessageTagsFilter(MultiFilterMixin, TaggitListFilter): @admin.register(FoiMessage) class FoiMessageAdmin(admin.ModelAdmin): save_on_top = True - list_display = ( - "is_draft", + list_display = [ "subject", "timestamp", "message_page", @@ -532,7 +532,7 @@ class FoiMessageAdmin(admin.ModelAdmin): "registered_mail_date", "kind", "get_deliverystatus_display", - ) + ] list_filter = ( "kind", "is_response", @@ -752,6 +752,22 @@ def add_comment( ) +@admin.register(FoiMessageDraft) +class FoiMessageDraftAdmin(FoiMessageAdmin): + list_display = ("pk", "request_page", "timestamp", "kind", "user") + + def request_page(self, obj): + return format_html( + '{}', obj.request.get_absolute_short_url(), _("on site") + ) + + def user(self, obj): + return obj.request.user + + def get_queryset(self, request): + return super().get_queryset(request).prefetch_related("deliverystatus") + + @admin.register(MessageTag) class MessageTagAdmin(admin.ModelAdmin): prepopulated_fields = {"slug": ("name",)} diff --git a/froide/foirequest/api_views/message.py b/froide/foirequest/api_views/message.py index 1c181a188..c8b20ba78 100644 --- a/froide/foirequest/api_views/message.py +++ b/froide/foirequest/api_views/message.py @@ -2,6 +2,11 @@ from django_filters import rest_framework as filters from rest_framework import permissions, viewsets +from rest_framework.decorators import action +from rest_framework.response import Response +from rest_framework.viewsets import mixins + +from froide.foirequest.models.message import FoiMessageDraft from ..auth import ( get_read_foimessage_queryset, @@ -11,7 +16,11 @@ OnlyEditableWhenDraftPermission, WriteFoiRequestPermission, ) -from ..serializers import FoiMessageSerializer, optimize_message_queryset +from ..serializers import ( + FoiMessageDraftSerializer, + FoiMessageSerializer, + optimize_message_queryset, +) User = get_user_model() @@ -19,13 +28,36 @@ class FoiMessageFilter(filters.FilterSet): class Meta: model = FoiMessage - fields = ("request", "kind", "is_response", "is_draft") + fields = ("request", "kind", "is_response") + + +class FoiMessageDraftFilter(filters.FilterSet): + class Meta: + model = FoiMessageDraft + fields = ("request",) -class FoiMessageViewSet(viewsets.ModelViewSet): +class FoiMessageViewSet(viewsets.ReadOnlyModelViewSet): serializer_class = FoiMessageSerializer filter_backends = (filters.DjangoFilterBackend,) filterset_class = FoiMessageFilter + + def get_queryset(self): + qs = get_read_foimessage_queryset(self.request).order_by() + return self.optimize_query(qs) + + def optimize_query(self, qs): + return optimize_message_queryset(self.request, qs) + + +class FoiMessageDraftViewSet( + FoiMessageViewSet, + mixins.CreateModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, +): + serializer_class = FoiMessageDraftSerializer + filterset_class = FoiMessageDraftFilter required_scopes = ["make:message"] permission_classes = [ permissions.IsAuthenticatedOrReadOnly, @@ -34,8 +66,18 @@ class FoiMessageViewSet(viewsets.ModelViewSet): ] def get_queryset(self): - qs = get_read_foimessage_queryset(self.request).order_by() + qs = get_read_foimessage_queryset( + self.request, queryset=FoiMessageDraft.objects.all() + ).order_by() return self.optimize_query(qs) - def optimize_query(self, qs): - return optimize_message_queryset(self.request, qs) + @action(detail=True, methods=["post"]) + def publish(self, request, pk=None): + message = self.get_object() + message.is_draft = False + message.save() + + serializer = FoiMessageSerializer( + message, context=self.get_serializer_context() + ) + return Response(serializer.data) diff --git a/froide/foirequest/apps.py b/froide/foirequest/apps.py index c02d1e065..ae0607980 100644 --- a/froide/foirequest/apps.py +++ b/froide/foirequest/apps.py @@ -24,7 +24,10 @@ def ready(self): from froide.api import api_router from froide.foirequest import signals # noqa from froide.foirequest.api_views.attachment import FoiAttachmentViewSet - from froide.foirequest.api_views.message import FoiMessageViewSet + from froide.foirequest.api_views.message import ( + FoiMessageViewSet, + FoiMessageDraftViewSet, + ) from froide.foirequest.api_views.request import FoiRequestViewSet from froide.helper.search import search_registry from froide.team import team_changed @@ -48,6 +51,9 @@ def ready(self): account_confirmed.connect(send_request_when_account_confirmed) api_router.register(r"request", FoiRequestViewSet, basename="request") + api_router.register( + r"message/draft", FoiMessageDraftViewSet, basename="message-draft" + ) api_router.register(r"message", FoiMessageViewSet, basename="message") api_router.register(r"attachment", FoiAttachmentViewSet, basename="attachment") diff --git a/froide/foirequest/models/__init__.py b/froide/foirequest/models/__init__.py index bb84511d1..75059411d 100644 --- a/froide/foirequest/models/__init__.py +++ b/froide/foirequest/models/__init__.py @@ -1,7 +1,13 @@ from .attachment import FoiAttachment from .deferred import DeferredMessage from .draft import RequestDraft -from .message import DeliveryStatus, FoiMessage, MessageTag, TaggedMessage +from .message import ( + DeliveryStatus, + FoiMessage, + FoiMessageDraft, + MessageTag, + TaggedMessage, +) from .project import FoiProject from .request import FoiRequest, TaggedFoiRequest from .suggestion import PublicBodySuggestion @@ -12,6 +18,7 @@ "FoiRequest", "TaggedFoiRequest", "FoiMessage", + "FoiMessageDraft", "FoiAttachment", "FoiEvent", "DeferredMessage", diff --git a/froide/foirequest/models/message.py b/froide/foirequest/models/message.py index 06afd8ac4..6a6b2f28e 100644 --- a/froide/foirequest/models/message.py +++ b/froide/foirequest/models/message.py @@ -37,8 +37,18 @@ def get_throttle_filter(self, queryset, user, extra_filters=None): qs = qs.filter(**extra_filters) return qs, "timestamp" - def get_drafts(self, drafts=True): - return super().get_queryset().filter(is_draft=drafts) + def get_queryset(self): + return super().get_queryset().filter(is_draft=False) + + +class FoiMessageDraftManager(FoiMessageManager): + def get_queryset(self): + # need to call models.Manager, since FoiMessageManager removes drafts + return super(models.Manager, self).get_queryset().filter(is_draft=True) + + def create(self, **kwargs): + kwargs.setdefault("is_draft", True) + return super().create(**kwargs) class MessageTag(TagBase): @@ -746,6 +756,15 @@ def set_cached_rendered_content(self, authenticated_read, content): FoiMessage.objects.filter(id=self.id).update(**update) +class FoiMessageDraft(FoiMessage): + objects = FoiMessageDraftManager() + + class Meta: + proxy = True + verbose_name = _("Freedom of Information Message Draft") + verbose_name_plural = _("Freedom of Information Message Drafts") + + class Delivery(models.TextChoices): STATUS_UNKNOWN = ("unknown", _("unknown")) STATUS_SENDING = ("sending", _("sending")) diff --git a/froide/foirequest/serializers.py b/froide/foirequest/serializers.py index 158cc192b..462c38705 100644 --- a/froide/foirequest/serializers.py +++ b/froide/foirequest/serializers.py @@ -1,4 +1,5 @@ from django.db.models import Prefetch +from django.template.defaultfilters import default from django.utils import timezone from django.utils.translation import gettext as _ @@ -7,7 +8,11 @@ from froide.document.api_views import DocumentSerializer from froide.foirequest.forms.message import TransferUploadForm -from froide.foirequest.models.message import MESSAGE_KIND_USER_ALLOWED, MessageKind +from froide.foirequest.models.message import ( + MESSAGE_KIND_USER_ALLOWED, + FoiMessageDraft, + MessageKind, +) from froide.helper.text_diff import get_differences from froide.publicbody.models import PublicBody from froide.publicbody.serializers import ( @@ -175,6 +180,26 @@ def get_queryset(self): return get_write_foirequest_queryset(request) +class FoiMessageRelatedField(serializers.HyperlinkedRelatedField): + def __init__(self, **kwargs): + super().__init__("api:message-detail", **kwargs) + + def get_url(self, obj, view_name, request, format): + # Unsaved objects will not yet have a valid URL. + if hasattr(obj, "pk") and obj.pk in (None, ""): + return None + + lookup_value = getattr(obj, self.lookup_field) + kwargs = {self.lookup_url_kwarg: lookup_value} + + if isinstance(obj, FoiMessageDraft): + view_name = "api:message-draft-detail" + else: + view_name = "api:message-detail" + + return self.reverse(view_name, kwargs=kwargs, request=request, format=format) + + class FoiMessageSerializer(serializers.HyperlinkedModelSerializer): resource_uri = serializers.HyperlinkedIdentityField(view_name="api:message-detail") request = FoiRequestRelatedField() @@ -193,8 +218,8 @@ class FoiMessageSerializer(serializers.HyperlinkedModelSerializer): sender = serializers.CharField(read_only=True) url = serializers.CharField(source="get_absolute_domain_url", read_only=True) status = serializers.ChoiceField(choices=FoiRequest.STATUS.choices, required=False) - kind = serializers.ChoiceField(choices=MessageKind.choices, required=True) - is_draft = serializers.BooleanField(required=False) + kind = serializers.ChoiceField(choices=MessageKind.choices, default="post") + is_draft = serializers.BooleanField(read_only=True) status_name = serializers.CharField(source="get_status_display", read_only=True) not_publishable = serializers.BooleanField(read_only=True) timestamp = serializers.DateTimeField(default=timezone.now) @@ -289,6 +314,15 @@ def validate_request(self, value): return value +class FoiMessageDraftSerializer(FoiMessageSerializer): + resource_uri = serializers.HyperlinkedIdentityField( + view_name="api:message-draft-detail" + ) + + class Meta(FoiMessageSerializer.Meta): + model = FoiMessageDraft + + class FoiAttachmentSerializer(serializers.HyperlinkedModelSerializer): resource_uri = serializers.HyperlinkedIdentityField( view_name="api:attachment-detail", lookup_field="pk" @@ -303,9 +337,7 @@ class FoiAttachmentSerializer(serializers.HyperlinkedModelSerializer): lookup_field="pk", read_only=True, ) - belongs_to = serializers.HyperlinkedRelatedField( - read_only=True, view_name="api:message-detail" - ) + belongs_to = FoiMessageRelatedField(read_only=True) document = DocumentSerializer() site_url = serializers.CharField(source="get_absolute_domain_url", read_only=True) anchor_url = serializers.CharField(source="get_domain_anchor_url", read_only=True) diff --git a/froide/foirequest/tasks.py b/froide/foirequest/tasks.py index 36514f557..74f939a31 100644 --- a/froide/foirequest/tasks.py +++ b/froide/foirequest/tasks.py @@ -442,10 +442,10 @@ def unpack_zipfile_attachment_task(instance_id): @celery_app.task(name="froide.foirequest.tasks.remove_old_drafts", time_limit=10) def remove_old_drafts(): - from .models import FoiMessage + from .models import FoiMessageDraft - FoiMessage.objects.filter( - is_draft=True, last_modified_at__lt=timezone.now() - timedelta(days=30) + FoiMessageDraft.objects.filter( + last_modified_at__lt=timezone.now() - timedelta(days=30) ).delete() diff --git a/froide/foirequest/tests/test_api_message.py b/froide/foirequest/tests/test_api_message.py index e4c68e870..2e3981cc9 100644 --- a/froide/foirequest/tests/test_api_message.py +++ b/froide/foirequest/tests/test_api_message.py @@ -1,5 +1,3 @@ -import json - from django.test import Client from django.urls import reverse @@ -14,64 +12,55 @@ def test_message_draft(client: Client, user): ok = client.login(email=user.email, password="froide") assert ok - # must specify kind + # can't create an email response = client.post( - "/api/v1/message/", + "/api/v1/message/draft/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, + "kind": "email", }, content_type="application/json", ) assert response.status_code == 400 - # can't create an email + # must use draft endpoint response = client.post( "/api/v1/message/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, - "kind": "email", + "kind": "post", }, content_type="application/json", ) - assert response.status_code == 400 + assert response.status_code == 405 # create message draft response = client.post( - "/api/v1/message/", + "/api/v1/message/draft/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, "kind": "post", }, content_type="application/json", ) assert response.status_code == 201 + assert "/draft/" in response.json()["resource_uri"] - message_id = json.loads(response.content)["id"] - resource_uri = reverse("api:message-detail", kwargs={"pk": message_id}) + message_id = response.json()["id"] + resource_uri = reverse("api:message-draft-detail", kwargs={"pk": message_id}) response = client.patch( resource_uri, data={"status": "resolved"}, content_type="application/json" ) - data = json.loads(response.content) assert response.status_code == 200 - assert data["status"] == "resolved" + assert response.json()["status"] == "resolved" response = client.delete(resource_uri) assert response.status_code == 204 - -@pytest.mark.django_db -def test_message_not_editable(client: Client, user): - ok = client.login(email=user.email, password="froide") - assert ok - request = factories.FoiRequestFactory.create(user=user) - - # not a draft + # create message draft response = client.post( - "/api/v1/message/", + "/api/v1/message/draft/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), "kind": "post", @@ -80,34 +69,27 @@ def test_message_not_editable(client: Client, user): ) assert response.status_code == 201 - message_id = json.loads(response.content)["id"] - resource_uri = reverse("api:message-detail", kwargs={"pk": message_id}) + message_id = response.json()["id"] + resource_uri = reverse("api:message-draft-detail", kwargs={"pk": message_id}) - response = client.delete(resource_uri) - assert response.status_code == 403 + # doesn't appear in regular messages + response = client.get(reverse("api:message-detail", kwargs={"pk": message_id})) + assert response.status_code == 404 - # first draft, then not - response = client.post( - "/api/v1/message/", - data={ - "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, - "kind": "post", - }, - content_type="application/json", - ) - assert response.status_code == 201 + # publish + resource_uri = reverse("api:message-draft-publish", kwargs={"pk": message_id}) + response = client.post(resource_uri) + assert response.status_code == 200 + assert "/draft/" not in response.json()["resource_uri"] - message_id = json.loads(response.content)["id"] + # can't delete anymore resource_uri = reverse("api:message-detail", kwargs={"pk": message_id}) + response = client.delete(resource_uri) + assert response.status_code == 405 - response = client.patch( - resource_uri, data={"is_draft": False}, content_type="application/json" - ) - assert response.status_code == 200 - + resource_uri = reverse("api:message-draft-detail", kwargs={"pk": message_id}) response = client.delete(resource_uri) - assert response.status_code == 403 + assert response.status_code == 404 @pytest.mark.django_db @@ -118,10 +100,9 @@ def test_auth(client, user): # need to be logged in client.logout() response = client.post( - "/api/v1/message/", + "/api/v1/message/draft/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, "kind": "post", }, content_type="application/json", @@ -131,10 +112,9 @@ def test_auth(client, user): # needs to be own request client.login(email=user.email, password="froide") response = client.post( - "/api/v1/message/", + "/api/v1/message/draft/", data={ "request": reverse("api:request-detail", kwargs={"pk": request.pk}), - "is_draft": True, "kind": "post", }, content_type="application/json",