Compare commits
26
Commits
v1.5.3
..
9c6b4415cd
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9c6b4415cd
|
||
|
|
74df3f5ba8
|
||
|
|
f8f211929c | ||
|
|
61a40f0e8d | ||
|
|
88a8d0ae5c
|
||
|
|
e2da45a1b0
|
||
|
|
dfa29745df | ||
|
|
54a8b906f7
|
||
|
|
b3ed3a67d5 | ||
|
|
908b7fcd1f
|
||
|
|
b17adf02ff
|
||
|
|
a23490ec80 | ||
|
|
1552bf4b5f
|
||
|
|
f8762e1095
|
||
|
|
c9d18fec74
|
||
|
|
ab609898d6
|
||
|
|
4bc8e80837 | ||
|
|
28f586d207
|
||
|
|
e097b7a0bf | ||
|
|
572aaf5b49
|
||
|
|
116394f2ff | ||
|
|
dda8386c1f | ||
|
|
7278309bf5 | ||
|
|
5ce42f0003
|
||
|
|
76530878b5 | ||
|
|
6031a75012 |
+45
-113
@@ -7,56 +7,31 @@ on:
|
||||
- develop
|
||||
pull_request:
|
||||
|
||||
# Jobs that need PHP run inside the shared CI images maintained in the
|
||||
# Unsupervised/ci-php repository. PHP, Composer, the intl and zip extensions
|
||||
# and the GNU CLI tools are already in the image, so there is no toolchain
|
||||
# setup step in any job here.
|
||||
#
|
||||
# The registry path is written out at each use because
|
||||
# jobs.<id>.container.image cannot read the `env` context.
|
||||
|
||||
jobs:
|
||||
# PHPCS and PHPStan share a job so the two of them draw once on Setup PHP
|
||||
# rather than twice. That step is slow and intermittently fails on 8.3
|
||||
# (see #178), so every job that can be folded into another is one less
|
||||
# chance for a run to fall over. They run as separate steps, and PHPCS
|
||||
# failing stops the job before PHPStan reports.
|
||||
quality:
|
||||
name: Coding Standards & Static Analysis
|
||||
phpcs:
|
||||
name: Coding Standards
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: git.unsupervised.ca/unsupervised/ci-php:8.3
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# setup-php installs PHP 8.3+ through php-builder, which apt-installs ~70
|
||||
# -dev packages before unpacking the build (#178). Ubuntu's image deletes
|
||||
# the .debs after install, so every job re-downloads them. Keeping them
|
||||
# and restoring them from the cache server, which lives in the cluster,
|
||||
# turns a WAN download into a local one.
|
||||
#
|
||||
# Keyed per PHP version because the install path differs by version and
|
||||
# the package sets are not interchangeable: 8.3+ pulls the ~70 -dev
|
||||
# packages through php-builder, while 8.1 and 8.2 come from the ondrej
|
||||
# PPA as a handful of runtime packages. Sharing one key across both lets
|
||||
# whichever job finishes first decide what the others restore, and 8.1 is
|
||||
# always first.
|
||||
#
|
||||
# Only the .debs are cached, never /var/lib/apt/lists — a stale index is
|
||||
# how you get 404s mid-install.
|
||||
- name: Keep downloaded .debs
|
||||
run: |
|
||||
sudo rm -f /etc/apt/apt.conf.d/docker-clean
|
||||
echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' \
|
||||
| sudo tee /etc/apt/apt.conf.d/99keep-downloaded-packages >/dev/null
|
||||
|
||||
- name: Cache apt packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /var/cache/apt/archives/*.deb
|
||||
key: apt-php8.3-${{ runner.arch }}-v1
|
||||
|
||||
- name: Setup PHP
|
||||
uses: shivammathur/setup-php@v2
|
||||
with:
|
||||
php-version: '8.3'
|
||||
tools: composer:v2
|
||||
|
||||
# COMPOSER_HOME is /composer in the image, so that is where the
|
||||
# download cache lives. composer.lock is what fingerprints the
|
||||
# dependency set.
|
||||
- name: Cache Composer packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/.composer/cache
|
||||
key: composer-${{ hashFiles('composer.json') }}
|
||||
path: /composer/cache
|
||||
key: composer-${{ hashFiles('composer.lock') }}
|
||||
|
||||
- name: Install dependencies
|
||||
run: composer install --prefer-dist --no-progress --no-interaction
|
||||
@@ -64,15 +39,36 @@ jobs:
|
||||
- name: Run PHPCS
|
||||
run: composer cs
|
||||
|
||||
phpstan:
|
||||
name: Static Analysis
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: git.unsupervised.ca/unsupervised/ci-php:8.3
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- name: Cache Composer packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /composer/cache
|
||||
key: composer-${{ hashFiles('composer.lock') }}
|
||||
|
||||
- name: Install dependencies
|
||||
run: composer install --prefer-dist --no-progress --no-interaction
|
||||
|
||||
- name: Run PHPStan
|
||||
run: composer lint
|
||||
|
||||
test:
|
||||
name: Tests (PHP ${{ matrix.php }})
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: git.unsupervised.ca/unsupervised/ci-php:${{ matrix.php }}
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
# A version can only be added here once ci-php publishes the matching
|
||||
# tag.
|
||||
php:
|
||||
- '8.1'
|
||||
- '8.2'
|
||||
@@ -81,46 +77,11 @@ jobs:
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# setup-php installs PHP 8.3+ through php-builder, which apt-installs ~70
|
||||
# -dev packages before unpacking the build (#178). Ubuntu's image deletes
|
||||
# the .debs after install, so every job re-downloads them. Keeping them
|
||||
# and restoring them from the cache server, which lives in the cluster,
|
||||
# turns a WAN download into a local one.
|
||||
#
|
||||
# Keyed per PHP version because the install path differs by version and
|
||||
# the package sets are not interchangeable: 8.3+ pulls the ~70 -dev
|
||||
# packages through php-builder, while 8.1 and 8.2 come from the ondrej
|
||||
# PPA as a handful of runtime packages. Sharing one key across both lets
|
||||
# whichever job finishes first decide what the others restore, and 8.1 is
|
||||
# always first.
|
||||
#
|
||||
# Only the .debs are cached, never /var/lib/apt/lists — a stale index is
|
||||
# how you get 404s mid-install.
|
||||
- name: Keep downloaded .debs
|
||||
run: |
|
||||
sudo rm -f /etc/apt/apt.conf.d/docker-clean
|
||||
echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' \
|
||||
| sudo tee /etc/apt/apt.conf.d/99keep-downloaded-packages >/dev/null
|
||||
|
||||
- name: Cache apt packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /var/cache/apt/archives/*.deb
|
||||
key: apt-php${{ matrix.php }}-${{ runner.arch }}-v1
|
||||
|
||||
- name: Setup PHP
|
||||
uses: shivammathur/setup-php@v2
|
||||
with:
|
||||
php-version: ${{ matrix.php }}
|
||||
extensions: mbstring, intl
|
||||
coverage: none
|
||||
tools: composer:v2
|
||||
|
||||
- name: Cache Composer packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/.composer/cache
|
||||
key: ${{ matrix.php }}-composer-${{ hashFiles('composer.json') }}
|
||||
path: /composer/cache
|
||||
key: ${{ matrix.php }}-composer-${{ hashFiles('composer.lock') }}
|
||||
|
||||
- name: Install dependencies
|
||||
run: composer install --prefer-dist --no-progress --no-interaction
|
||||
@@ -128,6 +89,8 @@ jobs:
|
||||
- name: Run PHPUnit
|
||||
run: composer test
|
||||
|
||||
# Runs on the runner image rather than a container: it needs no PHP, and it
|
||||
# uses GNU grep's --include.
|
||||
no-debug:
|
||||
name: No Debug Code
|
||||
runs-on: ubuntu-latest
|
||||
@@ -144,46 +107,15 @@ jobs:
|
||||
build:
|
||||
name: Build Plugin Zip
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: git.unsupervised.ca/unsupervised/ci-php:8.3
|
||||
# Only build a shippable artifact once changes land on main, and only
|
||||
# after the quality gates pass.
|
||||
needs: [quality, test, no-debug]
|
||||
needs: [phpcs, phpstan, test, no-debug]
|
||||
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
# setup-php installs PHP 8.3+ through php-builder, which apt-installs ~70
|
||||
# -dev packages before unpacking the build (#178). Ubuntu's image deletes
|
||||
# the .debs after install, so every job re-downloads them. Keeping them
|
||||
# and restoring them from the cache server, which lives in the cluster,
|
||||
# turns a WAN download into a local one.
|
||||
#
|
||||
# Keyed per PHP version because the install path differs by version and
|
||||
# the package sets are not interchangeable: 8.3+ pulls the ~70 -dev
|
||||
# packages through php-builder, while 8.1 and 8.2 come from the ondrej
|
||||
# PPA as a handful of runtime packages. Sharing one key across both lets
|
||||
# whichever job finishes first decide what the others restore, and 8.1 is
|
||||
# always first.
|
||||
#
|
||||
# Only the .debs are cached, never /var/lib/apt/lists — a stale index is
|
||||
# how you get 404s mid-install.
|
||||
- name: Keep downloaded .debs
|
||||
run: |
|
||||
sudo rm -f /etc/apt/apt.conf.d/docker-clean
|
||||
echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' \
|
||||
| sudo tee /etc/apt/apt.conf.d/99keep-downloaded-packages >/dev/null
|
||||
|
||||
- name: Cache apt packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /var/cache/apt/archives/*.deb
|
||||
key: apt-php8.3-${{ runner.arch }}-v1
|
||||
|
||||
- name: Setup PHP
|
||||
uses: shivammathur/setup-php@v2
|
||||
with:
|
||||
php-version: '8.3'
|
||||
tools: composer:v2
|
||||
|
||||
- name: Build plugin zip
|
||||
run: composer build
|
||||
|
||||
|
||||
@@ -15,15 +15,13 @@ jobs:
|
||||
release:
|
||||
name: Build and Publish Release
|
||||
runs-on: ubuntu-latest
|
||||
# The shared CI image carries composer, curl, jq and the GNU coreutils
|
||||
# the steps below shell out to. See docs/ci.md.
|
||||
container:
|
||||
image: git.unsupervised.ca/unsupervised/ci-php:8.3
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- name: Setup PHP
|
||||
uses: shivammathur/setup-php@v2
|
||||
with:
|
||||
php-version: '8.3'
|
||||
tools: composer:v2
|
||||
|
||||
# A tag that disagrees with the plugin header would make sites see a
|
||||
# phantom update forever (or never see a real one), so fail fast.
|
||||
- name: Verify tag matches plugin version
|
||||
@@ -37,6 +35,14 @@ jobs:
|
||||
fi
|
||||
echo "version=${header_version}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
# COMPOSER_HOME is /composer in the image, so that is where the
|
||||
# download cache lives.
|
||||
- name: Cache Composer packages
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /composer/cache
|
||||
key: composer-${{ hashFiles('composer.lock') }}
|
||||
|
||||
- name: Install dependencies
|
||||
run: composer install --prefer-dist --no-progress --no-interaction
|
||||
|
||||
@@ -143,6 +149,50 @@ jobs:
|
||||
{ print }
|
||||
' CHANGELOG.md > CHANGELOG.md.tmp && mv CHANGELOG.md.tmp CHANGELOG.md
|
||||
|
||||
# main requires signed commits, and Gitea refuses to merge a pull request
|
||||
# that carries an unsigned one. The key Gitea signs merge commits with
|
||||
# lives on the server and is not reachable from a runner, so the bump
|
||||
# commit is signed here with a dedicated release-bot key that the instance
|
||||
# trusts via TRUSTED_SSH_KEYS. Generating that key, trusting it and storing
|
||||
# the secret is documented in docs/ci.md.
|
||||
- name: Configure signing as Release Bot
|
||||
env:
|
||||
SIGNING_KEY: ${{ secrets.RELEASE_BOT_SIGNING_KEY }}
|
||||
run: |
|
||||
if [ -z "${SIGNING_KEY}" ]; then
|
||||
echo "RELEASE_BOT_SIGNING_KEY is not set - the bump commit would be unsigned and unmergeable." >&2
|
||||
exit 1
|
||||
fi
|
||||
if ! command -v ssh-keygen > /dev/null; then
|
||||
echo "ssh-keygen is missing from the runner image; git cannot make SSH signatures without it." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# The secret holds an OpenSSH private key ("-----BEGIN OPENSSH PRIVATE
|
||||
# KEY-----"). git signs by shelling out to ssh-keygen, which wants that
|
||||
# key on disk next to the .pub it is pointed at, readable only by us,
|
||||
# and rejects it unless the final newline survived the round trip.
|
||||
keydir="${RUNNER_TEMP:-${TMPDIR:-/tmp}}/release-bot-signing"
|
||||
install -m 700 -d "${keydir}"
|
||||
printf '%s\n' "${SIGNING_KEY}" | tr -d '\r' > "${keydir}/key"
|
||||
chmod 600 "${keydir}/key"
|
||||
# Doubles as a format check: a truncated or re-wrapped key fails here,
|
||||
# with a clearer cause than "gpg failed to sign the data" later on.
|
||||
if ! ssh-keygen -y -f "${keydir}/key" < /dev/null > "${keydir}/key.pub"; then
|
||||
echo "RELEASE_BOT_SIGNING_KEY is not a usable OpenSSH private key (passphrase-protected, truncated, or re-wrapped on paste)." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# No Gitea account backs this address; TRUSTED_SSH_KEYS verifies the
|
||||
# signature without an account lookup, so it is a label, not an identity.
|
||||
git config user.name 'Release Bot'
|
||||
git config user.email '[email protected]'
|
||||
# Named gpg.format for historical reasons; "ssh" is what switches git
|
||||
# over to signing with the SSH key above rather than a GPG key.
|
||||
git config gpg.format ssh
|
||||
git config user.signingkey "${keydir}/key.pub"
|
||||
git config commit.gpgsign true
|
||||
|
||||
- name: Open pull request
|
||||
env:
|
||||
TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
@@ -151,10 +201,14 @@ jobs:
|
||||
branch="release/bump-${next}"
|
||||
api="${GITHUB_SERVER_URL}/api/v1/repos/${GITHUB_REPOSITORY}"
|
||||
|
||||
git config user.name 'Release Bot'
|
||||
git config user.email '[email protected]'
|
||||
git checkout -b "${branch}"
|
||||
git commit -am "Bump version to ${next} and open changelog section"
|
||||
# A commit that came out unsigned would otherwise go unnoticed until
|
||||
# someone tried to merge the PR, so fail here instead.
|
||||
if ! git cat-file commit HEAD | grep -q '^gpgsig'; then
|
||||
echo "Bump commit is unsigned; refusing to push it." >&2
|
||||
exit 1
|
||||
fi
|
||||
git push origin "${branch}"
|
||||
|
||||
curl -fsS -X POST "${api}/pulls" \
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
vendor/
|
||||
composer.lock
|
||||
coverage/
|
||||
.phpunit.result.cache
|
||||
*.log
|
||||
|
||||
@@ -11,6 +11,20 @@ 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.6]
|
||||
|
||||
## [1.5.5]
|
||||
|
||||
### Changed
|
||||
- **Payments moved up to version 21 of Stripe's PHP library**, from version 17. Being four major versions behind also meant asking Stripe to behave like an older version of its API; the plugin now uses API version `2026-07-29.dahlia`. Taking a card payment and handling a webhook are unchanged — the same charge is raised, the same events are honoured, and a forged webhook is still rejected. Nothing to do on your side.
|
||||
|
||||
## [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
|
||||
|
||||
@@ -23,6 +23,10 @@ INCLUDE=(
|
||||
"$SLUG.php"
|
||||
"uninstall.php"
|
||||
"composer.json"
|
||||
# Staged so the production install below resolves to the locked versions
|
||||
# rather than whatever is newest that day. Both are deleted again before
|
||||
# the zip is written.
|
||||
"composer.lock"
|
||||
"src"
|
||||
"templates"
|
||||
"assets"
|
||||
|
||||
+4
-1
@@ -5,7 +5,7 @@
|
||||
"license": "GPL-2.0-or-later",
|
||||
"require": {
|
||||
"php": ">=8.1",
|
||||
"stripe/stripe-php": "^17.0"
|
||||
"stripe/stripe-php": "^21.0"
|
||||
},
|
||||
"require-dev": {
|
||||
"phpunit/phpunit": "^10.5",
|
||||
@@ -36,6 +36,9 @@
|
||||
"build": "bash bin/build-zip.sh"
|
||||
},
|
||||
"config": {
|
||||
"platform": {
|
||||
"php": "8.1"
|
||||
},
|
||||
"allow-plugins": {
|
||||
"dealerdirect/phpcodesniffer-composer-installer": true
|
||||
}
|
||||
|
||||
Generated
+2589
File diff suppressed because it is too large
Load Diff
+101
@@ -0,0 +1,101 @@
|
||||
# CI
|
||||
|
||||
CI and release jobs do not install PHP. They run inside the shared images
|
||||
maintained in [Unsupervised/ci-php](https://git.unsupervised.ca/Unsupervised/ci-php):
|
||||
|
||||
```
|
||||
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 they pull anonymously — no registry
|
||||
credentials in any job here. What the images contain, how they are published,
|
||||
and how to add a PHP version are documented in that repository's README.
|
||||
|
||||
## Which job runs where
|
||||
|
||||
| Job | Runs in |
|
||||
|---|---|
|
||||
| Coding Standards (PHPCS) | `ci-php:8.3` |
|
||||
| Static Analysis (PHPStan) | `ci-php:8.3` |
|
||||
| Tests | `ci-php:${{ matrix.php }}` |
|
||||
| Build Plugin Zip | `ci-php:8.3` |
|
||||
| No Debug Code | runner image — no PHP, and it uses GNU `grep --include` |
|
||||
| Open next-version bump PR (release.yml) | runner image — no PHP |
|
||||
|
||||
PHPCS and PHPStan are separate jobs so a coding-standards failure still lets
|
||||
the static analysis result through. They run in parallel.
|
||||
|
||||
## Composer
|
||||
|
||||
`composer.lock` is committed, so every job installs the same dependency set
|
||||
and two builds of the same tag ship the same vendor tree. `bin/build-zip.sh`
|
||||
stages the lock into its build directory for the same reason, then removes it
|
||||
before writing the zip.
|
||||
|
||||
The Composer download cache lives at `/composer/cache` — `COMPOSER_HOME` is
|
||||
`/composer` in the image — and is keyed on `composer.lock`.
|
||||
|
||||
## Adding a PHP version to the test matrix
|
||||
|
||||
The image has to exist first. Add the version to the `php` matrix in
|
||||
`ci-php`'s `.gitea/workflows/publish.yml` and merge, then add it to the `test`
|
||||
matrix in `.gitea/workflows/ci.yml` here.
|
||||
|
||||
## Signing the version bump commit
|
||||
|
||||
`main` is a protected branch that requires signed commits, and Gitea will not
|
||||
merge a pull request containing an unsigned one. The `bump-version` job in
|
||||
`release.yml` therefore signs the commit it makes, using a dedicated
|
||||
`release-bot` SSH key rather than the key Gitea signs merge commits with —
|
||||
that one is `[repository.signing] SIGNING_KEY` on the server and no runner can
|
||||
reach it. Keeping the CI key separate also means it can be rotated on its own
|
||||
if the secret ever leaks.
|
||||
|
||||
There is deliberately no `release-bot` Gitea account. A key attached to an
|
||||
account is only consulted for signature checking after it has been through the
|
||||
web *Verify* flow, and that flow has no API — a bot account would need an
|
||||
interactive login to be worth anything. Listing the key under
|
||||
`TRUSTED_SSH_KEYS` instead makes Gitea verify commits signed with it without
|
||||
any account lookup, which is all the protected branch asks for.
|
||||
|
||||
Set up once for the instance, and again only if the key is rotated:
|
||||
|
||||
1. Generate a passphrase-less key (it has to be usable unattended):
|
||||
|
||||
```
|
||||
ssh-keygen -t ed25519 -C '[email protected]' -f release-bot -N ''
|
||||
```
|
||||
|
||||
2. Add the public half to `app.ini` and restart Gitea:
|
||||
|
||||
```ini
|
||||
[repository.signing]
|
||||
TRUSTED_SSH_KEYS = ssh-ed25519 AAAAC3Nza... [email protected]
|
||||
```
|
||||
|
||||
3. Store the private half as the **organisation** Actions secret
|
||||
`RELEASE_BOT_SIGNING_KEY` (Org → Settings → Actions → Secrets): the whole
|
||||
`release-bot` file verbatim, `-----BEGIN OPENSSH PRIVATE KEY-----` header
|
||||
and footer included — not the `.pub`, and not a GPG export. Organisation
|
||||
secrets are readable as `secrets.RELEASE_BOT_SIGNING_KEY` from every
|
||||
repository in the org, so no repository-level copy is needed. Delete both
|
||||
local files afterwards.
|
||||
|
||||
Two consequences of trusting the key instance-wide are worth knowing. Any
|
||||
commit signed with it verifies in *every* repository on the instance, not just
|
||||
these — the trust is in the key, not in a user with permissions you can scope.
|
||||
And the signature is attributed to `SIGNING_NAME` / `SIGNING_EMAIL`, not to the
|
||||
`Release Bot <release-bot@unsupervised.ca>` committer the job sets; that
|
||||
address backs no account and is only a label.
|
||||
|
||||
The job fails fast if the secret is missing or `ssh-keygen` is absent from the
|
||||
runner image, and it re-reads the commit it just made to confirm a signature
|
||||
is attached before pushing — an unsigned bump commit would otherwise look fine
|
||||
until someone tried to merge the PR.
|
||||
|
||||
Nothing else in the pipeline signs anything: release tags are made by a human
|
||||
through Gitea's release UI, and the merge commit is signed by the server when
|
||||
the PR is merged.
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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}>
|
||||
*/
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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( ... ) ) );
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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.6
|
||||
* 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.6');
|
||||
define('USC_PLUGIN_FILE', __FILE__);
|
||||
define('USC_PLUGIN_DIR', plugin_dir_path(__FILE__));
|
||||
define('USC_PLUGIN_URL', plugin_dir_url(__FILE__));
|
||||
|
||||
Reference in New Issue
Block a user