Unsupervised/unsupervised-scheduler
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 12 Wiki Activity

Require a name and birth year for every student

Browse Source 
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:
thatguygriff
2026-07-29 21:00:29 -03:00
co-authored by Claude Opus 5
parent 7e2bba79fe
commit 98e1bfb2f9
10 changed files with 215 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Unit/Auth/RegistrationPageTest.php
+76 -10
View File
@@ -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);
tests/Unit/Guardian/GuardianServiceTest.php
+28 -12
View File
@@ -116,20 +116,35 @@ class GuardianServiceTest extends TestCase
Functions\expect('wp_delete_user')->once()->with(42);
self::assertInstanceOf(\WP_Error::class, $this->service->createChild(5, 'Ada'));
self::assertInstanceOf(\WP_Error::class, $this->service->createChild(5, 'Ada', '2015'));
}
/**
* @dataProvider unusableBirthYears
*/
public function testCreateChildClearsAnUnusableBirthYear(string $submitted): void
public function testCreateChildRefusesAnUnusableBirthYear(string $submitted): void
{
Functions\when('wp_insert_user')->justReturn(42);
$this->guardians->shouldReceive('insert')->once()->andReturn(7);
// Refused before anything is written, so no orphan user is left behind.
Functions\expect('wp_insert_user')->never();
$this->guardians->shouldNotReceive('insert');
$this->service->createChild(5, 'Ada', $submitted);
$result = $this->service->createChild(5, 'Ada', $submitted);
self::assertArrayNotHasKey(GuardianService::META_BIRTH_YEAR, $this->meta[42] ?? []);
self::assertInstanceOf(\WP_Error::class, $result);
self::assertSame('missing_birth_year', $result->get_error_code());
self::assertArrayNotHasKey(42, $this->meta);
}
/** @dataProvider unusableBirthYears */
public function testUpdateChildRefusesAnUnusableBirthYear(string $submitted): void
{
$this->guardians->shouldReceive('isGuardianOf')->with(5, 42)->andReturn(true);
Functions\expect('wp_update_user')->never();
$result = $this->service->updateChild(5, 42, 'Ada', $submitted);
self::assertInstanceOf(\WP_Error::class, $result);
self::assertSame('missing_birth_year', $result->get_error_code());
}
/** @return array<string, array{string}> */
@@ -142,6 +157,7 @@ class GuardianServiceTest extends TestCase
'too many digits' => ['20155'],
'before 1900' => ['1899'],
'later than today' => ['2027'],
'left blank' => [''],
];
}
@@ -230,9 +246,8 @@ class GuardianServiceTest extends TestCase
}
/**
* Saving a child drops the legacy full date. Without that, clearing the birth
* year on a child who predates the change would leave the old date behind for
* the fallback above to resurrect on the next read.
* Saving a child drops the legacy full date, so the fallback above can never
* outrank a year the guardian has since corrected by hand.
*/
public function testSavingAChildClearsTheLegacyDateOfBirth(): void
{
@@ -241,15 +256,16 @@ class GuardianServiceTest extends TestCase
$this->guardians->shouldReceive('isGuardianOf')->with(5, 42)->andReturn(true);
Functions\when('wp_update_user')->justReturn(42);
self::assertNull($this->service->updateChild(5, 42, 'Ada L', ''));
self::assertNull($this->service->updateChild(5, 42, 'Ada L', '2016'));
self::assertArrayNotHasKey(GuardianService::META_DOB, $this->meta[42] ?? []);
self::assertArrayNotHasKey(GuardianService::META_BIRTH_YEAR, $this->meta[42] ?? []);
self::assertSame('2016', $this->meta[42][GuardianService::META_BIRTH_YEAR]);
// The corrected year is what is read back, not the year of the old date.
$this->guardians->shouldReceive('findByGuardian')->with(5)->andReturn([new GuardianLink(5, 42)]);
Functions\when('get_userdata')->justReturn($this->user(42, 'Ada', 'Lovelace'));
self::assertSame('', $this->service->children(5)[0]['birth_year']);
self::assertSame('2016', $this->service->children(5)[0]['birth_year']);
}
public function testBookableStudentsIsJustTheUserWithoutChildren(): void