Ask some registration questions of students only

Every account-signup question was asked of everybody who registered, on the
same terms: "school and grade" had to be put to an adult signing themselves
up, and a question a studio needed answered for each student could only be
made required by demanding it of everyone.

A question now carries an audience — everyone, or only the students someone
registers on behalf of — and its own required flag for each side, so optional
for you and required for every student you enrol is expressible. Both settings
are account-scope only: an offering asks its questions once, about the student
being booked, so there is no second audience to differ from, and an offering
question mirrors its single "required" into both columns.

Every caller reads askedOfSelf()/isRequiredForSelf()/isRequiredForChild()
rather than the raw flags, so a students-only question can neither block the
account holder nor have an answer filed against them by a crafted post. The
family screen, which only ever adds a student, is held to the students' rule.

is_required_child arrives from dbDelta defaulting to 0, which would quietly
stop every existing required question being required of the students a
guardian registers — the case it most likely existed for. A one-time backfill
copies is_required across, guarded by its own option so a question later made
optional for students stays that way.

Closes #163

Co-Authored-By: Claude Opus 5 <[email protected]>

tests/Unit/Auth/RegistrationPageTest.php

@@ -1017,7 +1017,7 @@ class RegistrationPageTest extends TestCase
         ];
 
         $this->ctx['questions']->shouldReceive('findByScope')->andReturn([
-            new Question(offeringId: null, label: 'Instrument', isRequired: true, scope: Question::SCOPE_ACCOUNT, id: 7),
+            new Question(offeringId: null, label: 'Instrument', isRequired: true, scope: Question::SCOPE_ACCOUNT, isRequiredChild: true, id: 7),
         ]);
 
         Functions\when('email_exists')->justReturn(false);
@@ -1247,4 +1247,157 @@ class RegistrationPageTest extends TestCase
             'The account holder answers the questions above the students they are adding.'
         );
     }
+
+    /**
+     * A "students only" question describes a child being registered, so it is put
+     * to each student and never to the account holder about themselves.
+     */
+    public function testAStudentsOnlyQuestionIsAskedOfTheStudentsAndNotOfTheAccountHolder(): void
+    {
+        $this->stubRenderContext();
+
+        $question = new Question(
+            null,
+            'School and grade',
+            scope: Question::SCOPE_ACCOUNT,
+            audience: Question::AUDIENCE_CHILD,
+            isRequiredChild: true,
+            id: 7
+        );
+        $this->ctx['questions']->shouldReceive('findByScope')->with(Question::SCOPE_ACCOUNT, Mockery::any())->andReturn([$question]);
+
+        $html = $this->ctx['page']->render([]);
+
+        self::assertStringNotContainsString('name="us_answers[7]"', $html);
+        self::assertStringContainsString('name="children[0][answers][7]"', $html);
+    }
+
+    /**
+     * The two required flags are read where each applies: the browser is asked to
+     * enforce the account holder's, and the students' block carries the marker
+     * without the attribute (it may not be in play at all).
+     */
+    public function testTheFormMarksAQuestionRequiredWhereItActuallyIs(): void
+    {
+        $this->stubRenderContext();
+
+        $question = new Question(
+            null,
+            'Previous experience',
+            scope: Question::SCOPE_ACCOUNT,
+            isRequired: false,
+            isRequiredChild: true,
+            id: 7
+        );
+        $this->ctx['questions']->shouldReceive('findByScope')->with(Question::SCOPE_ACCOUNT, Mockery::any())->andReturn([$question]);
+
+        $html = $this->ctx['page']->render([]);
+
+        // No `required` attribute on the account holder's copy, and no marker on
+        // its label — they may leave it blank.
+        self::assertStringContainsString('<input type="text" name="us_answers[7]" id="us-reg-q-7">', $html);
+        self::assertStringContainsString('<label for="us-reg-q-7">Previous experience</label>', $html);
+
+        // The student's copy is marked required, without the attribute: the block
+        // may not be in play at all, so the server is what enforces it.
+        self::assertStringContainsString('<label for="us-child-0-q-7">Previous experience <span class="us-required" aria-hidden="true">*</span></label>', $html);
+        self::assertStringContainsString('<input type="text" name="children[0][answers][7]" id="us-child-0-q-7">', $html);
+    }
+
+    /**
+     * The point of the two flags: an adult signing themselves up can leave the
+     * question blank, while every student they enrol must answer it.
+     */
+    public function testAQuestionOptionalForYouIsStillRequiredOfEachStudent(): void
+    {
+        $_POST = [
+            'password'           => 'thistle-marrow-42',
+            'display_name'       => 'Grace',
+            'birth_year'         => '1990',
+            'us_registering_for' => RegistrationPage::FOR_BOTH,
+            'us_answers'         => ['7' => '  '],
+            'children'           => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => '  ']]],
+        ];
+
+        $question = new Question(null, 'Instrument', isRequired: false, scope: Question::SCOPE_ACCOUNT, isRequiredChild: true, id: 7);
+        $this->ctx['questions']->shouldReceive('findByScope')->andReturn([$question]);
+        Functions\when('email_exists')->justReturn(false);
+        Functions\expect('wp_insert_user')->never();
+
+        $result = $this->submit(new Invite(email: '[email protected]', token: 'hash'), false);
+
+        // The student's blank is what stopped it — the account holder's was fine.
+        self::assertStringContainsString('for each student', $result);
+    }
+
+    public function testTheAccountHolderMayLeaveBlankWhatTheirStudentsMustAnswer(): void
+    {
+        $_POST = [
+            'password'           => 'thistle-marrow-42',
+            'display_name'       => 'Grace',
+            'birth_year'         => '1990',
+            'us_registering_for' => RegistrationPage::FOR_BOTH,
+            'us_answers'         => ['7' => '  '],
+            'children'           => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']]],
+        ];
+
+        $question = new Question(null, 'Instrument', isRequired: false, scope: Question::SCOPE_ACCOUNT, isRequiredChild: true, id: 7);
+        $this->ctx['questions']->shouldReceive('findByScope')->andReturn([$question]);
+        $this->ctx['guardians']->shouldReceive('createChild')->once()->andReturn(101);
+        $this->stubInviteSuccess();
+
+        $students = [];
+        $this->ctx['answers']->shouldReceive('insert')->andReturnUsing(
+            static function (Answer $answer) use (&$students): int {
+                $students[] = $answer->studentId;
+                return 1;
+            }
+        );
+
+        self::assertSame('invite', $this->submit(new Invite(email: '[email protected]', token: 'hash'), false));
+
+        // Only the student answered, so only the student has an answer stored.
+        self::assertSame([101], $students);
+    }
+
+    /**
+     * A question the account holder is never shown cannot be one they are held
+     * to, nor one an answer can be filed against them for — a crafted post that
+     * supplies both is ignored on both counts.
+     */
+    public function testAStudentsOnlyQuestionNeitherBlocksNorStoresAgainstTheAccountHolder(): void
+    {
+        $_POST = [
+            'password'           => 'thistle-marrow-42',
+            'display_name'       => 'Grace',
+            'birth_year'         => '1990',
+            'us_registering_for' => RegistrationPage::FOR_BOTH,
+            'us_answers'         => ['7' => 'Crafted by hand'],
+            'children'           => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Grade 4']]],
+        ];
+
+        $question = new Question(
+            null,
+            'School and grade',
+            isRequired: true,
+            scope: Question::SCOPE_ACCOUNT,
+            audience: Question::AUDIENCE_CHILD,
+            isRequiredChild: true,
+            id: 7
+        );
+        $this->ctx['questions']->shouldReceive('findByScope')->andReturn([$question]);
+        $this->ctx['guardians']->shouldReceive('createChild')->once()->andReturn(101);
+        $this->stubInviteSuccess();
+
+        $recorded = [];
+        $this->ctx['answers']->shouldReceive('insert')->andReturnUsing(
+            static function (Answer $answer) use (&$recorded): int {
+                $recorded[] = [$answer->studentId, $answer->answerValue];
+                return 1;
+            }
+        );
+
+        self::assertSame('invite', $this->submit(new Invite(email: '[email protected]', token: 'hash'), false));
+        self::assertSame([[101, 'Grade 4']], $recorded);
+    }
 }

tests/Unit/Guardian/FamilyPageTest.php

@@ -76,9 +76,21 @@ class FamilyPageTest extends TestCase
         return $page;
     }
 
+    /**
+     * 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
+     * the upgrade backfill leaves them in.
+     */
     private function question(int $id, bool $required): Question
     {
-        return new Question(offeringId: null, label: 'Instrument', isRequired: $required, scope: Question::SCOPE_ACCOUNT, id: $id);
+        return new Question(
+            offeringId: null,
+            label: 'Instrument',
+            isRequired: $required,
+            scope: Question::SCOPE_ACCOUNT,
+            isRequiredChild: $required,
+            id: $id
+        );
     }
 
     public function testLoggedOutVisitorIsOfferedALoginLink(): void
@@ -158,6 +170,71 @@ class FamilyPageTest extends TestCase
         self::assertNull($captured);
     }
 
+    /**
+     * This screen only ever adds a student, so the students' required-ness is the
+     * one that applies: a question required of the account holder alone must not
+     * stop a guardian adding a child.
+     */
+    public function testAddIsNotBlockedByAQuestionRequiredOnlyOfTheAccountHolder(): void
+    {
+        $_POST = [
+            'us_family_action' => 'add',
+            'child_name'       => 'Ada',
+            'child_birth_year' => '2015',
+            'us_answers'       => [7 => '   '],
+        ];
+
+        $question = new Question(
+            offeringId: null,
+            label: 'Instrument',
+            isRequired: true,
+            scope: Question::SCOPE_ACCOUNT,
+            isRequiredChild: false,
+            id: 7
+        );
+
+        $this->questions->shouldReceive('findByScope')->once()->andReturn([$question]);
+        $this->guardians->shouldReceive('createChild')->once()->andReturn(42);
+
+        // Nothing was typed, so nothing is stored — but the add went through.
+        $this->answers->shouldNotReceive('insert');
+
+        $captured = null;
+        $this->capturingPage($captured)->maybeHandleSubmit();
+
+        self::assertSame('https://studio.test/family/?us_family=added', $captured);
+    }
+
+    public function testAddIsBlockedByAQuestionRequiredOnlyOfTheStudents(): void
+    {
+        $_POST = [
+            'us_family_action' => 'add',
+            'child_name'       => 'Ada',
+            'child_birth_year' => '2015',
+            'us_answers'       => [7 => ''],
+        ];
+
+        $question = new Question(
+            offeringId: null,
+            label: 'Instrument',
+            isRequired: false,
+            scope: Question::SCOPE_ACCOUNT,
+            isRequiredChild: true,
+            id: 7
+        );
+
+        $this->questions->shouldReceive('findByScope')->once()->andReturn([$question]);
+        $this->guardians->shouldNotReceive('createChild');
+
+        $captured = null;
+        $page     = $this->capturingPage($captured);
+        $page->shouldNotReceive('redirect');
+
+        $page->maybeHandleSubmit();
+
+        self::assertNull($captured);
+    }
+
     public function testAddSurfacesAServiceErrorInsteadOfRedirecting(): void
     {
         $_POST = ['us_family_action' => 'add', 'child_name' => ''];

tests/Unit/Registration/QuestionFieldTest.php

@@ -0,0 +1,85 @@
+<?php
+declare(strict_types=1);
+
+namespace Unsupervised\Schedular\Tests\Unit\Registration;
+
+use Unsupervised\Schedular\Registration\Question;
+use Unsupervised\Schedular\Registration\QuestionField;
+use Unsupervised\Schedular\Tests\Unit\TestCase;
+
+class QuestionFieldTest extends TestCase
+{
+    public function testRendersATextInputNamedAndLabelledAsAsked(): void
+    {
+        $html = QuestionField::render(new Question(7, 'Your level?', id: 3), 'us_answers[3]', 'us-q-3');
+
+        self::assertStringContainsString('<label for="us-q-3">Your level?</label>', $html);
+        self::assertStringContainsString('<input type="text" name="us_answers[3]" id="us-q-3">', $html);
+    }
+
+    public function testARequiredQuestionIsMarkedAndEnforced(): void
+    {
+        $question = new Question(7, 'Your level?', isRequired: true, id: 3);
+
+        $html = QuestionField::render($question, 'us_answers[3]', 'us-q-3');
+
+        self::assertStringContainsString('us-required', $html);
+        self::assertStringContainsString(' required', $html);
+    }
+
+    /**
+     * A block that may not apply at all keeps the marker and drops the attribute,
+     * so the browser cannot refuse a submit over a field that is out of play.
+     */
+    public function testNotEnforcingRequiredKeepsTheMarkerButDropsTheAttribute(): void
+    {
+        $question = new Question(7, 'Your level?', isRequired: true, id: 3);
+
+        $html = QuestionField::render($question, 'us_answers[3]', 'us-q-3', enforceRequired: false);
+
+        self::assertStringContainsString('us-required', $html);
+        self::assertStringNotContainsString(' required>', $html);
+    }
+
+    /**
+     * Which of the question's two required flags applies depends on whose block
+     * this is, and only the caller knows that.
+     */
+    public function testTheCallerCanOverrideWhichRequiredFlagApplies(): void
+    {
+        $question = new Question(
+            null,
+            'Previous experience',
+            scope: Question::SCOPE_ACCOUNT,
+            isRequired: false,
+            isRequiredChild: true,
+            id: 3
+        );
+
+        $forSelf  = QuestionField::render($question, 'us_answers[3]', 'us-q-3', isRequired: $question->isRequiredForSelf());
+        $forChild = QuestionField::render($question, 'children[0][answers][3]', 'us-child-0-q-3', isRequired: $question->isRequiredForChild());
+
+        self::assertStringNotContainsString('us-required', $forSelf);
+        self::assertStringNotContainsString(' required', $forSelf);
+
+        self::assertStringContainsString('us-required', $forChild);
+        self::assertStringContainsString(' required', $forChild);
+    }
+
+    public function testASelectRendersItsOptionsBehindAnEmptyChoice(): void
+    {
+        $question = new Question(
+            7,
+            'Pick a level',
+            fieldType: Question::FIELD_SELECT,
+            options: ['Beginner', 'Advanced'],
+            id: 3
+        );
+
+        $html = QuestionField::render($question, 'us_answers[3]', 'us-q-3');
+
+        self::assertStringContainsString('<select name="us_answers[3]" id="us-q-3">', $html);
+        self::assertStringContainsString('<option value="Beginner">Beginner</option>', $html);
+        self::assertStringContainsString('<option value="Advanced">Advanced</option>', $html);
+    }
+}

tests/Unit/Registration/QuestionRepositoryTest.php

@@ -156,6 +156,59 @@ class QuestionRepositoryTest extends TestCase
         self::assertSame(30, $this->repo->insert($question));
     }
 
+    public function testInsertStoresAudienceAndTheStudentsRequiredFlag(): void
+    {
+        Functions\expect('current_time')->andReturn('2026-04-01 12:00:00');
+
+        $this->db->shouldReceive('insert')
+            ->once()
+            ->with(
+                'wp_us_questions',
+                Mockery::on(static function (array $data): bool {
+                    return $data['audience'] === Question::AUDIENCE_CHILD
+                        && $data['is_required'] === 0
+                        && $data['is_required_child'] === 1;
+                }),
+                // One placeholder per column, in the same order.
+                Mockery::on(static fn (array $format): bool => count($format) === 11)
+            );
+
+        $this->db->insert_id = 31;
+
+        $question = new Question(
+            null,
+            'School and grade',
+            scope: Question::SCOPE_ACCOUNT,
+            audience: Question::AUDIENCE_CHILD,
+            isRequiredChild: true
+        );
+
+        self::assertSame(31, $this->repo->insert($question));
+    }
+
+    public function testBackfillChildRequiredCopiesTheOldRequiredFlagAcross(): void
+    {
+        $this->db->shouldReceive('prepare')
+            ->once()
+            ->with(Mockery::pattern('/UPDATE %i SET is_required_child = 1 WHERE is_required = 1/'), 'wp_us_questions')
+            ->andReturn('UPDATE `wp_us_questions` SET is_required_child = 1 WHERE is_required = 1');
+
+        $this->db->shouldReceive('query')
+            ->once()
+            ->with('UPDATE `wp_us_questions` SET is_required_child = 1 WHERE is_required = 1')
+            ->andReturn(2);
+
+        self::assertTrue($this->repo->backfillChildRequired());
+    }
+
+    public function testBackfillChildRequiredReportsFailureWhenQueryFails(): void
+    {
+        $this->db->shouldReceive('prepare')->once()->andReturn('UPDATE ...');
+        $this->db->shouldReceive('query')->once()->andReturn(false);
+
+        self::assertFalse($this->repo->backfillChildRequired());
+    }
+
     public function testFindByScopeActiveOnlyPreparesQuery(): void
     {
         $this->db->shouldReceive('prepare')

tests/Unit/Registration/QuestionTest.php

@@ -101,11 +101,112 @@ class QuestionTest extends TestCase
         $question = new Question(7, 'Label', Question::FIELD_TEXT, id: 9);
         $arr      = $question->toArray();
 
-        foreach (['id', 'offering_id', 'scope', 'label', 'field_type', 'options', 'is_required', 'sort_order', 'is_active'] as $key) {
+        foreach (['id', 'offering_id', 'scope', 'label', 'field_type', 'options', 'audience', 'is_required', 'is_required_child', 'sort_order', 'is_active'] as $key) {
             self::assertArrayHasKey($key, $arr);
         }
     }
 
+    public function testDefaultsToBeingAskedOfEveryoneAndRequiredOfNobody(): void
+    {
+        $question = new Question(null, 'Instrument', scope: Question::SCOPE_ACCOUNT);
+
+        self::assertSame(Question::AUDIENCE_ALL, $question->audience);
+        self::assertTrue($question->askedOfSelf());
+        self::assertFalse($question->isRequiredForSelf());
+        self::assertFalse($question->isRequiredForChild());
+    }
+
+    public function testAChildAudienceQuestionIsNeverAskedOfTheAccountHolder(): void
+    {
+        $question = new Question(
+            null,
+            'School and grade',
+            scope: Question::SCOPE_ACCOUNT,
+            audience: Question::AUDIENCE_CHILD,
+            isRequired: true,
+            isRequiredChild: true
+        );
+
+        self::assertFalse($question->askedOfSelf());
+
+        // Required-ness cannot outlive the audience: a question the account
+        // holder is never shown must never be one they are held to.
+        self::assertFalse($question->isRequiredForSelf());
+        self::assertTrue($question->isRequiredForChild());
+    }
+
+    public function testAQuestionCanBeOptionalForYouAndRequiredForYourStudents(): void
+    {
+        $question = new Question(
+            null,
+            'Previous experience',
+            scope: Question::SCOPE_ACCOUNT,
+            isRequired: false,
+            isRequiredChild: true
+        );
+
+        self::assertTrue($question->askedOfSelf());
+        self::assertFalse($question->isRequiredForSelf());
+        self::assertTrue($question->isRequiredForChild());
+    }
+
+    public function testFromRowReadsAudienceAndTheStudentsRequiredFlag(): void
+    {
+        $row = (object) [
+            'id'                => '6',
+            'offering_id'       => null,
+            'scope'             => Question::SCOPE_ACCOUNT,
+            'label'             => 'School and grade',
+            'field_type'        => Question::FIELD_TEXT,
+            'options'           => null,
+            'audience'          => Question::AUDIENCE_CHILD,
+            'is_required'       => '0',
+            'is_required_child' => '1',
+            'sort_order'        => '0',
+            'is_active'         => '1',
+        ];
+
+        $question = Question::fromRow($row);
+
+        self::assertSame(Question::AUDIENCE_CHILD, $question->audience);
+        self::assertFalse($question->askedOfSelf());
+        self::assertTrue($question->isRequiredForChild());
+    }
+
+    /**
+     * A row read before dbDelta has added the columns — or one carrying a value
+     * no longer recognised — falls back to the behaviour every question had
+     * before the distinction existed: asked of everyone.
+     */
+    public function testFromRowFallsBackToEveryoneWhenAudienceIsMissingOrUnknown(): void
+    {
+        $base = [
+            'id'          => '7',
+            'offering_id' => null,
+            'scope'       => Question::SCOPE_ACCOUNT,
+            'label'       => 'Instrument',
+            'field_type'  => Question::FIELD_TEXT,
+            'options'     => null,
+            'is_required' => '1',
+            'sort_order'  => '0',
+            'is_active'   => '1',
+        ];
+
+        $missing = Question::fromRow((object) $base);
+        $unknown = Question::fromRow((object) ($base + ['audience' => 'grown-ups']));
+
+        self::assertSame(Question::AUDIENCE_ALL, $missing->audience);
+        self::assertTrue($missing->isRequiredForSelf());
+        self::assertFalse($missing->isRequiredForChild());
+        self::assertSame(Question::AUDIENCE_ALL, $unknown->audience);
+    }
+
+    public function testValidAudienceConstants(): void
+    {
+        self::assertContains(Question::AUDIENCE_ALL, Question::VALID_AUDIENCES);
+        self::assertContains(Question::AUDIENCE_CHILD, Question::VALID_AUDIENCES);
+    }
+
     public function testValidFieldTypeConstants(): void
     {
         self::assertContains(Question::FIELD_TEXT, Question::VALID_FIELD_TYPES);