Skip to content

Commit abbdfa9

Browse files
Backend: Refactor calculate_distinct_sorted_leaderboard_data to use sets for banned email IDs and participant teams, improving performance and ensuring distinct team listings. Add unit tests to verify behavior with empty and None banned email IDs. (#4997)
1 parent 454da01 commit abbdfa9

File tree

2 files changed

+333
-5
lines changed

2 files changed

+333
-5
lines changed

apps/jobs/utils.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,10 @@ def calculate_distinct_sorted_leaderboard_data(
460460
"submission__is_verified_by_host",
461461
)
462462

463-
all_banned_participant_team = []
463+
all_banned_participant_team = set()
464+
all_banned_email_ids_set = (
465+
set(all_banned_email_ids) if all_banned_email_ids else set()
466+
)
464467

465468
# Apply query limit to prevent slow queries on popular challenges
466469
max_limit = getattr(settings, "MAX_LEADERBOARD_QUERY_LIMIT", 10000)
@@ -489,8 +492,8 @@ def calculate_distinct_sorted_leaderboard_data(
489492
participant_team_id, []
490493
)
491494
for participant_email in all_participants_email_ids:
492-
if participant_email in all_banned_email_ids:
493-
all_banned_participant_team.append(participant_team_id)
495+
if participant_email in all_banned_email_ids_set:
496+
all_banned_participant_team.add(participant_team_id)
494497
break
495498
if leaderboard_item["error"] is None:
496499
leaderboard_item.update(filtering_error=0)
@@ -508,7 +511,7 @@ def calculate_distinct_sorted_leaderboard_data(
508511
reverse=True if is_leaderboard_order_descending else False,
509512
)
510513
distinct_sorted_leaderboard_data = []
511-
team_list = []
514+
team_list = set()
512515
for data in sorted_leaderboard_data:
513516
if (
514517
data["submission__participant_team__team_name"] in team_list
@@ -520,7 +523,7 @@ def calculate_distinct_sorted_leaderboard_data(
520523
distinct_sorted_leaderboard_data.append(data)
521524
else:
522525
distinct_sorted_leaderboard_data.append(data)
523-
team_list.append(data["submission__participant_team__team_name"])
526+
team_list.add(data["submission__participant_team__team_name"])
524527

525528
leaderboard_labels = challenge_phase_split.leaderboard.schema["labels"]
526529
show_scores = challenge_phase_split.show_scores_on_leaderboard

tests/unit/jobs/test_utils.py

Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ def setUp(self):
524524
self.challenge_phase_split.show_leaderboard_by_latest_submission = (
525525
False
526526
)
527+
self.challenge_phase_split.show_scores_on_leaderboard = True
527528

528529
@patch("jobs.utils.ParticipantTeam")
529530
@patch("challenges.models.LeaderboardData.objects")
@@ -799,6 +800,330 @@ def test_multiple_banned_participants_in_team(
799800
self.assertEqual(len(result), 1)
800801
self.assertEqual(result[0]["submission__participant_team"], 2)
801802

803+
@patch("jobs.utils.ParticipantTeam")
804+
@patch("challenges.models.LeaderboardData.objects")
805+
@patch("hosts.utils.is_user_a_staff_or_host")
806+
def test_empty_banned_email_ids_includes_all_teams(
807+
self,
808+
mock_is_user_a_staff_or_host,
809+
mock_leaderboard_data_objects,
810+
mock_participant_team,
811+
):
812+
"""Test that empty banned_email_ids excludes no teams."""
813+
mock_is_user_a_staff_or_host.return_value = False
814+
self.challenge_obj.banned_email_ids = []
815+
816+
test_data = [
817+
{
818+
"submission__participant_team": 1,
819+
"submission__participant_team__team_name": "Team1",
820+
"submission__is_baseline": False,
821+
"error": None,
822+
"filtering_score": 10,
823+
"result": {"score": 10, "time": 5},
824+
},
825+
{
826+
"submission__participant_team": 2,
827+
"submission__participant_team__team_name": "Team2",
828+
"submission__is_baseline": False,
829+
"error": None,
830+
"filtering_score": 9,
831+
"result": {"score": 9, "time": 4},
832+
},
833+
]
834+
835+
self._create_mock_leaderboard_chain(
836+
mock_leaderboard_data_objects, test_data
837+
)
838+
839+
mock_team1 = Mock()
840+
mock_team1.id = 1
841+
mock_p1 = Mock()
842+
mock_p1.user.email = "user1@example.com"
843+
mock_team1.participants.all.return_value = [mock_p1]
844+
845+
mock_team2 = Mock()
846+
mock_team2.id = 2
847+
mock_p2 = Mock()
848+
mock_p2.user.email = "user2@example.com"
849+
mock_team2.participants.all.return_value = [mock_p2]
850+
851+
mock_participant_team.objects.filter.return_value.prefetch_related.return_value = [
852+
mock_team1,
853+
mock_team2,
854+
]
855+
856+
result, status_code = calculate_distinct_sorted_leaderboard_data(
857+
self.user,
858+
self.challenge_obj,
859+
self.challenge_phase_split,
860+
False,
861+
"score",
862+
)
863+
864+
self.assertEqual(status_code, 200)
865+
self.assertEqual(len(result), 2)
866+
867+
@patch("jobs.utils.ParticipantTeam")
868+
@patch("challenges.models.LeaderboardData.objects")
869+
@patch("hosts.utils.is_user_a_staff_or_host")
870+
def test_none_banned_email_ids_includes_all_teams(
871+
self,
872+
mock_is_user_a_staff_or_host,
873+
mock_leaderboard_data_objects,
874+
mock_participant_team,
875+
):
876+
"""Test that None banned_email_ids excludes no teams (set conversion)."""
877+
mock_is_user_a_staff_or_host.return_value = False
878+
self.challenge_obj.banned_email_ids = None
879+
880+
test_data = [
881+
{
882+
"submission__participant_team": 1,
883+
"submission__participant_team__team_name": "Team1",
884+
"submission__is_baseline": False,
885+
"error": None,
886+
"filtering_score": 10,
887+
"result": {"score": 10, "time": 5},
888+
},
889+
]
890+
891+
self._create_mock_leaderboard_chain(
892+
mock_leaderboard_data_objects, test_data
893+
)
894+
895+
mock_team1 = Mock()
896+
mock_team1.id = 1
897+
mock_p1 = Mock()
898+
mock_p1.user.email = "user1@example.com"
899+
mock_team1.participants.all.return_value = [mock_p1]
900+
901+
mock_participant_team.objects.filter.return_value.prefetch_related.return_value = [
902+
mock_team1,
903+
]
904+
905+
result, status_code = calculate_distinct_sorted_leaderboard_data(
906+
self.user,
907+
self.challenge_obj,
908+
self.challenge_phase_split,
909+
False,
910+
"score",
911+
)
912+
913+
self.assertEqual(status_code, 200)
914+
self.assertEqual(len(result), 1)
915+
916+
test_data = [
917+
{
918+
"submission__participant_team": 1,
919+
"submission__participant_team__team_name": "Team1",
920+
"submission__is_baseline": False,
921+
"error": None,
922+
"filtering_score": 10,
923+
"result": {"score": 10, "time": 5},
924+
},
925+
{
926+
"submission__participant_team": 2,
927+
"submission__participant_team__team_name": "Team2",
928+
"submission__is_baseline": False,
929+
"error": None,
930+
"filtering_score": 9,
931+
"result": {"score": 9, "time": 4},
932+
},
933+
]
934+
935+
self._create_mock_leaderboard_chain(
936+
mock_leaderboard_data_objects, test_data
937+
)
938+
939+
mock_team1 = Mock()
940+
mock_team1.id = 1
941+
mock_p1 = Mock()
942+
mock_p1.user.email = "user1@example.com"
943+
mock_team1.participants.all.return_value = [mock_p1]
944+
945+
mock_team2 = Mock()
946+
mock_team2.id = 2
947+
mock_p2 = Mock()
948+
mock_p2.user.email = "user2@example.com"
949+
mock_team2.participants.all.return_value = [mock_p2]
950+
951+
mock_participant_team.objects.filter.return_value.prefetch_related.return_value = [
952+
mock_team1,
953+
mock_team2,
954+
]
955+
956+
result, status_code = calculate_distinct_sorted_leaderboard_data(
957+
self.user,
958+
self.challenge_obj,
959+
self.challenge_phase_split,
960+
False,
961+
"score",
962+
)
963+
964+
self.assertEqual(status_code, 200)
965+
self.assertEqual(len(result), 2)
966+
967+
@patch("jobs.utils.ParticipantTeam")
968+
@patch("challenges.models.LeaderboardData.objects")
969+
@patch("hosts.utils.is_user_a_staff_or_host")
970+
def test_distinct_team_list_with_many_duplicates(
971+
self,
972+
mock_is_user_a_staff_or_host,
973+
mock_leaderboard_data_objects,
974+
mock_participant_team,
975+
):
976+
"""Test set-based team_list gives correct distinct result with many entries."""
977+
mock_is_user_a_staff_or_host.return_value = False
978+
979+
# Many entries from same teams - Team1 appears 5x, Team2 appears 3x
980+
test_data = [
981+
{
982+
"submission__participant_team": 1,
983+
"submission__participant_team__team_name": "Team1",
984+
"submission__is_baseline": False,
985+
"error": None,
986+
"filtering_score": float(i),
987+
"result": {"score": str(i), "time": "0"},
988+
}
989+
for i in range(10, 5, -1)
990+
] + [
991+
{
992+
"submission__participant_team": 2,
993+
"submission__participant_team__team_name": "Team2",
994+
"submission__is_baseline": False,
995+
"error": None,
996+
"filtering_score": float(i),
997+
"result": {"score": str(i), "time": "0"},
998+
}
999+
for i in range(9, 6, -1)
1000+
]
1001+
1002+
self._create_mock_leaderboard_chain(
1003+
mock_leaderboard_data_objects, test_data
1004+
)
1005+
1006+
mock_team1 = Mock()
1007+
mock_team1.id = 1
1008+
mock_p1 = Mock()
1009+
mock_p1.user.email = "user1@example.com"
1010+
mock_team1.participants.all.return_value = [mock_p1]
1011+
1012+
mock_team2 = Mock()
1013+
mock_team2.id = 2
1014+
mock_p2 = Mock()
1015+
mock_p2.user.email = "user2@example.com"
1016+
mock_team2.participants.all.return_value = [mock_p2]
1017+
1018+
mock_participant_team.objects.filter.return_value.prefetch_related.return_value = [
1019+
mock_team1,
1020+
mock_team2,
1021+
]
1022+
1023+
result, status_code = calculate_distinct_sorted_leaderboard_data(
1024+
self.user,
1025+
self.challenge_obj,
1026+
self.challenge_phase_split,
1027+
False,
1028+
"score",
1029+
)
1030+
1031+
self.assertEqual(status_code, 200)
1032+
# Only 2 distinct teams (best entry per team)
1033+
self.assertEqual(len(result), 2)
1034+
self.assertEqual(
1035+
result[0]["submission__participant_team__team_name"], "Team1"
1036+
)
1037+
self.assertEqual(
1038+
result[1]["submission__participant_team__team_name"], "Team2"
1039+
)
1040+
1041+
@patch("jobs.utils.ParticipantTeam")
1042+
@patch("challenges.models.LeaderboardData.objects")
1043+
@patch("hosts.utils.is_user_a_staff_or_host")
1044+
def test_baseline_entries_always_included(
1045+
self,
1046+
mock_is_user_a_staff_or_host,
1047+
mock_leaderboard_data_objects,
1048+
mock_participant_team,
1049+
):
1050+
"""Test baseline entries are always included regardless of team_list."""
1051+
mock_is_user_a_staff_or_host.return_value = False
1052+
1053+
test_data = [
1054+
{
1055+
"submission__participant_team": 1,
1056+
"submission__participant_team__team_name": "Team1",
1057+
"submission__is_baseline": False,
1058+
"error": None,
1059+
"filtering_score": 10,
1060+
"result": {"score": "10", "time": "0"},
1061+
},
1062+
{
1063+
"submission__participant_team": 2,
1064+
"submission__participant_team__team_name": "Baseline",
1065+
"submission__is_baseline": True,
1066+
"error": None,
1067+
"filtering_score": 5,
1068+
"result": {"score": "5", "time": "0"},
1069+
},
1070+
{
1071+
"submission__participant_team": 2,
1072+
"submission__participant_team__team_name": "Baseline",
1073+
"submission__is_baseline": True,
1074+
"error": None,
1075+
"filtering_score": 3,
1076+
"result": {"score": "3", "time": "0"},
1077+
},
1078+
]
1079+
1080+
self._create_mock_leaderboard_chain(
1081+
mock_leaderboard_data_objects, test_data
1082+
)
1083+
1084+
mock_team1 = Mock()
1085+
mock_team1.id = 1
1086+
mock_p1 = Mock()
1087+
mock_p1.user.email = "user1@example.com"
1088+
mock_team1.participants.all.return_value = [mock_p1]
1089+
1090+
mock_team2 = Mock()
1091+
mock_team2.id = 2
1092+
mock_p2 = Mock()
1093+
mock_p2.user.email = "baseline@example.com"
1094+
mock_team2.participants.all.return_value = [mock_p2]
1095+
1096+
mock_participant_team.objects.filter.return_value.prefetch_related.return_value = [
1097+
mock_team1,
1098+
mock_team2,
1099+
]
1100+
1101+
result, status_code = calculate_distinct_sorted_leaderboard_data(
1102+
self.user,
1103+
self.challenge_obj,
1104+
self.challenge_phase_split,
1105+
False,
1106+
"score",
1107+
)
1108+
1109+
self.assertEqual(status_code, 200)
1110+
# Team1 (1) + 2 baseline entries = 3
1111+
self.assertEqual(len(result), 3)
1112+
1113+
def _create_mock_leaderboard_chain(
1114+
self, mock_leaderboard_data_objects, test_data
1115+
):
1116+
"""Helper to create mock chain for LeaderboardData queryset."""
1117+
mock_qs = MagicMock()
1118+
mock_filter_result = MagicMock()
1119+
mock_leaderboard_data_objects.filter.return_value = mock_filter_result
1120+
mock_filter_result.exclude.return_value = mock_qs
1121+
mock_qs.filter.return_value = mock_qs
1122+
mock_qs.order_by.return_value = mock_qs
1123+
mock_qs.annotate.return_value = mock_qs
1124+
mock_qs.values.return_value = test_data
1125+
return mock_qs
1126+
8021127
def test_all_comparisons(self):
8031128
def dummy_comparator(a, b):
8041129
return a - b

0 commit comments

Comments
 (0)