From df3462a8b3f34167cd666ccf172419afba518e48 Mon Sep 17 00:00:00 2001 From: James Griffin Date: Thu, 30 Jul 2026 16:34:57 -0300 Subject: [PATCH] Show a series' policy acceptances on every occurrence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A weekly booking reserves a series of lessons, but the student answers the intake and ticks the policy boxes once — so BookingEndpoint records both against the anchor lesson alone. The admin detail view looked them up by whichever lesson id was being viewed, so every occurrence after the first showed no answers and no acceptances at all. LessonDetail now takes the Lesson rather than a bare id and resolves the registration to `series_id ?? id`, so each occurrence reads the anchor's records. This is the same seam PaymentService already uses to find a series lesson's payment on the anchor. Nothing was ever missing from the database, so existing bookings read correctly with no migration and no schema change. Closes #167 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 3 ++ docs/features/lesson-booking.md | 2 +- src/Booking/LessonController.php | 4 +- src/Booking/LessonDetail.php | 22 ++++++-- tests/Unit/Booking/LessonControllerTest.php | 6 ++- tests/Unit/Booking/LessonDetailTest.php | 57 ++++++++++++++++++++- 6 files changed, 83 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc8c6fa..4f40d50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ each change under the current top section as you work. - **You can now edit your own details on the profile page**, not just your students'. The page is called **Your profile**, and until now the one person on it you could not change was yourself: a mistyped name at signup, or a name that had since changed, meant asking the studio to fix it. **Your details** now sits at the top of the page with your name, your birth year, and whether you take lessons yourself. Your email address is shown but not editable — it is also how you sign in, so changing it stays a studio-side job. - **"I take lessons myself" can be corrected after signup.** Signup asks whether you are registering just yourself, only on behalf of students, or both, and the answer decides whether you are offered as a student when booking. Choosing wrongly — or taking up lessons later alongside the children you book for — used to leave you asking the studio to change it. Ticking the box makes you bookable again and asks for your birth year like any other student; unticking it takes you back off the list without discarding the birth year you already gave, so ticking it back on costs you nothing. +### Fixed +- **A recurring lesson now shows the policies the student accepted on every week of it, not just the first.** Booking a weekly lesson reserves a series of them, and the student answers the intake questions and agrees to the studio's policies once, for the whole reservation. Opening any week after the first showed no answers and no policies accepted — as though nothing had been agreed to. Nothing was ever missing: the agreement was recorded against the first lesson of the series and every other week was looking for one of its own. Each week of a series now shows the intake answers and the full acceptance record — policy, version, when it was accepted, and from where — captured when the reservation was booked. Existing bookings read correctly straight away; there is nothing to re-collect from anyone. + ## [1.4.1] ### Added diff --git a/docs/features/lesson-booking.md b/docs/features/lesson-booking.md index 0d34633..3629fe7 100644 --- a/docs/features/lesson-booking.md +++ b/docs/features/lesson-booking.md @@ -160,7 +160,7 @@ instructor may only open their own lessons; the studio **Scheduler** may open an - Model: `Unsupervised\Schedular\Booking\Lesson` - Registration gate: `Unsupervised\Schedular\Registration\RegistrationGate` — validates and records intake answers + booking-scoped policy acceptances; shared with group enrolment - Admin controller: `Unsupervised\Schedular\Booking\LessonController` -- Admin lesson detail presenter: `Unsupervised\Schedular\Booking\LessonDetail` (per-lesson intake answers + policy acceptances), template `templates/admin/lesson-detail.php` +- Admin lesson detail presenter: `Unsupervised\Schedular\Booking\LessonDetail` (per-lesson intake answers + policy acceptances), template `templates/admin/lesson-detail.php`. A weekly series is answered for and agreed to once, against the anchor lesson, so the presenter reads `series_id ?? id` — every occurrence shows the same intake and audit trail, not just the first. - REST endpoint: `Unsupervised\Schedular\Booking\BookingEndpoint` - Frontend: `Unsupervised\Schedular\Booking\BookingPage`, `Unsupervised\Schedular\Auth\LoginPage` - Upcoming-lessons panel: rendered client-side into `#us-my-lessons` by `assets/js/booking.js` (`lessonRowHtml`/`renderMyLessons`), mirrored for the editor by `BlockPreview::upcomingLessons()` — keep the two markup shapes in step. diff --git a/src/Booking/LessonController.php b/src/Booking/LessonController.php index 52f7a38..5c8bba2 100644 --- a/src/Booking/LessonController.php +++ b/src/Booking/LessonController.php @@ -75,8 +75,8 @@ class LessonController { $accepts = []; } else { $row = $this->row( $lesson ); - $answers = $this->detail->answers( $lessonId ); - $accepts = $this->detail->acceptances( $lessonId ); + $answers = $this->detail->answers( $lesson ); + $accepts = $this->detail->acceptances( $lesson ); } include USC_PLUGIN_DIR . 'templates/admin/lesson-detail.php'; diff --git a/src/Booking/LessonDetail.php b/src/Booking/LessonDetail.php index bbff5f6..596124a 100644 --- a/src/Booking/LessonDetail.php +++ b/src/Booking/LessonDetail.php @@ -17,6 +17,12 @@ use Unsupervised\Schedular\Registration\QuestionRepository; * * Scoped to a single lesson (the `lesson` registration type), mirroring the * per-student history in {@see \Unsupervised\Schedular\Auth\StudentHistory}. + * + * A weekly reservation is answered for and agreed to once, so its answers and + * acceptances hang off the series anchor. Every occurrence therefore reads its + * series' registration, not its own id — otherwise only the first lesson of a + * series showed the intake and the audit trail, and the rest looked as though + * nothing had been accepted. */ class LessonDetail { @@ -33,7 +39,7 @@ class LessonDetail { * * @return list */ - public function answers( int $lessonId ): array { + public function answers( Lesson $lesson ): array { return array_map( function ( Answer $answer ): array { $question = $this->questions->findById( $answer->questionId ); @@ -44,7 +50,7 @@ class LessonDetail { 'answer' => '' === $value ? '—' : $value, ]; }, - $this->answers->findByRegistration( Answer::REG_LESSON, $lessonId ) + $this->answers->findByRegistration( Answer::REG_LESSON, $this->registrationId( $lesson ) ) ); } @@ -54,7 +60,7 @@ class LessonDetail { * * @return list */ - public function acceptances( int $lessonId ): array { + public function acceptances( Lesson $lesson ): array { return array_map( function ( PolicyAcceptance $acceptance ): array { $version = $this->versions->findById( $acceptance->policyVersionId ); @@ -67,7 +73,15 @@ class LessonDetail { 'ip' => $acceptance->ipAddress ?? '', ]; }, - $this->acceptances->findByRegistration( PolicyAcceptance::REG_LESSON, $lessonId ) + $this->acceptances->findByRegistration( PolicyAcceptance::REG_LESSON, $this->registrationId( $lesson ) ) ); } + + /** + * The lesson id the booking's answers and acceptances were recorded against: + * the series anchor for a weekly reservation, the lesson itself otherwise. + */ + private function registrationId( Lesson $lesson ): int { + return $lesson->seriesId ?? (int) $lesson->id; + } } diff --git a/tests/Unit/Booking/LessonControllerTest.php b/tests/Unit/Booking/LessonControllerTest.php index 6c048da..c6db1bb 100644 --- a/tests/Unit/Booking/LessonControllerTest.php +++ b/tests/Unit/Booking/LessonControllerTest.php @@ -233,10 +233,12 @@ class LessonControllerTest extends TestCase $this->bookings->shouldReceive('findById')->once()->with(1)->andReturn($lesson); $this->availability->shouldReceive('findById')->once()->with(10)->andReturn($slot); $this->offerings->shouldReceive('findById')->once()->with(8)->andReturn($offering); - $this->detail->shouldReceive('answers')->once()->with(1)->andReturn([ + // The lesson itself is handed over, so the presenter can follow a series + // occurrence back to the anchor its answers and acceptances hang off. + $this->detail->shouldReceive('answers')->once()->with($lesson)->andReturn([ ['question' => 'Skill level', 'answer' => 'Beginner'], ]); - $this->detail->shouldReceive('acceptances')->once()->with(1)->andReturn([ + $this->detail->shouldReceive('acceptances')->once()->with($lesson)->andReturn([ ['policy' => 'Cancellation', 'version' => 'v2', 'accepted_at' => '2026-07-01 10:00:00', 'ip' => '1.2.3.4'], ]); diff --git a/tests/Unit/Booking/LessonDetailTest.php b/tests/Unit/Booking/LessonDetailTest.php index ad7fe09..f198c49 100644 --- a/tests/Unit/Booking/LessonDetailTest.php +++ b/tests/Unit/Booking/LessonDetailTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Unsupervised\Schedular\Tests\Unit\Booking; use Mockery; +use Unsupervised\Schedular\Booking\Lesson; use Unsupervised\Schedular\Booking\LessonDetail; use Unsupervised\Schedular\Policy\AcceptanceRepository; use Unsupervised\Schedular\Policy\Policy; @@ -60,7 +61,7 @@ class LessonDetailTest extends TestCase ['question' => 'Skill level', 'answer' => 'Beginner'], ['question' => '#9', 'answer' => '—'], ], - $this->detail->answers(7) + $this->detail->answers($this->lesson(7)) ); } @@ -89,7 +90,59 @@ class LessonDetailTest extends TestCase 'ip' => '1.2.3.4', ], ], - $this->detail->acceptances(7) + $this->detail->acceptances($this->lesson(7)) + ); + } + + public function testSeriesOccurrenceReadsTheAnchorsAnswersAndAcceptances(): void + { + // Occurrence #12 of a weekly reservation anchored on lesson 7: the intake + // and the agreement were recorded once, against the anchor. + $occurrence = $this->lesson(12, seriesId: 7); + + $this->answers->shouldReceive('findByRegistration')->once()->with(Answer::REG_LESSON, 7)->andReturn([ + new Answer(questionId: 2, registrationType: Answer::REG_LESSON, registrationId: 7, studentId: 5, answerValue: 'Beginner'), + ]); + $this->questions->shouldReceive('findById')->with(2)->andReturn(new Question(offeringId: 1, label: 'Skill level', id: 2)); + + $this->acceptances->shouldReceive('findByRegistration')->once()->with(PolicyAcceptance::REG_LESSON, 7)->andReturn([ + new PolicyAcceptance( + policyVersionId: 4, + studentId: 5, + registrationType: PolicyAcceptance::REG_LESSON, + registrationId: 7, + ipAddress: '1.2.3.4', + acceptedAt: '2026-07-01 10:00:00' + ), + ]); + $this->versions->shouldReceive('findById')->with(4)->andReturn(new PolicyVersion(policyId: 3, versionNumber: 2, id: 4)); + $this->policies->shouldReceive('findById')->with(3)->andReturn(new Policy(title: 'Cancellation', slug: 'cancellation', id: 3)); + + self::assertSame( + [['question' => 'Skill level', 'answer' => 'Beginner']], + $this->detail->answers($occurrence) + ); + self::assertSame( + [[ + 'policy' => 'Cancellation', + 'version' => 'v2', + 'accepted_at' => '2026-07-01 10:00:00', + 'ip' => '1.2.3.4', + ]], + $this->detail->acceptances($occurrence) + ); + } + + private function lesson(int $id, ?int $seriesId = null): Lesson + { + return new Lesson( + slotId: 1, + studentId: 5, + instructorId: 9, + offeringId: 1, + recurrence: null === $seriesId ? Lesson::RECURRENCE_SINGLE : Lesson::RECURRENCE_WEEKLY, + seriesId: $seriesId, + id: $id ); } } -- 2.54.0