Fix

Author: Akshat Singh

tests/unit/challenges/test_views.py

@@ -5961,8 +5961,11 @@ def test_request_challenge_approval_with_successful_subscription_email(
         )
         response = self.client.get(url)
 
+        # Verify email function was called with correct challenge
         mock_send_email.assert_called_once_with(self.challenge)
 
+        # Verify success logging
+
         mock_logger.info.assert_any_call(
             "Subscription plans email sent successfully for challenge {}".format(
                 self.challenge.pk
@@ -6001,14 +6004,19 @@ def test_request_challenge_approval_with_email_failure_continues_approval(
         )
         response = self.client.get(url)
 
+        # Verify email function was called
         mock_send_email.assert_called_once_with(self.challenge)
 
+        # Verify error logging
+
         mock_logger.error.assert_any_call(
             "Failed to send subscription plans email for challenge {}: {}".format(
                 self.challenge.pk, "Email service unavailable"
             )
         )
 
+        # Verify approval process continues despite email failure
+
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(
             response.data,
@@ -6025,10 +6033,12 @@ def test_request_challenge_approval_challenge_not_found(
         """Test that email is not sent when challenge doesn't exist"""
         url = reverse_lazy(
             "challenges:request_challenge_approval_by_pk",
-            kwargs={"challenge_pk": 99999},
+            kwargs={"challenge_pk": 99999},  # Non-existent challenge
         )
         response = self.client.get(url)
 
+        # Verify email function was not called for non-existent challenge
+
         mock_send_email.assert_not_called()
 
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
@@ -6038,6 +6048,8 @@ def test_request_challenge_approval_challenge_not_found(
     def test_request_challenge_approval_user_not_host(self, mock_send_email):
         """Test that email is not sent when user is not challenge host"""
 
+        # Create a different user who is not a challenge host
+
         other_user = User.objects.create(
             username="otheruser",
             password="other_password",
@@ -6050,6 +6062,8 @@ def test_request_challenge_approval_user_not_host(self, mock_send_email):
             verified=True,
         )
 
+        # Authenticate as the other user
+
         self.client.force_authenticate(user=other_user)
 
         url = reverse_lazy(
@@ -6058,6 +6072,8 @@ def test_request_challenge_approval_user_not_host(self, mock_send_email):
         )
         response = self.client.get(url)
 
+        # Verify email function was not called for unauthorized user
+
         mock_send_email.assert_not_called()
 
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
@@ -6071,7 +6087,7 @@ def test_request_challenge_approval_webhook_failure_after_email_success(
         responses.add(
             responses.POST,
             settings.APPROVAL_WEBHOOK_URL,
-            body=b"error",
+            body=b"error",  # Simulate webhook failure
             status=200,
             content_type="text/plain",
         )
@@ -6082,8 +6098,11 @@ def test_request_challenge_approval_webhook_failure_after_email_success(
         )
         response = self.client.get(url)
 
+        # Verify email function was called despite webhook failure
         mock_send_email.assert_called_once_with(self.challenge)
 
+        # Webhook failure should result in error response
+
         self.assertEqual(response.status_code, status.HTTP_406_NOT_ACCEPTABLE)
         self.assertIn("error", response.data)
 
@@ -6099,6 +6118,8 @@ def test_request_challenge_approval_with_smtp_error(self, mock_send_email):
             content_type="text/plain",
         )
 
+        # Simulate SMTP error
+
         from smtplib import SMTPException
 
         mock_send_email.side_effect = SMTPException(
@@ -6111,8 +6132,11 @@ def test_request_challenge_approval_with_smtp_error(self, mock_send_email):
         )
         response = self.client.get(url)
 
+        # Verify email function was called
         mock_send_email.assert_called_once_with(self.challenge)
 
+        # Approval should continue despite SMTP error
+
         self.assertEqual(response.status_code, status.HTTP_200_OK)
 
     @responses.activate
@@ -6122,6 +6146,8 @@ def test_request_challenge_approval_email_integration_with_challenge_phases(
     ):
         """Test email integration with challenge that has multiple phases"""
 
+        # Create additional challenge phase
+
         with self.settings(MEDIA_ROOT="/tmp/evalai"):
             additional_phase = ChallengePhase.objects.create(
                 name="Additional Phase",
@@ -6138,9 +6164,12 @@ def test_request_challenge_approval_email_integration_with_challenge_phases(
                 ),
             )
 
+        # Create a finished submission for the additional phase to satisfy the submission check
         from jobs.models import Submission
         from participants.models import Participant
 
+        # Ensure participant team is associated with the challenge and user is a participant
+
         self.challenge.participant_teams.add(self.participant_team)
         Participant.objects.get_or_create(
             user=self.user,
@@ -6152,7 +6181,7 @@ def test_request_challenge_approval_email_integration_with_challenge_phases(
             participant_team=self.participant_team,
             challenge_phase=additional_phase,
             created_by=self.user,
-            status="submitted",
+            status="submitted",  # Start with submitted status
             input_file=SimpleUploadedFile(
                 "test_input.txt", b"test input", content_type="text/plain"
             ),
@@ -6163,6 +6192,8 @@ def test_request_challenge_approval_email_integration_with_challenge_phases(
             is_public=True,
         )
 
+        # Manually update the status to finished after creation to bypass any automatic processing
+
         submission.status = "finished"
         submission.save()
 
@@ -6180,6 +6211,8 @@ def test_request_challenge_approval_email_integration_with_challenge_phases(
         )
         response = self.client.get(url)
 
+        # Verify email function was called with the challenge
+
         mock_send_email.assert_called_once_with(self.challenge)
 
         self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -6191,6 +6224,8 @@ def test_request_challenge_approval_email_not_sent_when_submissions_incomplete(
     ):
         """Test that email is not sent when submission check fails"""
 
+        # Create a challenge phase without finished submissions
+
         with self.settings(MEDIA_ROOT="/tmp/evalai"):
             ChallengePhase.objects.create(
                 name="Unfinished Phase",
@@ -6213,8 +6248,11 @@ def test_request_challenge_approval_email_not_sent_when_submissions_incomplete(
         )
         response = self.client.get(url)
 
+        # Email should NOT be sent when submission check fails
         mock_send_email.assert_not_called()
 
+        # The request should fail due to unfinished submissions
+
         self.assertEqual(response.status_code, status.HTTP_406_NOT_ACCEPTABLE)
         self.assertIn(
             "do not have finished submissions", response.data["error"]
@@ -6325,6 +6363,8 @@ def test_create_challenge_using_github_success(self):
         self.assertEqual(Leaderboard.objects.count(), 1)
         self.assertEqual(ChallengePhaseSplit.objects.count(), 1)
 
+        # Verify github_repository is properly stored
+
         challenge = Challenge.objects.first()
         self.assertEqual(
             challenge.github_repository,