Repository navigation
Warn before approving a request that double-books the teacher - #50
Merged
Merged
Conversation
The double-booking check existed only where the teacher types a time herself. Approving a lesson request or a proposed new time went straight through — and those are exactly the two places where the time was chosen by someone else, so she is least likely to have looked at her own calendar first. Reproduced against the running app: a parent requested a lesson at an hour another student already had. The dialog said only "סטטוס: ממתין לאישור", approving it said only "הבקשה אושרה", and the database then held two Scheduled lessons for the same hour. Both approval paths now use the warning that already existed, in two layers: the clash is named in the dialog, so she can decline instead of approving, and approving anyway asks first. A free slot produces neither. Not covered, deliberately: creating a recurring series. Its occurrences are generated on the server, which skips Jewish holidays, so a client-side check would either miss occurrences or invent ones. That check belongs next to the generator.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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.
The app already warns about double-booking — but only where the teacher types a time herself.
preSaveWarningruns when she creates a lesson and when she edits its time. It does not run when she approves a request.Those are exactly the two places where the time was chosen by someone else, so they are where she is least likely to have checked her own calendar first.
Reproduced against the running app
A parent requested a lesson for נועה ברגר at 16/09 17:00 — an hour when יעל כהן already had a scheduled lesson.
And in the database:
Two students, one hour, no warning anywhere. The same held for a parent's proposed new time on an existing lesson.
The fix
Both approval paths now use the warning that already existed, in two layers:
In the dialog, so she can decline instead of approving:
On approve, so it cannot be skimmed past:
Same for a proposed new time:
And a free slot produces neither — no false positives:
The conflict lookup runs against the full lesson list (
lessons()is loaded unfiltered), so a proposed date outside the month on screen is still checked.Deliberately not covered: recurring series
saveRecurringLessonhas no conflict check either, and I left it that way. Its occurrences are generated on the server byLessonSeriesGenerator, which skips Jewish holidays and rolls forward on a horizon — so a client-side check would either miss occurrences or warn about ones that will never be created. A warning that silently checks only the first week is worse than none, because "no warning" would read as "the whole series is clear". That check belongs next to the generator, as a separate change.Testing
npx ng build— clean.npx vitest run— 30 passing.Demo data restored and verified: 12 / 12 / 15 lessons per student, 4 teachers, 0 overlapping scheduled lessons, 0 change requests. The lesson the test moved was put back to 2026-09-16 14:00:00+00.