Compare commits

..
5 Commits
Author SHA1 Message Date
thatguygriffandClaude Opus 5 794df2cb4f Publish prebuilt CI images to the Gitea container registry
CI Images / Build CI image (PHP 8.1) (pull_request) Successful in 58s
CI Images / Build CI image (PHP 8.2) (pull_request) Successful in 1m12s
CI Images / Build CI image (PHP 8.3) (pull_request) Successful in 1m55s
CI / Tests (PHP 8.1) (pull_request) Successful in 1m2s
CI / Tests (PHP 8.2) (pull_request) Successful in 59s
CI / No Debug Code (pull_request) Successful in 2s
CI Images / Build CI image (PHP 8.5) (pull_request) Successful in 2m30s
CI / Tests (PHP 8.5) (pull_request) Successful in 4m32s
CI / Coding Standards & Static Analysis (pull_request) Successful in 13m28s
CI / Tests (PHP 8.3) (pull_request) Failing after 12m19s
CI / Build Plugin Zip (pull_request) Skipped
setup-php installs PHP 8.3+ from apt on these arm64 runners: a ~145s floor
against ~35s for 8.1/8.2, with a tail that twice ran past the step timeout
and failed the run (#178). Caching the .debs softened it without removing
the apt step, and 8.5 has the same problem.

Add a per-version CI image built on php:<version>-cli-alpine and a workflow
that publishes it to git.unsupervised.ca/unsupervised/ci-php:<version>. The
org is public, so the packages pull anonymously.

The image carries bash and nodejs because act_runner runs JavaScript actions
inside the job container, GNU coreutils/grep/sed because the workflow scripts
use `tac` and `grep --include`, and curl/jq/git/zip for release.yml and
bin/build-zip.sh. Composer 2 and the intl and zip extensions round it out.

Nothing consumes the images yet — ci.yml and release.yml switch over in a
follow-up, because a job cannot run in an image that has not been published.

Part of #187

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01D9acV1mHktGAb1uyvNmrR2
2026-08-24 19:49:17 -03:00
thatguygriff 7278309bf5 Merge pull request 'Let the studio register the students who cannot register themselves' (#186) from fix/185-book-for-child-and-vet-group-enrolment into main
CI / Coding Standards & Static Analysis (push) Successful in 13m23s
CI / Tests (PHP 8.1) (push) Successful in 55s
CI / Tests (PHP 8.2) (push) Successful in 1m7s
CI / Tests (PHP 8.3) (push) Successful in 6m59s
CI / Tests (PHP 8.5) (push) Successful in 12m55s
CI / No Debug Code (push) Successful in 3s
CI / Build Plugin Zip (push) Successful in 5m53s
Release / Build and Publish Release (push) Successful in 8m5s
Release / Open next-version bump PR (push) Successful in 5s
Reviewed-on: #186
2026-08-24 22:03:11 +00:00
thatguygriffandClaude Opus 5 5ce42f0003 Let the studio register the students who cannot register themselves
CI / No Debug Code (pull_request) Successful in 4s
CI / Tests (PHP 8.2) (pull_request) Successful in 50s
CI / Tests (PHP 8.1) (pull_request) Successful in 1m3s
CI / Tests (PHP 8.5) (pull_request) Successful in 2m48s
CI / Tests (PHP 8.3) (pull_request) Successful in 3m24s
CI / Coding Standards & Static Analysis (pull_request) Successful in 8m21s
CI / Build Plugin Zip (pull_request) Skipped
The Book a lesson for a student panel built its picker from the us_student
role but vetted the submission with the book_lesson capability. ChildLoginGate
and RegistrationLoginGate withhold that capability from accounts that keep the
role, so the panel offered every guardian-managed child and every unapproved
signup and then refused them — with a message claiming no student had been
chosen, and a form cleared of all five fields.

Withholding book_lesson stops those accounts registering in their own name. It
was never meant to stop the studio acting for them, which is what the panel is
for, and for a child is the only route to a lesson besides their guardian.

Guard the student role instead, via a new RoleManager::isStudent() shared with
every picker and guard on the staff side so the two cannot drift apart again.
Group enrolment gets the same predicate: addDirect() and grantAccess() vetted
their posted ids not at all, and would enrol an instructor, an administrator,
or an account deleted since the page was drawn — raising a real payment against
them for a priced class.

Keep a refused booking's fields as submitted, reading the form through one
LessonController::submittedBooking() so what gets booked and what is shown
again cannot disagree about a field name. A booking that succeeds still leaves
an empty form, so the next one does not inherit it.

Closes #185

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01XunBYk2sFEc1oL14sUiuBU
2026-08-24 18:42:59 -03:00
thatguygriff 76530878b5 Merge pull request 'Bump version to 1.5.4' (#184) from release/bump-1.5.4 into main
CI / Tests (PHP 8.1) (push) Successful in 58s
CI / No Debug Code (push) Successful in 2s
CI / Tests (PHP 8.5) (push) Successful in 4m20s
CI / Coding Standards & Static Analysis (push) Successful in 5m21s
CI / Tests (PHP 8.2) (push) Successful in 6m4s
CI / Tests (PHP 8.3) (push) Successful in 12m56s
CI / Build Plugin Zip (push) Successful in 2m39s
Reviewed-on: #184
2026-08-24 17:59:40 +00:00
Release Bot 6031a75012 Bump version to 1.5.4 and open changelog section 2026-08-24 17:56:38 +00:00
16 changed files with 599 additions and 33 deletions
+60
View File
@@ -0,0 +1,60 @@
# CI image for unsupervised-scheduler, one tag per PHP version.
#
# Built and published by .gitea/workflows/ci-images.yml to
# git.unsupervised.ca/unsupervised/ci-php:<php-version>. CI and release jobs
# run inside it via `container:`, so nothing installs PHP at job time.
#
# Why: setup-php installs 8.3+ through apt/the ondrej PPA on these arm64
# runners — a ~145s floor against ~35s for 8.1/8.2, with a tail that has
# twice crossed into hard failure (#178). Pulling a ~120MB image from the
# registry in our own cluster replaces that entirely (#187).
ARG PHP_VERSION=8.3
FROM php:${PHP_VERSION}-cli-alpine
# bash and nodejs are not optional: act_runner executes JavaScript actions
# (actions/checkout, actions/cache, actions/upload-artifact) *inside* the job
# container, and shells `run:` steps through bash.
#
# coreutils, gawk, grep and sed replace the busybox applets with the GNU ones
# the workflow scripts are written against (`tac`, `grep --include`).
#
# jq, curl, git and zip/unzip are used by release.yml and bin/build-zip.sh.
RUN apk add --no-cache \
bash \
coreutils \
curl \
gawk \
git \
grep \
jq \
nodejs \
sed \
unzip \
zip \
icu-libs \
libzip \
&& apk add --no-cache --virtual .build-deps \
$PHPIZE_DEPS \
icu-dev \
libzip-dev \
&& docker-php-ext-install -j"$(nproc)" intl zip \
&& apk del --no-network .build-deps
# mbstring is compiled into the official php images; intl and zip are added
# above. That covers what phpunit, phpstan, phpcs and Composer need.
COPY --from=composer:2 /usr/bin/composer /usr/bin/composer
# Jobs run as root inside the container, and never answer prompts.
ENV COMPOSER_ALLOW_SUPERUSER=1 \
COMPOSER_NO_INTERACTION=1 \
COMPOSER_HOME=/composer
RUN mkdir -p "$COMPOSER_HOME" \
&& php -v \
&& php -m | grep -qx intl \
&& php -m | grep -qx mbstring \
&& composer --version
CMD ["/bin/bash"]
+94
View File
@@ -0,0 +1,94 @@
name: CI Images
# Publishes the per-PHP-version images that ci.yml and release.yml run inside
# (#187). Nothing else consumes them, so this workflow is the only place the
# registry path is written down.
#
# Triggers:
# - the Dockerfile or this workflow changing on main, so an edit ships;
# - the same paths on a pull request, which builds but does not push, so a
# broken Dockerfile is caught before it reaches main;
# - workflow_dispatch, to rebuild on demand;
# - weekly, so PHP patch releases and Alpine security updates land without
# anyone remembering to ask.
on:
push:
branches:
- main
paths:
- '.gitea/ci/Dockerfile'
- '.gitea/workflows/ci-images.yml'
pull_request:
paths:
- '.gitea/ci/Dockerfile'
- '.gitea/workflows/ci-images.yml'
schedule:
- cron: '17 4 * * 1'
workflow_dispatch:
env:
# The instance ROOT_URL host — the container registry lives on the same host.
REGISTRY: git.unsupervised.ca
IMAGE: unsupervised/ci-php
jobs:
build:
name: Build CI image (PHP ${{ matrix.php }})
runs-on: ubuntu-latest
strategy:
# One version failing should not hide whether the others built.
fail-fast: false
matrix:
# Keep in step with the test matrix in ci.yml.
php:
- '8.1'
- '8.2'
- '8.3'
- '8.5'
steps:
- uses: actions/checkout@v4
# The images are built natively, so they carry the runner's
# architecture only. Fine while every runner is arm64; if a runner of a
# different architecture ever joins the pool it will overwrite these
# tags with its own arch and the others will fail to pull.
- name: Check Docker is available
run: |
if ! docker info >/dev/null 2>&1; then
echo "No usable Docker daemon in the job container." >&2
echo "act_runner needs container.docker_host set (or left empty to autodetect)." >&2
exit 1
fi
docker version --format 'client {{.Client.Version}} / server {{.Server.Version}} / arch {{.Server.Arch}}'
# secrets.GITHUB_TOKEN is the Actions task token and can write packages
# for the repository owner. REGISTRY_TOKEN is an escape hatch: set it to
# a PAT with package:write if the task token is ever refused.
- name: Log in to the container registry
if: github.event_name != 'pull_request'
run: |
echo "${{ secrets.REGISTRY_TOKEN || secrets.GITHUB_TOKEN }}" \
| docker login "${REGISTRY}" -u "${{ vars.REGISTRY_USER || github.actor }}" --password-stdin
- name: Build
run: |
docker build \
--pull \
--build-arg "PHP_VERSION=${{ matrix.php }}" \
--tag "${REGISTRY}/${IMAGE}:${{ matrix.php }}" \
--file .gitea/ci/Dockerfile \
.gitea/ci
# Pull requests build only — the tags on the registry are what the other
# workflows run inside, so only main and a manual dispatch move them.
- name: Push
if: github.event_name != 'pull_request'
run: |
image="${REGISTRY}/${IMAGE}:${{ matrix.php }}"
docker push "${image}"
echo "Published ${image}"
- name: Log out
if: always() && github.event_name != 'pull_request'
run: docker logout "${REGISTRY}" || true
+7
View File
@@ -11,6 +11,13 @@ When a `v*` tag is pushed, `.gitea/workflows/release.yml` publishes the matching
the plugin to the next patch version and adds a fresh section here for it. Record
each change under the current top section as you work.
## [1.5.4]
### Fixed
- **Adding students to a group class now checks that each one is actually a student.** **Add students directly** and **Make available** acted on whatever ids the form posted without confirming they named students at all, so a stale page — or a tampered submission — could put an instructor, an administrator, or an account that had since been deleted onto a class roster, raising a real payment against them. Both controls now skip anything that is not a student, and the "%d student(s) added" count tells you how many actually went through. Children and students awaiting approval are unaffected: they are students, and adding them is what these controls are for.
- **A refused booking no longer empties the Book a lesson for a student form.** Whatever the reason it came back — the time taken while you were typing, a weekly reservation asked for on a time that does not repeat — the panel reopens with the student, time, lesson type, both ticks and your note exactly as you left them, so a correction is one field, not five. A booking that goes through still leaves an empty form behind for the next one.
- **Booking a lesson for a child, or for a student you have not approved yet, no longer fails with "Choose a student to book for."** The **Book a lesson for a student** panel offered every student it could see, but then refused a good half of them: a parent's child and a self-signup still awaiting approval both appeared in the list, and both were rejected on submit — with an error that read as though no student had been chosen, and which cleared the form. Neither of those accounts is allowed to book *in their own name* (a child's is never signed in to at all, and an unapproved signup waits for you), and the panel was mistakenly applying that same restriction to the studio booking on their behalf, which is precisely the case it was built for. Anyone the panel offers can now be booked for.
## [1.5.3]
### Added
+70
View File
@@ -0,0 +1,70 @@
# CI images
CI and release jobs do not install PHP. They run inside prebuilt images
published to the Gitea container registry:
```
git.unsupervised.ca/unsupervised/ci-php:8.1
git.unsupervised.ca/unsupervised/ci-php:8.2
git.unsupervised.ca/unsupervised/ci-php:8.3
git.unsupervised.ca/unsupervised/ci-php:8.5
```
The `Unsupervised` org is public, so the packages pull anonymously — jobs need
no registry credentials to use them.
## Why
`shivammathur/setup-php` installs PHP 8.3+ from apt/the ondrej PPA on these
arm64 runners. That was a ~145s floor against ~35s for 8.1 and 8.2, with a
tail that twice ran past the step timeout and failed the run outright
(#178). Caching the `.deb`s helped, but the apt step itself remained, and PHP
8.5 has the same shape of problem. Pulling a ~120MB image from a registry
inside the cluster replaces the whole thing (#187).
## What is in the image
`.gitea/ci/Dockerfile` builds on `php:<version>-cli-alpine` and adds:
- **`bash` and `nodejs`** — act_runner runs JavaScript actions
(`actions/checkout`, `actions/cache`, `actions/upload-artifact`) *inside*
the job container and shells `run:` steps through bash. Without these, the
first step of every job fails.
- **`coreutils`, `gawk`, `grep`, `sed`** — GNU versions, because the workflow
scripts use `tac` and `grep --include`, which busybox does not provide.
- **`curl`, `jq`, `git`, `zip`, `unzip`** — used by `release.yml` and
`bin/build-zip.sh`.
- **`intl` and `zip` PHP extensions**, plus Composer 2. `mbstring` is already
compiled into the official images.
## Publishing
`.gitea/workflows/ci-images.yml` builds and pushes them. It runs when the
Dockerfile changes on `main`, weekly (so PHP patch releases and Alpine
security updates land on their own), and on `workflow_dispatch`. On a pull
request it builds without pushing, so a broken Dockerfile is caught before it
reaches `main`.
## Adding or dropping a PHP version
1. Add the version to the `php` matrix in `.gitea/workflows/ci-images.yml`.
2. Merge to `main`, or dispatch the workflow, and wait for the tag to appear.
3. Add the version to the `test` matrix in `.gitea/workflows/ci.yml`.
Steps 2 and 3 cannot be one commit: a job cannot run in an image that has not
been published yet.
## Architecture
The images are built natively on whichever runner picks the job, so they carry
that runner's architecture only. Every runner in the pool is arm64 today. If
one of a different architecture ever joins, it will overwrite these tags with
its own arch and the rest will fail to pull — at which point the build needs
`docker buildx` and a multi-arch manifest.
## If a push is refused
The build authenticates with `secrets.GITHUB_TOKEN`, the Actions task token.
If the registry ever refuses it, create a personal access token with
`package:write`, store it as the `REGISTRY_TOKEN` secret, and optionally set
the `REGISTRY_USER` variable — the workflow prefers both when present.
+11
View File
@@ -192,6 +192,17 @@ controls beneath it:
a **pending** invite, the grant is attached to that invite and **no second link is
sent**. An address that already has an account is treated as **Make available** instead.
Both student-picking controls vet every posted id with `Auth\RoleManager::isStudent()`
before acting on it — the same predicate the picker is built from, and the same one
`Booking\AdminBooking` guards a staff booking with. A posted id naming an instructor,
an administrator, or an account deleted since the page was drawn is skipped rather
than enrolled, so nothing can put a non-student on a roster or raise a payment
against one. Being a student is a matter of the **role**, not the `book_lesson`
capability, so a guardian's child and a signup still awaiting approval are both
fully enrollable — neither may enrol *themselves*, which is exactly what the studio
adding them is for. The reported count is what was actually added, so a skipped id
shows up as a smaller number.
When an email-invited person completes registration, `RegistrationPage` links their new
account to the grant (`GroupAccessRepository::linkStudentByEmail`), so the invite-only
class becomes enrollable for them — they choose whether to enrol.
+15 -2
View File
@@ -96,11 +96,13 @@ lesson. The times offered are the open slots of the next eight weeks — every
instructor's on the studio **Scheduler**, only the instructor's own on **My
Lessons**, which `AdminBooking::book()` re-checks rather than trusting the
posted slot id. The result is reported as a notice above the panel saying what
was booked and what it left owing; a refusal reopens the panel with the reason.
was booked and what it left owing; a refusal reopens the panel with the reason
and every field as it was submitted, so only the mistake needs correcting. A
booking that succeeds clears the form, so the next one does not inherit it.
It is the same booking a student makes — `LessonBooker` claims the slot(s),
writes the lesson row(s), and raises the payment exactly as `POST /bookings`
does — and differs in three deliberate ways:
does — and differs in four deliberate ways:
1. **No intake answers or policy acceptances are recorded at booking time.**
Those are the student's to give; staff ticking the boxes for them would be an
@@ -113,6 +115,17 @@ does — and differs in three deliberate ways:
whole series) is `confirmed` at once. Without the tick a pending payment is
raised at the lesson type's price, per-occurrence for a weekly reservation,
and the lesson confirms when it settles like any other.
4. **It can book for a student who cannot book at all.** The guard is
`Auth\RoleManager::isStudent()` — the student *role*, not the `book_lesson`
capability — so it covers a guardian's child and a self-signup still awaiting
approval alike, and is shared with the group-class **Add students directly**
and **Make available** controls so the two paths cannot drift. Both hold the role;
both have `book_lesson` withheld (`Guardian\ChildLoginGate`,
`Auth\RegistrationLoginGate`) so that neither can book in their own name.
That restriction is on them, not on the studio acting for them — and for a
child, whose account is never signed in to, it is the only route to a lesson
besides their guardian's. The picker and the guard therefore accept exactly
the same set, so nothing offered in the panel can be refused as ineligible.
A weekly reservation needs a time that actually repeats: asked for one on a
one-off slot, the form refuses (`not_weekly`) rather than quietly booking a
+21
View File
@@ -60,6 +60,27 @@ class RoleManager {
self::CAP_EXPORT_PAYMENTS,
];
/**
* Whether a user account is a student the studio may act for.
*
* Deliberately the role and not the `book_lesson` capability: that capability
* is withheld from a guardian's child ({@see \Unsupervised\Schedular\Guardian\ChildLoginGate})
* and from a self-signup still awaiting approval
* ({@see \Unsupervised\Schedular\Auth\RegistrationLoginGate}), so that neither
* can book or enrol *in their own name*. Staff booking or enrolling on their
* behalf is the case those restrictions exist to leave open — and for a child,
* whose account is never signed in to, it is the only route there is.
*
* Use this for every "may the studio register this person?" check, so the
* pickers staff choose from and the guards that vet their choice cannot drift
* into offering someone who is then refused.
*/
public static function isStudent( int $userId ): bool {
$user = $userId > 0 ? get_userdata( $userId ) : false;
return $user instanceof \WP_User && in_array( self::STUDENT, (array) $user->roles, true );
}
public function __construct( private AccessSettings $access = new AccessSettings() ) {}
public function register(): void {
+13 -4
View File
@@ -19,7 +19,7 @@ use Unsupervised\Schedular\Val;
* flow.
*
* It reuses `LessonBooker` — the same offering rules, the same atomic slot claim,
* the same billing — and differs from a student's own booking in exactly three
* the same billing — and differs from a student's own booking in exactly four
* ways, each deliberate:
*
* 1. **No intake questions or policy acceptances are recorded.** They are the
@@ -30,6 +30,10 @@ use Unsupervised\Schedular\Val;
* slot of the instructor's, including one only reachable past a deadline.
* 3. **It can be booked at no charge**, for a make-up or goodwill lesson, which
* skips the payment entirely and confirms the lesson at once.
* 4. **It can book for a student who cannot book at all** — a guardian's child,
* or someone still awaiting approval. Both hold the student role but have
* `book_lesson` withheld so that neither can book in their own name; that is
* a limit on them, never on the studio acting for them.
*/
class AdminBooking {
@@ -54,7 +58,10 @@ class AdminBooking {
* @return string|\WP_Error Success notice, or why nothing was booked.
*/
public function book( int $studentId, int $slotId, int $offeringId, string $recurrence, bool $noCharge, string $notes, int $onlyInstructorId = 0 ): string|\WP_Error {
if ( $studentId <= 0 || ! user_can( $studentId, RoleManager::CAP_BOOK_LESSON ) ) {
// The student role, not the `book_lesson` capability — see
// {@see RoleManager::isStudent()} for why a child and an unapproved signup
// must both be bookable for.
if ( ! RoleManager::isStudent( $studentId ) ) {
return new \WP_Error( 'invalid_student', __( 'Choose a student to book for.', 'unsupervised-schedular' ) );
}
@@ -250,8 +257,10 @@ class AdminBooking {
}
/**
* Everyone who can be booked for, by name — students and the children a
* guardian books for alike, since both hold `book_lesson`.
* Everyone who can be booked for, by name — every holder of the student role,
* which is exactly the set {@see book()} accepts. That deliberately includes
* the children a guardian books for and students still awaiting approval:
* neither may book in their own name, both may be booked for.
*
* @return list<array{id: int, name: string}>
*/
+50 -9
View File
@@ -177,7 +177,7 @@ class LessonController {
*
* @param list<array<string, mixed>> $rows
*/
// phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter -- $notice and $error are read by the included template.
// phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter -- $notice is read by the included template.
private function renderLessonsPage( array $rows, string $pageSlug, int $onlyInstructorId, string $notice, string $error ): void {
// View-state query params only (which view, which week) — nothing is
// mutated from them, so no nonce applies.
@@ -193,6 +193,11 @@ class LessonController {
$baseUrl = admin_url( 'admin.php?page=' . $pageSlug );
$bookForm = $this->adminBooking->formData( $onlyInstructorId );
// A refused booking is shown again as it was typed — losing five fields to a
// single mistake is what made the panel infuriating to correct. A successful
// one starts empty, so the next booking does not inherit the last one's.
$bookValues = '' !== $error ? $this->submittedBooking() : $this->emptyBooking();
include USC_PLUGIN_DIR . 'templates/admin/lessons.php';
}
@@ -229,23 +234,59 @@ class LessonController {
* @return array{string, string}
*/
private function bookForStudent( int $onlyInstructorId ): array {
// phpcs:disable WordPress.Security.NonceVerification.Missing -- nonce checked by the caller.
$submitted = $this->submittedBooking();
$result = $this->adminBooking->book(
absint( Val::int( $_POST['student_id'] ?? 0 ) ),
absint( Val::int( $_POST['slot_id'] ?? 0 ) ),
absint( Val::int( $_POST['offering_id'] ?? 0 ) ),
isset( $_POST['recurrence_weekly'] ) ? Lesson::RECURRENCE_WEEKLY : Lesson::RECURRENCE_SINGLE,
isset( $_POST['no_charge'] ),
sanitize_text_field( Val::string( wp_unslash( $_POST['notes'] ?? '' ) ) ),
$submitted['student_id'],
$submitted['slot_id'],
$submitted['offering_id'],
$submitted['weekly'] ? Lesson::RECURRENCE_WEEKLY : Lesson::RECURRENCE_SINGLE,
$submitted['no_charge'],
$submitted['notes'],
$onlyInstructorId
);
// phpcs:enable WordPress.Security.NonceVerification.Missing
return $result instanceof \WP_Error
? [ '', $result->get_error_message() ]
: [ $result, '' ];
}
/**
* The book-for-a-student form exactly as submitted. Read in one place so what
* gets booked and what the form shows again after a refusal cannot drift apart
* on a field name.
*
* @return array{student_id: int, slot_id: int, offering_id: int, weekly: bool, no_charge: bool, notes: string}
*/
private function submittedBooking(): array {
// phpcs:disable WordPress.Security.NonceVerification.Missing -- read only after handleFormAction() has verified the nonce: to book, or to re-render (escaped) a form it refused.
return [
'student_id' => absint( Val::int( $_POST['student_id'] ?? 0 ) ),
'slot_id' => absint( Val::int( $_POST['slot_id'] ?? 0 ) ),
'offering_id' => absint( Val::int( $_POST['offering_id'] ?? 0 ) ),
'weekly' => isset( $_POST['recurrence_weekly'] ),
'no_charge' => isset( $_POST['no_charge'] ),
'notes' => sanitize_text_field( Val::string( wp_unslash( $_POST['notes'] ?? '' ) ) ),
];
// phpcs:enable WordPress.Security.NonceVerification.Missing
}
/**
* An untouched book-for-a-student form.
*
* @return array{student_id: int, slot_id: int, offering_id: int, weekly: bool, no_charge: bool, notes: string}
*/
private function emptyBooking(): array {
return [
'student_id' => 0,
'slot_id' => 0,
'offering_id' => 0,
'weekly' => false,
'no_charge' => false,
'notes' => '',
];
}
/**
* Apply a per-lesson payment override. When $onlyOwn, the payment must belong
* to the current instructor.
+10 -2
View File
@@ -640,7 +640,15 @@ class GroupClassController {
}
/**
* The de-duplicated positive student ids posted from a multi-select.
* The de-duplicated student ids posted from a multi-select, keeping only ids
* that are actually students.
*
* The select is built from {@see studentOptions()}, but nothing stops a posted
* id naming an instructor, an administrator, or an account deleted since the
* page was drawn and enrolling one would write a roster row, and bill it,
* against someone who is not in the class. Vetting here covers both actions at
* once, and against the same {@see RoleManager::isStudent()} the picker uses,
* so a child or an unapproved signup is still perfectly enrollable.
*
* @return list<int>
*/
@@ -650,7 +658,7 @@ class GroupClassController {
$raw = (array) ( $_POST['student_ids'] ?? [] );
$ids = array_filter( array_map( static fn( mixed $v ): int => absint( Val::int( $v ) ), $raw ) );
return array_values( array_unique( $ids ) );
return array_values( array_filter( array_unique( $ids ), RoleManager::isStudent( ... ) ) );
}
/**
+7 -6
View File
@@ -16,6 +16,7 @@ if (! defined('ABSPATH')) {
* @var string $notice
* @var string $error
* @var array{students: list<array{id: int, name: string}>, offerings: list<array{id: int, label: string}>, slots: list<array{id: int, label: string, weekly: bool}>} $bookForm
* @var array{student_id: int, slot_id: int, offering_id: int, weekly: bool, no_charge: bool, notes: string} $bookValues
*/
?>
<div class="wrap">
@@ -54,7 +55,7 @@ if (! defined('ABSPATH')) {
<select name="student_id" id="usc-book-student" required>
<option value=""><?php esc_html_e('Choose a student', 'unsupervised-schedular'); ?></option>
<?php foreach ($bookForm['students'] as $student) : ?>
<option value="<?php echo esc_attr((string) $student['id']); ?>"><?php echo esc_html($student['name']); ?></option>
<option value="<?php echo esc_attr((string) $student['id']); ?>" <?php selected($bookValues['student_id'], $student['id']); ?>><?php echo esc_html($student['name']); ?></option>
<?php endforeach; ?>
</select>
</td>
@@ -65,7 +66,7 @@ if (! defined('ABSPATH')) {
<select name="slot_id" id="usc-book-slot" required style="max-width:100%;">
<option value=""><?php esc_html_e('Choose an open time', 'unsupervised-schedular'); ?></option>
<?php foreach ($bookForm['slots'] as $slot) : ?>
<option value="<?php echo esc_attr((string) $slot['id']); ?>"><?php echo esc_html($slot['label']); ?></option>
<option value="<?php echo esc_attr((string) $slot['id']); ?>" <?php selected($bookValues['slot_id'], $slot['id']); ?>><?php echo esc_html($slot['label']); ?></option>
<?php endforeach; ?>
</select>
</td>
@@ -76,7 +77,7 @@ if (! defined('ABSPATH')) {
<select name="offering_id" id="usc-book-offering" style="max-width:100%;">
<option value="0"><?php esc_html_e('Use the time\'s own lesson type', 'unsupervised-schedular'); ?></option>
<?php foreach ($bookForm['offerings'] as $offering) : ?>
<option value="<?php echo esc_attr((string) $offering['id']); ?>"><?php echo esc_html($offering['label']); ?></option>
<option value="<?php echo esc_attr((string) $offering['id']); ?>" <?php selected($bookValues['offering_id'], $offering['id']); ?>><?php echo esc_html($offering['label']); ?></option>
<?php endforeach; ?>
</select>
<p class="description"><?php esc_html_e('A time already tied to a lesson type is booked as that type; a general time needs one chosen here.', 'unsupervised-schedular'); ?></p>
@@ -86,12 +87,12 @@ if (! defined('ABSPATH')) {
<th scope="row"><?php esc_html_e('Options', 'unsupervised-schedular'); ?></th>
<td>
<label>
<input type="checkbox" name="recurrence_weekly" value="1">
<input type="checkbox" name="recurrence_weekly" value="1" <?php checked($bookValues['weekly']); ?>>
<?php esc_html_e('Reserve this time weekly for the rest of the term', 'unsupervised-schedular'); ?>
</label>
<p class="description"><?php esc_html_e('Only for a time that repeats weekly. Billed upfront as one payment.', 'unsupervised-schedular'); ?></p>
<label>
<input type="checkbox" name="no_charge" value="1">
<input type="checkbox" name="no_charge" value="1" <?php checked($bookValues['no_charge']); ?>>
<?php esc_html_e('No charge — book it free and confirm it now', 'unsupervised-schedular'); ?>
</label>
<p class="description"><?php esc_html_e('For a make-up or goodwill lesson. Otherwise a pending payment is raised at the lesson type\'s price.', 'unsupervised-schedular'); ?></p>
@@ -99,7 +100,7 @@ if (! defined('ABSPATH')) {
</tr>
<tr>
<th scope="row"><label for="usc-book-notes"><?php esc_html_e('Notes', 'unsupervised-schedular'); ?></label></th>
<td><input type="text" name="notes" id="usc-book-notes" class="regular-text" maxlength="500"></td>
<td><input type="text" name="notes" id="usc-book-notes" class="regular-text" maxlength="500" value="<?php echo esc_attr($bookValues['notes']); ?>"></td>
</tr>
</table>
<p>
+44
View File
@@ -155,4 +155,48 @@ class RoleManagerTest extends TestCase
(new RoleManager())->createRoles();
}
/**
* The predicate every staff-side "register this person" path shares. It is
* deliberately the role and not `book_lesson`, so the two accounts that have
* that capability withheld a guardian's child and an unapproved signup are
* still people the studio can act for.
*/
public function testIsStudentAcceptsAnyHolderOfTheStudentRole(): void
{
Functions\when('get_userdata')->justReturn($this->userWithRoles([RoleManager::STUDENT]));
self::assertTrue(RoleManager::isStudent(5));
}
public function testIsStudentRejectsSomeoneWhoIsNotAStudent(): void
{
Functions\when('get_userdata')->justReturn($this->userWithRoles([RoleManager::INSTRUCTOR]));
self::assertFalse(RoleManager::isStudent(5));
}
public function testIsStudentRejectsAnAccountThatNoLongerExists(): void
{
Functions\when('get_userdata')->justReturn(false);
self::assertFalse(RoleManager::isStudent(5));
}
public function testIsStudentRejectsNoOneChosenWithoutLookingAnyoneUp(): void
{
Functions\expect('get_userdata')->never();
self::assertFalse(RoleManager::isStudent(0));
}
/** @param list<string> $roles */
private function userWithRoles(array $roles): \WP_User
{
$user = \Mockery::mock(\WP_User::class);
$user->ID = 5;
$user->roles = $roles;
return $user;
}
}
+72 -6
View File
@@ -5,6 +5,7 @@ namespace Unsupervised\Schedular\Tests\Unit\Booking;
use Brain\Monkey\Functions;
use Mockery;
use Unsupervised\Schedular\Auth\RoleManager;
use Unsupervised\Schedular\Availability\AvailabilityRepository;
use Unsupervised\Schedular\Availability\AvailabilitySlot;
use Unsupervised\Schedular\Booking\AdminBooking;
@@ -36,10 +37,10 @@ class AdminBookingTest extends TestCase
Functions\when('mysql2date')->alias(
static fn (string $format, string $date): string => date($format, (int) strtotime($date))
);
Functions\when('get_userdata')->justReturn(false);
// The picker offers holders of the student role, and that is what the guard
// accepts; it is exercised on its own below.
Functions\when('get_userdata')->alias(fn (int $id): \WP_User => $this->user($id, 'Jane Doe'));
Functions\when('get_users')->justReturn([]);
// Everyone offered in the picker can book; the guard is exercised on its own.
Functions\when('user_can')->justReturn(true);
// The staff member doing the booking; stamped on the lesson as booked_by.
Functions\when('get_current_user_id')->justReturn(3);
@@ -175,9 +176,9 @@ class AdminBookingTest extends TestCase
self::assertSame('slot_taken', $result->get_error_code());
}
public function testSomeoneWhoCannotBookLessonsIsRefused(): void
public function testSomeoneWhoIsNotAStudentIsRefused(): void
{
Functions\when('user_can')->justReturn(false);
Functions\when('get_userdata')->alias(fn (int $id): \WP_User => $this->user($id, 'Jane Doe', [RoleManager::INSTRUCTOR]));
$this->availability->shouldReceive('findById')->never();
$result = $this->admin->book(42, 7, 3, Lesson::RECURRENCE_SINGLE, false, '');
@@ -186,6 +187,69 @@ class AdminBookingTest extends TestCase
self::assertSame('invalid_student', $result->get_error_code());
}
public function testAnAccountThatNoLongerExistsIsRefused(): void
{
Functions\when('get_userdata')->justReturn(false);
$this->availability->shouldReceive('findById')->never();
$result = $this->admin->book(42, 7, 3, Lesson::RECURRENCE_SINGLE, false, '');
self::assertInstanceOf(\WP_Error::class, $result);
self::assertSame('invalid_student', $result->get_error_code());
}
public function testNoStudentChosenIsRefusedWithoutLookingAnyoneUp(): void
{
Functions\expect('get_userdata')->never();
$this->availability->shouldReceive('findById')->never();
$result = $this->admin->book(0, 7, 3, Lesson::RECURRENCE_SINGLE, false, '');
self::assertInstanceOf(\WP_Error::class, $result);
self::assertSame('invalid_student', $result->get_error_code());
}
/**
* A child holds the student role but never `book_lesson` withheld so the
* account cannot book in its own name. The studio booking for them is the only
* route a child has to a lesson, so it must not be blocked by that.
*/
public function testBooksForAGuardiansChildWhoCannotBookThemselves(): void
{
Functions\when('user_can')->justReturn(false);
$this->availability->shouldReceive('findById')->with(7)->andReturn($this->slot());
$this->offerings->shouldReceive('findById')->with(3)->andReturn($this->offering());
$this->availability->shouldReceive('claim')->once()->with(7)->andReturn(true);
$this->bookings->shouldReceive('insert')->once()->andReturn(100);
$this->payments->shouldReceive('createForRegistration')->once()->andReturn($this->pendingPayment());
$notice = $this->admin->book(42, 7, 3, Lesson::RECURRENCE_SINGLE, false, '');
self::assertIsString($notice);
self::assertStringContainsString('30 min piano', $notice);
}
/**
* Same for a self-signup the studio has not approved yet: the front desk can
* still get them onto the calendar while the paperwork catches up.
*/
public function testBooksForAStudentStillAwaitingApproval(): void
{
Functions\when('user_can')->justReturn(false);
$this->availability->shouldReceive('findById')->with(7)->andReturn($this->slot());
$this->offerings->shouldReceive('findById')->with(3)->andReturn($this->offering());
$this->availability->shouldReceive('claim')->once()->with(7)->andReturn(true);
$this->bookings->shouldReceive('insert')->once()->with(Mockery::on(
static fn (Lesson $l): bool => 42 === $l->studentId
))->andReturn(100);
$this->payments->shouldReceive('createForRegistration')->once()->andReturn($this->pendingPayment());
$notice = $this->admin->book(42, 7, 3, Lesson::RECURRENCE_SINGLE, false, '');
self::assertIsString($notice);
self::assertStringContainsString('pending payment', $notice);
}
public function testATiedTimeCannotBeBookedAsADifferentLessonType(): void
{
$this->availability->shouldReceive('findById')->with(7)->andReturn($this->slot(offeringId: 3));
@@ -299,10 +363,12 @@ class AdminBookingTest extends TestCase
);
}
private function user(int $id, string $name): \WP_User
/** @param list<string> $roles */
private function user(int $id, string $name, array $roles = [RoleManager::STUDENT]): \WP_User
{
$user = Mockery::mock(\WP_User::class);
$user->ID = $id;
$user->roles = $roles;
$user->first_name = '';
$user->last_name = '';
$user->nickname = $name;
@@ -316,6 +316,54 @@ class LessonControllerTest extends TestCase
self::assertStringContainsString('name="usc_action" value="book_for_student"', $html);
}
/**
* A refusal used to clear all five fields, so one mistake meant retyping the
* whole form and the panel is only ever reopened *because* something was
* refused.
*/
public function testARefusedBookingComesBackWithEveryFieldStillFilledIn(): void
{
$this->postBooking();
$this->offerPanel();
$this->adminBooking->shouldReceive('book')->once()->andReturn(
new \WP_Error('slot_taken', 'That time has already been booked.')
);
$this->bookings->shouldReceive('findAllUpcoming')->once()->andReturn([]);
$html = $this->render();
self::assertStringContainsString('That time has already been booked.', $html);
// Attribute spacing in the template is not what is under test here.
$tags = (string) preg_replace('/\s+/', ' ', $html);
// The panel is reopened, showing the student, time and lesson type as posted.
self::assertStringContainsString(' open>', $html);
self::assertStringContainsString('value="42" selected=\'selected\'', $tags);
self::assertStringContainsString('value="7" selected=\'selected\'', $tags);
self::assertStringContainsString('value="3" selected=\'selected\'', $tags);
// Both ticks and the note survive too.
self::assertSame(2, substr_count($html, "checked='checked'"));
self::assertStringContainsString('value="Make-up lesson"', $html);
}
public function testASuccessfulBookingLeavesAnEmptyFormForTheNextOne(): void
{
$this->postBooking();
$this->offerPanel();
$this->adminBooking->shouldReceive('book')->once()->andReturn('Booked.');
$this->bookings->shouldReceive('findAllUpcoming')->once()->andReturn([]);
$html = $this->render();
self::assertStringContainsString('Booked.', $html);
// Nothing carried over, or the next booking silently inherits this one's.
self::assertStringNotContainsString("selected='selected'", $html);
self::assertStringNotContainsString("checked='checked'", $html);
self::assertStringContainsString('value=""', $html);
}
public function testTheStudioSchedulerBooksAgainstAnyInstructorsTime(): void
{
$this->postBooking();
@@ -390,6 +438,16 @@ class LessonControllerTest extends TestCase
Functions\when('absint')->alias(static fn ($v): int => abs((int) $v));
}
/** The panel with one of each choice, so a re-selected value has somewhere to land. */
private function offerPanel(): void
{
$this->adminBooking->shouldReceive('formData')->once()->andReturn([
'students' => [['id' => 42, 'name' => 'Ada Lovelace']],
'offerings' => [['id' => 3, 'label' => '30 min piano (30 min)']],
'slots' => [['id' => 7, 'label' => 'Wed Jul 1, 2026 10:00 AM (30 min)', 'weekly' => false]],
]);
}
public function testAStaffBookedLessonOffersTheRecordIntakeForm(): void
{
$_GET['lesson_id'] = '1';
@@ -7,6 +7,7 @@ use Brain\Monkey\Functions;
use Mockery;
use Unsupervised\Schedular\Auth\InviteRepository;
use Unsupervised\Schedular\Auth\RegistrationMailer;
use Unsupervised\Schedular\Auth\RoleManager;
use Unsupervised\Schedular\GroupClass\Enrollment;
use Unsupervised\Schedular\GroupClass\EnrollmentRepository;
use Unsupervised\Schedular\GroupClass\GroupAccessRepository;
@@ -87,7 +88,8 @@ class GroupClassControllerTest extends TestCase
{
[$first, $last] = array_pad(explode(' ', $full, 2), 2, '');
$user = Mockery::mock(\WP_User::class);
$user = Mockery::mock(\WP_User::class);
$user->roles = [RoleManager::STUDENT];
$user->first_name = $first;
$user->last_name = $last;
$user->nickname = $full;
@@ -406,6 +408,8 @@ class GroupClassControllerTest extends TestCase
Functions\when('wp_unslash')->returnArg();
Functions\when('sanitize_email')->returnArg();
Functions\when('absint')->alias(static fn ($v) => abs((int) $v));
// Every posted id is vetted as a student before it is enrolled or granted.
Functions\when('get_userdata')->justReturn($this->userNamed('Ada Lovelace'));
// Render tail: no classes/enrolments to draw so the assertion targets the notice.
$this->offerings->shouldReceive('findAll')->with(3, Offering::KIND_GROUP_CLASS)->andReturn([]);
@@ -544,6 +548,64 @@ class GroupClassControllerTest extends TestCase
$this->audit->shouldReceive('acceptances')->with($enrollment)->andReturn([]);
}
/**
* The multi-select is built from the studio's students, but a posted id is just
* a number: it could name an instructor, an administrator, or an account
* deleted since the page was drawn. Enrolling one would put a non-student on
* the roster and raise a payment against them.
*/
public function testAddDirectIgnoresAnIdThatIsNotAStudent(): void
{
$_POST = ['usc_action' => 'add_direct', 'offering_id' => 8, 'student_ids' => [5]];
$this->stubActionContext();
$instructor = Mockery::mock(\WP_User::class);
$instructor->roles = [RoleManager::INSTRUCTOR];
Functions\when('get_userdata')->justReturn($instructor);
$this->offerings->shouldReceive('findById')->with(8)->andReturn($this->inviteOnlyOffering(100.0));
$this->enrollments->shouldReceive('insert')->never();
$this->paymentService->shouldReceive('createForRegistration')->never();
$this->access->shouldReceive('markEnrolled')->never();
$html = $this->renderInstructor();
self::assertStringContainsString('0 student(s) added to the class.', $html);
}
public function testAddDirectIgnoresAnAccountThatNoLongerExists(): void
{
$_POST = ['usc_action' => 'add_direct', 'offering_id' => 8, 'student_ids' => [5]];
$this->stubActionContext();
Functions\when('get_userdata')->justReturn(false);
$this->offerings->shouldReceive('findById')->with(8)->andReturn($this->inviteOnlyOffering(100.0));
$this->enrollments->shouldReceive('insert')->never();
$this->paymentService->shouldReceive('createForRegistration')->never();
$html = $this->renderInstructor();
self::assertStringContainsString('0 student(s) added to the class.', $html);
}
public function testGrantAccessIgnoresAnIdThatIsNotAStudent(): void
{
$_POST = ['usc_action' => 'grant_access', 'offering_id' => 8, 'student_ids' => [5]];
$this->stubActionContext();
$instructor = Mockery::mock(\WP_User::class);
$instructor->roles = [RoleManager::INSTRUCTOR];
Functions\when('get_userdata')->justReturn($instructor);
$this->offerings->shouldReceive('findById')->with(8)->andReturn($this->inviteOnlyOffering());
$this->access->shouldReceive('insert')->never();
$this->mailer->shouldReceive('sendClassAccessGranted')->never();
$html = $this->renderInstructor();
self::assertStringContainsString('0 student(s) granted access.', $html);
}
public function testGrantAccessCreatesGrantAndEmailsStudent(): void
{
$_POST = ['usc_action' => 'grant_access', 'offering_id' => 8, 'student_ids' => [5]];
@@ -554,7 +616,8 @@ class GroupClassControllerTest extends TestCase
$this->access->shouldReceive('hasGrant')->with(8, 5)->andReturn(false);
$this->access->shouldReceive('insert')->once()->andReturn(1);
$user = Mockery::mock(\WP_User::class);
$user = Mockery::mock(\WP_User::class);
$user->roles = [RoleManager::STUDENT];
$user->user_email = '[email protected]';
Functions\when('get_userdata')->justReturn($user);
$this->mailer->shouldReceive('sendClassAccessGranted')->once()->with($user, 'Private Choir')->andReturn(true);
+2 -2
View File
@@ -3,7 +3,7 @@
* Plugin Name: Unsupervised Scheduler
* Plugin URI: https://git.unsupervised.ca/Unsupervised/unsupervised-scheduler
* Description: Instructor/student lesson scheduling for WordPress.
* Version: 1.5.3
* Version: 1.5.4
* Requires at least: 6.2
* Requires PHP: 8.1
* Author: Unsupervised
@@ -21,7 +21,7 @@ if (! defined('ABSPATH')) {
exit;
}
define('USC_VERSION', '1.5.3');
define('USC_VERSION', '1.5.4');
define('USC_PLUGIN_FILE', __FILE__);
define('USC_PLUGIN_DIR', plugin_dir_path(__FILE__));
define('USC_PLUGIN_URL', plugin_dir_url(__FILE__));