From 8122c158cf8ee6ca4e5639ba94a82dace5e7c0bb Mon Sep 17 00:00:00 2001 From: James Griffin Date: Wed, 29 Jul 2026 16:06:54 -0300 Subject: [PATCH] Keep the guardian service within the PHP the plugin supports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `true` as a return type is PHP 8.2, but the plugin advertises 8.1, so the family screen's two service calls fataled on the 8.1 test job while every other job passed. They now return `?\WP_Error` — null on success — which matches RegistrationGate::validate() and works on 8.1. PHPStan was analysing against whatever PHP happened to be running (8.3 in CI, newer locally), so `composer lint` was green on syntax the plugin promises not to use. It is now pinned to the supported 8.1-8.3 range, which reproduces this failure at lint time instead of three jobs later. Co-Authored-By: Claude Opus 5 --- phpstan.neon | 7 +++++++ src/Guardian/FamilyPage.php | 8 ++++---- src/Guardian/GuardianService.php | 12 ++++++++---- tests/Unit/Guardian/FamilyPageTest.php | 4 ++-- tests/Unit/Guardian/GuardianServiceTest.php | 4 ++-- 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 5c4121c..b0a270c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,6 +3,13 @@ includes: parameters: level: 10 + # Analyse against the whole supported range, not whatever PHP happens to be + # running. Without this, syntax newer than the `Requires PHP: 8.1` header + # promises passes lint on a modern local PHP and only fails in the 8.1 test + # job — which is how a PHP 8.2 `true` return type once reached CI. + phpVersion: + min: 80100 + max: 80300 paths: - src bootstrapFiles: diff --git a/src/Guardian/FamilyPage.php b/src/Guardian/FamilyPage.php index a73468f..e93543f 100644 --- a/src/Guardian/FamilyPage.php +++ b/src/Guardian/FamilyPage.php @@ -144,18 +144,18 @@ class FamilyPage { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce checked by the caller. $childId = absint( Val::int( $_POST['child_id'] ?? 0 ) ); - $result = $this->guardians->updateChild( $guardianId, $childId, $this->postString( 'child_name' ), $this->postString( 'child_dob' ) ); + $error = $this->guardians->updateChild( $guardianId, $childId, $this->postString( 'child_name' ), $this->postString( 'child_dob' ) ); - return $result instanceof \WP_Error ? $result : self::RESULT_UPDATED; + return $error ?? self::RESULT_UPDATED; } private function handleRemove( int $guardianId ): string|\WP_Error { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce checked by the caller. $childId = absint( Val::int( $_POST['child_id'] ?? 0 ) ); - $result = $this->guardians->removeChild( $guardianId, $childId ); + $error = $this->guardians->removeChild( $guardianId, $childId ); - return $result instanceof \WP_Error ? $result : self::RESULT_REMOVED; + return $error ?? self::RESULT_REMOVED; } /** diff --git a/src/Guardian/GuardianService.php b/src/Guardian/GuardianService.php index 52a6cb7..315d7f0 100644 --- a/src/Guardian/GuardianService.php +++ b/src/Guardian/GuardianService.php @@ -99,8 +99,10 @@ class GuardianService { * Rename a child and update their date of birth. Refuses a student the caller * is not the guardian of, so the family screen cannot be turned into an * arbitrary user editor by posting someone else's id. + * + * Returns null on success, mirroring {@see \Unsupervised\Schedular\Registration\RegistrationGate::validate()}. */ - public function updateChild( int $guardianId, int $studentId, string $name, string $dateOfBirth = '' ): true|\WP_Error { + public function updateChild( int $guardianId, int $studentId, string $name, string $dateOfBirth = '' ): ?\WP_Error { if ( ! $this->guardians->isGuardianOf( $guardianId, $studentId ) ) { return new \WP_Error( 'forbidden', __( 'That is not one of your children.', 'unsupervised-schedular' ) ); } @@ -124,7 +126,7 @@ class GuardianService { $this->setDateOfBirth( $studentId, $dateOfBirth ); - return true; + return null; } /** @@ -132,8 +134,10 @@ class GuardianService { * lesson or enrolment history: their id is referenced by lessons, payments and * credits, and deleting the user would orphan all of it. A studio admin * handles those cases by hand. + * + * Returns null on success. */ - public function removeChild( int $guardianId, int $studentId ): true|\WP_Error { + public function removeChild( int $guardianId, int $studentId ): ?\WP_Error { if ( ! $this->guardians->isGuardianOf( $guardianId, $studentId ) ) { return new \WP_Error( 'forbidden', __( 'That is not one of your children.', 'unsupervised-schedular' ) ); } @@ -148,7 +152,7 @@ class GuardianService { $this->guardians->delete( $guardianId, $studentId ); $this->deleteUser( $studentId ); - return true; + return null; } /** diff --git a/tests/Unit/Guardian/FamilyPageTest.php b/tests/Unit/Guardian/FamilyPageTest.php index 4a35d66..b7f183b 100644 --- a/tests/Unit/Guardian/FamilyPageTest.php +++ b/tests/Unit/Guardian/FamilyPageTest.php @@ -182,7 +182,7 @@ class FamilyPageTest extends TestCase 'child_dob' => '2015-04-02', ]; - $this->guardians->shouldReceive('updateChild')->once()->with(5, 42, 'Ada L', '2015-04-02')->andReturn(true); + $this->guardians->shouldReceive('updateChild')->once()->with(5, 42, 'Ada L', '2015-04-02')->andReturn(null); $captured = null; $this->capturingPage($captured)->maybeHandleSubmit(); @@ -194,7 +194,7 @@ class FamilyPageTest extends TestCase { $_POST = ['us_family_action' => 'remove', 'child_id' => '42']; - $this->guardians->shouldReceive('removeChild')->once()->with(5, 42)->andReturn(true); + $this->guardians->shouldReceive('removeChild')->once()->with(5, 42)->andReturn(null); $captured = null; $this->capturingPage($captured)->maybeHandleSubmit(); diff --git a/tests/Unit/Guardian/GuardianServiceTest.php b/tests/Unit/Guardian/GuardianServiceTest.php index 07b2beb..805aaae 100644 --- a/tests/Unit/Guardian/GuardianServiceTest.php +++ b/tests/Unit/Guardian/GuardianServiceTest.php @@ -236,7 +236,7 @@ class GuardianServiceTest extends TestCase ->with(['ID' => 42, 'display_name' => 'Ada L', 'nickname' => 'Ada L']) ->andReturn(42); - self::assertTrue($this->service->updateChild(5, 42, 'Ada L', '2015-04-02')); + self::assertNull($this->service->updateChild(5, 42, 'Ada L', '2015-04-02')); self::assertSame('2015-04-02', $this->meta[42][GuardianService::META_DOB]); } @@ -249,7 +249,7 @@ class GuardianServiceTest extends TestCase Functions\expect('wp_delete_user')->once()->with(42); - self::assertTrue($this->service->removeChild(5, 42)); + self::assertNull($this->service->removeChild(5, 42)); } /**