Let the account holder edit their own profile details
CI / Tests (PHP 8.1) (pull_request) Successful in 45s
CI / No Debug Code (pull_request) Successful in 2s
CI / Tests (PHP 8.2) (pull_request) Successful in 55s
CI / PHPStan (pull_request) Successful in 2m57s
CI / Coding Standards (pull_request) Successful in 3m3s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m42s
CI / Build Plugin Zip (pull_request) Skipped
CI / Tests (PHP 8.1) (pull_request) Successful in 45s
CI / No Debug Code (pull_request) Successful in 2s
CI / Tests (PHP 8.2) (pull_request) Successful in 55s
CI / PHPStan (pull_request) Successful in 2m57s
CI / Coding Standards (pull_request) Successful in 3m3s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m42s
CI / Build Plugin Zip (pull_request) Skipped
The Profile block is headed "Your profile", but the one person on it you could not change was yourself: your name, your birth year, and whether you take lessons yourself were fixed at whatever signup recorded, and correcting any of them meant asking a studio admin. A "Your details" section now opens the page, saved through the same nonce-checked template_redirect post/redirect/get path the child rows use: - Your name, written to display_name and nickname together, for the reason updateChild() does — UserName reads the nickname first, and leaving it behind would put the account's email address back on every screen that names a person. - "I take lessons myself", the positive of us_guardian_only. This makes good on the claim already in bookableStudents() and the feature doc that a guardian-only account can put itself right from the profile page. - Your birth year, held to the same normaliseBirthYear() rule as every other student. The email is shown but not editable: it is the account's user_login as well as its address, so changing it stays a studio-side job. The birth-year field deliberately carries no `required` attribute. It is asked of a student only, and this page loads no JavaScript, so a browser-enforced `required` would leave a guardian who books solely for other people unable to submit the form at all; handleSelf() enforces it against the checkbox instead. Unticking the box does not clear a stored birth year — it says who books, not "forget what is on file". Closes #165 Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -74,6 +74,23 @@ class BlockPreviewTest extends TestCase
|
||||
self::assertStringContainsString('us-editor-note', $html);
|
||||
}
|
||||
|
||||
/**
|
||||
* The preview is what someone placing the block styles against, so it has to
|
||||
* show both halves of the page — your own details as well as your students'.
|
||||
*/
|
||||
public function testFamilyPreviewShowsTheAccountHoldersDetailsAndTheirStudents(): void
|
||||
{
|
||||
$html = BlockPreview::family();
|
||||
|
||||
self::assertStringContainsString('class="us-family-self"', $html);
|
||||
self::assertStringContainsString('id="us-own-name"', $html);
|
||||
self::assertStringContainsString('id="us-own-birth-year"', $html);
|
||||
self::assertStringContainsString('I take lessons myself', $html);
|
||||
self::assertStringContainsString('class="us-family-list"', $html);
|
||||
self::assertStringContainsString('class="us-family-add"', $html);
|
||||
self::assertStringContainsString('us-editor-note', $html);
|
||||
}
|
||||
|
||||
public function testRegistrationPreviewShowsADisabledSampleForm(): void
|
||||
{
|
||||
$html = BlockPreview::registration();
|
||||
|
||||
@@ -76,6 +76,21 @@ class FamilyPageTest extends TestCase
|
||||
return $page;
|
||||
}
|
||||
|
||||
/**
|
||||
* The account holder's own details, which every render reads.
|
||||
*
|
||||
* @param array{name?: string, email?: string, birth_year?: string, is_student?: bool} $overrides
|
||||
*/
|
||||
private function expectAccountHolder(array $overrides = []): void
|
||||
{
|
||||
$this->guardians->shouldReceive('accountHolder')->with(5)->andReturn($overrides + [
|
||||
'name' => 'Grace',
|
||||
'email' => '[email protected]',
|
||||
'birth_year' => '1984',
|
||||
'is_student' => true,
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* A question required of everyone, or of nobody — the shape every question
|
||||
* had before the account holder and the students could differ, and the shape
|
||||
@@ -105,6 +120,7 @@ class FamilyPageTest extends TestCase
|
||||
|
||||
public function testRenderListsTheGuardiansChildren(): void
|
||||
{
|
||||
$this->expectAccountHolder();
|
||||
$this->guardians->shouldReceive('children')->once()->with(5)->andReturn([
|
||||
['id' => 42, 'name' => 'Ada', 'birth_year' => '2015', 'relationship' => 'Parent'],
|
||||
]);
|
||||
@@ -117,6 +133,101 @@ class FamilyPageTest extends TestCase
|
||||
self::assertStringContainsString('Add a student', $html);
|
||||
}
|
||||
|
||||
public function testRenderShowsTheAccountHoldersOwnDetails(): void
|
||||
{
|
||||
$this->expectAccountHolder();
|
||||
$this->guardians->shouldReceive('children')->andReturn([]);
|
||||
$this->questions->shouldReceive('findByScope')->andReturn([]);
|
||||
|
||||
$html = $this->page->render([]);
|
||||
|
||||
self::assertStringContainsString('Your details', $html);
|
||||
self::assertStringContainsString('value="Grace"', $html);
|
||||
self::assertStringContainsString('[email protected]', $html);
|
||||
self::assertStringContainsString('value="1984"', $html);
|
||||
// A student in their own right has the box ticked.
|
||||
self::assertStringContainsString("checked='checked'", $html);
|
||||
}
|
||||
|
||||
public function testAGuardianOnlyAccountRendersTheStudentBoxUnticked(): void
|
||||
{
|
||||
$this->expectAccountHolder(['is_student' => false]);
|
||||
$this->guardians->shouldReceive('children')->andReturn([]);
|
||||
$this->questions->shouldReceive('findByScope')->andReturn([]);
|
||||
|
||||
$html = $this->page->render([]);
|
||||
|
||||
self::assertStringContainsString('name="is_student"', $html);
|
||||
self::assertStringNotContainsString("checked='checked'", $html);
|
||||
}
|
||||
|
||||
/**
|
||||
* The birth-year field must not carry `required`: it is asked of a student
|
||||
* only, and the browser would otherwise block a guardian who books solely
|
||||
* for other people from ever saving the form.
|
||||
*/
|
||||
public function testTheOwnBirthYearFieldIsNotBrowserRequired(): void
|
||||
{
|
||||
$this->expectAccountHolder(['is_student' => false, 'birth_year' => '']);
|
||||
$this->guardians->shouldReceive('children')->andReturn([]);
|
||||
$this->questions->shouldReceive('findByScope')->andReturn([]);
|
||||
|
||||
$html = $this->page->render([]);
|
||||
|
||||
self::assertMatchesRegularExpression('/<input[^>]*name="own_birth_year"(?![^>]*\brequired\b)[^>]*>/', $html);
|
||||
}
|
||||
|
||||
public function testSavingOwnDetailsDelegatesToTheServiceAndRedirects(): void
|
||||
{
|
||||
$_POST = [
|
||||
'us_family_action' => 'self',
|
||||
'own_name' => 'Grace H',
|
||||
'own_birth_year' => '1984',
|
||||
'is_student' => '1',
|
||||
];
|
||||
|
||||
$this->guardians->shouldReceive('updateSelf')->once()->with(5, 'Grace H', '1984', true)->andReturn(null);
|
||||
|
||||
$captured = null;
|
||||
$this->capturingPage($captured)->maybeHandleSubmit();
|
||||
|
||||
self::assertSame('https://studio.test/family/?us_family=self', $captured);
|
||||
}
|
||||
|
||||
/** An unticked checkbox is simply absent from the post — that is the "no". */
|
||||
public function testAnAbsentStudentBoxSavesTheAccountAsGuardianOnly(): void
|
||||
{
|
||||
$_POST = [
|
||||
'us_family_action' => 'self',
|
||||
'own_name' => 'Grace H',
|
||||
'own_birth_year' => '',
|
||||
];
|
||||
|
||||
$this->guardians->shouldReceive('updateSelf')->once()->with(5, 'Grace H', '', false)->andReturn(null);
|
||||
|
||||
$captured = null;
|
||||
$this->capturingPage($captured)->maybeHandleSubmit();
|
||||
|
||||
self::assertSame('https://studio.test/family/?us_family=self', $captured);
|
||||
}
|
||||
|
||||
public function testOwnDetailsRefusalIsShownRatherThanRedirected(): void
|
||||
{
|
||||
$_POST = ['us_family_action' => 'self', 'own_name' => 'Grace', 'is_student' => '1'];
|
||||
|
||||
$this->guardians->shouldReceive('updateSelf')->once()->andReturn(
|
||||
new \WP_Error('missing_birth_year', 'Please give your birth year.')
|
||||
);
|
||||
|
||||
$captured = null;
|
||||
$page = $this->capturingPage($captured);
|
||||
$page->shouldNotReceive('redirect');
|
||||
|
||||
$page->maybeHandleSubmit();
|
||||
|
||||
self::assertNull($captured);
|
||||
}
|
||||
|
||||
public function testAddCreatesTheChildRecordsItsAnswersAndRedirects(): void
|
||||
{
|
||||
$_POST = [
|
||||
@@ -347,6 +458,7 @@ class FamilyPageTest extends TestCase
|
||||
{
|
||||
$_GET = ['us_family' => 'added'];
|
||||
|
||||
$this->expectAccountHolder();
|
||||
$this->guardians->shouldReceive('children')->andReturn([]);
|
||||
$this->questions->shouldReceive('findByScope')->andReturn([]);
|
||||
|
||||
|
||||
@@ -321,6 +321,87 @@ class GuardianServiceTest extends TestCase
|
||||
self::assertSame('2015', $this->meta[42][GuardianService::META_BIRTH_YEAR]);
|
||||
}
|
||||
|
||||
public function testUpdateSelfRenamesAndStoresTheBirthYear(): void
|
||||
{
|
||||
$this->meta[5][GuardianService::META_GUARDIAN_ONLY] = '1';
|
||||
|
||||
Functions\expect('wp_update_user')
|
||||
->once()
|
||||
->with(['ID' => 5, 'display_name' => 'Grace H', 'nickname' => 'Grace H'])
|
||||
->andReturn(5);
|
||||
|
||||
self::assertNull($this->service->updateSelf(5, 'Grace H', '1984', true));
|
||||
|
||||
self::assertSame('1984', $this->meta[5][GuardianService::META_BIRTH_YEAR]);
|
||||
// Saying they take lessons makes them a bookable student again.
|
||||
self::assertFalse(GuardianService::isGuardianOnly(5));
|
||||
}
|
||||
|
||||
public function testUpdateSelfMarksTheAccountGuardianOnly(): void
|
||||
{
|
||||
Functions\when('wp_update_user')->justReturn(5);
|
||||
|
||||
self::assertNull($this->service->updateSelf(5, 'Grace H', '', false));
|
||||
|
||||
self::assertSame('1', $this->meta[5][GuardianService::META_GUARDIAN_ONLY]);
|
||||
}
|
||||
|
||||
/**
|
||||
* "I only book for other people" says who books, not "forget my birth year" —
|
||||
* ticking the box back on should not have cost them what was on file.
|
||||
*/
|
||||
public function testUpdateSelfKeepsAStoredBirthYearWhenTheyAreNoLongerAStudent(): void
|
||||
{
|
||||
$this->meta[5][GuardianService::META_BIRTH_YEAR] = '1984';
|
||||
|
||||
Functions\when('wp_update_user')->justReturn(5);
|
||||
|
||||
self::assertNull($this->service->updateSelf(5, 'Grace H', '', false));
|
||||
|
||||
self::assertSame('1984', $this->meta[5][GuardianService::META_BIRTH_YEAR]);
|
||||
}
|
||||
|
||||
public function testUpdateSelfRejectsABlankName(): void
|
||||
{
|
||||
Functions\expect('wp_update_user')->never();
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $this->service->updateSelf(5, ' ', '1984', true));
|
||||
}
|
||||
|
||||
/**
|
||||
* The browser cannot enforce the year conditionally, so the server is the
|
||||
* only thing standing between a student and a nonsense age on their record.
|
||||
*
|
||||
* @dataProvider unusableBirthYears
|
||||
*/
|
||||
public function testUpdateSelfRefusesAnUnusableBirthYearFromAStudent(string $submitted): void
|
||||
{
|
||||
Functions\expect('wp_update_user')->never();
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $this->service->updateSelf(5, 'Grace H', $submitted, true));
|
||||
}
|
||||
|
||||
public function testAccountHolderReportsTheirOwnDetails(): void
|
||||
{
|
||||
$this->meta[5][GuardianService::META_BIRTH_YEAR] = '1984';
|
||||
|
||||
Functions\when('get_userdata')->justReturn($this->user(5, 'Grace', 'Hopper', email: '[email protected]'));
|
||||
|
||||
self::assertSame(
|
||||
['name' => 'Grace Hopper', 'email' => '[email protected]', 'birth_year' => '1984', 'is_student' => true],
|
||||
$this->service->accountHolder(5)
|
||||
);
|
||||
}
|
||||
|
||||
public function testAccountHolderReportsAGuardianOnlyAccountAsNotAStudent(): void
|
||||
{
|
||||
$this->meta[5][GuardianService::META_GUARDIAN_ONLY] = '1';
|
||||
|
||||
Functions\when('get_userdata')->justReturn($this->user(5, 'Grace', 'Hopper', email: '[email protected]'));
|
||||
|
||||
self::assertFalse($this->service->accountHolder(5)['is_student']);
|
||||
}
|
||||
|
||||
public function testRemoveChildUnlinksAndDeletesAChildWithNoHistory(): void
|
||||
{
|
||||
$this->guardians->shouldReceive('isGuardianOf')->with(5, 42)->andReturn(true);
|
||||
|
||||
Reference in New Issue
Block a user