diff --git a/CHANGELOG.md b/CHANGELOG.md index 57251e42..be4aff7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to - ✨(backend) update a room's attributes from the external API - 🔊(backend) log request duration in Gunicorn workers - 📈(frontend) track missing lobby participant on accept/reject +- ✨(backend) sort waiting participants by their arrival time ### Changed diff --git a/src/backend/core/services/lobby.py b/src/backend/core/services/lobby.py index 240560c2..a361abcf 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -9,6 +9,7 @@ from uuid import UUID from django.conf import settings from django.core.cache import cache +from django.utils import timezone from core import models, utils @@ -46,6 +47,7 @@ class LobbyParticipant: username: str color: str id: str + entered_at: str def to_dict(self) -> Dict[str, str]: """Serialize the participant object to a dict representation.""" @@ -54,6 +56,7 @@ class LobbyParticipant: "username": self.username, "id": self.id, "color": self.color, + "entered_at": self.entered_at, } @classmethod @@ -68,6 +71,7 @@ class LobbyParticipant: username=data["username"], id=data["id"], color=data["color"], + entered_at=data["entered_at"], ) except (KeyError, ValueError) as e: logger.exception("Error creating Participant from dict:") @@ -203,6 +207,7 @@ class LobbyService: username=username, id=participant_id, color=utils.generate_color(participant_id), + entered_at=timezone.now().isoformat(), ) else: participant.status = LobbyParticipantStatus.ACCEPTED @@ -264,6 +269,7 @@ class LobbyService: username=username, id=participant_id, color=color, + entered_at=timezone.now().isoformat(), ) try: @@ -338,6 +344,8 @@ class LobbyService: self._index_remove(room_id, *dead_ids) + waiting_participants.sort(key=lambda p: p["entered_at"], reverse=True) + return tuple(waiting_participants) def handle_participant_entry( diff --git a/src/backend/core/tests/rooms/test_api_rooms_lobby.py b/src/backend/core/tests/rooms/test_api_rooms_lobby.py index 164603d8..04df0df6 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_lobby.py +++ b/src/backend/core/tests/rooms/test_api_rooms_lobby.py @@ -9,6 +9,7 @@ from unittest import mock from django.core.cache import cache import pytest +from freezegun import freeze_time from rest_framework.test import APIClient from ... import utils @@ -24,6 +25,7 @@ pytestmark = pytest.mark.django_db # Tests for request_entry endpoint +@freeze_time("2025-01-01 10:00:00") def test_request_entry_anonymous(settings): """Anonymous users should be allowed to request entry to a room.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -59,6 +61,7 @@ def test_request_entry_anonymous(settings): "username": "test_user", "status": "waiting", "color": "mocked-color", + "entered_at": "2025-01-01T10:00:00+00:00", "livekit": None, } @@ -71,6 +74,7 @@ def test_request_entry_anonymous(settings): assert participant_data.get("username") == "test_user" +@freeze_time("2025-01-01 10:00:00") def test_request_entry_authenticated_user(settings): """Authenticated users should be allowed to request entry.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -108,6 +112,7 @@ def test_request_entry_authenticated_user(settings): "username": "test_user", "status": "waiting", "color": "mocked-color", + "entered_at": "2025-01-01T10:00:00+00:00", "livekit": None, } @@ -120,6 +125,7 @@ def test_request_entry_authenticated_user(settings): assert participant_data.get("username") == "test_user" +@freeze_time("2025-01-01 10:00:00") def test_request_entry_with_existing_participants(settings): """Anonymous users should be allowed to request entry to a room with existing participants.""" # Create a restricted access room @@ -138,6 +144,7 @@ def test_request_entry_with_existing_participants(settings): "username": "user1", "status": "waiting", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", }, ) cache.set( @@ -147,6 +154,7 @@ def test_request_entry_with_existing_participants(settings): "username": "user2", "status": "accepted", "color": "#654321", + "entered_at": "2025-01-01T10:00:00+00:00", }, ) @@ -178,6 +186,7 @@ def test_request_entry_with_existing_participants(settings): assert response.json() == { "id": participant_id, "username": "test_user", + "entered_at": "2025-01-01T10:00:00+00:00", "status": "waiting", "color": "mocked-color", "livekit": None, @@ -192,6 +201,7 @@ def test_request_entry_with_existing_participants(settings): assert participant_data.get("username") == "test_user" +@freeze_time("2025-01-01 10:00:00") def test_request_entry_public_room(settings): """Entry requests to public rooms should return ACCEPTED status with LiveKit config.""" room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) @@ -230,6 +240,7 @@ def test_request_entry_public_room(settings): assert response.json() == { "id": "123", "username": "test_user", + "entered_at": "2025-01-01T10:00:00+00:00", "status": "accepted", "color": "mocked-color", "livekit": {"token": "test-token"}, @@ -240,6 +251,7 @@ def test_request_entry_public_room(settings): assert not lobby_keys +@freeze_time("2025-01-01 10:00:00") def test_request_entry_authenticated_user_public_room(settings): """While authenticated, entry request to public rooms should get accepted.""" room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) @@ -282,6 +294,7 @@ def test_request_entry_authenticated_user_public_room(settings): assert response.json() == { "id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", "username": "test_user", + "entered_at": "2025-01-01T10:00:00+00:00", "status": "accepted", "color": "mocked-color", "livekit": {"token": "test-token"}, @@ -292,6 +305,7 @@ def test_request_entry_authenticated_user_public_room(settings): assert not lobby_keys +@freeze_time("2025-01-01 10:00:00") def test_request_entry_waiting_participant_public_room(settings): """While waiting, entry request to public rooms should get accepted.""" room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) @@ -308,6 +322,7 @@ def test_request_entry_waiting_participant_public_room(settings): "username": "user1", "status": "waiting", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", }, ) @@ -338,6 +353,7 @@ def test_request_entry_waiting_participant_public_room(settings): "username": "user1", "status": "accepted", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", "livekit": {"token": "test-token"}, } @@ -443,6 +459,7 @@ def test_allow_participant_to_enter_success(settings, allow_entry, updated_statu "status": "waiting", "username": "foo", "color": "123", + "entered_at": "2025-01-01T10:00:00+00:00", }, ) @@ -578,6 +595,7 @@ def test_list_waiting_participants_success(settings): "username": "user1", "status": "waiting", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", }, ) cache.set( @@ -587,6 +605,7 @@ def test_list_waiting_participants_success(settings): "username": "user2", "status": "waiting", "color": "#654321", + "entered_at": "2025-01-01T10:05:00+00:00", }, ) lobby_service = LobbyService() @@ -597,21 +616,24 @@ def test_list_waiting_participants_success(settings): assert response.status_code == 200 - participants = response.json().get("participants") - assert sorted(participants, key=lambda p: p["id"]) == [ - { - "id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", - "username": "user1", - "status": "waiting", - "color": "#123456", - }, - { - "id": "f4ca3ab8a6c04ad88097b8da33f60f10", - "username": "user2", - "status": "waiting", - "color": "#654321", - }, - ] + assert response.json() == { + "participants": [ + { + "id": "f4ca3ab8a6c04ad88097b8da33f60f10", + "username": "user2", + "status": "waiting", + "color": "#654321", + "entered_at": "2025-01-01T10:05:00+00:00", + }, + { + "id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", + "username": "user1", + "status": "waiting", + "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", + }, + ] + } def test_list_waiting_participants_empty(settings): diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index 5d062784..f2bb695b 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -14,6 +14,7 @@ from django.core.cache import cache from django.http import HttpResponse import pytest +from freezegun import freeze_time from core.factories import RoomFactory, UserFactory, UserResourceAccessFactory from core.models import RoleChoices, RoomAccessLevel @@ -55,6 +56,7 @@ def participant_dict(): "username": "test-username", "id": "test-participant-id", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } @@ -66,6 +68,7 @@ def participant_data(): username="test-username", id="test-participant-id", color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) @@ -77,6 +80,7 @@ def test_lobby_participant_to_dict(participant_data): assert result["username"] == "test-username" assert result["id"] == "test-participant-id" assert result["color"] == "#123456" + assert result["entered_at"] == "2025-01-01T10:00:00+00:00" def test_lobby_participant_from_dict_success(participant_dict): @@ -87,6 +91,20 @@ def test_lobby_participant_from_dict_success(participant_dict): assert participant.username == "test-username" assert participant.id == "test-participant-id" assert participant.color == "#123456" + assert participant.entered_at == "2025-01-01T10:00:00+00:00" + + +def test_lobby_participant_from_dict_missing_entered_at(): + """`entered_at` is mandatory; data without it is rejected.""" + data = { + "status": "waiting", + "username": "test-username", + "id": "test-participant-id", + "color": "#123456", + } + + with pytest.raises(LobbyParticipantParsingError, match="Invalid participant data"): + LobbyParticipant.from_dict(data) def test_lobby_participant_from_dict_default_status(): @@ -95,6 +113,7 @@ def test_lobby_participant_from_dict_default_status(): "username": "test-username", "id": "test-participant-id", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } participant = LobbyParticipant.from_dict(data_without_status) @@ -120,6 +139,7 @@ def test_lobby_participant_from_dict_invalid_status(): "username": "test-username", "id": "test-participant-id", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } with pytest.raises(LobbyParticipantParsingError, match="Invalid participant data"): @@ -264,6 +284,7 @@ def test_request_entry_public_room( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) @@ -302,6 +323,7 @@ def test_request_entry_trusted_room( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) @@ -344,6 +366,7 @@ def test_request_entry_new_participant( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) mock_enter.return_value = participant_data @@ -371,6 +394,7 @@ def test_request_entry_waiting_participant( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) @@ -399,6 +423,7 @@ def test_request_entry_accepted_participant( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) @@ -439,6 +464,7 @@ def test_request_entry_participant_with_role( username=username, id=participant_id, color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ) lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) @@ -479,6 +505,7 @@ def test_refresh_waiting_status(mock_cache, lobby_service, participant_id): @mock.patch("core.utils.generate_color") @mock.patch("core.utils.notify_participants") @mock.patch("core.services.lobby.LobbyService._index_add") +@freeze_time("2025-01-01 10:00:00") def test_enter_success( mock_index_add, mock_notify, @@ -500,6 +527,7 @@ def test_enter_success( assert participant.username == username assert participant.id == participant_id assert participant.color == "#123456" + assert participant.entered_at == "2025-01-01T10:00:00+00:00" lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) @@ -629,6 +657,7 @@ def test_list_waiting_participants_multiple(mock_cache, lobby_service): "username": "user1", "id": "participant1", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } participant2 = { @@ -636,6 +665,7 @@ def test_list_waiting_participants_multiple(mock_cache, lobby_service): "username": "user2", "id": "participant2", "color": "#654321", + "entered_at": "2025-01-01T10:05:00+00:00", } lobby_service._index_members = mock.Mock( @@ -651,9 +681,10 @@ def test_list_waiting_participants_multiple(mock_cache, lobby_service): assert len(result) == 2 - # Verify both participants are in the result - assert any(p["id"] == "participant1" and p["username"] == "user1" for p in result) - assert any(p["id"] == "participant2" and p["username"] == "user2" for p in result) + # Most recent entry comes first + assert [p["id"] for p in result] == ["participant2", "participant1"] + assert result[0]["username"] == "user2" + assert result[1]["username"] == "user1" # Verify all participants have waiting status assert all(p["status"] == "waiting" for p in result) @@ -689,6 +720,7 @@ def test_list_waiting_participants_partially_corrupted(mock_cache, lobby_service "username": "user2", "id": "participant2", "color": "#654321", + "entered_at": "2025-01-01T10:00:00+00:00", } corrupted_participant = {"invalid": "data"} @@ -729,12 +761,14 @@ def test_list_waiting_participants_non_waiting(mock_cache, lobby_service): "username": "user1", "id": "participant1", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } participant2 = { "status": "accepted", "username": "user2", "id": "participant2", "color": "#654321", + "entered_at": "2025-01-01T10:00:00+00:00", } lobby_service._index_members = mock.Mock( @@ -832,6 +866,7 @@ def test_update_participant_status_success(mock_cache, lobby_service, participan "username": "test-username", "id": participant_id, "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } mock_cache.get.return_value = participant_dict @@ -850,6 +885,7 @@ def test_update_participant_status_success(mock_cache, lobby_service, participan "username": "test-username", "id": participant_id, "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } mock_cache.set.assert_called_once_with( "mocked_cache_key", expected_data, timeout=60 @@ -875,6 +911,7 @@ def test_clear_room_cache(settings, lobby_service): username="participant1", id="participant1", color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ), timeout=settings.LOBBY_WAITING_TIMEOUT, ) @@ -885,6 +922,7 @@ def test_clear_room_cache(settings, lobby_service): username="participant2", id="participant2", color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ), timeout=settings.LOBBY_ACCEPTED_TIMEOUT, ) @@ -895,6 +933,7 @@ def test_clear_room_cache(settings, lobby_service): username="participant3", id="participant3", color="#123456", + entered_at="2025-01-01T10:00:00+00:00", ), timeout=settings.LOBBY_DENIED_TIMEOUT, ) @@ -930,6 +969,7 @@ def test_clear_participant_cache(lobby_service): "username": "test-username", "id": participant_id, "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", } cache.set(cache_key, participant_data, timeout=settings.LOBBY_WAITING_TIMEOUT) lobby_service._index_add(room_id, participant_id) @@ -1000,6 +1040,7 @@ def test_list_waiting_participants_prunes_stale_index_ids(settings, lobby_servic "username": "user1", "status": "waiting", "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", }, timeout=100, ) diff --git a/src/frontend/src/features/participants/api/listWaitingParticipants.ts b/src/frontend/src/features/participants/api/listWaitingParticipants.ts index ad1153b5..0665719c 100644 --- a/src/frontend/src/features/participants/api/listWaitingParticipants.ts +++ b/src/frontend/src/features/participants/api/listWaitingParticipants.ts @@ -8,6 +8,7 @@ export type WaitingParticipant = { status: string username: string color: string + entered_at: string } export type WaitingParticipantsResponse = { diff --git a/src/frontend/src/features/participants/hooks/useWaitingParticipants.ts b/src/frontend/src/features/participants/hooks/useWaitingParticipants.ts index 5db3d471..777a30b8 100644 --- a/src/frontend/src/features/participants/hooks/useWaitingParticipants.ts +++ b/src/frontend/src/features/participants/hooks/useWaitingParticipants.ts @@ -9,6 +9,14 @@ import { } from '../../participants/api/listWaitingParticipants' import { reportError } from '@/features/analytics/telemetry' +const toTimestamp = (participant: WaitingParticipant): number => + Date.parse(participant.entered_at) + +export const sortWaitingParticipants = ( + participants: WaitingParticipant[] +): WaitingParticipant[] => + [...participants].sort((a, b) => toTimestamp(a) - toTimestamp(b)) + export const useWaitingParticipants = () => { const roomData = useRoomData() const roomId = roomData?.id || '' // FIXME - bad practice @@ -22,7 +30,10 @@ export const useWaitingParticipants = () => { }) const waitingParticipants = useMemo( - () => (canManageLobby ? waitingData?.participants || [] : []), + () => + canManageLobby + ? sortWaitingParticipants(waitingData?.participants || []) + : [], [waitingData, canManageLobby] )