diff --git a/pyproject.toml b/pyproject.toml
index ad9da6437b783650494d4f2a7a3bcc3d1615144e..15e4cade3952ee576715e8527baa5a7d9fb66d97 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -35,7 +35,8 @@ disable_error_code = ["annotation-unchecked"]
exclude = [
"migrations",
"tests",
- "dev"
+ "dev",
+ "scripts",
]
[tool.black]
diff --git a/running_exercise/lib/email_client.py b/running_exercise/lib/email_client.py
index 7bf5392a9abd7d23a85b0d55c924817a3cf39499..e58a3fd5096af67bb88417eeae8aee49acd79893 100644
--- a/running_exercise/lib/email_client.py
+++ b/running_exercise/lib/email_client.py
@@ -67,47 +67,39 @@ def send_email(
def _validate_participants(
participant_addresses: List[str], exercise: Exercise
-) -> List[EmailParticipant]:
- if len(set(participant_addresses)) < 2:
+) -> QuerySet[EmailParticipant]:
+ distinct_addresses = set(participant_addresses)
+ if len(distinct_addresses) < 2:
raise RunningExerciseOperationException(
"A thread must have at least 2 participants"
)
- email_between_teams = exercise.config.has_enabled(
- Feature.EMAIL_BETWEEN_TEAMS
+ participants = EmailParticipant.objects.filter(
+ exercise_id=exercise.id, address__in=distinct_addresses
)
- participants: List[EmailParticipant] = []
- previous_definition_participant: Optional[EmailParticipant] = None
- has_team = False
-
- for address in participant_addresses:
- # TODO: Handle email address that is not included in the definition,
- # needs to be logged somewhere, however action logs do not seem as
- # the best choice, because I am not sure if this should be included
- # in the trainee visible logs
- # maybe add a new action log type for errors, which can be used for filtering
- participant = get_model(
- EmailParticipant, exercise=exercise, address=address
+
+ if len(participants) != len(distinct_addresses):
+ # how much do we care about the error message?
+ raise RunningExerciseOperationException(
+ f"Some address/es from this list `{distinct_addresses}` don't exist in the exercise"
)
- if participant.is_definition():
- if previous_definition_participant is not None:
- raise RunningExerciseOperationException(
- f"Cannot send email to these addresses at the same time: "
- f"'{previous_definition_participant.address}',"
- f" '{participant.address}'"
- )
- previous_definition_participant = participant
- else:
- # this is probably an extremely rare edge case, however there is a possibility
- # that a team randomly guesses another teams address and without this check
- # the 'email_between_teams' feature could be bypassed
- if has_team and not email_between_teams:
- raise RunningExerciseOperationException(
- "Email communication between teams is disabled"
- )
- has_team = True
-
- participants.append(participant)
+
+ team_participants_count = sum(
+ map(lambda participant: participant.team_id is not None, participants)
+ )
+
+ if team_participants_count == 0:
+ raise RunningExerciseOperationException(
+ f"A thread must contain at least 1 team address"
+ )
+
+ if team_participants_count > 1 and not exercise.config.has_enabled(
+ Feature.EMAIL_BETWEEN_TEAMS
+ ):
+ raise RunningExerciseOperationException(
+ f"Email between teams is not enabled"
+ )
+
return participants
@@ -118,17 +110,21 @@ def _activate_thread_definition_milestones(
This method handles proper milestone activation for milestones specified in
EmailAddress
"""
- team_email_sent = (
- thread.emails.filter(sender__definition_address__isnull=True).count()
- > 0
- )
+ # Instructor emails should not activate milestones
+ if sender.is_definition():
+ return
- if team_email_sent or sender.is_definition():
+ # If a team already sent an email to this thread, do not activate milestones again
+ team_email_sent = thread.emails.filter(
+ sender__definition_address__isnull=True
+ ).exists()
+
+ if team_email_sent:
return
- if (
- definition_participant := thread.get_definition_participant()
- ) is not None:
+ for definition_participant in thread.participants.filter(
+ definition_address__isnull=False
+ ).select_related("definition_address__control"):
_update_thread_milestones(
thread, definition_participant.definition_address.control
)
@@ -147,9 +143,7 @@ class EmailClient:
)
new_thread.participants.add(*participants)
- SubscriptionHandler.broadcast_email_thread(
- new_thread, participants[0].exercise
- )
+ SubscriptionHandler.broadcast_email_thread(new_thread, exercise)
return new_thread
@staticmethod
diff --git a/running_exercise/schema/query.py b/running_exercise/schema/query.py
index 5b7ea3b5eb803ec46362124c4b34edfcc04f16ac..afaf33d3b0e2f1f1cc67eb07b01d5e6280e595aa 100644
--- a/running_exercise/schema/query.py
+++ b/running_exercise/schema/query.py
@@ -353,17 +353,20 @@ class Query(graphene.ObjectType):
@protected(User.AuthGroup.INSTRUCTOR)
def resolve_thread_templates(
self, info, thread_id: int
- ) -> List[EmailTemplate]:
+ ) -> QuerySet[EmailTemplate]:
thread = get_model(EmailThread, id=thread_id)
exercise_access(info.context, thread.exercise_id)
- definition_participant = thread.get_definition_participant()
- if definition_participant is None:
- return []
+ definition_participants = thread.participants.filter(
+ definition_address__isnull=False
+ ).select_related("definition_address")
- return list(
- definition_participant.get_definition_address().templates.all()
- )
+ templates = EmailTemplate.objects.none()
+
+ for participant in definition_participants:
+ templates |= participant.definition_address.templates.all()
+
+ return templates
@protected(User.AuthGroup.INSTRUCTOR)
def resolve_thread_template(self, info, template_id: str) -> EmailTemplate:
diff --git a/running_exercise/tests/email_tests.py b/running_exercise/tests/email_tests.py
index 5b20b1de131632d2fc33d22aa16eff1951491c17..152787219a29ae8243b36a4ba2cf950d3c52b7cd 100644
--- a/running_exercise/tests/email_tests.py
+++ b/running_exercise/tests/email_tests.py
@@ -128,22 +128,15 @@ class EmailTests(TestCase):
participants, self.subject, str(self.exercise.id)
)
+ participants.append("team-1@mail.com")
participants.append("nonexistentaddress@mail.com")
- with self.assertRaises(ModelNotFoundException) as _:
- EmailClient.create_thread(
- participants, self.subject, str(self.exercise.id)
- )
-
- # multiple definition addresses
- participants[1] = "test@mail.ex"
with self.assertRaises(RunningExerciseOperationException) as _:
EmailClient.create_thread(
participants, self.subject, str(self.exercise.id)
)
_disable_feature(self.exercise, Feature.EMAIL_BETWEEN_TEAMS)
- participants[1] = "team-1@mail.com"
- participants.append("team-2@mail.com")
+ participants[0] = "team-2@mail.com"
with self.assertRaises(RunningExerciseOperationException) as _:
EmailClient.create_thread(
participants, self.subject, str(self.exercise.id)
@@ -190,6 +183,17 @@ class EmailTests(TestCase):
self.subject,
)
+ # thread with multiple definition addresses
+ participants = ["team-1@mail.com", "test@mail.ex", "doe@mail.ex"]
+ thread = EmailClient.create_thread(
+ participants, self.subject, str(self.exercise.id)
+ )
+
+ self._compare_expected_participants(
+ participants, thread.participants.all()
+ )
+ self.assertEqual(thread.subject, self.subject)
+
def test_get_thread(self):
# each combination consist of a definition and a team address,
# the team address is first
@@ -355,8 +359,11 @@ class EmailTests(TestCase):
)
def test_team_email(self):
- participants = ["doe@mail.ex", "team-1@mail.com"]
- milestone = self.definition.milestones.get(name="email_doe")
+ participants = ["doe@mail.ex", "test@mail.ex", "team-1@mail.com"]
+ email_doe = self.definition.milestones.get(name="email_doe")
+ email_sent_to_test = self.definition.milestones.get(
+ name="email_sent_to_test"
+ )
team_sender = self.participants["team-1@mail.com"]
definition_sender = self.participants["doe@mail.ex"]
email_channel = self.definition.channels.get(type=InjectTypes.EMAIL)
@@ -365,7 +372,10 @@ class EmailTests(TestCase):
)
# check that milestones from the definition address were not activated on thread create
- self.assertFalse(is_milestone_reached(team_sender.team, milestone))
+ self.assertFalse(is_milestone_reached(team_sender.team, email_doe))
+ self.assertFalse(
+ is_milestone_reached(team_sender.team, email_sent_to_test)
+ )
file_info = self.definition.files.first()
action_logs = EmailClient.send_email(
create_input(
@@ -392,7 +402,10 @@ class EmailTests(TestCase):
)
# check that after first email was sent the thread milestones were not activated
# because it was not sent by a team
- self.assertFalse(is_milestone_reached(team_sender.team, milestone))
+ self.assertFalse(is_milestone_reached(team_sender.team, email_doe))
+ self.assertFalse(
+ is_milestone_reached(team_sender.team, email_sent_to_test)
+ )
action_logs = EmailClient.send_email(
create_input(
@@ -411,9 +424,12 @@ class EmailTests(TestCase):
f"'{sent_email.sender=}' and '{team_sender}'",
)
- self.assertTrue(is_milestone_reached(team_sender.team, milestone))
+ self.assertTrue(is_milestone_reached(team_sender.team, email_doe))
+ self.assertTrue(
+ is_milestone_reached(team_sender.team, email_sent_to_test)
+ )
- deactivate_milestones(team_sender.team, [milestone])
+ deactivate_milestones(team_sender.team, [email_doe, email_sent_to_test])
action_logs = EmailClient.send_email(
create_input(
@@ -426,4 +442,7 @@ class EmailTests(TestCase):
self._check_action_logs_channel(action_logs, email_channel)
# check that further emails do not activate the thread milestones again
- self.assertFalse(is_milestone_reached(team_sender.team, milestone))
+ self.assertFalse(is_milestone_reached(team_sender.team, email_doe))
+ self.assertFalse(
+ is_milestone_reached(team_sender.team, email_sent_to_test)
+ )