From c7d5d72c9ce98737d68fd6b56e17da930095a1ee Mon Sep 17 00:00:00 2001 From: James Griffin Date: Sun, 5 Jul 2026 22:09:27 -0300 Subject: [PATCH] Require an offering on every lesson booking, with a student-facing picker Generic slots (no tied offering) were bookable with no offering at all: free, instantly confirmed, and with no intake questions. POST /bookings now rejects offering-less bookings (400 offering_required), and a student-chosen offering must be an active private-lesson type owned by the slot's instructor whose duration matches the slot. The registration form gains a Lesson type field: locked to the slot's tied offering (title, duration, price) so the student sees what they are booking, or a required picker of fitting offerings for generic slots, with intake questions following the selection. Fixes #55 Co-Authored-By: Claude Fable 5 --- assets/js/booking.js | 112 +++++++++++++++++++-- docs/features/lesson-booking.md | 9 +- src/Booking/BookingEndpoint.php | 33 ++++-- tests/Unit/Booking/BookingEndpointTest.php | 99 ++++++++++++++++-- 4 files changed, 228 insertions(+), 25 deletions(-) diff --git a/assets/js/booking.js b/assets/js/booking.js index 6a0839b..f695cac 100644 --- a/assets/js/booking.js +++ b/assets/js/booking.js @@ -210,23 +210,84 @@ `; } + // Active private-lesson offerings per instructor, so revisiting the + // registration form does not refetch the same catalog. + const offeringCache = new Map(); + + function instructorOfferings(instructorId) { + if (offeringCache.has(instructorId)) { + return Promise.resolve(offeringCache.get(instructorId)); + } + return apiFetch(`offerings?instructor_id=${instructorId}&kind=private_lesson`).then((list) => { + offeringCache.set(instructorId, list); + return list; + }); + } + + // "Piano Lesson (60 min — $50.00 CAD)" / "Trial Lesson (Free)" + function offeringLabel(o) { + const duration = o.duration_minutes ? `${o.duration_minutes} min — ` : ''; + const price = Number(o.price) > 0 + ? `$${Number(o.price).toFixed(2)} ${o.currency}` + : 'Free'; + return `${o.title} (${duration}${price})`; + } + function openRegistration(slot) { clearError(); - const offeringId = Number(slot.offering_id) || 0; - const qPath = offeringId ? `offerings/${offeringId}/questions` : null; - Promise.all([ - qPath ? apiFetch(qPath) : Promise.resolve([]), + instructorOfferings(Number(slot.instructor_id)), apiFetch('policies?scope=booking'), ]) - .then(([questions, policies]) => { - renderRegistration(slot, offeringId, questions, policies); + .then(([offerings, policies]) => { + renderRegistration(slot, offerings, policies); }) .catch((err) => showError(err.message)); } - function renderRegistration(slot, offeringId, questions, policies) { + function offeringFieldHtml(tied, tiedId, choices) { + if (tiedId) { + // The slot is tied to one offering: show it locked so the student + // sees exactly what they are booking. + const label = tied ? offeringLabel(tied) : `Offering #${tiedId}`; + return ` +

+ +

`; + } + return ` +

+ +

`; + } + + function renderRegistration(slot, offerings, policies) { + const tiedId = Number(slot.offering_id) || 0; + const tied = tiedId ? offerings.find((o) => Number(o.id) === tiedId) : null; + + // Generic slots offer every lesson type that fits the slot's length. + const choices = tiedId + ? [] + : offerings.filter((o) => !o.duration_minutes || Number(o.duration_minutes) === Number(slot.duration_minutes)); + + if (!tiedId && !choices.length) { + // The server rejects offering-less bookings, so without a matching + // lesson type this time cannot be booked online. + slotList.innerHTML = ` +
+

This time cannot be booked online right now. Please contact the instructor.

+

+
`; + document.getElementById('us-cancel').addEventListener('click', loadSlots); + return; + } + const weekly = slot.recurrence_group ? `

` : ''; @@ -235,7 +296,8 @@

${escHtml(dayLabel(dayKey(slot.start_dt)))} · ${escHtml(timeOf(slot.start_dt))}–${escHtml(timeOf(slot.end_dt))}

- ${questions.map(questionField).join('')} + ${offeringFieldHtml(tied, tiedId, choices)} +
${policies.map(policyField).join('')} ${weekly}

@@ -245,10 +307,42 @@

`; + // The intake questions belong to the selected offering, so they follow + // the picker instead of being fixed at render time. + let selectedId = tiedId; + let questions = []; + + const questionsBox = document.getElementById('us-questions'); + + function loadQuestions() { + questions = []; + questionsBox.innerHTML = ''; + if (!selectedId) return; + apiFetch(`offerings/${selectedId}/questions`) + .then((qs) => { + questions = qs; + questionsBox.innerHTML = qs.map(questionField).join(''); + }) + .catch((err) => showError(err.message)); + } + + if (!tiedId) { + document.getElementById('us-offering').addEventListener('change', (e) => { + selectedId = Number(e.target.value) || 0; + loadQuestions(); + }); + } + + loadQuestions(); + document.getElementById('us-cancel').addEventListener('click', loadSlots); document.getElementById('us-register-form').addEventListener('submit', (e) => { e.preventDefault(); - submitBooking(e.target, slot, offeringId, questions); + if (!selectedId) { + showError('Please choose a lesson type.'); + return; + } + submitBooking(e.target, slot, selectedId, questions); }); } diff --git a/docs/features/lesson-booking.md b/docs/features/lesson-booking.md index 6cce231..fd15845 100644 --- a/docs/features/lesson-booking.md +++ b/docs/features/lesson-booking.md @@ -21,12 +21,12 @@ Students register for a private lesson by choosing an offering, picking a time ( ## Registration Flow 1. Student opens the page with the `[us_booking]` shortcode and browses open slots as an agenda list or a weekly calendar (view toggle with previous/next-week navigation; times shown in 12-hour AM/PM form). -2. Student picks an **offering** (a 30 or 60-minute private-lesson type) and a slot. +2. Student picks a slot and an **offering** (a 30 or 60-minute private-lesson type). When the slot is tied to an offering the form shows it locked (the student sees exactly what they are booking); otherwise the form presents the instructor's active private-lesson offerings whose duration fits the slot. Every booking requires an offering — a generic slot with no fitting offering cannot be booked online. 3. For a `weekly` reservation, the same weekday/time is held for the rest of the offering's term. 4. Student answers the offering's questions (`GET /offerings/{id}/questions`). 5. Student accepts the current published policy versions (`GET /policies`) — required to continue. 6. Payment is taken per the student's billing method (card by default; `pending` for e-transfer; skipped for comp). See `payments.md`. -7. `POST /bookings` creates the lesson row(s) (`status = pending`), records answers and policy acceptances, marks `us_availability.is_booked = 1`, and links the payment. A booking with nothing owed (no offering, or a free offering) creates no payment and is `confirmed` immediately. +7. `POST /bookings` creates the lesson row(s) (`status = pending`), records answers and policy acceptances, marks `us_availability.is_booked = 1`, and links the payment. A booking with nothing owed (a free offering) creates no payment and is `confirmed` immediately. 8. On successful payment (or comp) the lesson is `confirmed` and a receipt is emailed. 9. Instructor sees the booking under **My Lessons** and may update status via `PATCH /bookings/{id}/status`. 10. The booking page also shows the student their upcoming lessons (`GET /bookings`) with a per-lesson status badge (pending payment / confirmed) and a **Cancel** button. @@ -61,6 +61,11 @@ offering). `status`, and `payment` — a `{id, method, status}` summary, or `null` when nothing is owed (the front end then skips the payment step). +An offering is always required (`400 offering_required` otherwise): a slot tied +to an offering uses that offering regardless of the request, while a generic +slot uses the student's `offering_id`, which must be one of the instructor's +active `private_lesson` offerings whose `duration_minutes` matches the slot. + `GET /bookings` returns the caller's upcoming, non-cancelled lessons (their own for students; the instructor's for callers with `manage_availability`), each with the slot's `start_dt`/`end_dt`. diff --git a/src/Booking/BookingEndpoint.php b/src/Booking/BookingEndpoint.php index d0af35c..92670ea 100644 --- a/src/Booking/BookingEndpoint.php +++ b/src/Booking/BookingEndpoint.php @@ -5,6 +5,7 @@ namespace Unsupervised\Schedular\Booking; use Unsupervised\Schedular\Availability\AvailabilityRepository; use Unsupervised\Schedular\Auth\RoleManager; +use Unsupervised\Schedular\Offering\Offering; use Unsupervised\Schedular\Offering\OfferingRepository; use Unsupervised\Schedular\Payment\Payment; use Unsupervised\Schedular\Payment\PaymentService; @@ -165,15 +166,35 @@ class BookingEndpoint { $offeringId = $requestedOfferingId; } - $offering = $offeringId > 0 ? $this->offerings->findById( $offeringId ) : null; - if ( $offeringId > 0 && null === $offering ) { + // Every lesson books against an offering: it carries the price, intake + // questions, and payment routing. Without one the booking would silently + // be free and unquestioned, so generic slots require the student's choice. + if ( $offeringId <= 0 ) { + return new \WP_Error( 'offering_required', __( 'Choose a lesson type to book this slot.', 'unsupervised-schedular' ), [ 'status' => 400 ] ); + } + + $offering = $this->offerings->findById( $offeringId ); + if ( null === $offering ) { return new \WP_Error( 'invalid_offering', __( 'Offering not found.', 'unsupervised-schedular' ), [ 'status' => 400 ] ); } - if ( null !== $offering && $offering->instructorId !== $slot->instructorId ) { + if ( $offering->instructorId !== $slot->instructorId ) { return new \WP_Error( 'offering_mismatch', __( 'That offering is not available for this slot.', 'unsupervised-schedular' ), [ 'status' => 400 ] ); } + // A slot-tied offering was the instructor's explicit choice and is honoured + // as-is; a student-chosen one must be something the catalog actually offers + // for this slot: an active private-lesson type whose length fits the slot. + if ( 0 === $slotOfferingId ) { + if ( ! $offering->isActive || Offering::KIND_PRIVATE_LESSON !== $offering->kind ) { + return new \WP_Error( 'invalid_offering', __( 'That offering cannot be booked as a private lesson.', 'unsupervised-schedular' ), [ 'status' => 400 ] ); + } + + if ( null !== $offering->durationMinutes && $offering->durationMinutes !== $slot->durationMinutes ) { + return new \WP_Error( 'offering_mismatch', __( 'That offering does not match this slot\'s lesson length.', 'unsupervised-schedular' ), [ 'status' => 400 ] ); + } + } + $answers = $this->answers( $request ); $acceptedVersionIds = array_values( array_map( static fn( mixed $v ): int => absint( Val::int( $v ) ), (array) $request->get_param( 'accepted_policy_version_ids' ) ) ); @@ -192,7 +213,7 @@ class BookingEndpoint { slotId: $slotId, studentId: $studentId, instructorId: $slot->instructorId, - offeringId: $offeringId > 0 ? $offeringId : null, + offeringId: $offeringId, recurrence: $recurrence, notes: '' !== $notes ? $notes : null, ); @@ -227,14 +248,14 @@ class BookingEndpoint { $payment = null; $status = Lesson::STATUS_PENDING; - if ( null !== $offering && $offering->price > 0.0 ) { + if ( $offering->price > 0.0 ) { $payment = $this->payments->createForRegistration( Payment::REG_LESSON, $anchorId, $studentId, $slot->instructorId, $offering->price, $offering->currency, $offering->etransferEmail ); if ( null !== $payment && $payment->isPaid() ) { $status = Lesson::STATUS_CONFIRMED; } } else { - // Nothing owed: there is no payment step that would confirm these + // Free offering: there is no payment step that would confirm these // lessons later, so they are confirmed at booking time. foreach ( $ids as $lessonId ) { $this->bookings->updateStatus( $lessonId, Lesson::STATUS_CONFIRMED ); diff --git a/tests/Unit/Booking/BookingEndpointTest.php b/tests/Unit/Booking/BookingEndpointTest.php index 9d4d737..c9ac497 100644 --- a/tests/Unit/Booking/BookingEndpointTest.php +++ b/tests/Unit/Booking/BookingEndpointTest.php @@ -98,13 +98,16 @@ class BookingEndpointTest extends TestCase public function testBookReturns409WhenSlotClaimFails(): void { - // Generic slot, no offering; another request wins the claim first. + // Generic slot; another request wins the claim first. $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, null)); + $this->offerings->shouldReceive('findById')->with(8)->andReturn( + new Offering(instructorId: 3, kind: Offering::KIND_PRIVATE_LESSON, title: 'Lesson', price: 0.0, id: 8) + ); $this->gate->shouldReceive('validate')->andReturn(null); $this->availability->shouldReceive('claim')->with(10)->once()->andReturn(false); $this->bookings->shouldNotReceive('insert'); - $request = new \WP_REST_Request(['slot_id' => 10]); + $request = new \WP_REST_Request(['slot_id' => 10, 'offering_id' => 8]); $result = $this->endpoint->book($request); self::assertInstanceOf(\WP_Error::class, $result); @@ -135,22 +138,102 @@ class BookingEndpointTest extends TestCase self::assertNull($result->get_data()['payment']); } - public function testBookWithoutOfferingConfirmsImmediatelyWithNoPayment(): void + public function testBookWithoutOfferingIsRejected(): void { + // A booking with no offering would be silently free, so it must be refused. $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, null)); + $this->offerings->shouldNotReceive('findById'); + $this->availability->shouldNotReceive('claim'); + $this->bookings->shouldNotReceive('insert'); + + $request = new \WP_REST_Request(['slot_id' => 10]); + $result = $this->endpoint->book($request); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('offering_required', $result->get_error_code()); + } + + public function testBookUsesSlotTiedOfferingWhenRequestOmitsIt(): void + { + // Slot tied to offering 5: the booking must charge that offering even + // though the client sent no offering_id. + $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, 5)); + $this->offerings->shouldReceive('findById')->with(5)->andReturn( + new Offering(instructorId: 3, kind: Offering::KIND_PRIVATE_LESSON, title: 'Lesson', price: 50.0, id: 5) + ); $this->gate->shouldReceive('validate')->andReturn(null); $this->availability->shouldReceive('claim')->with(10)->once()->andReturn(true); - $this->bookings->shouldReceive('insert')->once()->andReturn(77); + $this->bookings->shouldReceive('insert') + ->once() + ->with(Mockery::on(static fn (Lesson $l): bool => 5 === $l->offeringId)) + ->andReturn(77); $this->gate->shouldReceive('record')->once(); - $this->payments->shouldNotReceive('createForRegistration'); - $this->bookings->shouldReceive('updateStatus')->with(77, Lesson::STATUS_CONFIRMED)->once()->andReturn(true); + $this->payments->shouldReceive('createForRegistration') + ->once() + ->with(Payment::REG_LESSON, 77, 5, 3, 50.0, 'CAD', null) + ->andReturn(new Payment( + studentId: 5, + instructorId: 3, + registrationType: Payment::REG_LESSON, + registrationId: 77, + amount: 50.0, + method: Payment::METHOD_ETRANSFER, + status: Payment::STATUS_PENDING, + id: 12, + )); + $this->bookings->shouldNotReceive('updateStatus'); $request = new \WP_REST_Request(['slot_id' => 10]); $result = $this->endpoint->book($request); self::assertInstanceOf(\WP_REST_Response::class, $result); - self::assertSame(Lesson::STATUS_CONFIRMED, $result->get_data()['status']); - self::assertNull($result->get_data()['payment']); + self::assertSame(Lesson::STATUS_PENDING, $result->get_data()['status']); + } + + public function testBookRejectsInactiveStudentChosenOffering(): void + { + $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, null)); + $this->offerings->shouldReceive('findById')->with(8)->andReturn( + new Offering(instructorId: 3, kind: Offering::KIND_PRIVATE_LESSON, title: 'Retired', price: 50.0, isActive: false, id: 8) + ); + $this->availability->shouldNotReceive('claim'); + + $request = new \WP_REST_Request(['slot_id' => 10, 'offering_id' => 8]); + $result = $this->endpoint->book($request); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('invalid_offering', $result->get_error_code()); + } + + public function testBookRejectsGroupClassOfferingForLessonSlot(): void + { + $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, null)); + $this->offerings->shouldReceive('findById')->with(8)->andReturn( + new Offering(instructorId: 3, kind: Offering::KIND_GROUP_CLASS, title: 'Choir', price: 10.0, id: 8) + ); + $this->availability->shouldNotReceive('claim'); + + $request = new \WP_REST_Request(['slot_id' => 10, 'offering_id' => 8]); + $result = $this->endpoint->book($request); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('invalid_offering', $result->get_error_code()); + } + + public function testBookRejectsOfferingWhoseDurationDoesNotFitSlot(): void + { + // Slot is 60 minutes (the helper default); a 30-minute offering cannot book it. + $this->availability->shouldReceive('findById')->with(10)->andReturn($this->slot(10, 3, null)); + $this->offerings->shouldReceive('findById')->with(8)->andReturn( + new Offering(instructorId: 3, kind: Offering::KIND_PRIVATE_LESSON, title: 'Short', price: 25.0, durationMinutes: 30, id: 8) + ); + $this->availability->shouldNotReceive('claim'); + + $request = new \WP_REST_Request(['slot_id' => 10, 'offering_id' => 8]); + $result = $this->endpoint->book($request); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('offering_mismatch', $result->get_error_code()); } public function testBookWithPricedOfferingStaysPendingAndReturnsPaymentSummary(): void -- 2.54.0