fix: return one calendar event per lesson, not one per teacher - #105
Merged
Merged
Conversation
Aula's calendar answers with a separate lesson row for every adult attached to a lesson: same title, timeslot, room and child, but a distinct id and a different teacher in participants. Passed through as they arrive, a co-taught lesson shows up two or three times in a row, every day. merge_duplicate_lessons folds those rows back into one event, collecting the adults into new teacher_names and substitute_names lists. Rows merge only when title, timeslot, room and child all match, so two genuinely different events that merely overlap in time keep their own entries. The scalar teacher_name and substitute_name stay and point at the first adult, leaving existing callers working unchanged. Teachers are also read per row rather than one-per-role, since a single row has been seen carrying several participants in the same role. The two CLI summaries rebuilt their teacher lists from the duplicate rows, so they now read teacher_names instead and would otherwise have regressed to naming a single adult.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Aula's calendar returns a separate
lessonrow for every adult attached to alesson. The rows carry identical
title,startDateTime,endDateTime,primaryResourceandbelongsToProfiles, differing only inidand theteacher in
participants.get_calendar_eventsmapped each row straight to aCalendarEvent, so alesson with two adults — common in the younger years — came back twice:
Every consumer inherited the duplicates, all day, every day.
Change
merge_duplicate_lessons()folds those rows back into one event each,collecting the adults into new
teacher_names/substitute_nameslists.The merge key is deliberately conservative — title, timeslot, room and
child must all match. Two different subjects in the same slot, the same subject
in two periods, and a lesson shared across two children all stay separate, so
nothing real is hidden. The helper returns new events via
dataclasses.replacerather than rewriting its inputs, keeps the first row's
idand_raw, andsets
has_substitutewhen any row reported one._teacher_names()reads every participant in a role instead of just the first.Aula normally splits co-taught lessons across rows, but a single row carrying
several same-role participants has also been seen; handling both shapes means
the teacher list is complete either way.
Compatibility
teacher_nameandsubstitute_nameare unchanged and point at the firstadult, so existing callers keep working without edits. The new lists are
additive and appear in
dict(event)for JSON consumers.One follow-on was required rather than optional: both CLI summaries rebuilt
their teacher list from the duplicate rows (
dict.fromkeys(ev.teacher_name ...)).Left alone they would have regressed to naming a single adult once the rows
merged, so they now read
teacher_namesthrough a shared_unique_nameshelper — which also removes a copy-pasted comprehension.
Tests
11 model tests covering the merge and each case that must not merge, plus two
client-level tests: the real duplicated-row payload, and multiple teachers on a
single row.
813 passed, 4 skipped;ruff checkandruff formatclean.