feat: Introduce a CLAIMED state for moderation submissions, requiring claims before approval or rejection, and add a scheduled task to expire stale claims.

2026-02-05 09:45:17 -05:00 · 2026-01-07 13:41:52 -05:00
parent 28c9ec56da
commit 40cba5bdb2
8 changed files with 450 additions and 94 deletions
--- a/backend/apps/moderation/tests/test_comprehensive.py
+++ b/backend/apps/moderation/tests/test_comprehensive.py
@@ -399,11 +399,17 @@ class EditSubmissionTransitionTests(TestCase):
            reason="Test reason",
        )

-    def test_pending_to_approved_transition(self):
-        """Test transition from PENDING to APPROVED."""
+    def test_pending_to_claimed_to_approved_transition(self):
+        """Test transition from PENDING to CLAIMED to APPROVED (mandatory flow)."""
        submission = self._create_submission()
        self.assertEqual(submission.status, "PENDING")

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can approve
        submission.transition_to_approved(user=self.moderator)
        submission.handled_by = self.moderator
        submission.handled_at = timezone.now()
@@ -414,11 +420,17 @@ class EditSubmissionTransitionTests(TestCase):
        self.assertEqual(submission.handled_by, self.moderator)
        self.assertIsNotNone(submission.handled_at)

-    def test_pending_to_rejected_transition(self):
-        """Test transition from PENDING to REJECTED."""
+    def test_pending_to_claimed_to_rejected_transition(self):
+        """Test transition from PENDING to CLAIMED to REJECTED (mandatory flow)."""
        submission = self._create_submission()
        self.assertEqual(submission.status, "PENDING")

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can reject
        submission.transition_to_rejected(user=self.moderator)
        submission.handled_by = self.moderator
        submission.handled_at = timezone.now()
@@ -430,11 +442,17 @@ class EditSubmissionTransitionTests(TestCase):
        self.assertEqual(submission.handled_by, self.moderator)
        self.assertIn("Rejected", submission.notes)

-    def test_pending_to_escalated_transition(self):
-        """Test transition from PENDING to ESCALATED."""
+    def test_pending_to_claimed_to_escalated_transition(self):
+        """Test transition from PENDING to CLAIMED to ESCALATED (mandatory flow)."""
        submission = self._create_submission()
        self.assertEqual(submission.status, "PENDING")

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can escalate
        submission.transition_to_escalated(user=self.moderator)
        submission.handled_by = self.moderator
        submission.handled_at = timezone.now()
@@ -487,9 +505,15 @@ class EditSubmissionTransitionTests(TestCase):
            submission.transition_to_approved(user=self.moderator)

    def test_approve_wrapper_method(self):
-        """Test the approve() wrapper method."""
+        """Test the approve() wrapper method (requires CLAIMED state first)."""
        submission = self._create_submission()

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can approve
        submission.approve(self.moderator)

        submission.refresh_from_db()
@@ -498,9 +522,15 @@ class EditSubmissionTransitionTests(TestCase):
        self.assertIsNotNone(submission.handled_at)

    def test_reject_wrapper_method(self):
-        """Test the reject() wrapper method."""
+        """Test the reject() wrapper method (requires CLAIMED state first)."""
        submission = self._create_submission()

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can reject
        submission.reject(self.moderator, reason="Not enough evidence")

        submission.refresh_from_db()
@@ -508,9 +538,15 @@ class EditSubmissionTransitionTests(TestCase):
        self.assertIn("Not enough evidence", submission.notes)

    def test_escalate_wrapper_method(self):
-        """Test the escalate() wrapper method."""
+        """Test the escalate() wrapper method (requires CLAIMED state first)."""
        submission = self._create_submission()

+        # Must claim first
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        self.assertEqual(submission.status, "CLAIMED")
+
+        # Now can escalate
        submission.escalate(self.moderator, reason="Needs admin approval")

        submission.refresh_from_db()
@@ -846,18 +882,23 @@ class TransitionLoggingTestCase(TestCase):
            reason="Test reason",
        )

+        # Must claim first (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
        # Perform transition
        submission.transition_to_approved(user=self.moderator)
        submission.save()

        # Check log was created
        submission_ct = ContentType.objects.get_for_model(submission)
-        log = StateLog.objects.filter(content_type=submission_ct, object_id=submission.id).first()
+        log = StateLog.objects.filter(
+            content_type=submission_ct, object_id=submission.id, state="APPROVED"
+        ).first()

        self.assertIsNotNone(log, "StateLog entry should be created")
        self.assertEqual(log.state, "APPROVED")
        self.assertEqual(log.by, self.moderator)
-        self.assertIn("approved", log.transition.lower())

    def test_multiple_transitions_logged(self):
        """Test that multiple transitions are all logged."""
@@ -875,20 +916,28 @@ class TransitionLoggingTestCase(TestCase):

        submission_ct = ContentType.objects.get_for_model(submission)

-        # First transition
+        # First claim (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
+        # First transition: CLAIMED -> ESCALATED
        submission.transition_to_escalated(user=self.moderator)
        submission.save()

-        # Second transition
+        # Second transition: ESCALATED -> APPROVED
        submission.transition_to_approved(user=self.moderator)
        submission.save()

-        # Check multiple logs created
-        logs = StateLog.objects.filter(content_type=submission_ct, object_id=submission.id).order_by("timestamp")
+        # Check logs created (excluding the claim transition log)
+        logs = StateLog.objects.filter(
+            content_type=submission_ct, object_id=submission.id
+        ).order_by("timestamp")

-        self.assertEqual(logs.count(), 2, "Should have 2 log entries")
-        self.assertEqual(logs[0].state, "ESCALATED")
-        self.assertEqual(logs[1].state, "APPROVED")
+        # Should have at least 2 entries for ESCALATED and APPROVED
+        self.assertGreaterEqual(logs.count(), 2, "Should have at least 2 log entries")
+        states = [log.state for log in logs]
+        self.assertIn("ESCALATED", states)
+        self.assertIn("APPROVED", states)

    def test_history_endpoint_returns_logs(self):
        """Test history API endpoint returns transition logs."""
@@ -907,6 +956,10 @@ class TransitionLoggingTestCase(TestCase):
            reason="Test reason",
        )

+        # Must claim first (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
        # Perform transition to create log
        submission.transition_to_approved(user=self.moderator)
        submission.save()
@@ -918,7 +971,7 @@ class TransitionLoggingTestCase(TestCase):
        self.assertEqual(response.status_code, 200)

    def test_system_transitions_without_user(self):
-        """Test that system transitions work without a user."""
+        """Test that system transitions work without a user (admin/cron operations)."""
        from django_fsm_log.models import StateLog

        submission = EditSubmission.objects.create(
@@ -931,13 +984,19 @@ class TransitionLoggingTestCase(TestCase):
            reason="Test reason",
        )

-        # Perform transition without user
+        # Must claim first (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
+        # Perform transition without user (simulating system/cron action)
        submission.transition_to_rejected(user=None)
        submission.save()

        # Check log was created even without user
        submission_ct = ContentType.objects.get_for_model(submission)
-        log = StateLog.objects.filter(content_type=submission_ct, object_id=submission.id).first()
+        log = StateLog.objects.filter(
+            content_type=submission_ct, object_id=submission.id, state="REJECTED"
+        ).first()

        self.assertIsNotNone(log)
        self.assertEqual(log.state, "REJECTED")
@@ -957,13 +1016,19 @@ class TransitionLoggingTestCase(TestCase):
            reason="Test reason",
        )

+        # Must claim first (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
        # Perform transition
        submission.transition_to_approved(user=self.moderator)
        submission.save()

        # Check log
        submission_ct = ContentType.objects.get_for_model(submission)
-        log = StateLog.objects.filter(content_type=submission_ct, object_id=submission.id).first()
+        log = StateLog.objects.filter(
+            content_type=submission_ct, object_id=submission.id, state="APPROVED"
+        ).first()

        self.assertIsNotNone(log)
        # Description field exists and can be used for audit trails
@@ -986,6 +1051,10 @@ class TransitionLoggingTestCase(TestCase):

        submission_ct = ContentType.objects.get_for_model(submission)

+        # Must claim first (FSM requirement)
+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+
        # Create multiple transitions
        submission.transition_to_escalated(user=self.moderator)
        submission.save()
@@ -996,9 +1065,11 @@ class TransitionLoggingTestCase(TestCase):
        # Get logs ordered by timestamp
        logs = list(StateLog.objects.filter(content_type=submission_ct, object_id=submission.id).order_by("timestamp"))

-        # Verify ordering
-        self.assertEqual(len(logs), 2)
-        self.assertTrue(logs[0].timestamp <= logs[1].timestamp)
+        # Verify ordering - should have at least 2 logs (escalated and approved)
+        self.assertGreaterEqual(len(logs), 2)
+        # Verify timestamps are ordered
+        for i in range(len(logs) - 1):
+            self.assertTrue(logs[i].timestamp <= logs[i + 1].timestamp)


 # ============================================================================
@@ -1065,10 +1136,16 @@ class ModerationActionTests(TestCase):


 class PhotoSubmissionTransitionTests(TestCase):
-    """Comprehensive tests for PhotoSubmission FSM transitions."""
+    """Comprehensive tests for PhotoSubmission FSM transitions.
+    
+    Note: All approve/reject/escalate transitions require CLAIMED state first.
+    """

    def setUp(self):
        """Set up test fixtures."""
+        from datetime import timedelta
+        from django_cloudflareimages_toolkit.models import CloudflareImage
+        
        self.user = User.objects.create_user(
            username="testuser", email="test@example.com", password="testpass123", role="USER"
        )
@@ -1082,43 +1159,60 @@ class PhotoSubmissionTransitionTests(TestCase):
            name="Test Operator", description="Test Description", roles=["OPERATOR"]
        )
        self.content_type = ContentType.objects.get_for_model(Operator)
-
-    def _create_mock_photo(self):
-        """Create a mock CloudflareImage for testing."""
-        from unittest.mock import Mock
-
-        mock_photo = Mock()
-        mock_photo.pk = 1
-        mock_photo.id = 1
-        return mock_photo
+        
+        # Create a real CloudflareImage for tests (required by FK constraint)
+        self.mock_image = CloudflareImage.objects.create(
+            cloudflare_id=f"test-cf-photo-{id(self)}",
+            user=self.user,
+            expires_at=timezone.now() + timedelta(days=365),
+        )

    def _create_submission(self, status="PENDING"):
-        """Helper to create a PhotoSubmission."""
-        # Create using direct database creation to bypass FK validation
-        from unittest.mock import Mock, patch
+        """Helper to create a PhotoSubmission with proper CloudflareImage."""
+        submission = PhotoSubmission.objects.create(
+            user=self.user,
+            content_type=self.content_type,
+            object_id=self.operator.id,
+            photo=self.mock_image,
+            caption="Test Photo",
+            status="PENDING",  # Always create as PENDING first
+        )
+        
+        # For non-PENDING states, we need to transition through CLAIMED
+        if status == "CLAIMED":
+            submission.claim(user=self.moderator)
+            submission.refresh_from_db()
+        elif status in ("APPROVED", "REJECTED", "ESCALATED"):
+            # First claim, then transition to target state
+            submission.claim(user=self.moderator)
+            if status == "APPROVED":
+                submission.transition_to_approved(user=self.moderator)
+            elif status == "REJECTED":
+                submission.transition_to_rejected(user=self.moderator)
+            elif status == "ESCALATED":
+                submission.transition_to_escalated(user=self.moderator)
+            submission.save()
+            submission.refresh_from_db()
+        
+        return submission

-        with patch.object(PhotoSubmission, "photo", Mock()):
-            submission = PhotoSubmission(
-                user=self.user,
-                content_type=self.content_type,
-                object_id=self.operator.id,
-                caption="Test Photo",
-                status=status,
-            )
-            # Bypass model save to avoid FK constraint on photo
-            submission.photo_id = 1
-            submission.save(update_fields=None)
-            # Force status after creation for non-PENDING states
-            if status != "PENDING":
-                PhotoSubmission.objects.filter(pk=submission.pk).update(status=status)
-                submission.refresh_from_db()
-            return submission
-
-    def test_pending_to_approved_transition(self):
-        """Test transition from PENDING to APPROVED."""
+    def test_pending_to_claimed_transition(self):
+        """Test transition from PENDING to CLAIMED."""
        submission = self._create_submission()
        self.assertEqual(submission.status, "PENDING")

+        submission.claim(user=self.moderator)
+        submission.refresh_from_db()
+        
+        self.assertEqual(submission.status, "CLAIMED")
+        self.assertEqual(submission.claimed_by, self.moderator)
+        self.assertIsNotNone(submission.claimed_at)
+
+    def test_claimed_to_approved_transition(self):
+        """Test transition from CLAIMED to APPROVED (mandatory flow)."""
+        submission = self._create_submission(status="CLAIMED")
+        self.assertEqual(submission.status, "CLAIMED")
+
        submission.transition_to_approved(user=self.moderator)
        submission.handled_by = self.moderator
        submission.handled_at = timezone.now()
@@ -1129,10 +1223,10 @@ class PhotoSubmissionTransitionTests(TestCase):
        self.assertEqual(submission.handled_by, self.moderator)
        self.assertIsNotNone(submission.handled_at)

-    def test_pending_to_rejected_transition(self):
-        """Test transition from PENDING to REJECTED."""
-        submission = self._create_submission()
-        self.assertEqual(submission.status, "PENDING")
+    def test_claimed_to_rejected_transition(self):
+        """Test transition from CLAIMED to REJECTED (mandatory flow)."""
+        submission = self._create_submission(status="CLAIMED")
+        self.assertEqual(submission.status, "CLAIMED")

        submission.transition_to_rejected(user=self.moderator)
        submission.handled_by = self.moderator
@@ -1145,10 +1239,10 @@ class PhotoSubmissionTransitionTests(TestCase):
        self.assertEqual(submission.handled_by, self.moderator)
        self.assertIn("Rejected", submission.notes)

-    def test_pending_to_escalated_transition(self):
-        """Test transition from PENDING to ESCALATED."""
-        submission = self._create_submission()
-        self.assertEqual(submission.status, "PENDING")
+    def test_claimed_to_escalated_transition(self):
+        """Test transition from CLAIMED to ESCALATED (mandatory flow)."""
+        submission = self._create_submission(status="CLAIMED")
+        self.assertEqual(submission.status, "CLAIMED")

        submission.transition_to_escalated(user=self.moderator)
        submission.handled_by = self.moderator
@@ -1199,28 +1293,22 @@ class PhotoSubmissionTransitionTests(TestCase):
        with self.assertRaises(TransitionNotAllowed):
            submission.transition_to_approved(user=self.moderator)

+
    def test_reject_wrapper_method(self):
-        """Test the reject() wrapper method."""
-        from unittest.mock import patch
+        """Test the reject() wrapper method (requires CLAIMED state first)."""
+        submission = self._create_submission(status="CLAIMED")

-        submission = self._create_submission()
-
-        # Mock the photo creation part since we don't have actual photos
-        with patch.object(submission, "transition_to_rejected"):
-            submission.reject(self.moderator, notes="Not suitable")
+        submission.reject(self.moderator, notes="Not suitable")

        submission.refresh_from_db()
        self.assertEqual(submission.status, "REJECTED")
        self.assertIn("Not suitable", submission.notes)

    def test_escalate_wrapper_method(self):
-        """Test the escalate() wrapper method."""
-        from unittest.mock import patch
+        """Test the escalate() wrapper method (requires CLAIMED state first)."""
+        submission = self._create_submission(status="CLAIMED")

-        submission = self._create_submission()
-
-        with patch.object(submission, "transition_to_escalated"):
-            submission.escalate(self.moderator, notes="Needs admin review")
+        submission.escalate(self.moderator, notes="Needs admin review")

        submission.refresh_from_db()
        self.assertEqual(submission.status, "ESCALATED")
@@ -1230,7 +1318,7 @@ class PhotoSubmissionTransitionTests(TestCase):
        """Test that transitions create StateLog entries."""
        from django_fsm_log.models import StateLog

-        submission = self._create_submission()
+        submission = self._create_submission(status="CLAIMED")

        # Perform transition
        submission.transition_to_approved(user=self.moderator)
@@ -1248,10 +1336,10 @@ class PhotoSubmissionTransitionTests(TestCase):
        """Test that multiple transitions are all logged."""
        from django_fsm_log.models import StateLog

-        submission = self._create_submission()
+        submission = self._create_submission(status="CLAIMED")
        submission_ct = ContentType.objects.get_for_model(submission)

-        # First transition: PENDING -> ESCALATED
+        # First transition: CLAIMED -> ESCALATED
        submission.transition_to_escalated(user=self.moderator)
        submission.save()

@@ -1268,10 +1356,7 @@ class PhotoSubmissionTransitionTests(TestCase):

    def test_handled_by_and_handled_at_updated(self):
        """Test that handled_by and handled_at are properly updated."""
-        submission = self._create_submission()
-
-        self.assertIsNone(submission.handled_by)
-        self.assertIsNone(submission.handled_at)
+        submission = self._create_submission(status="CLAIMED")

        before_time = timezone.now()
        submission.transition_to_approved(user=self.moderator)
@@ -1287,7 +1372,7 @@ class PhotoSubmissionTransitionTests(TestCase):

    def test_notes_field_updated_on_rejection(self):
        """Test that notes field is updated with rejection reason."""
-        submission = self._create_submission()
+        submission = self._create_submission(status="CLAIMED")
        rejection_reason = "Image contains watermarks"

        submission.transition_to_rejected(user=self.moderator)
@@ -1299,7 +1384,7 @@ class PhotoSubmissionTransitionTests(TestCase):

    def test_notes_field_updated_on_escalation(self):
        """Test that notes field is updated with escalation reason."""
-        submission = self._create_submission()
+        submission = self._create_submission(status="CLAIMED")
        escalation_reason = "Potentially copyrighted content"

        submission.transition_to_escalated(user=self.moderator)
@@ -1308,3 +1393,4 @@ class PhotoSubmissionTransitionTests(TestCase):

        submission.refresh_from_db()
        self.assertEqual(submission.notes, escalation_reason)
+