diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a90a08..c5577b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,14 @@ each change under the current top section as you work. ## [1.4.1] ### Added -- **Group classes now appear in "Your upcoming lessons".** A class is stored as a term rather than as bookable slots, so nothing that listed lessons could ever show one — a student whose whole term was a group class saw an empty schedule, and an instructor teaching one saw nothing on their My Lessons page. Each remaining session of a class you are enrolled in now sorts in among your lessons by date, labelled **group class**; instructors see every session of the classes they teach, one row per session however many students are in it. A session has no Cancel button, because there is no such thing as cancelling one date of a term — withdrawing from the class is still done from the class page. A class whose date, time or length has not been filled in yields no sessions and is left out rather than shown at a time nobody chose. +- **Group classes now appear in "Your upcoming lessons".** A class is stored as a term rather than as bookable slots, so nothing that listed lessons could ever show one — a student whose whole term was a group class saw an empty schedule, and an instructor teaching one saw nothing on their My Lessons page. Each remaining session of a class you are enrolled in now sorts in among your lessons by date, labelled **group class**; instructors see every session of the classes they teach, one row per session however many students are in it. A session has no Cancel button, because there is no such thing as cancelling one date of a term — withdrawing from the class is still done from the class page. +- A class you are enrolled in shows up **whether or not its schedule is pinned to a clock**. Class time and duration are both optional on the offering form, and the schedule note is there so a studio can simply write "Tuesdays 4:00pm" — so a class with a time but no duration lists its dates and says when each session starts rather than guessing when it ends, and a class with no time at all gets a single row carrying its schedule note (or its term dates) where the time would go. Only a class whose last day has passed drops off the list. - The same group-class sessions now appear in **Upcoming lessons** on a student's admin detail page, so one table answers "what are they booked into next week?". Only upcoming ones — the **Group-class enrolments** table below already holds the history. - **A policy can be renamed.** The title was fixed at creation, so a typo or a change of wording meant creating a second policy and re-collecting everyone's acceptance. Renaming changes only what students read above the policy text: the slug stays put, so every version already accepted stays attached. ### Fixed +- **Deleting a parent now removes the students they booked for.** A managed student account has no login of its own and exists only so its parent has somebody to book for — with the parent gone nobody can reach it, book for it, or be billed for it, so it was left stranded on the roster still holding lesson times. Deleting a parent now releases each of their students' upcoming lessons and enrolments on the same terms as their own, and deletes the accounts. Removing a student from the family screen is unchanged and still refuses one with lessons on record. +- **The upcoming-lessons panel no longer collapses onto itself in some themes.** Rows could render on top of one another and the status badge's colour could stop short of the text inside it. Both came from the same thing: the panel never stated its own line spacing, so a theme setting a line height of zero anywhere above it — a common icon-font reset — was inherited straight through, leaving each line of text taller than the space allotted to it. The panel now sets its own. - **Deleting a student now gives back what they had booked.** WordPress deletes a user without knowing anything about lessons, so their bookings were left behind: the times stayed marked as booked and nobody else could take them, the lessons stayed on the instructor's schedule under a name that no longer resolved, and a group class kept a seat filled by nobody. Deleting an account now cancels each of its upcoming lessons, frees the time for rebooking, cancels its active class enrolments, and voids any payment still pending on them. Past lessons are left exactly as they are — they happened, and the payment report has to keep adding up. A paid lesson is not credited back: a credit could only be spent on the account being deleted, so a refund owed to someone who has left stays the studio's decision to make. - **A weak password is now caught before the form is submitted, not after.** The strength meter scores the password as you type, but zxcvbn's dictionary arrives a moment after the page loads — so a password typed straight away was never scored at all, and the first you heard of it was the server rejecting the whole form. The password is now re-scored on submit, so the verdict is always the one your password actually earns. diff --git a/assets/css/frontend.css b/assets/css/frontend.css index 6f10988..73da434 100644 --- a/assets/css/frontend.css +++ b/assets/css/frontend.css @@ -104,18 +104,41 @@ * explicit flex rules above, but the text itself still sits in inline elements * a theme is free to take out of normal flow — an absolutely positioned, * floated or negatively offset span drops the date/time on top of the title and - * the status pill on top of the Cancel button. Pinning the three properties - * that would have to change keeps the leaves in flow, at the same id-level + * the status pill on top of the Cancel button. Pinning the properties that + * would have to change keeps the leaves in flow, at the same id-level * specificity the rules above rely on. + * + * `line-height` is pinned for the same reason and is the subtler one, because a + * theme does not have to target this panel to break it — it is inherited, so a + * `line-height: 0` anywhere above (the usual icon-font or sprite reset) reaches + * these elements untouched. Below 1 it produces both halves of the same bug: a + * line box shorter than its glyphs, so stacked lines in the details column + * overlap, and an inline-block pill whose background is shorter than the text + * sitting in it. Nothing here should ever inherit a line-height, so the panel + * states its own. */ +#us-booking-app .us-my-lesson, +#us-booking-app .us-my-lesson-info, +#us-booking-app .us-my-lesson-actions, #us-booking-app .us-my-lesson-title, #us-booking-app .us-my-lesson-when, #us-booking-app .us-my-lesson-duration, #us-booking-app .us-my-lesson-who, +#us-booking-app .us-my-lesson-kind, +#us-booking-app .us-lesson-status { + line-height: 1.45; +} + +#us-booking-app .us-my-lesson-title, +#us-booking-app .us-my-lesson-when, +#us-booking-app .us-my-lesson-duration, +#us-booking-app .us-my-lesson-who, +#us-booking-app .us-my-lesson-kind, #us-booking-app .us-lesson-status { position: static; float: none; margin: 0; + vertical-align: middle; } /* diff --git a/assets/js/booking.js b/assets/js/booking.js index 299fa45..6c704b5 100644 --- a/assets/js/booking.js +++ b/assets/js/booking.js @@ -613,6 +613,21 @@ return l.kind === 'group_class'; } + // When a row meets. A class with no class time set has no clock to put it on, + // so it carries `schedule` — the studio's own wording, or its term dates — and + // that is shown verbatim in place of a date and time. A dated row with no + // duration knows when it starts but not when it ends, and says only that + // rather than inventing a finish. + function lessonWhenHtml(l) { + if (l.schedule) { + return escHtml(String(l.schedule)); + } + + const when = `${escHtml(dayLabel(dayKey(l.start_dt)))} · ${escHtml(timeOf(l.start_dt))}`; + + return l.end_dt ? `${when}–${escHtml(timeOf(l.end_dt))}` : when; + } + function lessonRowHtml(l) { const group = isGroupSession(l); const title = l.offering_title ? escHtml(String(l.offering_title)) : (group ? 'Group class' : 'Lesson'); @@ -626,7 +641,7 @@
${title}${duration}${badge}${lessonWhoHtml(l)} - ${escHtml(dayLabel(dayKey(l.start_dt)))} · ${escHtml(timeOf(l.start_dt))}–${escHtml(timeOf(l.end_dt))} + ${lessonWhenHtml(l)}
${escHtml(lessonStatusLabel(String(l.status)))} diff --git a/docs/features/group-classes.md b/docs/features/group-classes.md index e8f5b30..cccfdfd 100644 --- a/docs/features/group-classes.md +++ b/docs/features/group-classes.md @@ -47,9 +47,27 @@ the class meets. one row per session however many students are enrolled; enrolments are not consulted, because a class still has to be taught if nobody has signed up yet. -A class whose schedule is not fully specified (no time, or no duration) yields no -windows and so contributes no rows — better absent from a dated list than shown at -a time nobody chose. +**A class you are enrolled in must never silently vanish from these lists.** Both +the class time and the duration are optional on the offering form, and the +schedule note exists precisely so a studio can write "Tuesdays 4:00pm" rather than +pin the class to a clock. So the schedule degrades instead of disappearing: + +| Class has | What the list gets | +|---|---| +| date + time + duration | one dated row per remaining session, with an end time | +| date + time, no duration | one dated row per remaining session, `end_dt` empty — when it starts is worth showing without guessing when it ends | +| no class time | **one** row for the class as a whole, sorted by term start (or by "now" once the term is under way), with `schedule` text from `Offering::scheduleLabel()` — the studio's note, else the term dates, else "Schedule to be confirmed" | +| a term whose last day has passed | nothing | + +`schedule` is the tell: non-null means "a class, described in words, not a session +at a known time", and every renderer shows that text in place of a date and time. +An undated row's `start_dt` is a **sort key only** — never displayed. + +`Offering::sessionStarts()` is the split that makes this work: it needs only the +date and the time, because knowing *when* a class meets is a separate question +from knowing how long it runs. `sessionWindows()` is that plus the duration, and +still returns nothing without one — availability blocking and per-session billing +need both ends of a window. Consumers mark these rows `kind = 'group_class'` (`SessionSchedule::KIND`) and withhold the per-lesson actions from them: a session is one date in a term, not a diff --git a/docs/features/student-administration.md b/docs/features/student-administration.md index fa01b86..f2b2bf1 100644 --- a/docs/features/student-administration.md +++ b/docs/features/student-administration.md @@ -69,6 +69,19 @@ pending payment voided. Without it the slots stayed marked booked and unbookable by anyone else, the lessons stayed on the instructor's schedule under a name that no longer resolved, and a class kept a seat filled by nobody. +**A guardian takes their children with them.** A child account is login-less and +exists only so the guardian has somebody to book for; without the guardian nobody +can reach it, book for it, or be billed for it, so leaving it behind leaves an +unreachable student on the roster holding slots that will never be used. Each +child's bookings are released on the same terms, the `us_guardians` link row is +deleted, and the account goes. Deleting a child fires `delete_user` again and +re-enters the same handler; a `handled` set of user ids makes that a no-op and +also stops a self-referential or circular link recursing. + +(This is a different rule from the family screen's **Remove**, which still refuses +a child with any lesson or enrolment history — that is a guardian tidying up, not +an admin deleting an account, and `GuardianService::removeChild()` is unchanged.) + Past lessons are deliberately untouched: they happened, they may have been paid for, and the payment report has to keep adding up. No account credit is issued for a paid lesson either, unlike a cancellation the student asks for — a credit diff --git a/src/Auth/DeletedUserCleanup.php b/src/Auth/DeletedUserCleanup.php index b12e833..737b939 100644 --- a/src/Auth/DeletedUserCleanup.php +++ b/src/Auth/DeletedUserCleanup.php @@ -8,10 +8,13 @@ use Unsupervised\Schedular\Booking\BookingRepository; use Unsupervised\Schedular\Booking\Lesson; use Unsupervised\Schedular\GroupClass\Enrollment; use Unsupervised\Schedular\GroupClass\EnrollmentRepository; +use Unsupervised\Schedular\Guardian\GuardianRepository; +use Unsupervised\Schedular\Guardian\GuardianService; use Unsupervised\Schedular\Payment\PaymentService; /** - * Gives back what a deleted account was holding. + * Gives back what a deleted account was holding, and takes the accounts that + * only existed underneath it with it. * * WordPress deletes a user without knowing anything about lessons, so a student * removed from **Users → Delete** used to leave their bookings behind: the @@ -24,6 +27,13 @@ use Unsupervised\Schedular\Payment\PaymentService; * lessons are deliberately left alone: they happened, they may have been paid * for, and the payment report has to keep adding up. * + * A **guardian** takes their children with them. A child account is login-less + * and exists only so the guardian has somebody to book for; without the + * guardian nobody can reach it, book for it, or be billed for it, so leaving it + * behind leaves an unreachable student on the roster holding slots that will + * never be used. Each child's bookings are released on the same terms, the link + * row goes, and the account is deleted. + * * No account credit is issued for a paid lesson, unlike a cancellation the * student asks for. A credit only has value against future billing on the * account it belongs to, and that account is being deleted; a refund owed to @@ -32,38 +42,62 @@ use Unsupervised\Schedular\Payment\PaymentService; */ class DeletedUserCleanup { + /** + * Accounts already dealt with this request, so deleting a guardian's child + * — which fires `delete_user` again and re-enters this very handler — cannot + * loop or redo work. It also makes a self-referential or circular guardian + * link, however it got into the table, terminate rather than recurse. + * + * @var array + */ + private array $handled = []; + public function __construct( private BookingRepository $bookings, private AvailabilityRepository $availability, private EnrollmentRepository $enrollments, private PaymentService $payments, + private GuardianRepository $links, + private GuardianService $guardians, ) {} public function register(): void { // `delete_user` fires before the row goes, which is what lets the lookups - // below still find the account's bookings. `wpmu_delete_user` is the - // multisite equivalent for a user removed from the network entirely. + // below still find the account's bookings and children. `wpmu_delete_user` + // is the multisite equivalent for a user removed from the network entirely. add_action( 'delete_user', [ $this, 'releaseBookings' ] ); add_action( 'wpmu_delete_user', [ $this, 'releaseBookings' ] ); } /** - * Cancel and release everything the account had booked ahead of it. + * Release everything the account had booked ahead of it, then remove any + * children that only existed to be booked for. */ public function releaseBookings( int $userId ): void { - if ( $userId <= 0 ) { + if ( $userId <= 0 || isset( $this->handled[ $userId ] ) ) { return; } + $this->handled[ $userId ] = true; + + $this->release( $userId ); + $this->removeChildren( $userId ); + } + + /** + * Cancel one account's upcoming lessons and active enrolments, freeing the + * slot and voiding the pending payment behind each. + */ + private function release( int $studentId ): void { // Upcoming and not already cancelled — the only bookings that are still // holding anything. - foreach ( $this->bookings->findUpcomingForStudent( $userId ) as $lesson ) { + foreach ( $this->bookings->findUpcomingForStudent( $studentId ) as $lesson ) { $this->bookings->updateStatus( (int) $lesson->id, Lesson::STATUS_CANCELLED ); $this->availability->release( $lesson->slotId ); $this->payments->voidPending( $lesson->paymentId ); } - foreach ( $this->enrollments->findByStudent( $userId ) as $enrollment ) { + foreach ( $this->enrollments->findByStudent( $studentId ) as $enrollment ) { if ( Enrollment::STATUS_ACTIVE !== $enrollment->status ) { continue; } @@ -72,4 +106,25 @@ class DeletedUserCleanup { $this->payments->voidPending( $enrollment->paymentId ); } } + + /** + * Delete every child linked to a departing guardian, releasing what each was + * holding first. Each child is marked handled *before* it is deleted, so the + * `delete_user` this fires re-enters and returns without redoing the release. + */ + private function removeChildren( int $guardianId ): void { + foreach ( $this->links->findByGuardian( $guardianId ) as $link ) { + $childId = $link->studentId; + + if ( $childId <= 0 || $childId === $guardianId || isset( $this->handled[ $childId ] ) ) { + continue; + } + + $this->handled[ $childId ] = true; + + $this->release( $childId ); + $this->links->delete( $guardianId, $childId ); + $this->guardians->deleteUser( $childId ); + } + } } diff --git a/src/Auth/StudentController.php b/src/Auth/StudentController.php index 2db29b0..0bf02e9 100644 --- a/src/Auth/StudentController.php +++ b/src/Auth/StudentController.php @@ -205,6 +205,7 @@ class StudentController { return [ 'id' => (int) $lesson->id, 'kind' => 'lesson', + 'schedule' => null, 'start_dt' => $slot ? $slot->startDt : '', 'end_dt' => $slot ? $slot->endDt : '', 'offering' => $offering ? $offering->title : '—', @@ -231,6 +232,9 @@ class StudentController { 'kind' => SessionSchedule::KIND, 'start_dt' => $session['start_dt'], 'end_dt' => $session['end_dt'], + // Set when the class has no time to put on a clock; shown in the + // When column in place of a date. See GroupClass\SessionSchedule. + 'schedule' => $session['schedule'], 'offering' => $session['offering_title'], 'instructor' => $instructor ? $instructor->display_name : (string) $session['instructor_id'], 'status' => $session['status'], diff --git a/src/GroupClass/SessionSchedule.php b/src/GroupClass/SessionSchedule.php index d96a59e..f738ff0 100644 --- a/src/GroupClass/SessionSchedule.php +++ b/src/GroupClass/SessionSchedule.php @@ -13,13 +13,25 @@ use Unsupervised\Schedular\Offering\OfferingRepository; * A group class is stored as a term (`term_start`, `term_end`, `class_time`) * rather than as rows in `us_availability`, which is why an enrolment on its own * has no date on it and why nothing that listed lessons ever showed one. The - * concrete windows come from {@see Offering::sessionWindows()} — the same - * derivation the billing scan and the class-slot reconciler use, so a student's - * list, an instructor's list and the invoice all agree on when the class meets. + * dates come from {@see Offering::sessionStarts()} — the same derivation the + * billing scan and the class-slot reconciler build on, so a student's list, an + * instructor's list and the invoice all agree on when the class meets. * - * A class whose schedule is not fully specified yields no windows and so - * contributes no rows: better to leave it out of a dated list than to invent a - * time for it. + * **A class you are enrolled in must never silently vanish from the list.** Both + * the class time and the duration are optional on the offering form, and the + * schedule note exists precisely so a studio can write "Tuesdays 4:00pm" instead + * of pinning the class to a clock. So the schedule degrades rather than + * disappearing: + * + * - date **and** time set — one dated row per remaining session, closed off with + * the duration when there is one and left open-ended when there is not; + * - no time to derive dates from — a single row for the class as a whole, sorted + * by when the term starts and labelled with `schedule` text + * ({@see Offering::scheduleLabel()}) in place of a time. + * + * A row's `schedule` is the tell: non-null means "this is a class, described in + * words, not a session at a known time", and every renderer shows that text + * instead of a date and time. */ class SessionSchedule { @@ -42,7 +54,7 @@ class SessionSchedule { * kept, since "completed" describes the enrolment's billing state and says * nothing about whether the class has met yet. * - * @return list + * @return list */ public function upcomingForStudent( int $studentId, string $now ): array { $rows = []; @@ -57,18 +69,10 @@ class SessionSchedule { continue; } - foreach ( $this->windowsFrom( $offering, $now ) as $window ) { - $rows[] = [ - 'enrollment_id' => (int) $enrollment->id, - 'offering_id' => (int) $offering->id, - 'offering_title' => $offering->title, - 'instructor_id' => $enrollment->instructorId, - 'status' => $enrollment->status, - 'start_dt' => $window['start'], - 'end_dt' => $window['end'], - 'duration_minutes' => $offering->durationMinutes, - ]; - } + $rows = array_merge( + $rows, + $this->rowsFor( $offering, $now, (int) $enrollment->id, $enrollment->instructorId, $enrollment->status ) + ); } return self::sortedByStart( $rows ); @@ -80,7 +84,7 @@ class SessionSchedule { * are not consulted at all: a class the instructor has to turn up and teach * belongs on their schedule whether or not anyone has signed up yet. * - * @return list + * @return list */ public function upcomingForInstructor( int $instructorId, string $now ): array { $rows = []; @@ -88,43 +92,119 @@ class SessionSchedule { $classes = $this->offerings->findAll( $instructorId, Offering::KIND_GROUP_CLASS, activeOnly: true ); foreach ( $classes as $offering ) { - foreach ( $this->windowsFrom( $offering, $now ) as $window ) { - $rows[] = [ - 'enrollment_id' => 0, - 'offering_id' => (int) $offering->id, - 'offering_title' => $offering->title, - 'instructor_id' => $instructorId, - 'status' => Enrollment::STATUS_ACTIVE, - 'start_dt' => $window['start'], - 'end_dt' => $window['end'], - 'duration_minutes' => $offering->durationMinutes, - ]; - } + $rows = array_merge( $rows, $this->rowsFor( $offering, $now, 0, $instructorId, Enrollment::STATUS_ACTIVE ) ); } return self::sortedByStart( $rows ); } /** - * The class's session windows that have not started yet. + * One class's contribution to an upcoming list: its remaining dated sessions, + * or — when it has no time to derive dates from — a single row describing the + * class in words. Empty only when the class has demonstrably finished. * - * @return list + * @return list */ - private function windowsFrom( Offering $offering, string $now ): array { - return array_values( - array_filter( - $offering->sessionWindows(), - static fn( array $window ): bool => $window['start'] >= $now - ) - ); + private function rowsFor( Offering $offering, string $now, int $enrollmentId, int $instructorId, string $status ): array { + $base = [ + 'enrollment_id' => $enrollmentId, + 'offering_id' => (int) $offering->id, + 'offering_title' => $offering->title, + 'instructor_id' => $instructorId, + 'status' => $status, + 'duration_minutes' => $offering->durationMinutes, + ]; + + $starts = $offering->sessionStarts(); + + // Dated: the class says exactly when it meets, so list what is left of it + // — and nothing at all once the term is over. + if ( [] !== $starts ) { + $rows = []; + + foreach ( $starts as $start ) { + if ( $start < $now ) { + continue; + } + + $rows[] = $base + [ + 'start_dt' => $start, + // Left open when no duration is set. Knowing a class starts at + // four o'clock is worth showing even without knowing when it + // ends; guessing an end time is not. + 'end_dt' => $this->endOf( $offering, $start ), + 'schedule' => null, + ]; + } + + return $rows; + } + + // Undated: no class time, so there is nothing to put on a clock. The class + // still gets a row — it is enrolled in and running — described by the + // studio's own schedule note or its term dates. + if ( ! $this->isStillRunning( $offering, $now ) ) { + return []; + } + + return [ + $base + [ + // A sort key, not a claim about when the class meets: a class yet to + // start sorts to its first day, one already under way to right now. + // `schedule` is what any renderer actually shows. + 'start_dt' => $this->sortKeyFor( $offering, $now ), + 'end_dt' => '', + 'schedule' => $offering->scheduleLabel(), + ], + ]; + } + + /** + * When a session that starts at `$start` finishes, or an empty string when the + * class has no duration to close it off with. + */ + private function endOf( Offering $offering, string $start ): string { + if ( null === $offering->durationMinutes || $offering->durationMinutes <= 0 ) { + return ''; + } + + return ( new \DateTimeImmutable( $start ) ) + ->add( new \DateInterval( 'PT' . $offering->durationMinutes . 'M' ) ) + ->format( 'Y-m-d H:i:s' ); + } + + /** + * Whether an undated class still has life in it: its last day has not passed, + * or it has no dates at all (in which case nothing says it has ended, and + * dropping it would be the very disappearance this class exists to prevent). + */ + private function isStillRunning( Offering $offering, string $now ): bool { + $lastDay = $offering->lastClassDay(); + + return null === $lastDay || $lastDay >= substr( $now, 0, 10 ); + } + + /** + * Where an undated class sits in a list ordered by time: at its first day when + * that is still ahead, otherwise at `$now`, so a term already under way reads + * as current rather than as ancient history. + */ + private function sortKeyFor( Offering $offering, string $now ): string { + if ( null === $offering->termStart ) { + return $now; + } + + $firstDay = $offering->termStart . ' 00:00:00'; + + return $firstDay > $now ? $firstDay : $now; } /** * Soonest session first, so classes from separate enrolments interleave by * date rather than arriving grouped by class. * - * @param list $rows - * @return list + * @param list $rows + * @return list */ private static function sortedByStart( array $rows ): array { usort( $rows, static fn( array $a, array $b ): int => strcmp( $a['start_dt'], $b['start_dt'] ) ); diff --git a/src/Offering/Offering.php b/src/Offering/Offering.php index 153df26..85fc15a 100644 --- a/src/Offering/Offering.php +++ b/src/Offering/Offering.php @@ -175,22 +175,19 @@ class Offering { } /** - * The concrete start/end datetimes of every session of this group class, - * derived from the class date(s), the class time, and the duration. A weekly - * class yields one window per week from `term_start` through `term_end`; a - * one-off class yields a single window. Returns an empty list unless the - * schedule is fully specified (date, time, and a positive duration), so it can - * never fabricate a session window from partial data. + * The datetime each session of this group class starts, derived from the class + * date(s) and the class time. A weekly class yields one per week from + * `term_start` through `term_end`; a one-off class yields a single one. * - * @return list + * Deliberately does **not** need a duration: knowing *when* a class meets is a + * separate question from knowing how long it runs, and a studio can quite + * reasonably set the first without the second. Returns an empty list when + * there is no date or no time, since neither can be invented. + * + * @return list `Y-m-d H:i:s` starts, earliest first. */ - public function sessionWindows(): array { - if ( - null === $this->termStart - || null === $this->classTime - || null === $this->durationMinutes - || $this->durationMinutes <= 0 - ) { + public function sessionStarts(): array { + if ( null === $this->termStart || null === $this->classTime ) { return []; } @@ -200,25 +197,85 @@ class Offering { } $lastDay = null !== $this->termEnd ? $this->termEnd : $this->termStart; - $step = new \DateInterval( 'PT' . $this->durationMinutes . 'M' ); - $windows = []; + $starts = []; $cursor = $first; $cursorDay = $cursor->format( 'Y-m-d' ); // Cap the walk at ten years of weeks so a term_end before term_start (or a // bad value) can never spin into an unbounded loop. for ( $i = 0; $i < 520 && $cursorDay <= $lastDay; $i++ ) { - $windows[] = [ - 'start' => $cursor->format( 'Y-m-d H:i:s' ), - 'end' => $cursor->add( $step )->format( 'Y-m-d H:i:s' ), - ]; + $starts[] = $cursor->format( 'Y-m-d H:i:s' ); $cursor = $cursor->modify( '+7 days' ); $cursorDay = $cursor->format( 'Y-m-d' ); } - return $windows; + return $starts; + } + + /** + * The concrete start/end datetimes of every session of this group class: + * {@see sessionStarts()} closed off with the class duration. Returns an empty + * list unless the schedule is fully specified (date, time, *and* a positive + * duration), so it can never fabricate a session window from partial data — + * callers that block availability or bill per session need both ends. + * + * @return list + */ + public function sessionWindows(): array { + if ( null === $this->durationMinutes || $this->durationMinutes <= 0 ) { + return []; + } + + $step = new \DateInterval( 'PT' . $this->durationMinutes . 'M' ); + + return array_map( + static fn( string $start ): array => [ + 'start' => $start, + 'end' => ( new \DateTimeImmutable( $start ) )->add( $step )->format( 'Y-m-d H:i:s' ), + ], + $this->sessionStarts() + ); + } + + /** + * The last day this class meets, or null when it has no dates at all. + */ + public function lastClassDay(): ?string { + return $this->termEnd ?? $this->termStart; + } + + /** + * Plain-language wording for when this class meets, for the places that have + * to say something about a class whose schedule cannot be resolved to dates. + * Prefers the studio's own note ("Tuesdays 4:00pm") — that field exists + * precisely so a class can describe its schedule without pinning it to a + * time — then the term dates, and finally an honest admission that nothing + * has been set. + */ + public function scheduleLabel(): string { + $note = null !== $this->scheduleNote ? trim( $this->scheduleNote ) : ''; + if ( '' !== $note ) { + return $note; + } + + if ( null === $this->termStart ) { + return __( 'Schedule to be confirmed', 'unsupervised-schedular' ); + } + + $start = (string) mysql2date( 'M j, Y', $this->termStart ); + + if ( null === $this->termEnd || $this->termEnd === $this->termStart ) { + return $start; + } + + return sprintf( + /* translators: 1: first class date, 2: last class date. */ + __( '%1$s – %2$s', 'unsupervised-schedular' ), + $start, + (string) mysql2date( 'M j, Y', $this->termEnd ) + ); } public static function fromRow( \stdClass $row ): self { diff --git a/src/Plugin.php b/src/Plugin.php index 5efacb6..9b82f3e 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -110,7 +110,7 @@ class Plugin { ( new RegistrationLoginGate() )->register(); ( new ChildLoginGate() )->register(); ( new StudentAdminGuard() )->register(); - ( new DeletedUserCleanup( $bookings, $availability, $enrollments, $paymentService ) )->register(); + ( new DeletedUserCleanup( $bookings, $availability, $enrollments, $paymentService, $guardianRepo, $guardians ) )->register(); ( new EmailConfirmationHandler( $settings, $registrationMailer ) )->register(); ( new AdminMenu( $availability, $bookings, $offerings, $questions, $answers, $policies, $policyVersions, $policyService, $acceptances, $invites, $enrollments, $groupAccess, $settings, $paymentRepo, $paymentService, $resolver, $registrationMailer, $creditRepo, $guardians ) )->register(); ( new RestRegistrar( $availability, $bookings, $offerings, $questions, $policies, $policyVersions, $policyService, $registrationGate, $enrollments, $groupAccess, $paymentService, $guardians ) )->register(); diff --git a/templates/admin/student-detail.php b/templates/admin/student-detail.php index f1869c2..24a21df 100644 --- a/templates/admin/student-detail.php +++ b/templates/admin/student-detail.php @@ -9,8 +9,8 @@ use Unsupervised\Schedular\GroupClass\SessionSchedule; /** * @var \WP_User $student - * @var list $upcoming Booked lessons and upcoming group-class sessions, soonest first. - * @var list $past + * @var list $upcoming Booked lessons and upcoming group-class sessions, soonest first. `schedule` is wording to show instead of a date, for a class with no class time set. + * @var list $past * @var list $enrolments * @var list $acceptances * @var list $registrationInfo @@ -53,7 +53,18 @@ $renderLessons = static function (array $rows, bool $withActions = false): void - + + + diff --git a/tests/Unit/Auth/DeletedUserCleanupTest.php b/tests/Unit/Auth/DeletedUserCleanupTest.php index cf6e6ce..b2dd518 100644 --- a/tests/Unit/Auth/DeletedUserCleanupTest.php +++ b/tests/Unit/Auth/DeletedUserCleanupTest.php @@ -11,6 +11,9 @@ use Unsupervised\Schedular\Booking\BookingRepository; use Unsupervised\Schedular\Booking\Lesson; use Unsupervised\Schedular\GroupClass\Enrollment; use Unsupervised\Schedular\GroupClass\EnrollmentRepository; +use Unsupervised\Schedular\Guardian\GuardianLink; +use Unsupervised\Schedular\Guardian\GuardianRepository; +use Unsupervised\Schedular\Guardian\GuardianService; use Unsupervised\Schedular\Payment\PaymentService; use Unsupervised\Schedular\Tests\Unit\TestCase; @@ -20,6 +23,8 @@ class DeletedUserCleanupTest extends TestCase private AvailabilityRepository&Mockery\MockInterface $availability; private EnrollmentRepository&Mockery\MockInterface $enrollments; private PaymentService&Mockery\MockInterface $payments; + private GuardianRepository&Mockery\MockInterface $links; + private GuardianService&Mockery\MockInterface $guardians; private DeletedUserCleanup $cleanup; protected function setUp(): void @@ -30,12 +35,19 @@ class DeletedUserCleanupTest extends TestCase $this->availability = Mockery::mock(AvailabilityRepository::class); $this->enrollments = Mockery::mock(EnrollmentRepository::class); $this->payments = Mockery::mock(PaymentService::class); + $this->links = Mockery::mock(GuardianRepository::class); + $this->guardians = Mockery::mock(GuardianService::class); + + // Most accounts have nobody linked to them; the guardian tests say so. + $this->links->shouldReceive('findByGuardian')->andReturn([])->byDefault(); $this->cleanup = new DeletedUserCleanup( $this->bookings, $this->availability, $this->enrollments, - $this->payments + $this->payments, + $this->links, + $this->guardians ); } @@ -134,4 +146,89 @@ class DeletedUserCleanupTest extends TestCase $this->cleanup->releaseBookings(0); } + + /** + * A child account is login-less and exists only so its guardian has somebody + * to book for. Without the guardian nobody can reach it, book for it, or be + * billed for it — so it goes too, and what it was holding goes back. + */ + public function testDeletingAGuardianReleasesAndDeletesEachChild(): void + { + $this->bookings->shouldReceive('findUpcomingForStudent')->with(5)->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([]); + + $this->links->shouldReceive('findByGuardian')->with(5)->andReturn([ + new GuardianLink(guardianId: 5, studentId: 42, id: 1), + new GuardianLink(guardianId: 5, studentId: 43, id: 2), + ]); + + $this->bookings->shouldReceive('findUpcomingForStudent')->with(42)->andReturn([ + new Lesson(slotId: 7, studentId: 42, instructorId: 3, status: Lesson::STATUS_CONFIRMED, paymentId: 40, id: 12), + ]); + $this->bookings->shouldReceive('findUpcomingForStudent')->with(43)->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(42)->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(43)->andReturn([ + new Enrollment(offeringId: 8, studentId: 43, instructorId: 3, paymentId: 41, id: 40), + ]); + + // The child's lesson is cancelled and its time freed, exactly as the + // guardian's own would have been. + $this->bookings->shouldReceive('updateStatus')->once()->with(12, Lesson::STATUS_CANCELLED)->andReturn(true); + $this->availability->shouldReceive('release')->once()->with(7)->andReturn(true); + $this->enrollments->shouldReceive('updateStatus')->once()->with(40, Enrollment::STATUS_CANCELLED)->andReturn(true); + $this->payments->shouldReceive('voidPending')->with(40)->once(); + $this->payments->shouldReceive('voidPending')->with(41)->once(); + + // Then the link row and the account itself. + $this->links->shouldReceive('delete')->once()->with(5, 42)->andReturn(true); + $this->links->shouldReceive('delete')->once()->with(5, 43)->andReturn(true); + $this->guardians->shouldReceive('deleteUser')->once()->with(42); + $this->guardians->shouldReceive('deleteUser')->once()->with(43); + + $this->cleanup->releaseBookings(5); + } + + /** + * Deleting a child fires `delete_user` again, which lands back in this same + * handler. It must return without redoing the release — and a self-link, + * however it got into the table, must not recurse for ever. + */ + public function testAChildAlreadyDealtWithIsNotProcessedTwice(): void + { + $this->bookings->shouldReceive('findUpcomingForStudent')->with(5)->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([]); + + $this->links->shouldReceive('findByGuardian')->with(5)->andReturn([ + new GuardianLink(guardianId: 5, studentId: 42, id: 1), + // A duplicate row, and a self-link: neither may cause a second pass. + new GuardianLink(guardianId: 5, studentId: 42, id: 2), + new GuardianLink(guardianId: 5, studentId: 5, id: 3), + ]); + + $this->bookings->shouldReceive('findUpcomingForStudent')->with(42)->once()->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(42)->once()->andReturn([]); + $this->links->shouldReceive('delete')->once()->with(5, 42)->andReturn(true); + $this->guardians->shouldReceive('deleteUser')->once()->with(42); + + $this->cleanup->releaseBookings(5); + + // The re-entrant call the child's own deletion triggers is a no-op. + $this->cleanup->releaseBookings(42); + } + + /** + * A child's own deletion (from the family screen, say) touches nothing but + * that child — they have nobody linked beneath them. + */ + public function testDeletingAStudentWithNoChildrenDeletesNobodyElse(): void + { + $this->bookings->shouldReceive('findUpcomingForStudent')->with(42)->andReturn([]); + $this->enrollments->shouldReceive('findByStudent')->with(42)->andReturn([]); + $this->links->shouldReceive('findByGuardian')->with(42)->andReturn([]); + + $this->guardians->shouldNotReceive('deleteUser'); + $this->links->shouldNotReceive('delete'); + + $this->cleanup->releaseBookings(42); + } } diff --git a/tests/Unit/GroupClass/SessionScheduleTest.php b/tests/Unit/GroupClass/SessionScheduleTest.php index 7b3290b..2b9b773 100644 --- a/tests/Unit/GroupClass/SessionScheduleTest.php +++ b/tests/Unit/GroupClass/SessionScheduleTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Unsupervised\Schedular\Tests\Unit\GroupClass; +use Brain\Monkey\Functions; use Mockery; use Unsupervised\Schedular\GroupClass\Enrollment; use Unsupervised\Schedular\GroupClass\EnrollmentRepository; @@ -56,6 +57,7 @@ class SessionScheduleTest extends TestCase array_column($rows, 'start_dt') ); self::assertSame('2026-09-08 17:00:00', $rows[0]['end_dt']); + self::assertNull($rows[0]['schedule']); self::assertSame('Choir', $rows[0]['offering_title']); self::assertSame(40, $rows[0]['enrollment_id']); self::assertSame(3, $rows[0]['instructor_id']); @@ -115,26 +117,132 @@ class SessionScheduleTest extends TestCase } /** - * A class with no time set has no derivable sessions, so it is left out of a - * dated list rather than shown at a time nobody chose. + * Both the class time and the duration are optional on the offering form, and + * the schedule note exists so a studio can say "Tuesdays 4:00pm" instead of + * pinning the class to a clock. A class configured that way used to vanish + * from the list entirely — the bug this covers. It now gets one row carrying + * the note in place of a date. */ - public function testAClassWithoutAScheduleYieldsNoRows(): void + public function testAClassWithNoClassTimeStillGetsARowDescribedInWords(): void { - $undated = new Offering( + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([ + new Enrollment(offeringId: 8, studentId: 5, instructorId: 3, id: 40), + ]); + $this->offerings->shouldReceive('findById')->with(8)->andReturn(new Offering( instructorId: 3, kind: Offering::KIND_GROUP_CLASS, title: 'Choir', durationMinutes: 60, termStart: '2026-09-08', + termEnd: '2026-12-08', + scheduleNote: 'Tuesdays 4:00pm', id: 8, + )); + + $rows = $this->schedule->upcomingForStudent(5, '2026-09-01 00:00:00'); + + self::assertCount(1, $rows); + self::assertSame('Tuesdays 4:00pm', $rows[0]['schedule']); + self::assertSame('Choir', $rows[0]['offering_title']); + // A sort key, not a claim about the time: the class has not started yet, + // so it sorts to its first day. + self::assertSame('2026-09-08 00:00:00', $rows[0]['start_dt']); + self::assertSame('', $rows[0]['end_dt']); + } + + /** Without a note, the term dates do the describing. */ + public function testAnUndatedClassFallsBackToItsTermDates(): void + { + Functions\when('mysql2date')->alias( + static fn (string $format, string $date): string => date($format, (int) strtotime($date)) ); $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([ new Enrollment(offeringId: 8, studentId: 5, instructorId: 3, id: 40), ]); - $this->offerings->shouldReceive('findById')->with(8)->andReturn($undated); + $this->offerings->shouldReceive('findById')->with(8)->andReturn(new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-12-08', + id: 8, + )); - self::assertSame([], $this->schedule->upcomingForStudent(5, '2026-09-01 00:00:00')); + $rows = $this->schedule->upcomingForStudent(5, '2026-09-01 00:00:00'); + + self::assertSame('Sep 8, 2026 – Dec 8, 2026', $rows[0]['schedule']); + } + + /** + * A term already under way sorts to "now" rather than to a start date in the + * past, so an ongoing class reads as current instead of dropping to the + * bottom of a list ordered by time. + */ + public function testAnUndatedClassAlreadyUnderWaySortsToNow(): void + { + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([ + new Enrollment(offeringId: 8, studentId: 5, instructorId: 3, id: 40), + ]); + $this->offerings->shouldReceive('findById')->with(8)->andReturn(new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-12-08', + scheduleNote: 'Tuesdays 4:00pm', + id: 8, + )); + + $rows = $this->schedule->upcomingForStudent(5, '2026-10-01 09:00:00'); + + self::assertSame('2026-10-01 09:00:00', $rows[0]['start_dt']); + } + + /** An undated class whose last day has passed is over, and drops out. */ + public function testAnUndatedClassThatHasFinishedIsDropped(): void + { + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([ + new Enrollment(offeringId: 8, studentId: 5, instructorId: 3, id: 40), + ]); + $this->offerings->shouldReceive('findById')->with(8)->andReturn(new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-12-08', + scheduleNote: 'Tuesdays 4:00pm', + id: 8, + )); + + self::assertSame([], $this->schedule->upcomingForStudent(5, '2026-12-09 00:00:00')); + } + + /** + * A class time but no duration: the dates are still known, so they are still + * listed — the row just says when it starts and not when it ends, rather than + * inventing a finish. + */ + public function testAClassWithNoDurationKeepsItsDatesAndLeavesTheEndOpen(): void + { + $this->enrollments->shouldReceive('findByStudent')->with(5)->andReturn([ + new Enrollment(offeringId: 8, studentId: 5, instructorId: 3, id: 40), + ]); + $this->offerings->shouldReceive('findById')->with(8)->andReturn(new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-09-15', + classTime: '16:00:00', + id: 8, + )); + + $rows = $this->schedule->upcomingForStudent(5, '2026-09-01 00:00:00'); + + self::assertSame(['2026-09-08 16:00:00', '2026-09-15 16:00:00'], array_column($rows, 'start_dt')); + self::assertSame(['', ''], array_column($rows, 'end_dt')); + self::assertNull($rows[0]['schedule']); } public function testADeletedOfferingIsSkippedRatherThanFatal(): void diff --git a/tests/Unit/Offering/OfferingTest.php b/tests/Unit/Offering/OfferingTest.php index 025919f..f758bf6 100644 --- a/tests/Unit/Offering/OfferingTest.php +++ b/tests/Unit/Offering/OfferingTest.php @@ -128,6 +128,96 @@ class OfferingTest extends TestCase self::assertSame([], $noDuration->sessionWindows()); } + /** + * Knowing *when* a class meets is a separate question from knowing how long + * it runs, so the dates survive a missing duration even though the windows + * (which need both ends) do not. + */ + public function testSessionStartsNeedsNoDuration(): void + { + $noDuration = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-09-22', + classTime: '16:00:00', + ); + + self::assertSame( + ['2026-09-08 16:00:00', '2026-09-15 16:00:00', '2026-09-22 16:00:00'], + $noDuration->sessionStarts() + ); + self::assertSame([], $noDuration->sessionWindows()); + } + + public function testSessionStartsEmptyWithoutADateOrATime(): void + { + $noTime = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + ); + self::assertSame([], $noTime->sessionStarts()); + + $noDate = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + classTime: '16:00:00', + ); + self::assertSame([], $noDate->sessionStarts()); + } + + public function testLastClassDayPrefersTheTermEnd(): void + { + $run = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-12-08', + ); + self::assertSame('2026-12-08', $run->lastClassDay()); + + $oneOff = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Recital', + termStart: '2026-09-08', + ); + self::assertSame('2026-09-08', $oneOff->lastClassDay()); + + $undated = new Offering(instructorId: 3, kind: Offering::KIND_GROUP_CLASS, title: 'Choir'); + self::assertNull($undated->lastClassDay()); + } + + /** + * The studio's own wording wins: the schedule-note field exists precisely so + * a class can describe when it meets without being pinned to a clock. + */ + public function testScheduleLabelPrefersTheScheduleNote(): void + { + $offering = new Offering( + instructorId: 3, + kind: Offering::KIND_GROUP_CLASS, + title: 'Choir', + termStart: '2026-09-08', + termEnd: '2026-12-08', + scheduleNote: ' Tuesdays 4:00pm ', + ); + + self::assertSame('Tuesdays 4:00pm', $offering->scheduleLabel()); + } + + public function testScheduleLabelSaysSoWhenNothingIsSet(): void + { + $offering = new Offering(instructorId: 3, kind: Offering::KIND_GROUP_CLASS, title: 'Choir'); + + self::assertSame('Schedule to be confirmed', $offering->scheduleLabel()); + } + public function testDefaults(): void { $offering = new Offering(1, Offering::KIND_GROUP_CLASS, 'Choir');