From abe767e239380f32ad2f0ed0baec42c1b92e7d0f Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Sat, 8 Aug 2026 22:51:24 +0500 Subject: [PATCH] Enforce phase start/end window on submission creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase.is_active returned True unconditionally when end was unset, never checking start — a not-yet-started phase with no end date was treated as active. SubmissionCreationSerializer.validate() also never checked is_active or can_user_make_submissions(), so submissions could be created via the API before a phase started or after it ended. - Fix Phase.is_active to check start regardless of whether end is set - Reject submission creation when the target phase is not active or the user has hit their submission limit, returning a 400 error - Add tests for both new fixes --- src/apps/api/serializers/submissions.py | 7 +++ src/apps/api/tests/test_submissions.py | 61 +++++++++++++++++++ src/apps/competitions/models.py | 2 +- .../competitions/tests/test_submissions.py | 29 +++++++++ 4 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/apps/api/serializers/submissions.py b/src/apps/api/serializers/submissions.py index 9c91737ca..4069965c4 100644 --- a/src/apps/api/serializers/submissions.py +++ b/src/apps/api/serializers/submissions.py @@ -161,6 +161,13 @@ def validate(self, attrs): if not is_in_competition: raise PermissionDenied("You do not have access to this competition to make a submission") + if not data["phase"].is_active: + raise ValidationError("This phase is not currently accepting submissions.") + + can_make_submission, reason_why_not = data["phase"].can_user_make_submissions(self.context["request"].user) + if not can_make_submission: + raise ValidationError(reason_why_not) + return data def update(self, submission, validated_data): diff --git a/src/apps/api/tests/test_submissions.py b/src/apps/api/tests/test_submissions.py index 895fa142e..dea9903aa 100644 --- a/src/apps/api/tests/test_submissions.py +++ b/src/apps/api/tests/test_submissions.py @@ -1,4 +1,5 @@ import random +from datetime import timedelta from unittest import mock from django.urls import reverse @@ -731,3 +732,63 @@ def test_organization_is_removed_from_soft_deleted_submission(self): self.organization_submission.refresh_from_db() assert self.organization_submission.is_soft_deleted is True assert self.organization_submission.organization is None + + +class PhaseActiveSubmissionTests(APITestCase): + """a submission must only be creatable while its phase is active + (has started and, if it has an end date, has not ended).""" + + def setUp(self): + self.creator = UserFactory() + self.comp = CompetitionFactory(created_by=self.creator) + self.participant = UserFactory() + CompetitionParticipantFactory(user=self.participant, competition=self.comp, status=CompetitionParticipant.APPROVED) + self.dataset = DataFactory(type='submission', created_by=self.participant) + self.url_submission = reverse('submission-list') + + def post_submission(self, phase): + self.client.force_login(user=self.participant) + data = {'phase': phase.id, 'data': self.dataset.key} + # Mock _send_to_compute_worker so submissions don't actually run + with mock.patch('competitions.tasks._send_to_compute_worker'): + return self.client.post(self.url_submission, data=data) + + def test_cannot_submit_before_phase_starts(self): + phase = PhaseFactory(competition=self.comp, start=now() + timedelta(days=1), end=None) + resp = self.post_submission(phase) + assert resp.status_code == 400 + assert "This phase is not currently accepting submissions." in str(resp.data) + + def test_cannot_submit_before_phase_starts_even_with_end_date_set(self): + phase = PhaseFactory( + competition=self.comp, + start=now() + timedelta(days=1), + end=now() + timedelta(days=2), + ) + resp = self.post_submission(phase) + assert resp.status_code == 400 + assert "This phase is not currently accepting submissions." in str(resp.data) + + def test_cannot_submit_after_phase_ends(self): + phase = PhaseFactory( + competition=self.comp, + start=now() - timedelta(days=2), + end=now() - timedelta(days=1), + ) + resp = self.post_submission(phase) + assert resp.status_code == 400 + assert "This phase is not currently accepting submissions." in str(resp.data) + + def test_can_submit_during_active_phase_with_no_end_date(self): + phase = PhaseFactory(competition=self.comp, start=now() - timedelta(days=1), end=None) + resp = self.post_submission(phase) + assert resp.status_code == 201 + + def test_can_submit_during_active_phase_with_future_end_date(self): + phase = PhaseFactory( + competition=self.comp, + start=now() - timedelta(days=1), + end=now() + timedelta(days=1), + ) + resp = self.post_submission(phase) + assert resp.status_code == 201 diff --git a/src/apps/competitions/models.py b/src/apps/competitions/models.py index f919ae432..4fbaa78a5 100644 --- a/src/apps/competitions/models.py +++ b/src/apps/competitions/models.py @@ -358,7 +358,7 @@ def can_user_make_submissions(self, user): def is_active(self): """ Returns true when this phase of the competition is on-going. """ if not self.end: - return True + return self.start < now() else: return self.start < now() < self.end diff --git a/src/apps/competitions/tests/test_submissions.py b/src/apps/competitions/tests/test_submissions.py index ee5cdc850..e6fe9340c 100644 --- a/src/apps/competitions/tests/test_submissions.py +++ b/src/apps/competitions/tests/test_submissions.py @@ -446,3 +446,32 @@ def test_cancelling_parent_submission_cancels_all_children(self): assert self.parent_submission.status == Submission.FAILED for sub in self.parent_submission.children.all(): assert sub.status == Submission.FAILED + + +class PhaseIsActiveTests(SubmissionTestCase): + """Tests for Phase.is_active""" + + def test_active_when_started_and_no_end_date(self): + self.phase.start = timezone.now() - timedelta(days=1) + self.phase.end = None + assert self.phase.is_active + + def test_not_active_when_not_yet_started_and_no_end_date(self): + self.phase.start = timezone.now() + timedelta(days=1) + self.phase.end = None + assert not self.phase.is_active + + def test_not_active_when_not_yet_started_and_end_date_in_future(self): + self.phase.start = timezone.now() + timedelta(days=1) + self.phase.end = timezone.now() + timedelta(days=2) + assert not self.phase.is_active + + def test_active_when_within_start_and_end_range(self): + self.phase.start = timezone.now() - timedelta(days=1) + self.phase.end = timezone.now() + timedelta(days=1) + assert self.phase.is_active + + def test_not_active_when_end_date_has_passed(self): + self.phase.start = timezone.now() - timedelta(days=2) + self.phase.end = timezone.now() - timedelta(days=1) + assert not self.phase.is_active