Availability: adding a window shorter than the lesson length fails silently #130

New Issue

2026-07-29T01:42:34Z

thatguygriff commented

2026-07-29 01:42:34 +00:00

Reported

Adding availability for Thursday Sept 10, 5:30 PM–6:00 PM, repeating weekly for 41 weeks did nothing. No slots appeared, no error was shown — the page just reloaded.

Root cause (confirmed)

The lesson length select was left at its default of 60 minutes while the window is only 30 minutes long. The window therefore cannot fit a single lesson:

AvailabilitySlot::splitByDuration() (src/Availability/AvailabilitySlot.php:48) returns [] when the window is shorter than durationMinutes.
AvailabilityRepository::createFromWindow() (src/Availability/AvailabilityRepository.php:41) iterates that empty list, so zero rows are inserted and it returns [].
AvailabilityController::addSlot() (src/Availability/AvailabilityController.php:112) ignores the return value and renders the page as if nothing happened.

The REST endpoint already rejects this case with a 400 (invalid_window, src/Availability/AvailabilityEndpoint.php:148-150). The admin form path has no equivalent — that asymmetry is the bug.

Other silent failures in the same form

Every one of these currently returns without any message and leaves the instructor thinking the save worked:

Window shorter than lesson length — the reported bug. No rows, no notice.
Unparseable start/end — AvailabilityController.php:94 returns silently when normalizeDateTime() yields null.
End before/equal to start — same silent return at line 94.
Window spans two days — same silent return; the REST path returns a specific invalid_window message about using the weekly repeat instead.
Foreign / non-existent offering_id — the controller never validates offering ownership. AvailabilityEndpoint::create() does (:122-127, so a slot can never inherit another instructor's price or payment routing), but a crafted POST to the admin page attaches any offering id with no check. Also silent.
Ignored $wpdb->insert() result — AvailabilityRepository::insert() (:14) returns $this->db->insert_id without checking whether the insert succeeded. On a DB error it returns the previous statement's insert id, so a failed write looks like a successful one.
No success feedback at all — even a fully successful save renders no notice, so "nothing happened" and "41 weeks of slots created" look identical apart from scanning the table.
weeks is unbounded server-side. The input has max="52", but addSlot() only absint()s it. A hand-posted weeks=10000 creates 10,000 rows per chunk with no guard.

Requested fix

Client side — filter the lesson-length select down to durations that actually fit the entered window, and block submission when none do:

Recompute the duration_minutes options whenever start_dt or end_dt changes; hide/disable any option longer than the window.
If the currently selected length no longer fits, fall back to the longest one that does.
Disable the submit button (with an explanation) while the window fits no lesson length at all.
There is currently no JS on this admin screen, so this needs a new assets/js/availability-admin.js enqueued from AdminMenu/AvailabilityController.

Server side — client-side validation is a convenience, not a guarantee. addSlot() should return a result and the page should render it, matching the existing $notice / $error pattern used by Auth\StudentController + templates/admin/student-detail.php:81-85:

Distinct error messages for: invalid datetimes, end before start, multi-day window, window shorter than lesson length, offering not owned by the instructor.
Validate offering_id ownership exactly as AvailabilityEndpoint::create() does.
Clamp weeks to the documented 1–52 range.
Have AvailabilityRepository::insert() distinguish a failed insert from a successful one, and surface a generic error when a write fails.
Show a success notice on save, ideally with the count ("Added 41 slots").

Consider extracting the shared validation so the controller and the REST endpoint cannot drift apart again.

Acceptance criteria

Entering a 30-minute window leaves only lesson lengths ≤ 30 selectable.
Submitting a window that fits no lesson (by any route, including a hand-crafted POST) shows an explanatory error and creates nothing.
Each failure mode above produces a visible admin notice rather than a silent reload.
A successful add produces a success notice.
Posting an offering_id belonging to another instructor is rejected.
Unit tests in tests/Unit/Availability/ cover each validation branch.

Files

src/Availability/AvailabilityController.php
src/Availability/AvailabilityRepository.php
src/Availability/AvailabilityEndpoint.php
src/Availability/AvailabilitySlot.php
templates/admin/availability.php
assets/js/ (new file)
docs/features/availability-management.md

## Reported Adding availability for **Thursday Sept 10, 5:30 PM–6:00 PM, repeating weekly for 41 weeks** did nothing. No slots appeared, no error was shown — the page just reloaded. ## Root cause (confirmed) The lesson length select was left at its default of **60 minutes** while the window is only **30 minutes** long. The window therefore cannot fit a single lesson: - `AvailabilitySlot::splitByDuration()` (`src/Availability/AvailabilitySlot.php:48`) returns `[]` when the window is shorter than `durationMinutes`. - `AvailabilityRepository::createFromWindow()` (`src/Availability/AvailabilityRepository.php:41`) iterates that empty list, so **zero rows are inserted** and it returns `[]`. - `AvailabilityController::addSlot()` (`src/Availability/AvailabilityController.php:112`) ignores the return value and renders the page as if nothing happened. The REST endpoint already rejects this case with a 400 (`invalid_window`, `src/Availability/AvailabilityEndpoint.php:148-150`). The admin form path has no equivalent — that asymmetry is the bug. ## Other silent failures in the same form Every one of these currently returns without any message and leaves the instructor thinking the save worked: 1. **Window shorter than lesson length** — the reported bug. No rows, no notice. 2. **Unparseable start/end** — `AvailabilityController.php:94` returns silently when `normalizeDateTime()` yields `null`. 3. **End before/equal to start** — same silent `return` at line 94. 4. **Window spans two days** — same silent `return`; the REST path returns a specific `invalid_window` message about using the weekly repeat instead. 5. **Foreign / non-existent `offering_id`** — the controller never validates offering ownership. `AvailabilityEndpoint::create()` does (`:122-127`, so a slot can never inherit another instructor's price or payment routing), but a crafted POST to the admin page attaches any offering id with no check. Also silent. 6. **Ignored `$wpdb->insert()` result** — `AvailabilityRepository::insert()` (`:14`) returns `$this->db->insert_id` without checking whether the insert succeeded. On a DB error it returns the *previous* statement's insert id, so a failed write looks like a successful one. 7. **No success feedback at all** — even a fully successful save renders no notice, so "nothing happened" and "41 weeks of slots created" look identical apart from scanning the table. 8. **`weeks` is unbounded server-side.** The input has `max="52"`, but `addSlot()` only `absint()`s it. A hand-posted `weeks=10000` creates 10,000 rows per chunk with no guard. ## Requested fix **Client side** — filter the lesson-length select down to durations that actually fit the entered window, and block submission when none do: - Recompute the `duration_minutes` options whenever `start_dt` or `end_dt` changes; hide/disable any option longer than the window. - If the currently selected length no longer fits, fall back to the longest one that does. - Disable the submit button (with an explanation) while the window fits no lesson length at all. - There is currently no JS on this admin screen, so this needs a new `assets/js/availability-admin.js` enqueued from `AdminMenu`/`AvailabilityController`. **Server side** — client-side validation is a convenience, not a guarantee. `addSlot()` should return a result and the page should render it, matching the existing `$notice` / `$error` pattern used by `Auth\StudentController` + `templates/admin/student-detail.php:81-85`: - Distinct error messages for: invalid datetimes, end before start, multi-day window, window shorter than lesson length, offering not owned by the instructor. - Validate `offering_id` ownership exactly as `AvailabilityEndpoint::create()` does. - Clamp `weeks` to the documented 1–52 range. - Have `AvailabilityRepository::insert()` distinguish a failed insert from a successful one, and surface a generic error when a write fails. - Show a success notice on save, ideally with the count ("Added 41 slots"). Consider extracting the shared validation so the controller and the REST endpoint cannot drift apart again. ## Acceptance criteria - Entering a 30-minute window leaves only lesson lengths ≤ 30 selectable. - Submitting a window that fits no lesson (by any route, including a hand-crafted POST) shows an explanatory error and creates nothing. - Each failure mode above produces a visible admin notice rather than a silent reload. - A successful add produces a success notice. - Posting an `offering_id` belonging to another instructor is rejected. - Unit tests in `tests/Unit/Availability/` cover each validation branch. ## Files - `src/Availability/AvailabilityController.php` - `src/Availability/AvailabilityRepository.php` - `src/Availability/AvailabilityEndpoint.php` - `src/Availability/AvailabilitySlot.php` - `templates/admin/availability.php` - `assets/js/` (new file) - `docs/features/availability-management.md`

thatguygriff added the bug label 2026-07-29 01:42:34 +00:00

thatguygriff referenced this issue from a commit

2026-07-29 02:20:23 +00:00

Stop the availability form failing in silence

thatguygriff referenced a pull request that will close this issue

2026-07-29 02:20:49 +00:00

Stop the availability form failing in silence #137

thatguygriff closed this issue

2026-07-29 02:25:37 +00:00

Sign in to join this conversation.