Fix field-length saves, student wp-admin access, and empty instructor picker
CI / Tests (PHP 8.1) (pull_request) Successful in 49s
CI / Tests (PHP 8.2) (pull_request) Successful in 49s
CI / No Debug Code (pull_request) Successful in 2s
CI / Coding Standards (pull_request) Successful in 2m47s
CI / PHPStan (pull_request) Successful in 3m16s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m41s
CI / Build Plugin Zip (pull_request) Skipped
CI / Tests (PHP 8.1) (pull_request) Successful in 49s
CI / Tests (PHP 8.2) (pull_request) Successful in 49s
CI / No Debug Code (pull_request) Successful in 2s
CI / Coding Standards (pull_request) Successful in 2m47s
CI / PHPStan (pull_request) Successful in 3m16s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m41s
CI / Build Plugin Zip (pull_request) Skipped
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 <[email protected]>
This commit is contained in:
@@ -0,0 +1,88 @@
|
||||
<?php
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Unsupervised\Schedular\Tests\Unit\Auth;
|
||||
|
||||
use Brain\Monkey\Functions;
|
||||
use Unsupervised\Schedular\Auth\RoleManager;
|
||||
use Unsupervised\Schedular\Auth\StudentAdminGuard;
|
||||
use Unsupervised\Schedular\Tests\Unit\TestCase;
|
||||
|
||||
class StudentAdminGuardTest extends TestCase
|
||||
{
|
||||
private StudentAdminGuard $guard;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
$this->guard = new StudentAdminGuard();
|
||||
Functions\when('wp_doing_ajax')->justReturn(false);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<string> $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));
|
||||
}
|
||||
}
|
||||
@@ -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('<option value="3"', $html);
|
||||
}
|
||||
|
||||
public function testInstructorPickerExcludesAdministratorsWhenGrantDisabled(): void
|
||||
{
|
||||
// With the "admins are instructors" toggle off, an admin is not a teacher,
|
||||
// so only the explicit us_instructor role is queried.
|
||||
Functions\when('get_option')->justReturn('0');
|
||||
|
||||
$queriedRoles = null;
|
||||
Functions\when('get_users')->alias(static function (array $args) use (&$queriedRoles): array {
|
||||
$queriedRoles = $args['role__in'];
|
||||
return [];
|
||||
});
|
||||
$this->repository->shouldReceive('findAll')->andReturn([]);
|
||||
|
||||
$this->render();
|
||||
|
||||
self::assertSame(['us_instructor'], $queriedRoles);
|
||||
}
|
||||
|
||||
private function render(): string
|
||||
{
|
||||
ob_start();
|
||||
|
||||
@@ -118,4 +118,37 @@ class OfferingEndpointTest extends TestCase
|
||||
|
||||
self::assertArrayNotHasKey('etransfer_email', $data[0]);
|
||||
}
|
||||
|
||||
public function testCreateRejectsTitleLongerThanColumnLimit(): void
|
||||
{
|
||||
Functions\when('sanitize_text_field')->returnArg();
|
||||
Functions\when('sanitize_email')->returnArg();
|
||||
$this->repository->shouldNotReceive('insert');
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'kind' => Offering::KIND_GROUP_CLASS,
|
||||
'title' => str_repeat('a', Offering::MAX_TITLE_LENGTH + 1),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $response);
|
||||
self::assertSame(400, $response->error_data['invalid_offering']['status']);
|
||||
}
|
||||
|
||||
public function testCreateRejectsScheduleNoteLongerThanColumnLimit(): void
|
||||
{
|
||||
Functions\when('sanitize_text_field')->returnArg();
|
||||
Functions\when('sanitize_email')->returnArg();
|
||||
$this->repository->shouldNotReceive('insert');
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'kind' => Offering::KIND_GROUP_CLASS,
|
||||
'title' => 'Choir',
|
||||
'schedule_note' => str_repeat('a', Offering::MAX_SCHEDULE_NOTE_LENGTH + 1),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $response);
|
||||
self::assertSame(400, $response->error_data['invalid_offering']['status']);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
<?php
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Unsupervised\Schedular\Tests\Unit\Policy;
|
||||
|
||||
use Brain\Monkey\Functions;
|
||||
use Mockery;
|
||||
use Unsupervised\Schedular\Policy\Policy;
|
||||
use Unsupervised\Schedular\Policy\PolicyEndpoint;
|
||||
use Unsupervised\Schedular\Policy\PolicyRepository;
|
||||
use Unsupervised\Schedular\Policy\PolicyService;
|
||||
use Unsupervised\Schedular\Policy\PolicyVersionRepository;
|
||||
use Unsupervised\Schedular\Tests\Unit\TestCase;
|
||||
|
||||
class PolicyEndpointTest extends TestCase
|
||||
{
|
||||
private PolicyRepository&Mockery\MockInterface $policies;
|
||||
private PolicyService&Mockery\MockInterface $service;
|
||||
private PolicyEndpoint $endpoint;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
|
||||
Functions\when('sanitize_text_field')->returnArg();
|
||||
Functions\when('sanitize_title')->returnArg();
|
||||
|
||||
$this->policies = Mockery::mock(PolicyRepository::class);
|
||||
$this->service = Mockery::mock(PolicyService::class);
|
||||
$this->endpoint = new PolicyEndpoint(
|
||||
$this->policies,
|
||||
Mockery::mock(PolicyVersionRepository::class),
|
||||
$this->service,
|
||||
);
|
||||
}
|
||||
|
||||
public function testCreateRejectsTitleLongerThanColumnLimit(): void
|
||||
{
|
||||
$this->service->shouldNotReceive('createPolicy');
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'title' => str_repeat('a', Policy::MAX_TITLE_LENGTH + 1),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $response);
|
||||
self::assertSame(400, $response->error_data['invalid_policy']['status']);
|
||||
}
|
||||
|
||||
public function testCreateRejectsSlugLongerThanColumnLimit(): void
|
||||
{
|
||||
$this->service->shouldNotReceive('createPolicy');
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'title' => 'Cancellation',
|
||||
'slug' => str_repeat('a', Policy::MAX_SLUG_LENGTH + 1),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $response);
|
||||
self::assertSame(400, $response->error_data['invalid_policy']['status']);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Unsupervised\Schedular\Tests\Unit\Registration;
|
||||
|
||||
use Brain\Monkey\Functions;
|
||||
use Mockery;
|
||||
use Unsupervised\Schedular\Offering\Offering;
|
||||
use Unsupervised\Schedular\Offering\OfferingRepository;
|
||||
use Unsupervised\Schedular\Registration\Question;
|
||||
use Unsupervised\Schedular\Registration\QuestionEndpoint;
|
||||
use Unsupervised\Schedular\Registration\QuestionRepository;
|
||||
use Unsupervised\Schedular\Tests\Unit\TestCase;
|
||||
|
||||
class QuestionEndpointTest extends TestCase
|
||||
{
|
||||
private QuestionRepository&Mockery\MockInterface $questions;
|
||||
private OfferingRepository&Mockery\MockInterface $offerings;
|
||||
private QuestionEndpoint $endpoint;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
|
||||
Functions\when('get_current_user_id')->justReturn(5);
|
||||
Functions\when('current_user_can')->justReturn(false);
|
||||
Functions\when('absint')->alias(static fn ($v): int => abs((int) $v));
|
||||
Functions\when('sanitize_text_field')->returnArg();
|
||||
|
||||
$this->questions = Mockery::mock(QuestionRepository::class);
|
||||
$this->offerings = Mockery::mock(OfferingRepository::class);
|
||||
$this->endpoint = new QuestionEndpoint($this->questions, $this->offerings);
|
||||
|
||||
// The caller (instructor 5) owns offering 9, so the ownership gate passes
|
||||
// and validation is reached.
|
||||
$this->offerings->shouldReceive('findById')->with(9)->andReturn(
|
||||
new Offering(instructorId: 5, kind: Offering::KIND_GROUP_CLASS, title: 'Choir', id: 9)
|
||||
);
|
||||
}
|
||||
|
||||
public function testCreateRejectsLabelLongerThanColumnLimit(): void
|
||||
{
|
||||
// The insert must never be attempted for an over-long label — the bug was
|
||||
// that it reached the DB, silently failed, and returned success anyway.
|
||||
$this->questions->shouldNotReceive('insert');
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'offering_id' => 9,
|
||||
'label' => str_repeat('a', Question::MAX_LABEL_LENGTH + 1),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $response);
|
||||
self::assertSame(400, $response->error_data['invalid_question']['status']);
|
||||
}
|
||||
|
||||
public function testCreateAcceptsLabelAtColumnLimit(): void
|
||||
{
|
||||
$this->questions->shouldReceive('insert')->once()->andReturn(42);
|
||||
|
||||
$request = new \WP_REST_Request([
|
||||
'offering_id' => 9,
|
||||
'label' => str_repeat('a', Question::MAX_LABEL_LENGTH),
|
||||
]);
|
||||
$response = $this->endpoint->create($request);
|
||||
|
||||
self::assertInstanceOf(\WP_REST_Response::class, $response);
|
||||
self::assertSame(201, $response->get_status());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user