Require a name and birth year for every student
CI / Tests (PHP 8.1) (pull_request) Successful in 42s
CI / Coding Standards (pull_request) Successful in 2m56s
CI / PHPStan (pull_request) Successful in 2m56s
CI / Tests (PHP 8.2) (pull_request) Successful in 50s
CI / No Debug Code (pull_request) Successful in 2s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m45s
CI / Build Plugin Zip (pull_request) Skipped
CI / Tests (PHP 8.1) (pull_request) Successful in 42s
CI / Coding Standards (pull_request) Successful in 2m56s
CI / PHPStan (pull_request) Successful in 2m56s
CI / Tests (PHP 8.2) (pull_request) Successful in 50s
CI / No Debug Code (pull_request) Successful in 2s
CI / Tests (PHP 8.3) (pull_request) Successful in 2m45s
CI / Build Plugin Zip (pull_request) Skipped
Both fields are marked in their labels the same way a required registration question is, and enforced on the server whichever form they arrive from: GuardianService::createChild() and updateChild() now refuse a blank name or an unusable birth year, and the signup form checks the same rule up front, before it creates a single user, so a bad block never leaves a half-registered family behind. normaliseBirthYear() became public and static so both paths share one definition of what a usable year is. The signup form cannot lean on the browser here. Its child blocks are hidden until the parent/guardian box is ticked, and a `required` field inside a hidden container makes the whole form unsubmittable with no control the user can reach to fix — the same trap the guardian's own question panel already sidesteps by disabling rather than hiding. So register.js puts `required` on and takes it off along with the block itself, and the server is what makes the rule hold with JavaScript off. The profile screen has no such problem: its forms are always visible, so the attribute is static there. One behaviour change beyond the requirement: a child block with anything typed into it is now reported back instead of dropped. Previously any block without a name was silently discarded, which would now mean losing a birth year the guardian had filled in. A wholly untouched spare block — the one the form always renders for "add another" — is still ignored. Verified the required-toggling in a headless browser: unticked submits, ticked blocks an empty block, a cloned block inherits the requirement, and re-unticking leaves nothing behind to block a non-guardian signup. Closes #148 Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -38,7 +38,11 @@ class RegistrationPageTest extends TestCase
|
||||
Functions\when('sanitize_textarea_field')->alias(static fn ($v) => $v);
|
||||
Functions\when('sanitize_email')->alias(static fn ($v) => $v);
|
||||
Functions\when('absint')->alias(static fn ($v) => (int) $v);
|
||||
Functions\when('current_time')->justReturn('2024-01-01 00:00:00');
|
||||
// The birth-year check reads current_time('Y'), so answer that format
|
||||
// properly rather than leaving it to cast out of the datetime string.
|
||||
Functions\when('current_time')->alias(
|
||||
static fn (string $type = 'mysql'): string => 'Y' === $type ? '2024' : '2024-01-01 00:00:00'
|
||||
);
|
||||
Functions\when('wp_enqueue_style')->justReturn(null);
|
||||
Functions\when('wp_enqueue_script')->justReturn(null);
|
||||
|
||||
@@ -619,7 +623,7 @@ class RegistrationPageTest extends TestCase
|
||||
'us_is_guardian' => '1',
|
||||
'children' => [
|
||||
['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']],
|
||||
['name' => 'Alan', 'birth_year' => '', 'answers' => [7 => 'Violin']],
|
||||
['name' => 'Alan', 'birth_year' => '2017', 'answers' => [7 => 'Violin']],
|
||||
// An untouched spare block is dropped, not rejected.
|
||||
['name' => ' ', 'birth_year' => '', 'answers' => []],
|
||||
],
|
||||
@@ -634,7 +638,7 @@ class RegistrationPageTest extends TestCase
|
||||
Functions\when('is_wp_error')->alias(static fn ($thing): bool => $thing instanceof \WP_Error);
|
||||
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Ada', '2015')->andReturn(101);
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Alan', '')->andReturn(102);
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Alan', '2017')->andReturn(102);
|
||||
|
||||
$recorded = [];
|
||||
$this->ctx['answers']->shouldReceive('insert')->andReturnUsing(
|
||||
@@ -670,6 +674,68 @@ class RegistrationPageTest extends TestCase
|
||||
self::assertStringContainsString('at least one student', $result);
|
||||
}
|
||||
|
||||
/**
|
||||
* A block the guardian actually typed into is theirs to correct, not ours to
|
||||
* discard — only a wholly untouched spare is dropped. Losing the birth year
|
||||
* they filled in and registering a nameless student would be worse than
|
||||
* telling them what is missing.
|
||||
*/
|
||||
public function testGuardianSignupRejectsAHalfFilledChildRatherThanDroppingIt(): void
|
||||
{
|
||||
$_POST = [
|
||||
'password' => 'password123',
|
||||
'display_name' => 'Grace',
|
||||
'us_is_guardian' => '1',
|
||||
'children' => [
|
||||
['name' => 'Ada', 'birth_year' => '2015', 'answers' => []],
|
||||
['name' => '', 'birth_year' => '2017', 'answers' => []],
|
||||
],
|
||||
];
|
||||
|
||||
$this->ctx['questions']->shouldReceive('findByScope')->andReturn([]);
|
||||
Functions\when('email_exists')->justReturn(false);
|
||||
Functions\expect('wp_insert_user')->never();
|
||||
$this->ctx['guardians']->shouldNotReceive('createChild');
|
||||
|
||||
self::assertStringContainsString(
|
||||
'give each student a name',
|
||||
$this->submit(new Invite(email: '[email protected]', token: 'hash'), false)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider rejectedBirthYears
|
||||
*/
|
||||
public function testGuardianSignupRejectsAChildWithoutAUsableBirthYear(string $submitted): void
|
||||
{
|
||||
$_POST = [
|
||||
'password' => 'password123',
|
||||
'display_name' => 'Grace',
|
||||
'us_is_guardian' => '1',
|
||||
'children' => [['name' => 'Ada', 'birth_year' => $submitted, 'answers' => []]],
|
||||
];
|
||||
|
||||
$this->ctx['questions']->shouldReceive('findByScope')->andReturn([]);
|
||||
Functions\when('email_exists')->justReturn(false);
|
||||
Functions\expect('wp_insert_user')->never();
|
||||
$this->ctx['guardians']->shouldNotReceive('createChild');
|
||||
|
||||
self::assertStringContainsString(
|
||||
'birth year',
|
||||
$this->submit(new Invite(email: '[email protected]', token: 'hash'), false)
|
||||
);
|
||||
}
|
||||
|
||||
/** @return array<string, array{string}> */
|
||||
public static function rejectedBirthYears(): array
|
||||
{
|
||||
return [
|
||||
'left blank' => [''],
|
||||
'a full date' => ['2015-04-02'],
|
||||
'in the future' => ['2027'],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Required per-child answers are validated before any user exists, so a
|
||||
* missing one never leaves a half-registered family behind.
|
||||
@@ -681,8 +747,8 @@ class RegistrationPageTest extends TestCase
|
||||
'display_name' => 'Grace',
|
||||
'us_is_guardian' => '1',
|
||||
'children' => [
|
||||
['name' => 'Ada', 'birth_year' => '', 'answers' => [7 => 'Piano']],
|
||||
['name' => 'Alan', 'birth_year' => '', 'answers' => [7 => ' ']],
|
||||
['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']],
|
||||
['name' => 'Alan', 'birth_year' => '2017', 'answers' => [7 => ' ']],
|
||||
],
|
||||
];
|
||||
|
||||
@@ -708,8 +774,8 @@ class RegistrationPageTest extends TestCase
|
||||
'display_name' => 'Grace',
|
||||
'us_is_guardian' => '1',
|
||||
'children' => [
|
||||
['name' => 'Ada', 'birth_year' => '', 'answers' => []],
|
||||
['name' => 'Alan', 'birth_year' => '', 'answers' => []],
|
||||
['name' => 'Ada', 'birth_year' => '2015', 'answers' => []],
|
||||
['name' => 'Alan', 'birth_year' => '2017', 'answers' => []],
|
||||
],
|
||||
];
|
||||
|
||||
@@ -717,8 +783,8 @@ class RegistrationPageTest extends TestCase
|
||||
Functions\when('wp_insert_user')->justReturn(42);
|
||||
Functions\when('is_wp_error')->alias(static fn ($thing): bool => $thing instanceof \WP_Error);
|
||||
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Ada', '')->andReturn(101);
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Alan', '')
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Ada', '2015')->andReturn(101);
|
||||
$this->ctx['guardians']->shouldReceive('createChild')->once()->with(42, 'Alan', '2017')
|
||||
->andReturn(new \WP_Error('link_failed', 'Nope.'));
|
||||
|
||||
$deleted = [];
|
||||
@@ -745,7 +811,7 @@ class RegistrationPageTest extends TestCase
|
||||
'display_name' => 'Grace',
|
||||
'us_is_guardian' => '1',
|
||||
'accept' => [3],
|
||||
'children' => [['name' => 'Ada', 'birth_year' => '', 'answers' => []]],
|
||||
'children' => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => []]],
|
||||
];
|
||||
|
||||
$version = new PolicyVersion(policyId: 1, versionNumber: 1, body: 'Terms', status: PolicyVersion::STATUS_PUBLISHED, id: 3);
|
||||
|
||||
Reference in New Issue
Block a user