diff --git a/src/aula/api_client.py b/src/aula/api_client.py index f72fe83..5d94776 100644 --- a/src/aula/api_client.py +++ b/src/aula/api_client.py @@ -62,6 +62,7 @@ SecureDocument, VacationRegistration, WidgetConfiguration, + merge_duplicate_lessons, ) from .utils.mapping import get_in from .widgets import AulaWidgetsClient @@ -98,6 +99,25 @@ def _compact_payload_for_log(payload: Any, *, max_chars: int = 4000) -> str: return rendered +def _teacher_names(lesson: dict[str, Any], role: str) -> list[str]: + """Return every named participant on ``lesson`` holding ``role``. + + Aula usually splits a co-taught lesson across one row per adult, which + ``merge_duplicate_lessons`` folds back together, but a single row has also + been seen carrying several participants in the same role. Reading all of + them here means both shapes end up with a complete teacher list. + """ + participants = lesson.get("participants", []) + if not isinstance(participants, list): + return [] + names = [ + p.get("teacherName") + for p in participants + if isinstance(p, dict) and p.get("participantRole") == role + ] + return list(dict.fromkeys(name for name in names if name)) + + class AulaApiClient: """Async client for Aula API endpoints. @@ -1019,6 +1039,8 @@ async def get_calendar_events( teacher_name=teacher.get("teacherName", ""), has_substitute=has_substitute, substitute_name=substitute.get("teacherName"), + teacher_names=_teacher_names(lesson, "primaryTeacher"), + substitute_names=_teacher_names(lesson, "substituteTeacher"), location=location, belongs_to=next(iter(event.get("belongsToProfiles", [])), None), _raw=event, @@ -1032,7 +1054,7 @@ async def get_calendar_events( ) continue - return events + return merge_duplicate_lessons(events) async def get_calendar_event( self, event_id: int, occurrence_datetime: str | None = None diff --git a/src/aula/cli.py b/src/aula/cli.py index 8fa6e2c..bf5c4b8 100644 --- a/src/aula/cli.py +++ b/src/aula/cli.py @@ -9,7 +9,7 @@ import platform import sys from collections import defaultdict -from collections.abc import Awaitable, Callable, Sequence +from collections.abc import Awaitable, Callable, Iterable, Sequence from zoneinfo import ZoneInfo import click @@ -356,6 +356,11 @@ def _resolve_week(week: str | None) -> str: return week +def _unique_names(name_lists: Iterable[list[str]]) -> list[str]: + """Flatten per-event name lists into one de-duplicated, ordered list.""" + return list(dict.fromkeys(name for names in name_lists for name in names if name)) + + def _on_login_required(): """Notify the user that MitID authentication is needed.""" click.echo("Session expired or not found. Please open your MitID app to approve the login.") @@ -2930,15 +2935,9 @@ async def weekly_summary(ctx, child, week, providers): locations = list( dict.fromkeys(ev.location for ev in evs if ev.location) ) - teachers = list( - dict.fromkeys(ev.teacher_name for ev in evs if ev.teacher_name) - ) - substitutes = list( - dict.fromkeys( - ev.substitute_name - for ev in evs - if ev.has_substitute and ev.substitute_name - ) + teachers = _unique_names(ev.teacher_names for ev in evs) + substitutes = _unique_names( + ev.substitute_names for ev in evs if ev.has_substitute ) parts = [time_range, titles] if locations: @@ -4352,13 +4351,9 @@ async def daily_summary(ctx, child, target_date): time_range = f"{start_dt.strftime('%H:%M')}–{end_dt.strftime('%H:%M')}" titles = " / ".join(dict.fromkeys(ev.title or "Untitled" for ev in evs)) locations = list(dict.fromkeys(ev.location for ev in evs if ev.location)) - teachers = list(dict.fromkeys(ev.teacher_name for ev in evs if ev.teacher_name)) - substitutes = list( - dict.fromkeys( - ev.substitute_name - for ev in evs - if ev.has_substitute and ev.substitute_name - ) + teachers = _unique_names(ev.teacher_names for ev in evs) + substitutes = _unique_names( + ev.substitute_names for ev in evs if ev.has_substitute ) parts = [time_range, titles] if locations: diff --git a/src/aula/models/__init__.py b/src/aula/models/__init__.py index 7167094..dd8aa0f 100644 --- a/src/aula/models/__init__.py +++ b/src/aula/models/__init__.py @@ -6,6 +6,7 @@ from .attachment import AttachmentMedia as AttachmentMedia from .auto_reply import AutoReply as AutoReply from .calendar_event import CalendarEvent as CalendarEvent +from .calendar_event import merge_duplicate_lessons as merge_duplicate_lessons from .child import Child as Child from .comment import Comment as Comment from .consent import ConsentResponse as ConsentResponse diff --git a/src/aula/models/calendar_event.py b/src/aula/models/calendar_event.py index 3a43665..1c06c88 100644 --- a/src/aula/models/calendar_event.py +++ b/src/aula/models/calendar_event.py @@ -1,4 +1,7 @@ +import dataclasses import datetime +import itertools +from collections.abc import Iterable from dataclasses import dataclass, field from .base import AulaDataClass @@ -16,3 +19,74 @@ class CalendarEvent(AulaDataClass): location: str | None belongs_to: int | None _raw: dict | None = field(default=None, repr=False) + #: Every primary teacher on the lesson, in the order Aula listed them. + #: ``teacher_name`` is the first of these, kept so existing callers and the + #: JSON output keep working; read this when a lesson can have more than one + #: adult attached, which is the common case in the younger years. + teacher_names: list[str] = field(default_factory=list) + #: Every substitute on the lesson, same relationship to ``substitute_name``. + substitute_names: list[str] = field(default_factory=list) + + +def merge_duplicate_lessons(events: list[CalendarEvent]) -> list[CalendarEvent]: + """Collapse the rows Aula returns once per adult into one event each. + + When two or more adults are attached to the same lesson, Aula's calendar + answers with one ``lesson`` row per adult: identical ``title``, + ``startDateTime``, ``endDateTime``, ``primaryResource`` and + ``belongsToProfiles``, but a distinct ``id`` and a different teacher in + ``participants``. Rendered as-is that shows the same lesson two or three + times in a row, every day. + + Rows are merged only when the lesson they describe is indistinguishable -- + same title, timeslot, room and child. Two genuinely different events that + merely overlap in time keep their own entries, so nothing real is hidden. + + The surviving event keeps the first row's ``id`` and ``_raw``; the adults + from the rows folded into it are collected into ``teacher_names`` and + ``substitute_names``. ``has_substitute`` is true when any row reported one. + A new list of new events is returned; the inputs are left untouched. + Group order follows each lesson's first appearance in ``events``. + """ + + def key(event: CalendarEvent) -> tuple: + return ( + event.title, + event.start_datetime, + event.end_datetime, + event.location, + event.belongs_to, + ) + + groups: dict[tuple, list[CalendarEvent]] = {} + for event in events: + groups.setdefault(key(event), []).append(event) + + merged = [] + for group in groups.values(): + first = group[0] + teachers = _unique( + itertools.chain.from_iterable(e.teacher_names or [e.teacher_name] for e in group) + ) + substitutes = _unique( + itertools.chain.from_iterable(e.substitute_names or [e.substitute_name] for e in group) + ) + merged.append( + dataclasses.replace( + first, + # Keep the scalars pointing at the first adult so callers that + # only know about them read a real name, not an arbitrary one. + teacher_name=teachers[0] if teachers else first.teacher_name, + substitute_name=substitutes[0] if substitutes else None, + has_substitute=any(e.has_substitute for e in group), + teacher_names=teachers, + substitute_names=substitutes, + ) + ) + + return merged + + +def _unique(names: Iterable[str | None]) -> list[str]: + """Return the non-empty names, de-duplicated, in first-seen order.""" + return list(dict.fromkeys(name for name in names if name)) diff --git a/tests/models/test_calendar_event.py b/tests/models/test_calendar_event.py index af64881..9e57978 100644 --- a/tests/models/test_calendar_event.py +++ b/tests/models/test_calendar_event.py @@ -2,7 +2,7 @@ import datetime -from aula.models.calendar_event import CalendarEvent +from aula.models.calendar_event import CalendarEvent, merge_duplicate_lessons def test_calendar_event_creation(): @@ -76,3 +76,159 @@ def test_calendar_event_raw_preserved(): ) assert event._raw is raw assert "_raw" not in dict(event) + + +def _event( + event_id: int, + title: str = "DFA 2", + teacher: str | None = None, + *, + start: tuple[int, int] = (8, 15), + end: tuple[int, int] = (9, 0), + location: str | None = "Room 101", + belongs_to: int | None = 100, + has_substitute: bool = False, + substitute: str | None = None, +) -> CalendarEvent: + """Build a lesson row, defaulting to the shape Aula duplicates per adult.""" + return CalendarEvent( + id=event_id, + title=title, + start_datetime=datetime.datetime(2026, 9, 14, *start), + end_datetime=datetime.datetime(2026, 9, 14, *end), + teacher_name=teacher, + has_substitute=has_substitute, + substitute_name=substitute, + location=location, + belongs_to=belongs_to, + teacher_names=[teacher] if teacher else [], + substitute_names=[substitute] if substitute else [], + ) + + +def test_merge_collapses_one_row_per_teacher(): + """Aula's per-adult rows for one lesson become a single event.""" + merged = merge_duplicate_lessons( + [ + _event(1, teacher="Lærer Nummer 1"), + _event(2, teacher="Lærer Nummer 2"), + ] + ) + + assert len(merged) == 1 + assert merged[0].teacher_names == ["Lærer Nummer 1", "Lærer Nummer 2"] + # The scalar keeps pointing at the first adult for existing callers. + assert merged[0].teacher_name == "Lærer Nummer 1" + assert merged[0].id == 1 + + +def test_merge_keeps_distinct_lessons_in_the_same_slot(): + """Two different subjects at the same time are not one lesson.""" + merged = merge_duplicate_lessons( + [ + _event(1, title="DFA 2", teacher="A"), + _event(2, title="Idræt", teacher="B"), + ] + ) + + assert [e.title for e in merged] == ["DFA 2", "Idræt"] + + +def test_merge_keeps_lessons_in_different_rooms(): + """Same title and slot but a different room stays two events.""" + merged = merge_duplicate_lessons( + [ + _event(1, teacher="A", location="Room 101"), + _event(2, teacher="B", location="Room 102"), + ] + ) + + assert len(merged) == 2 + + +def test_merge_keeps_lessons_for_different_children(): + """A shared lesson is not collapsed across two children.""" + merged = merge_duplicate_lessons( + [ + _event(1, teacher="A", belongs_to=100), + _event(2, teacher="A", belongs_to=200), + ] + ) + + assert [e.belongs_to for e in merged] == [100, 200] + + +def test_merge_keeps_separate_timeslots(): + """The same subject in two periods stays two events.""" + merged = merge_duplicate_lessons( + [ + _event(1, teacher="A", start=(8, 15), end=(9, 0)), + _event(2, teacher="A", start=(9, 30), end=(10, 15)), + ] + ) + + assert len(merged) == 2 + + +def test_merge_reports_a_substitute_from_any_row(): + """One row flagging a substitute makes the merged lesson a substitute one.""" + merged = merge_duplicate_lessons( + [ + _event(1, teacher="A"), + _event(2, teacher="B", has_substitute=True, substitute="Vikar"), + ] + ) + + assert len(merged) == 1 + assert merged[0].has_substitute is True + assert merged[0].substitute_names == ["Vikar"] + assert merged[0].substitute_name == "Vikar" + + +def test_merge_deduplicates_repeated_teacher_names(): + """The same adult listed on both rows is named once.""" + merged = merge_duplicate_lessons([_event(1, teacher="A"), _event(2, teacher="A")]) + + assert merged[0].teacher_names == ["A"] + + +def test_merge_leaves_inputs_untouched(): + """Merging returns new events rather than rewriting the ones passed in.""" + rows = [_event(1, teacher="A"), _event(2, teacher="B")] + merge_duplicate_lessons(rows) + + assert rows[0].teacher_names == ["A"] + assert rows[1].teacher_names == ["B"] + + +def test_merge_normalises_an_untouched_single_event(): + """A lone row keeps its own values and is returned as-is.""" + merged = merge_duplicate_lessons([_event(1, teacher="A")]) + + assert len(merged) == 1 + assert merged[0].teacher_names == ["A"] + assert merged[0].has_substitute is False + + +def test_merge_preserves_first_appearance_order(): + """Groups come back in the order their first row appeared.""" + merged = merge_duplicate_lessons( + [ + _event(1, title="B-fag", teacher="X"), + _event(2, title="A-fag", teacher="Y"), + _event(3, title="B-fag", teacher="Z"), + ] + ) + + assert [e.title for e in merged] == ["B-fag", "A-fag"] + assert merged[0].teacher_names == ["X", "Z"] + + +def test_merged_event_serialises_teacher_lists(): + """``dict(event)`` carries the new lists for JSON consumers.""" + merged = merge_duplicate_lessons([_event(1, teacher="A"), _event(2, teacher="B")]) + + result = dict(merged[0]) + assert result["teacher_names"] == ["A", "B"] + assert result["substitute_names"] == [] + assert "_raw" not in result diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 9ca9779..0915b93 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -1356,6 +1356,76 @@ async def test_non_list_data_returns_empty(self, client): events = await client.get_calendar_events([100], datetime(2026, 3, 1), datetime(2026, 3, 1)) assert events == [] + @pytest.mark.asyncio + async def test_co_taught_lesson_returned_once(self, client): + """Aula's one-row-per-adult duplicates collapse to one event.""" + raw_events = [ + { + "id": "3120da43", + "title": "DFA 2", + "startDateTime": "2026-09-14T08:15:00+01:00", + "endDateTime": "2026-09-14T09:00:00+01:00", + "belongsToProfiles": [100], + "primaryResource": {"name": "Room 101"}, + "lesson": { + "participants": [ + {"participantRole": "primaryTeacher", "teacherName": "Laerer 1"}, + ], + }, + }, + { + "id": "eb86b3c9", + "title": "DFA 2", + "startDateTime": "2026-09-14T08:15:00+01:00", + "endDateTime": "2026-09-14T09:00:00+01:00", + "belongsToProfiles": [100], + "primaryResource": {"name": "Room 101"}, + "lesson": { + "participants": [ + {"participantRole": "primaryTeacher", "teacherName": "Laerer 2"}, + ], + }, + }, + ] + client._request_with_version_retry = AsyncMock( + return_value=HttpResponse(status_code=200, data={"data": raw_events}) + ) + + events = await client.get_calendar_events( + [100], datetime(2026, 9, 14), datetime(2026, 9, 14) + ) + + assert len(events) == 1 + assert events[0].teacher_names == ["Laerer 1", "Laerer 2"] + + @pytest.mark.asyncio + async def test_multiple_teachers_on_one_row(self, client): + """Several primary teachers inside a single row are all read.""" + raw_events = [ + { + "id": 1, + "title": "Math", + "startDateTime": "2026-03-01T08:00:00+01:00", + "endDateTime": "2026-03-01T09:00:00+01:00", + "belongsToProfiles": [100], + "lesson": { + "participants": [ + {"participantRole": "primaryTeacher", "teacherName": "Mrs. Jensen"}, + {"participantRole": "primaryTeacher", "teacherName": "Mr. Poulsen"}, + ], + }, + } + ] + client._request_with_version_retry = AsyncMock( + return_value=HttpResponse(status_code=200, data={"data": raw_events}) + ) + + events = await client.get_calendar_events([100], datetime(2026, 3, 1), datetime(2026, 3, 1)) + + assert len(events) == 1 + assert events[0].teacher_names == ["Mrs. Jensen", "Mr. Poulsen"] + assert events[0].teacher_name == "Mrs. Jensen" + class TestGetPosts: """Tests for AulaApiClient.get_posts method."""