diff --git a/.env.example b/.env.example index 25da1b0f..4b46938a 100644 --- a/.env.example +++ b/.env.example @@ -14,6 +14,7 @@ DRF_THROTTLE_AUTH_RESET_PASSWORD=3/min DRF_THROTTLE_TOKEN_OBTAIN=10/min DRF_THROTTLE_PROGRAM_REGISTER_NEW=10/min DRF_THROTTLE_APPLICATION_CREATE=10/min +DRF_THROTTLE_SUBMISSION_CREATE=10/min SENTRY_DSN= diff --git a/partner_programs/application_urls.py b/partner_programs/application_urls.py index d587596e..2a7dbb4b 100644 --- a/partner_programs/application_urls.py +++ b/partner_programs/application_urls.py @@ -5,10 +5,16 @@ ApplicationSubmitView, ApplicationWithdrawView, ) +from partner_programs.submission_views import ApplicationSubmissionListCreateView app_name = "applications" urlpatterns = [ + path( + "/submissions/", + ApplicationSubmissionListCreateView.as_view(), + name="submission-list-create", + ), path("/", ApplicationDetailView.as_view(), name="detail"), path( "/submit/", diff --git a/partner_programs/models.py b/partner_programs/models.py index 6672ae1b..a0b035e6 100644 --- a/partner_programs/models.py +++ b/partner_programs/models.py @@ -364,6 +364,9 @@ def clean(self): if ( self.submitted_by_id and self.submitted_by_id not in allowed_submitter_ids + and not ( + self.submitted_by.is_staff or self.submitted_by.is_superuser + ) ): errors["submitted_by"] = ( "Submitted by must match the application user or creator." diff --git a/partner_programs/serializers/__init__.py b/partner_programs/serializers/__init__.py index 224782d2..805f548e 100644 --- a/partner_programs/serializers/__init__.py +++ b/partner_programs/serializers/__init__.py @@ -16,6 +16,7 @@ ProgramProjectFilterRequestSerializer, UserProgramsSerializer, ) +from .submissions import SubmissionSerializer __all__ = [ "ApplicationSerializer", @@ -33,5 +34,6 @@ "PartnerProgramUserSerializer", "ProgramProjectCreateSerializer", "ProgramProjectFilterRequestSerializer", + "SubmissionSerializer", "UserProgramsSerializer", ] diff --git a/partner_programs/serializers/submissions.py b/partner_programs/serializers/submissions.py new file mode 100644 index 00000000..51d850cc --- /dev/null +++ b/partner_programs/serializers/submissions.py @@ -0,0 +1,78 @@ +from rest_framework import serializers + +from partner_programs.models import Submission + + +class SubmissionSerializer(serializers.ModelSerializer): + create_fields = frozenset( + { + "title", + "description", + "form_data", + "links", + "stage_key", + "version", + } + ) + update_fields = frozenset( + { + "title", + "description", + "form_data", + "links", + } + ) + + class Meta: + model = Submission + fields = ( + "id", + "application", + "program", + "submitted_by", + "title", + "description", + "form_data", + "links", + "status", + "stage_key", + "version", + "submitted_at", + "created_at", + "updated_at", + ) + read_only_fields = ( + "id", + "application", + "program", + "submitted_by", + "status", + "submitted_at", + "created_at", + "updated_at", + ) + extra_kwargs = { + "description": {"required": False}, + "form_data": {"required": False}, + "links": {"required": False}, + "stage_key": {"required": False}, + "version": {"required": False, "min_value": 1}, + } + + def validate(self, attrs): + allowed_fields = self.update_fields if self.instance else self.create_fields + unsupported_fields = set(self.initial_data).difference(allowed_fields) + if unsupported_fields: + raise serializers.ValidationError( + { + field: "This field is read-only." + for field in sorted(unsupported_fields) + } + ) + + if self.instance and not self.instance.can_edit: + raise serializers.ValidationError( + {"status": "Only draft or returned submissions can be updated."} + ) + + return attrs diff --git a/partner_programs/submission_urls.py b/partner_programs/submission_urls.py new file mode 100644 index 00000000..0a436621 --- /dev/null +++ b/partner_programs/submission_urls.py @@ -0,0 +1,23 @@ +from django.urls import path + +from partner_programs.submission_views import ( + SubmissionCancelView, + SubmissionDetailView, + SubmissionSubmitView, +) + +app_name = "submissions" + +urlpatterns = [ + path("/", SubmissionDetailView.as_view(), name="detail"), + path( + "/submit/", + SubmissionSubmitView.as_view(), + name="submit", + ), + path( + "/cancel/", + SubmissionCancelView.as_view(), + name="cancel", + ), +] diff --git a/partner_programs/submission_views.py b/partner_programs/submission_views.py new file mode 100644 index 00000000..07728745 --- /dev/null +++ b/partner_programs/submission_views.py @@ -0,0 +1,215 @@ +from django.core.exceptions import ValidationError as DjangoValidationError +from django.db import IntegrityError, transaction +from django.db.models import Max +from django.shortcuts import get_object_or_404 +from django.utils import timezone +from rest_framework import status +from rest_framework.exceptions import ValidationError +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.views import APIView + +from core.throttling import PostOnlyScopedRateThrottle +from partner_programs.models import Application, Submission +from partner_programs.serializers import SubmissionSerializer + + +def _application_queryset_for(user): + queryset = Application.objects.select_related("program", "user", "created_by") + if user.is_staff or user.is_superuser: + return queryset + return queryset.filter(user=user) + + +def _submission_queryset_for(user): + queryset = Submission.objects.select_related( + "application", + "application__user", + "application__created_by", + "program", + "submitted_by", + ) + if user.is_staff or user.is_superuser: + return queryset + return queryset.filter(application__user=user) + + +def _submission_response(submission, response_status=status.HTTP_200_OK): + return Response( + SubmissionSerializer(submission).data, + status=response_status, + ) + + +class ApplicationSubmissionListCreateView(APIView): + permission_classes = [IsAuthenticated] + throttle_classes = [PostOnlyScopedRateThrottle] + throttle_scope = "submission_create" + + def get_application(self, request, application_id): + return get_object_or_404( + _application_queryset_for(request.user), + pk=application_id, + ) + + def get(self, request, application_id): + application = self.get_application(request, application_id) + submissions = ( + Submission.objects.select_related("application", "program", "submitted_by") + .filter(application=application) + .order_by("-created_at", "-id") + ) + return Response(SubmissionSerializer(submissions, many=True).data) + + def post(self, request, application_id): + application = self.get_application(request, application_id) + if application.status not in ( + Application.STATUS_SUBMITTED, + Application.STATUS_APPROVED, + ): + raise ValidationError( + { + "application": ( + "Submissions require a submitted or approved application." + ) + } + ) + + serializer = SubmissionSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + submission_data = dict(serializer.validated_data) + stage_key = submission_data.pop("stage_key", "main") + validated_version = submission_data.pop("version", None) + requested_version = ( + validated_version if "version" in request.data else None + ) + version = requested_version + + try: + with transaction.atomic(): + locked_application = get_object_or_404( + _application_queryset_for(request.user).select_for_update(), + pk=application.pk, + ) + if locked_application.status not in ( + Application.STATUS_SUBMITTED, + Application.STATUS_APPROVED, + ): + raise ValidationError( + { + "application": ( + "Submissions require a submitted or approved application." + ) + } + ) + if version is None: + latest_version = ( + Submission.objects.filter( + application=locked_application, + stage_key=stage_key, + ).aggregate(max_version=Max("version"))["max_version"] + or 0 + ) + version = latest_version + 1 + + submission = Submission.objects.create( + application=locked_application, + program=locked_application.program, + submitted_by=request.user, + stage_key=stage_key, + version=version, + **submission_data, + ) + except (DjangoValidationError, IntegrityError) as exc: + if Submission.objects.filter( + application=application, + stage_key=stage_key, + version=version, + ).exists(): + raise ValidationError( + { + "version": ( + "A submission with this stage and version already exists." + ) + } + ) from exc + if isinstance(exc, DjangoValidationError): + raise ValidationError(exc.message_dict) from exc + raise + + return _submission_response(submission, status.HTTP_201_CREATED) + + +class SubmissionDetailView(APIView): + permission_classes = [IsAuthenticated] + + def get_object(self, request, submission_id): + return get_object_or_404( + _submission_queryset_for(request.user), + pk=submission_id, + ) + + def get(self, request, submission_id): + return _submission_response(self.get_object(request, submission_id)) + + def patch(self, request, submission_id): + submission = self.get_object(request, submission_id) + if not submission.can_edit: + raise ValidationError( + {"status": "Only draft or returned submissions can be updated."} + ) + + serializer = SubmissionSerializer( + submission, + data=request.data, + partial=True, + ) + serializer.is_valid(raise_exception=True) + try: + serializer.save() + except DjangoValidationError as exc: + raise ValidationError(exc.message_dict) from exc + return Response(serializer.data) + + +class SubmissionSubmitView(APIView): + permission_classes = [IsAuthenticated] + + def post(self, request, submission_id): + with transaction.atomic(): + submission = get_object_or_404( + _submission_queryset_for(request.user).select_for_update(), + pk=submission_id, + ) + if submission.status == Submission.STATUS_SUBMITTED: + return _submission_response(submission) + if not submission.can_submit: + raise ValidationError( + {"status": "Only draft or returned submissions can be submitted."} + ) + + submission.status = Submission.STATUS_SUBMITTED + submission.submitted_at = timezone.now() + submission.save(update_fields=["status", "submitted_at", "updated_at"]) + return _submission_response(submission) + + +class SubmissionCancelView(APIView): + permission_classes = [IsAuthenticated] + + def post(self, request, submission_id): + with transaction.atomic(): + submission = get_object_or_404( + _submission_queryset_for(request.user).select_for_update(), + pk=submission_id, + ) + if submission.status == Submission.STATUS_CANCELLED: + return _submission_response(submission) + if not submission.can_edit: + raise ValidationError( + {"status": "Only draft or returned submissions can be cancelled."} + ) + + submission.status = Submission.STATUS_CANCELLED + submission.save(update_fields=["status", "updated_at"]) + return _submission_response(submission) diff --git a/partner_programs/tests/test_submission_api.py b/partner_programs/tests/test_submission_api.py new file mode 100644 index 00000000..1465ec68 --- /dev/null +++ b/partner_programs/tests/test_submission_api.py @@ -0,0 +1,415 @@ +from django.conf import settings +from django.core.cache import cache +from django.test import TestCase, override_settings +from django.utils import timezone +from rest_framework.test import APIClient + +from partner_programs.models import Application, Submission +from partner_programs.tests.helpers import create_partner_program, create_user + + +def throttle_settings(**rates): + rest_framework = dict(settings.REST_FRAMEWORK) + rest_framework["DEFAULT_THROTTLE_RATES"] = { + **rest_framework.get("DEFAULT_THROTTLE_RATES", {}), + **rates, + } + return rest_framework + + +class SubmissionAPITests(TestCase): + def setUp(self): + cache.clear() + self.client = APIClient() + self.user = create_user(prefix="submission-api-owner") + self.other_user = create_user(prefix="submission-api-other") + self.staff_user = create_user(prefix="submission-api-staff", is_staff=True) + self.program = create_partner_program() + self.application = self.create_application() + + def authenticate(self, user=None): + self.client.force_authenticate(user=user or self.user) + + def create_application( + self, + *, + user=None, + status=Application.STATUS_SUBMITTED, + ): + owner = user or self.user + return Application.objects.create( + program=self.program, + user=owner, + created_by=owner, + status=status, + submitted_at=( + timezone.now() if status == Application.STATUS_SUBMITTED else None + ), + ) + + def create_submission(self, **overrides): + values = { + "application": self.application, + "program": self.program, + "submitted_by": self.user, + "title": "Solution", + } + values.update(overrides) + return Submission.objects.create(**values) + + @property + def list_url(self): + return f"/applications/{self.application.id}/submissions/" + + def test_unauthenticated_user_cannot_list_or_create_submissions(self): + list_response = self.client.get(self.list_url) + create_response = self.client.post( + self.list_url, + {"title": "Unauthorized solution"}, + format="json", + ) + + self.assertEqual(list_response.status_code, 401) + self.assertEqual(create_response.status_code, 401) + self.assertFalse(Submission.objects.exists()) + + def test_owner_can_list_own_application_submissions_newest_first(self): + first = self.create_submission(version=1) + second = self.create_submission(title="Version two", version=2) + self.authenticate() + + response = self.client.get(self.list_url) + + self.assertEqual(response.status_code, 200) + self.assertEqual([item["id"] for item in response.data], [second.id, first.id]) + + def test_listing_empty_application_does_not_create_submission(self): + self.authenticate() + + response = self.client.get(self.list_url) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, []) + self.assertFalse(Submission.objects.exists()) + + def test_user_cannot_list_another_users_application_submissions(self): + other_application = self.create_application(user=self.other_user) + self.authenticate() + + response = self.client.get( + f"/applications/{other_application.id}/submissions/" + ) + + self.assertEqual(response.status_code, 404) + + def test_owner_can_create_draft_for_submitted_or_approved_application(self): + self.authenticate() + + submitted_response = self.client.post( + self.list_url, + {"title": "Submitted application solution"}, + format="json", + ) + self.application.status = Application.STATUS_APPROVED + self.application.save() + approved_response = self.client.post( + self.list_url, + {"title": "Approved application solution"}, + format="json", + ) + + self.assertEqual(submitted_response.status_code, 201) + self.assertEqual(approved_response.status_code, 201) + self.assertEqual(submitted_response.data["status"], Submission.STATUS_DRAFT) + self.assertEqual(approved_response.data["status"], Submission.STATUS_DRAFT) + + def test_cannot_create_for_ineligible_application_status(self): + self.authenticate() + + for application_status in ( + Application.STATUS_DRAFT, + Application.STATUS_WITHDRAWN, + Application.STATUS_REJECTED, + Application.STATUS_CANCELLED, + ): + with self.subTest(status=application_status): + self.application.status = application_status + self.application.save() + + response = self.client.post( + self.list_url, + {"title": "Invalid application solution"}, + format="json", + ) + + self.assertEqual(response.status_code, 400) + + self.assertFalse(Submission.objects.exists()) + + def test_create_fills_program_and_submitted_by(self): + self.authenticate() + + response = self.client.post( + self.list_url, + {"title": "Owned solution"}, + format="json", + ) + + self.assertEqual(response.status_code, 201) + submission = Submission.objects.get() + self.assertEqual(submission.program, self.application.program) + self.assertEqual(submission.submitted_by, self.user) + self.assertEqual(response.data["program"], self.program.id) + self.assertEqual(response.data["submitted_by"], self.user.id) + + def test_staff_can_create_submission(self): + self.authenticate(self.staff_user) + + response = self.client.post( + self.list_url, + {"title": "Staff-created solution"}, + format="json", + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(Submission.objects.get().submitted_by, self.staff_user) + + def test_default_stage_key_is_main(self): + self.authenticate() + + response = self.client.post( + self.list_url, + {"title": "Main stage solution"}, + format="json", + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(response.data["stage_key"], "main") + + def test_default_version_uses_next_stage_version(self): + self.create_submission(stage_key="main", version=1) + self.create_submission(title="Other stage", stage_key="final", version=7) + self.authenticate() + + response = self.client.post( + self.list_url, + {"title": "Next main version", "stage_key": "main"}, + format="json", + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(response.data["version"], 2) + + def test_duplicate_stage_and_version_returns_validation_error(self): + self.create_submission(stage_key="main", version=1) + self.authenticate() + + response = self.client.post( + self.list_url, + {"title": "Duplicate", "stage_key": "main", "version": 1}, + format="json", + ) + + self.assertEqual(response.status_code, 400) + self.assertIn("version", response.data) + self.assertEqual(Submission.objects.count(), 1) + + def test_owner_can_get_submission_detail(self): + submission = self.create_submission() + self.authenticate() + + response = self.client.get(f"/submissions/{submission.id}/") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["id"], submission.id) + + def test_user_cannot_get_another_users_submission(self): + other_application = self.create_application(user=self.other_user) + submission = self.create_submission( + application=other_application, + submitted_by=self.other_user, + ) + self.authenticate() + + response = self.client.get(f"/submissions/{submission.id}/") + + self.assertEqual(response.status_code, 404) + + def test_patch_draft_updates_editable_fields(self): + submission = self.create_submission() + self.authenticate() + + response = self.client.patch( + f"/submissions/{submission.id}/", + { + "title": "Updated solution", + "description": "Updated description", + "form_data": {"answer": 42}, + "links": ["https://example.com/solution"], + }, + format="json", + ) + + self.assertEqual(response.status_code, 200) + submission.refresh_from_db() + self.assertEqual(submission.title, "Updated solution") + self.assertEqual(submission.description, "Updated description") + self.assertEqual(submission.form_data, {"answer": 42}) + self.assertEqual(submission.links, ["https://example.com/solution"]) + + def test_patch_non_editable_status_is_rejected(self): + submission = self.create_submission() + self.authenticate() + + for submission_status in ( + Submission.STATUS_SUBMITTED, + Submission.STATUS_FINAL, + Submission.STATUS_CANCELLED, + ): + with self.subTest(status=submission_status): + submission.status = submission_status + submission.save() + + response = self.client.patch( + f"/submissions/{submission.id}/", + {"title": "Should not change"}, + format="json", + ) + + self.assertEqual(response.status_code, 400) + + submission.refresh_from_db() + self.assertEqual(submission.title, "Solution") + + def test_patch_rejects_immutable_fields(self): + submission = self.create_submission() + other_application = self.create_application(user=self.other_user) + other_program = create_partner_program() + self.authenticate() + + response = self.client.patch( + f"/submissions/{submission.id}/", + { + "application": other_application.id, + "program": other_program.id, + "submitted_by": self.other_user.id, + "status": Submission.STATUS_FINAL, + "stage_key": "other", + "version": 9, + "submitted_at": timezone.now().isoformat(), + }, + format="json", + ) + + self.assertEqual(response.status_code, 400) + submission.refresh_from_db() + self.assertEqual(submission.application, self.application) + self.assertEqual(submission.program, self.program) + self.assertEqual(submission.submitted_by, self.user) + self.assertEqual(submission.status, Submission.STATUS_DRAFT) + self.assertEqual(submission.stage_key, "main") + self.assertEqual(submission.version, 1) + self.assertIsNone(submission.submitted_at) + + def test_submit_draft_sets_status_and_submitted_at(self): + submission = self.create_submission() + self.authenticate() + + response = self.client.post( + f"/submissions/{submission.id}/submit/", + {}, + format="json", + ) + + self.assertEqual(response.status_code, 200) + submission.refresh_from_db() + self.assertEqual(submission.status, Submission.STATUS_SUBMITTED) + self.assertIsNotNone(submission.submitted_at) + + def test_repeated_submit_preserves_submitted_at(self): + submission = self.create_submission() + self.authenticate() + url = f"/submissions/{submission.id}/submit/" + + first_response = self.client.post(url, {}, format="json") + submission.refresh_from_db() + first_submitted_at = submission.submitted_at + second_response = self.client.post(url, {}, format="json") + + self.assertEqual(first_response.status_code, 200) + self.assertEqual(second_response.status_code, 200) + submission.refresh_from_db() + self.assertEqual(submission.submitted_at, first_submitted_at) + + def test_submit_final_or_cancelled_returns_bad_request(self): + submission = self.create_submission() + self.authenticate() + + for submission_status in ( + Submission.STATUS_FINAL, + Submission.STATUS_CANCELLED, + ): + with self.subTest(status=submission_status): + submission.status = submission_status + submission.save() + + response = self.client.post( + f"/submissions/{submission.id}/submit/", + {}, + format="json", + ) + + self.assertEqual(response.status_code, 400) + + def test_cancel_draft_is_idempotent(self): + submission = self.create_submission() + self.authenticate() + url = f"/submissions/{submission.id}/cancel/" + + first_response = self.client.post(url, {}, format="json") + second_response = self.client.post(url, {}, format="json") + + self.assertEqual(first_response.status_code, 200) + self.assertEqual(second_response.status_code, 200) + submission.refresh_from_db() + self.assertEqual(submission.status, Submission.STATUS_CANCELLED) + + def test_cancel_submitted_or_final_returns_bad_request(self): + submission = self.create_submission() + self.authenticate() + + for submission_status in ( + Submission.STATUS_SUBMITTED, + Submission.STATUS_FINAL, + ): + with self.subTest(status=submission_status): + submission.status = submission_status + submission.save() + + response = self.client.post( + f"/submissions/{submission.id}/cancel/", + {}, + format="json", + ) + + self.assertEqual(response.status_code, 400) + + @override_settings(REST_FRAMEWORK=throttle_settings(submission_create="1/min")) + def test_submission_create_is_scoped_throttled(self): + self.authenticate() + + first_response = self.client.post( + self.list_url, + {"title": "First solution"}, + format="json", + REMOTE_ADDR="203.0.113.50", + ) + second_response = self.client.post( + self.list_url, + {"title": "Second solution"}, + format="json", + REMOTE_ADDR="203.0.113.50", + ) + + self.assertEqual(first_response.status_code, 201) + self.assertEqual(second_response.status_code, 429) diff --git a/procollab/settings.py b/procollab/settings.py index ec479fa6..4ae97cd7 100644 --- a/procollab/settings.py +++ b/procollab/settings.py @@ -182,6 +182,9 @@ "application_create": config( "DRF_THROTTLE_APPLICATION_CREATE", default="10/min", cast=str ), + "submission_create": config( + "DRF_THROTTLE_SUBMISSION_CREATE", default="10/min", cast=str + ), }, } diff --git a/procollab/urls.py b/procollab/urls.py index 2daef614..9b7f1309 100644 --- a/procollab/urls.py +++ b/procollab/urls.py @@ -59,6 +59,10 @@ "applications/", include("partner_programs.application_urls", namespace="applications"), ), + path( + "submissions/", + include("partner_programs.submission_urls", namespace="submissions"), + ), path("courses/", include("courses.urls", namespace="courses")), path("rate-project/", include(("project_rates.urls", "rate_projects"))), path("feed/", include("feed.urls", namespace="feed")),