From 721c4be1d69571a2a96f48383f5c30fd775c1269 Mon Sep 17 00:00:00 2001 From: James Griffin Date: Fri, 24 Jul 2026 20:22:04 -0300 Subject: [PATCH] Fix field-length saves, student wp-admin access, and empty instructor picker Three bug fixes for the 1.2.1 section: - Fixed-size fields (question labels, offering titles/notes/e-transfer email, policy titles/slugs) no longer silently fail to save when the value exceeds its column length. The REST endpoints reject over-long values with a 400, the admin controllers refuse to insert them, and the form inputs carry a maxlength so the browser blocks over-long entry. Limits are MAX_* constants on the value objects, kept in lockstep with the schema columns. - Students are kept out of wp-admin entirely. New StudentAdminGuard redirects front-end-only users (no back-office capability) away from the dashboard and hides the admin bar for them, while administrators, studio admins, and instructors keep full access. - The Add/Edit Offering instructor picker now includes WordPress administrators when they act as instructors (the default single-account setup), so a solo studio owner is selectable instead of the dropdown being empty. composer test (618), composer lint, composer cs all pass. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 5 + src/Auth/StudentAdminGuard.php | 103 ++++++++++++++++++ src/Offering/Offering.php | 9 ++ src/Offering/OfferingController.php | 36 +++++- src/Offering/OfferingEndpoint.php | 55 +++++++++- src/Plugin.php | 2 + src/Policy/Policy.php | 6 + src/Policy/PolicyController.php | 4 +- src/Policy/PolicyEndpoint.php | 18 +++ src/Registration/Question.php | 3 + src/Registration/QuestionController.php | 2 +- src/Registration/QuestionEndpoint.php | 25 ++++- templates/admin/offerings.php | 6 +- templates/admin/policies.php | 4 +- templates/admin/questions.php | 2 +- tests/Unit/Auth/StudentAdminGuardTest.php | 88 +++++++++++++++ .../Unit/Offering/OfferingControllerTest.php | 47 ++++++++ tests/Unit/Offering/OfferingEndpointTest.php | 33 ++++++ tests/Unit/Policy/PolicyEndpointTest.php | 63 +++++++++++ .../Registration/QuestionEndpointTest.php | 70 ++++++++++++ 20 files changed, 561 insertions(+), 20 deletions(-) create mode 100644 src/Auth/StudentAdminGuard.php create mode 100644 tests/Unit/Auth/StudentAdminGuardTest.php create mode 100644 tests/Unit/Policy/PolicyEndpointTest.php create mode 100644 tests/Unit/Registration/QuestionEndpointTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 31000c6..13a60b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ each change under the current top section as you work. ## [1.2.1] +### Fixed +- Registration questions, offering titles/notes, and policy names longer than their storage limit are no longer silently discarded. Previously typing a fixed-size field past its maximum length reported success but saved nothing — the database quietly rejected the over-long value. These fields now cap the input in the form, and the API rejects an over-long value with a clear error. +- Students can no longer reach the WordPress dashboard. A student who navigates to `wp-admin` is redirected to the site front end and the admin toolbar is hidden for them, so they only ever see the studio's booking pages. Anyone who runs the studio — administrators, studio admins, and instructors — keeps full `wp-admin` access. +- The instructor picker on the **Add/Edit Offering** form no longer comes up empty for a solo studio owner. When the person running the studio teaches from a WordPress administrator account (the default single-account setup), they now appear in the instructor dropdown and can be assigned to a class. + ## [1.2.0] ### Added diff --git a/src/Auth/StudentAdminGuard.php b/src/Auth/StudentAdminGuard.php new file mode 100644 index 0000000..2f468df --- /dev/null +++ b/src/Auth/StudentAdminGuard.php @@ -0,0 +1,103 @@ + + */ + private const BACK_OFFICE_CAPS = [ + 'manage_options', + RoleManager::CAP_MANAGE_INSTRUCTORS, + RoleManager::CAP_MANAGE_STUDENTS, + RoleManager::CAP_MANAGE_OFFERINGS, + RoleManager::CAP_MANAGE_QUESTIONS, + RoleManager::CAP_MANAGE_POLICIES, + RoleManager::CAP_MANAGE_BILLING, + RoleManager::CAP_MANAGE_AVAILABILITY, + RoleManager::CAP_VIEW_ALL_LESSONS, + RoleManager::CAP_VIEW_ALL_PAYMENTS, + RoleManager::CAP_VIEW_OWN_PAYMENTS, + RoleManager::CAP_EXPORT_PAYMENTS, + ]; + + public function register(): void { + add_action( 'admin_init', [ $this, 'redirectFromDashboard' ] ); + add_filter( 'show_admin_bar', [ $this, 'hideAdminBar' ] ); + } + + /** + * Redirect a front-end-only user away from any wp-admin page to the site + * home, so the dashboard and profile screens are never reachable. + */ + public function redirectFromDashboard(): void { + if ( ! $this->shouldBlockAdminAccess() ) { + return; + } + + wp_safe_redirect( home_url( '/' ) ); + exit; + } + + /** + * Whether the current request into wp-admin should be bounced to the front + * end. AJAX requests are always allowed through so front-end features that + * call admin-ajax keep working. + */ + public function shouldBlockAdminAccess(): bool { + if ( wp_doing_ajax() ) { + return false; + } + + if ( ! is_user_logged_in() ) { + return false; + } + + return ! $this->hasBackOfficeAccess(); + } + + /** + * Hide the admin bar for front-end-only users; leave it untouched for anyone + * with back-office access. + * + * @param bool $show Whether WordPress would otherwise show the admin bar. + */ + public function hideAdminBar( bool $show ): bool { + if ( is_user_logged_in() && ! $this->hasBackOfficeAccess() ) { + return false; + } + + return $show; + } + + /** + * Whether the current user holds any capability that warrants wp-admin access. + */ + private function hasBackOfficeAccess(): bool { + foreach ( self::BACK_OFFICE_CAPS as $cap ) { + if ( current_user_can( $cap ) ) { + return true; + } + } + + return false; + } +} diff --git a/src/Offering/Offering.php b/src/Offering/Offering.php index f8d1720..153df26 100644 --- a/src/Offering/Offering.php +++ b/src/Offering/Offering.php @@ -54,6 +54,15 @@ class Offering { */ public const VALID_ACCESS_MODES = [ self::ACCESS_PUBLIC, self::ACCESS_INVITE_ONLY ]; + /** Maximum length of the title, matching the `title` VARCHAR(191) column. */ + public const MAX_TITLE_LENGTH = 191; + + /** Maximum length of the schedule note, matching the `schedule_note` VARCHAR(191) column. */ + public const MAX_SCHEDULE_NOTE_LENGTH = 191; + + /** Maximum length of the e-transfer email, matching the `etransfer_email` VARCHAR(191) column. */ + public const MAX_ETRANSFER_EMAIL_LENGTH = 191; + public function __construct( public readonly int $instructorId, public readonly string $kind, diff --git a/src/Offering/OfferingController.php b/src/Offering/OfferingController.php index a23fd6e..9702eac 100644 --- a/src/Offering/OfferingController.php +++ b/src/Offering/OfferingController.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Unsupervised\Schedular\Offering; +use Unsupervised\Schedular\Auth\AccessSettings; use Unsupervised\Schedular\Auth\RoleManager; use Unsupervised\Schedular\Val; @@ -11,6 +12,7 @@ class OfferingController { public function __construct( private OfferingRepository $repository, private ClassSlotReconciler $reconciler, + private AccessSettings $access = new AccessSettings(), ) {} public function renderPage(): void { @@ -137,17 +139,28 @@ class OfferingController { } /** - * Registered instructors offered in the assignment select, by display name. + * Instructors offered in the assignment select, by display name. + * + * Includes everyone holding the `us_instructor` role plus, when the site owner + * has left administrators acting as instructors (the default single-account + * setup), WordPress administrators — who teach through the dynamic capability + * grant rather than the role. Without them a solo studio owner running the + * business from an admin account would find no one to assign a class to. * * @return list */ private function instructorOptions(): array { + $roles = [ RoleManager::INSTRUCTOR ]; + if ( $this->access->adminsAreInstructors() ) { + $roles[] = 'administrator'; + } + $users = array_filter( get_users( [ - 'role' => RoleManager::INSTRUCTOR, - 'orderby' => 'display_name', - 'order' => 'ASC', + 'role__in' => $roles, + 'orderby' => 'display_name', + 'order' => 'ASC', ] ), static fn( mixed $u ): bool => $u instanceof \WP_User @@ -184,6 +197,17 @@ class OfferingController { return null; } + $scheduleNote = $this->nullableText( sanitize_text_field( Val::string( wp_unslash( $_POST['schedule_note'] ?? '' ) ) ) ); + $etransferEmail = $this->nullableText( sanitize_email( Val::string( wp_unslash( $_POST['etransfer_email'] ?? '' ) ) ) ); + + // Reject over-long fixed-size fields rather than let the DB silently drop them. + if ( mb_strlen( $title ) > Offering::MAX_TITLE_LENGTH + || ( null !== $scheduleNote && mb_strlen( $scheduleNote ) > Offering::MAX_SCHEDULE_NOTE_LENGTH ) + || ( null !== $etransferEmail && mb_strlen( $etransferEmail ) > Offering::MAX_ETRANSFER_EMAIL_LENGTH ) + ) { + return null; + } + $billingMode = sanitize_key( Val::string( wp_unslash( $_POST['billing_mode'] ?? Offering::BILLING_ONE_TIME ) ) ); if ( ! in_array( $billingMode, Offering::VALID_BILLING_MODES, true ) ) { $billingMode = Offering::BILLING_ONE_TIME; @@ -234,8 +258,8 @@ class OfferingController { classTime: $classTime, enrollmentDeadline: $enrollmentDeadline, withdrawalDeadline: $withdrawalDeadline, - scheduleNote: $this->nullableText( sanitize_text_field( Val::string( wp_unslash( $_POST['schedule_note'] ?? '' ) ) ) ), - etransferEmail: $this->nullableText( sanitize_email( Val::string( wp_unslash( $_POST['etransfer_email'] ?? '' ) ) ) ), + scheduleNote: $scheduleNote, + etransferEmail: $etransferEmail, cancellationCutoffHours: $cutoffHours, accessMode: isset( $_POST['invite_only'] ) ? Offering::ACCESS_INVITE_ONLY : Offering::ACCESS_PUBLIC, isActive: isset( $_POST['is_active'] ), diff --git a/src/Offering/OfferingEndpoint.php b/src/Offering/OfferingEndpoint.php index 9532ebc..b887b25 100644 --- a/src/Offering/OfferingEndpoint.php +++ b/src/Offering/OfferingEndpoint.php @@ -148,6 +148,14 @@ class OfferingEndpoint { return $this->invalid( __( 'Invalid billing mode.', 'unsupervised-schedular' ) ); } + $scheduleNote = $this->nullableText( $request->get_param( 'schedule_note' ) ); + $etransferEmail = $this->nullableEmail( $request->get_param( 'etransfer_email' ) ); + + $lengthError = $this->checkLengths( $title, $scheduleNote, $etransferEmail ); + if ( $lengthError instanceof \WP_Error ) { + return $lengthError; + } + $offering = new Offering( instructorId: get_current_user_id(), kind: $kind, @@ -162,8 +170,8 @@ class OfferingEndpoint { termStart: $this->nullableText( $request->get_param( 'term_start' ) ), termEnd: $this->nullableText( $request->get_param( 'term_end' ) ), enrollmentDeadline: $this->nullableText( $request->get_param( 'enrollment_deadline' ) ), - scheduleNote: $this->nullableText( $request->get_param( 'schedule_note' ) ), - etransferEmail: $this->nullableEmail( $request->get_param( 'etransfer_email' ) ), + scheduleNote: $scheduleNote, + etransferEmail: $etransferEmail, cancellationCutoffHours: $this->nullableInt( $request->get_param( 'cancellation_cutoff_hours' ) ), accessMode: $this->accessMode( $request->get_param( 'access_mode' ), Offering::ACCESS_PUBLIC ), isActive: null === $request->get_param( 'is_active' ) ? true : (bool) $request->get_param( 'is_active' ), @@ -196,10 +204,19 @@ class OfferingEndpoint { return $this->invalid( __( 'Invalid billing mode.', 'unsupervised-schedular' ) ); } + $title = $request->has_param( 'title' ) ? sanitize_text_field( Val::string( $request->get_param( 'title' ) ) ) : $existing->title; + $scheduleNote = $request->has_param( 'schedule_note' ) ? $this->nullableText( $request->get_param( 'schedule_note' ) ) : $existing->scheduleNote; + $etransferEmail = $request->has_param( 'etransfer_email' ) ? $this->nullableEmail( $request->get_param( 'etransfer_email' ) ) : $existing->etransferEmail; + + $lengthError = $this->checkLengths( $title, $scheduleNote, $etransferEmail ); + if ( $lengthError instanceof \WP_Error ) { + return $lengthError; + } + $offering = new Offering( instructorId: $existing->instructorId, kind: $kind, - title: $request->has_param( 'title' ) ? sanitize_text_field( Val::string( $request->get_param( 'title' ) ) ) : $existing->title, + title: $title, price: $request->has_param( 'price' ) ? $this->price( $request->get_param( 'price' ) ) : $existing->price, currency: $request->has_param( 'currency' ) ? sanitize_text_field( Val::string( $request->get_param( 'currency' ) ) ) : $existing->currency, billingMode: $billingMode, @@ -210,8 +227,8 @@ class OfferingEndpoint { termStart: $request->has_param( 'term_start' ) ? $this->nullableText( $request->get_param( 'term_start' ) ) : $existing->termStart, termEnd: $request->has_param( 'term_end' ) ? $this->nullableText( $request->get_param( 'term_end' ) ) : $existing->termEnd, enrollmentDeadline: $request->has_param( 'enrollment_deadline' ) ? $this->nullableText( $request->get_param( 'enrollment_deadline' ) ) : $existing->enrollmentDeadline, - scheduleNote: $request->has_param( 'schedule_note' ) ? $this->nullableText( $request->get_param( 'schedule_note' ) ) : $existing->scheduleNote, - etransferEmail: $request->has_param( 'etransfer_email' ) ? $this->nullableEmail( $request->get_param( 'etransfer_email' ) ) : $existing->etransferEmail, + scheduleNote: $scheduleNote, + etransferEmail: $etransferEmail, cancellationCutoffHours: $request->has_param( 'cancellation_cutoff_hours' ) ? $this->nullableInt( $request->get_param( 'cancellation_cutoff_hours' ) ) : $existing->cancellationCutoffHours, accessMode: $request->has_param( 'access_mode' ) ? $this->accessMode( $request->get_param( 'access_mode' ), $existing->accessMode ) : $existing->accessMode, isActive: $request->has_param( 'is_active' ) ? (bool) $request->get_param( 'is_active' ) : $existing->isActive, @@ -266,6 +283,34 @@ class OfferingEndpoint { return new \WP_Error( 'invalid_offering', $message, [ 'status' => 400 ] ); } + /** + * Reject any fixed-size field whose value exceeds its column length, so an + * over-long value is refused with a clear 400 rather than silently dropped + * by the database. + */ + private function checkLengths( string $title, ?string $scheduleNote, ?string $etransferEmail ): ?\WP_Error { + $fields = [ + [ __( 'title', 'unsupervised-schedular' ), $title, Offering::MAX_TITLE_LENGTH ], + [ __( 'schedule note', 'unsupervised-schedular' ), $scheduleNote, Offering::MAX_SCHEDULE_NOTE_LENGTH ], + [ __( 'e-transfer email', 'unsupervised-schedular' ), $etransferEmail, Offering::MAX_ETRANSFER_EMAIL_LENGTH ], + ]; + + foreach ( $fields as [ $name, $value, $max ] ) { + if ( null !== $value && mb_strlen( $value ) > $max ) { + return $this->invalid( + sprintf( + /* translators: 1: field name, 2: maximum character count. */ + __( 'The %1$s must be %2$d characters or fewer.', 'unsupervised-schedular' ), + $name, + $max + ) + ); + } + } + + return null; + } + private function price( mixed $value ): float { return max( 0.0, Val::float( $value ) ); } diff --git a/src/Plugin.php b/src/Plugin.php index 090f690..46b5759 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -10,6 +10,7 @@ use Unsupervised\Schedular\Auth\RegistrationLoginGate; use Unsupervised\Schedular\Auth\RegistrationMailer; use Unsupervised\Schedular\Auth\RegistrationPage; use Unsupervised\Schedular\Auth\RoleManager; +use Unsupervised\Schedular\Auth\StudentAdminGuard; use Unsupervised\Schedular\Booking\BookingPage; use Unsupervised\Schedular\Availability\AvailabilityRepository; use Unsupervised\Schedular\Booking\BookingRepository; @@ -96,6 +97,7 @@ class Plugin { ( new UpdateChecker() )->register(); ( new RoleManager() )->register(); ( new RegistrationLoginGate() )->register(); + ( new StudentAdminGuard() )->register(); ( new EmailConfirmationHandler( $settings, $registrationMailer ) )->register(); ( new AdminMenu( $availability, $bookings, $offerings, $questions, $answers, $policies, $policyVersions, $policyService, $acceptances, $invites, $enrollments, $groupAccess, $settings, $paymentRepo, $paymentService, $resolver, $registrationMailer, $creditRepo ) )->register(); ( new RestRegistrar( $availability, $bookings, $offerings, $questions, $policies, $policyVersions, $policyService, $registrationGate, $enrollments, $groupAccess, $paymentService ) )->register(); diff --git a/src/Policy/Policy.php b/src/Policy/Policy.php index dad39fa..bc860d7 100644 --- a/src/Policy/Policy.php +++ b/src/Policy/Policy.php @@ -18,6 +18,12 @@ class Policy { */ public const VALID_SCOPES = [ self::SCOPE_SIGNUP, self::SCOPE_BOOKING, self::SCOPE_BOTH ]; + /** Maximum length of the title, matching the `title` VARCHAR(191) column. */ + public const MAX_TITLE_LENGTH = 191; + + /** Maximum length of the slug, matching the `slug` VARCHAR(191) column. */ + public const MAX_SLUG_LENGTH = 191; + public function __construct( public readonly string $title, public readonly string $slug, diff --git a/src/Policy/PolicyController.php b/src/Policy/PolicyController.php index 82fb8c6..66a2eea 100644 --- a/src/Policy/PolicyController.php +++ b/src/Policy/PolicyController.php @@ -47,7 +47,9 @@ class PolicyController { $scope = Policy::SCOPE_BOOKING; } - if ( '' !== $title && '' !== $slug && null === $this->policies->findBySlug( $slug ) ) { + $withinLimits = mb_strlen( $title ) <= Policy::MAX_TITLE_LENGTH && mb_strlen( $slug ) <= Policy::MAX_SLUG_LENGTH; + + if ( '' !== $title && '' !== $slug && $withinLimits && null === $this->policies->findBySlug( $slug ) ) { $this->service->createPolicy( $title, $slug, $scope ); } diff --git a/src/Policy/PolicyEndpoint.php b/src/Policy/PolicyEndpoint.php index b850759..d99c217 100644 --- a/src/Policy/PolicyEndpoint.php +++ b/src/Policy/PolicyEndpoint.php @@ -118,12 +118,30 @@ class PolicyEndpoint { if ( '' === $title ) { return $this->invalid( __( 'A policy title is required.', 'unsupervised-schedular' ) ); } + if ( mb_strlen( $title ) > Policy::MAX_TITLE_LENGTH ) { + return $this->invalid( + sprintf( + /* translators: %d: maximum character count. */ + __( 'The policy title must be %d characters or fewer.', 'unsupervised-schedular' ), + Policy::MAX_TITLE_LENGTH + ) + ); + } $slugParam = sanitize_text_field( Val::string( $request->get_param( 'slug' ) ) ); $slug = sanitize_title( '' !== $slugParam ? $slugParam : $title ); if ( '' === $slug ) { return $this->invalid( __( 'A valid policy slug is required.', 'unsupervised-schedular' ) ); } + if ( mb_strlen( $slug ) > Policy::MAX_SLUG_LENGTH ) { + return $this->invalid( + sprintf( + /* translators: %d: maximum character count. */ + __( 'The policy slug must be %d characters or fewer.', 'unsupervised-schedular' ), + Policy::MAX_SLUG_LENGTH + ) + ); + } if ( null !== $this->policies->findBySlug( $slug ) ) { return new \WP_Error( 'duplicate_slug', __( 'A policy with that slug already exists.', 'unsupervised-schedular' ), [ 'status' => 409 ] ); diff --git a/src/Registration/Question.php b/src/Registration/Question.php index 311dfd0..97a0405 100644 --- a/src/Registration/Question.php +++ b/src/Registration/Question.php @@ -12,6 +12,9 @@ class Question { public const FIELD_SELECT = 'select'; public const FIELD_CHECKBOX = 'checkbox'; + /** Maximum length of a question label, matching the `label` VARCHAR(255) column. */ + public const MAX_LABEL_LENGTH = 255; + /** Question is scoped to a single offering, asked at booking/enrolment time. */ public const SCOPE_OFFERING = 'offering'; diff --git a/src/Registration/QuestionController.php b/src/Registration/QuestionController.php index 0f55c46..61e7374 100644 --- a/src/Registration/QuestionController.php +++ b/src/Registration/QuestionController.php @@ -85,7 +85,7 @@ class QuestionController { $label = sanitize_text_field( Val::string( wp_unslash( $_POST['label'] ?? '' ) ) ); $fieldType = sanitize_key( Val::string( wp_unslash( $_POST['field_type'] ?? Question::FIELD_TEXT ) ) ); - if ( '' === $label || ! in_array( $fieldType, Question::VALID_FIELD_TYPES, true ) ) { + if ( '' === $label || mb_strlen( $label ) > Question::MAX_LABEL_LENGTH || ! in_array( $fieldType, Question::VALID_FIELD_TYPES, true ) ) { return; } diff --git a/src/Registration/QuestionEndpoint.php b/src/Registration/QuestionEndpoint.php index 3e9ad01..fc8bae3 100644 --- a/src/Registration/QuestionEndpoint.php +++ b/src/Registration/QuestionEndpoint.php @@ -79,6 +79,9 @@ class QuestionEndpoint { if ( '' === $label ) { return $this->invalid( __( 'A question label is required.', 'unsupervised-schedular' ) ); } + if ( mb_strlen( $label ) > Question::MAX_LABEL_LENGTH ) { + return $this->invalid( $this->tooLongMessage( __( 'question', 'unsupervised-schedular' ), Question::MAX_LABEL_LENGTH ) ); + } $fieldType = Val::string( $request->get_param( 'field_type' ) ?? Question::FIELD_TEXT ); if ( ! in_array( $fieldType, Question::VALID_FIELD_TYPES, true ) ) { @@ -118,9 +121,17 @@ class QuestionEndpoint { return $this->invalid( __( 'Invalid field type.', 'unsupervised-schedular' ) ); } + $label = $request->has_param( 'label' ) ? sanitize_text_field( Val::string( $request->get_param( 'label' ) ) ) : $existing->label; + if ( '' === $label ) { + return $this->invalid( __( 'A question label is required.', 'unsupervised-schedular' ) ); + } + if ( mb_strlen( $label ) > Question::MAX_LABEL_LENGTH ) { + return $this->invalid( $this->tooLongMessage( __( 'question', 'unsupervised-schedular' ), Question::MAX_LABEL_LENGTH ) ); + } + $question = new Question( offeringId: $existing->offeringId, - label: $request->has_param( 'label' ) ? sanitize_text_field( Val::string( $request->get_param( 'label' ) ) ) : $existing->label, + label: $label, fieldType: $fieldType, options: $request->has_param( 'options' ) ? $this->sanitizeOptions( $request->get_param( 'options' ) ) : $existing->options, isRequired: $request->has_param( 'is_required' ) ? (bool) $request->get_param( 'is_required' ) : $existing->isRequired, @@ -217,4 +228,16 @@ class QuestionEndpoint { private function invalid( string $message ): \WP_Error { return new \WP_Error( 'invalid_question', $message, [ 'status' => 400 ] ); } + + /** + * Build a uniform "too long" validation message for a named field. + */ + private function tooLongMessage( string $field, int $max ): string { + return sprintf( + /* translators: 1: field name, 2: maximum character count. */ + __( 'The %1$s must be %2$d characters or fewer.', 'unsupervised-schedular' ), + $field, + $max + ); + } } diff --git a/templates/admin/offerings.php b/templates/admin/offerings.php index ff085a4..8700ea1 100644 --- a/templates/admin/offerings.php +++ b/templates/admin/offerings.php @@ -45,7 +45,7 @@ if ($editing && null !== $editing->termStart && null !== $editing->termEnd && $e - + @@ -140,11 +140,11 @@ if ($editing && null !== $editing->termStart && null !== $editing->termEnd && $e - + - + diff --git a/templates/admin/policies.php b/templates/admin/policies.php index 65efaa9..1d5e3e9 100644 --- a/templates/admin/policies.php +++ b/templates/admin/policies.php @@ -24,12 +24,12 @@ if (! defined('ABSPATH')) {
- + diff --git a/templates/admin/questions.php b/templates/admin/questions.php index 102dd96..dd0bbf6 100644 --- a/templates/admin/questions.php +++ b/templates/admin/questions.php @@ -53,7 +53,7 @@ if (! defined('ABSPATH')) {
- +
- + diff --git a/tests/Unit/Auth/StudentAdminGuardTest.php b/tests/Unit/Auth/StudentAdminGuardTest.php new file mode 100644 index 0000000..fe7add9 --- /dev/null +++ b/tests/Unit/Auth/StudentAdminGuardTest.php @@ -0,0 +1,88 @@ +guard = new StudentAdminGuard(); + Functions\when('wp_doing_ajax')->justReturn(false); + } + + /** + * @param list $held Capabilities the user is treated as holding. + */ + private function stubUser(bool $loggedIn, array $held = []): void + { + Functions\when('is_user_logged_in')->justReturn($loggedIn); + Functions\when('current_user_can')->alias(static fn (string $cap): bool => in_array($cap, $held, true)); + } + + public function testBlocksStudentWithNoBackOfficeCapabilities(): void + { + // A student holds only front-end capabilities. + $this->stubUser(true, [RoleManager::CAP_BOOK_LESSON, RoleManager::CAP_VIEW_LESSONS]); + + self::assertTrue($this->guard->shouldBlockAdminAccess()); + } + + public function testAllowsInstructor(): void + { + $this->stubUser(true, [RoleManager::CAP_MANAGE_AVAILABILITY]); + + self::assertFalse($this->guard->shouldBlockAdminAccess()); + } + + public function testAllowsAdministrator(): void + { + $this->stubUser(true, ['manage_options']); + + self::assertFalse($this->guard->shouldBlockAdminAccess()); + } + + public function testDoesNotBlockLoggedOutRequests(): void + { + $this->stubUser(false); + + self::assertFalse($this->guard->shouldBlockAdminAccess()); + } + + public function testDoesNotBlockAjaxRequests(): void + { + Functions\when('wp_doing_ajax')->justReturn(true); + $this->stubUser(true, [RoleManager::CAP_BOOK_LESSON]); + + self::assertFalse($this->guard->shouldBlockAdminAccess()); + } + + public function testHidesAdminBarForStudent(): void + { + $this->stubUser(true, [RoleManager::CAP_BOOK_LESSON]); + + self::assertFalse($this->guard->hideAdminBar(true)); + } + + public function testKeepsAdminBarForInstructor(): void + { + $this->stubUser(true, [RoleManager::CAP_MANAGE_AVAILABILITY]); + + self::assertTrue($this->guard->hideAdminBar(true)); + } + + public function testLeavesAdminBarUntouchedForLoggedOutVisitor(): void + { + $this->stubUser(false); + + self::assertFalse($this->guard->hideAdminBar(false)); + } +} diff --git a/tests/Unit/Offering/OfferingControllerTest.php b/tests/Unit/Offering/OfferingControllerTest.php index f5f4aa0..02d38cc 100644 --- a/tests/Unit/Offering/OfferingControllerTest.php +++ b/tests/Unit/Offering/OfferingControllerTest.php @@ -32,6 +32,8 @@ class OfferingControllerTest extends TestCase Functions\when('current_user_can')->justReturn(true); Functions\when('get_current_user_id')->justReturn(3); Functions\when('get_users')->justReturn([]); + // Default single-account setup: admins act as instructors. + Functions\when('get_option')->justReturn('1'); Functions\when('check_admin_referer')->justReturn(true); Functions\when('admin_url')->justReturn('admin.php?page=us-offerings'); Functions\when('add_query_arg')->alias( @@ -450,6 +452,51 @@ class OfferingControllerTest extends TestCase self::assertStringNotContainsString('Edit Offering', $html); } + public function testInstructorPickerIncludesAdministratorsWhenTheyActAsInstructors(): void + { + // The reported bug: a solo studio owner runs the business from a WordPress + // administrator account and teaches through the dynamic capability grant, + // so they never hold the us_instructor role. The picker must still list + // them, otherwise there is no one to assign a class to. + Functions\when('get_option')->justReturn('1'); + + $admin = Mockery::mock(\WP_User::class); + $admin->ID = 3; + $admin->display_name = 'Studio Owner'; + + $queriedRoles = []; + Functions\when('get_users')->alias(static function (array $args) use (&$queriedRoles, $admin): array { + $queriedRoles = $args['role__in']; + return [$admin]; + }); + $this->repository->shouldReceive('findAll')->andReturn([]); + + $html = $this->render(); + + self::assertContains('us_instructor', $queriedRoles); + self::assertContains('administrator', $queriedRoles); + self::assertStringContainsString('Studio Owner', $html); + self::assertStringContainsString('