Ask who the signup is for, and ask each student the studio's questions #159
@@ -21,6 +21,7 @@ each change under the current top section as you work.
|
|||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
- Signup now asks **"Who are you registering?"** as a three-way choice — **just myself**, **on behalf of one or more students**, or **both** — in place of the single parent/guardian tick. The tick could only ever say "I have children to add"; it could not say whether the account holder was a student themselves, so every account was offered its own name in the **Who is this for?** picker whether or not anyone meant to book them a lesson. Choosing *on behalf of* now leaves the account holder out of that picker. Existing accounts are unaffected and stay bookable, since the flag records only the new "not a student" case.
|
- Signup now asks **"Who are you registering?"** as a three-way choice — **just myself**, **on behalf of one or more students**, or **both** — in place of the single parent/guardian tick. The tick could only ever say "I have children to add"; it could not say whether the account holder was a student themselves, so every account was offered its own name in the **Who is this for?** picker whether or not anyone meant to book them a lesson. Choosing *on behalf of* now leaves the account holder out of that picker. Existing accounts are unaffected and stay bookable, since the flag records only the new "not a student" case.
|
||||||
|
- The studio's **account-signup questions are now asked of anyone registering as a student**, including someone registering themselves alongside their children. Choosing **both** previously collected the questions per child only, so the account holder's own instrument, level and the rest were never asked for or stored, even though they could book lessons. Their answers are recorded against their own account, and a blank required answer now names them rather than blaming "each student".
|
||||||
- A student's **name and birth year are now required**, marked in the form the same way a required registration question is and enforced on the server whichever way they were submitted. On signup the requirement applies only once the parent/guardian box is ticked, so registering for yourself is unaffected. A student block you have started filling in is now reported back to you rather than silently dropped when the name is missing — only a completely untouched spare block is still ignored.
|
- A student's **name and birth year are now required**, marked in the form the same way a required registration question is and enforced on the server whichever way they were submitted. On signup the requirement applies only once the parent/guardian box is ticked, so registering for yourself is unaffected. A student block you have started filling in is now reported back to you rather than silently dropped when the name is missing — only a completely untouched spare block is still ignored.
|
||||||
- Signup and the profile page now ask for a **birth year** rather than a full date of birth — a four-digit year between 1900 and the current year, with anything else discarded rather than stored. Students added before this change keep showing a birth year, derived from the date already on file; that old full date is then dropped the first time the record is saved, so the studio ends up holding only what it now asks for. No bulk purge runs, so a site wanting the remaining old dates gone should clear the `us_date_of_birth` user meta directly.
|
- Signup and the profile page now ask for a **birth year** rather than a full date of birth — a four-digit year between 1900 and the current year, with anything else discarded rather than stored. Students added before this change keep showing a birth year, derived from the date already on file; that old full date is then dropped the first time the record is saved, so the studio ends up holding only what it now asks for. No bulk purge runs, so a site wanting the remaining old dates gone should clear the `us_date_of_birth` user meta directly.
|
||||||
- The interface now says **student** where it said "child" and **profile** where it said "family". The `[us_family]` page is headed **Your profile**, its form is **Add a student**, signup asks for a **Student's name**, and the wp-admin students list and student screen both label the relationship **Profile**. Two strings were reworded rather than swapped: the students list reads **Managed by _name_** (a bare "Student of _name_" would read as a teacher's pupil), and a managed account is described as a **managed student account** so it is not confused with the account holder. Internal names — database columns, request parameters, form field names, the `us_family` shortcode and the `us-scheduler/family` block — are unchanged, since they are contracts with existing installs and saved post content.
|
- The interface now says **student** where it said "child" and **profile** where it said "family". The `[us_family]` page is headed **Your profile**, its form is **Add a student**, signup asks for a **Student's name**, and the wp-admin students list and student screen both label the relationship **Profile**. Two strings were reworded rather than swapped: the students list reads **Managed by _name_** (a bare "Student of _name_" would read as a teacher's pupil), and a managed account is described as a **managed student account** so it is not confused with the account holder. Internal names — database columns, request parameters, form field names, the `us_family` shortcode and the `us-scheduler/family` block — are unchanged, since they are contracts with existing installs and saved post content.
|
||||||
|
|||||||
+20
-14
@@ -11,10 +11,10 @@
|
|||||||
* validation.
|
* validation.
|
||||||
* 2. **Who are you registering?** The student section is hidden until the
|
* 2. **Who are you registering?** The student section is hidden until the
|
||||||
* choice is "on behalf of students" or "both", and "Add another student"
|
* choice is "on behalf of students" or "both", and "Add another student"
|
||||||
* clones the student block. Either of those choices also takes the account
|
* clones the student block. "On behalf of students" *alone* also takes the
|
||||||
* holder's *own* question panel out of play — the questions are then asked
|
* account holder's own question panel out of play — they are not a student
|
||||||
* per student, so the server ignores those answers and the browser must not
|
* in that case, so the server ignores those answers and the browser must not
|
||||||
* demand them.
|
* demand them. Under "both" they are a student and do answer them.
|
||||||
* 3. **Password strength.** The password is scored with zxcvbn (via WordPress's
|
* 3. **Password strength.** The password is scored with zxcvbn (via WordPress's
|
||||||
* own `wp.passwordStrength`) and a weak one is refused. The server applies
|
* own `wp.passwordStrength`) and a weak one is refused. The server applies
|
||||||
* its own, coarser rule regardless — see `Auth\PasswordPolicy`.
|
* its own, coarser rule regardless — see `Auth\PasswordPolicy`.
|
||||||
@@ -220,15 +220,21 @@
|
|||||||
/**
|
/**
|
||||||
* Keep the form in step with the choice.
|
* Keep the form in step with the choice.
|
||||||
*
|
*
|
||||||
* Student blocks appear for "students" and "both". The account holder's
|
* Two independent questions, which is why "both" needs its own answer to
|
||||||
* own question panel is the mirror image: the studio's questions describe
|
* each:
|
||||||
* a student, so whenever students are being added they are asked per
|
*
|
||||||
* student instead, and the account holder's copy goes out of play.
|
* - Are student blocks in play? For "students" and "both".
|
||||||
* Disabling it rather than hiding it is what stops a `required` question
|
* - Does the account holder answer the studio's questions themselves? For
|
||||||
* the server will ignore from blocking submit.
|
* "self" and "both" — they are a student in those two, and the questions
|
||||||
|
* describe a student. A pure guardian answers them per student instead,
|
||||||
|
* so their own copy goes out of play; disabling it rather than hiding it
|
||||||
|
* is what stops a `required` question the server will ignore from
|
||||||
|
* blocking submit.
|
||||||
*/
|
*/
|
||||||
function sync() {
|
function sync() {
|
||||||
var wantsStudents = mode() !== 'self';
|
var current = mode();
|
||||||
|
var wantsStudents = current !== 'self';
|
||||||
|
var asksSelf = current !== 'students';
|
||||||
|
|
||||||
children.hidden = !wantsStudents;
|
children.hidden = !wantsStudents;
|
||||||
|
|
||||||
@@ -248,15 +254,15 @@
|
|||||||
|
|
||||||
var fields = steps.step2.querySelectorAll('input, select, textarea');
|
var fields = steps.step2.querySelectorAll('input, select, textarea');
|
||||||
for (var i = 0; i < fields.length; i++) {
|
for (var i = 0; i < fields.length; i++) {
|
||||||
fields[i].disabled = wantsStudents;
|
fields[i].disabled = !asksSelf;
|
||||||
}
|
}
|
||||||
|
|
||||||
// With the questions out of play there is no second step to advance to,
|
// With the questions out of play there is no second step to advance to,
|
||||||
// so "Next" would be a dead end — swap it for the submit.
|
// so "Next" would be a dead end — swap it for the submit.
|
||||||
steps.next.hidden = wantsStudents;
|
steps.next.hidden = !asksSelf;
|
||||||
|
|
||||||
if (steps.earlySubmit) {
|
if (steps.earlySubmit) {
|
||||||
steps.earlySubmit.hidden = !wantsStudents;
|
steps.earlySubmit.hidden = asksSelf;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -144,11 +144,22 @@ A **"Who are you registering?"** choice on the existing `[us_student_register]`
|
|||||||
form (all three signup paths — personal invite, group link, self-approval), as
|
form (all three signup paths — personal invite, group link, self-approval), as
|
||||||
three radios:
|
three radios:
|
||||||
|
|
||||||
| Choice | `us_registering_for` | Student blocks | Account holder is a student |
|
| Choice | `us_registering_for` | Student blocks | Account holder is a student | Answers the studio's questions |
|
||||||
|---|---|---|---|
|
|---|---|---|---|---|
|
||||||
| Just myself | `self` | no | yes |
|
| Just myself | `self` | no | yes | for themselves |
|
||||||
| On behalf of one or more students | `students` | yes | **no** |
|
| On behalf of one or more students | `students` | yes | **no** | per student only |
|
||||||
| Both — myself and one or more students | `both` | yes | yes |
|
| Both — myself and one or more students | `both` | yes | yes | **per student *and* for themselves** |
|
||||||
|
|
||||||
|
The last column follows from the third, and is the whole of it: the
|
||||||
|
account-scope questions describe a *student* — instrument, level, school — so
|
||||||
|
they are asked of everyone being registered as one. Under `both` that is each
|
||||||
|
student **and** the account holder, whose answers are stored against their own
|
||||||
|
user id, not shared with anyone. Under `students` the account holder is not a
|
||||||
|
student, so anything posted for them is ignored outright.
|
||||||
|
|
||||||
|
Required answers are checked in two passes rather than one, so the error can say
|
||||||
|
whose are missing: `both` would otherwise have to blame "each student" for the
|
||||||
|
account holder's own blank field.
|
||||||
|
|
||||||
Radios rather than checkboxes because the three answers are mutually exclusive:
|
Radios rather than checkboxes because the three answers are mutually exclusive:
|
||||||
"both" only means anything as a third choice alongside the other two. Either
|
"both" only means anything as a third choice alongside the other two. Either
|
||||||
|
|||||||
@@ -312,15 +312,21 @@ class RegistrationPage {
|
|||||||
|
|
||||||
$accountQuestions = $this->questions->findByScope( Question::SCOPE_ACCOUNT, activeOnly: true );
|
$accountQuestions = $this->questions->findByScope( Question::SCOPE_ACCOUNT, activeOnly: true );
|
||||||
|
|
||||||
// Registering as a parent/guardian turns the account-signup questions from
|
// The account-signup questions describe a *student* — instrument, level,
|
||||||
// "about you" into "about each child" — they describe the student
|
// school — not whoever holds the account. So they are asked of each
|
||||||
// (instrument, level, school), not the person holding the account.
|
// student being added, and of the account holder only when they are a
|
||||||
|
// student themselves. "Both" is both.
|
||||||
$registeringFor = $this->submittedRegisteringFor();
|
$registeringFor = $this->submittedRegisteringFor();
|
||||||
|
|
||||||
// "Students" and "both" both collect student blocks; only "self" does not.
|
// "Students" and "both" collect student blocks; only "self" does not.
|
||||||
$isGuardian = self::FOR_SELF !== $registeringFor;
|
$isGuardian = self::FOR_SELF !== $registeringFor;
|
||||||
$children = $isGuardian ? $this->submittedChildren() : [];
|
|
||||||
$answers = $isGuardian ? [] : $this->submittedAnswers();
|
// "Self" and "both" make the account holder a student, so they answer the
|
||||||
|
// questions in their own right. Only a pure guardian does not.
|
||||||
|
$asksSelf = self::FOR_STUDENTS !== $registeringFor;
|
||||||
|
|
||||||
|
$children = $isGuardian ? $this->submittedChildren() : [];
|
||||||
|
$answers = $asksSelf ? $this->submittedAnswers() : [];
|
||||||
|
|
||||||
// Everything is validated before a single user is created, so a bad child
|
// Everything is validated before a single user is created, so a bad child
|
||||||
// block never leaves a half-registered family behind.
|
// block never leaves a half-registered family behind.
|
||||||
@@ -342,16 +348,18 @@ class RegistrationPage {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach ( $isGuardian ? array_column( $children, 'answers' ) : [ $answers ] as $set ) {
|
// Checked as two passes rather than one so the message can say *whose*
|
||||||
foreach ( $accountQuestions as $question ) {
|
// answers are missing — under "both" a single message could not.
|
||||||
if ( $question->isRequired && '' === trim( (string) ( $set[ (int) $question->id ] ?? '' ) ) ) {
|
foreach ( array_column( $children, 'answers' ) as $set ) {
|
||||||
return $isGuardian
|
if ( $this->hasUnansweredRequired( $accountQuestions, $set ) ) {
|
||||||
? esc_html__( 'Please answer all required registration questions for each student.', 'unsupervised-schedular' )
|
return esc_html__( 'Please answer all required registration questions for each student.', 'unsupervised-schedular' );
|
||||||
: esc_html__( 'Please answer all required registration questions.', 'unsupervised-schedular' );
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ( $asksSelf && $this->hasUnansweredRequired( $accountQuestions, $answers ) ) {
|
||||||
|
return esc_html__( 'Please answer all required registration questions.', 'unsupervised-schedular' );
|
||||||
|
}
|
||||||
|
|
||||||
if ( email_exists( $email ) ) {
|
if ( email_exists( $email ) ) {
|
||||||
return esc_html__( 'An account already exists for this email.', 'unsupervised-schedular' );
|
return esc_html__( 'An account already exists for this email.', 'unsupervised-schedular' );
|
||||||
}
|
}
|
||||||
@@ -381,7 +389,11 @@ class RegistrationPage {
|
|||||||
if ( '' !== $failure ) {
|
if ( '' !== $failure ) {
|
||||||
return $failure;
|
return $failure;
|
||||||
}
|
}
|
||||||
} else {
|
}
|
||||||
|
|
||||||
|
// After the children, so a rollback that deletes this account cannot
|
||||||
|
// leave its answers behind pointing at a user that no longer exists.
|
||||||
|
if ( $asksSelf ) {
|
||||||
$this->recordAnswers( $accountQuestions, $answers, (int) $userId );
|
$this->recordAnswers( $accountQuestions, $answers, (int) $userId );
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -523,6 +535,22 @@ class RegistrationPage {
|
|||||||
return add_query_arg( 'us_confirm', rawurlencode( $rawToken ), $base );
|
return add_query_arg( 'us_confirm', rawurlencode( $rawToken ), $base );
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether any required question in `$questions` is left blank in `$answers`.
|
||||||
|
*
|
||||||
|
* @param list<Question> $questions
|
||||||
|
* @param array<int, string> $answers
|
||||||
|
*/
|
||||||
|
private function hasUnansweredRequired( array $questions, array $answers ): bool {
|
||||||
|
foreach ( $questions as $question ) {
|
||||||
|
if ( $question->isRequired && '' === trim( (string) ( $answers[ (int) $question->id ] ?? '' ) ) ) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Who this signup is for: {@see FOR_SELF}, {@see FOR_STUDENTS} or
|
* Who this signup is for: {@see FOR_SELF}, {@see FOR_STUDENTS} or
|
||||||
* {@see FOR_BOTH}.
|
* {@see FOR_BOTH}.
|
||||||
|
|||||||
@@ -825,6 +825,95 @@ class RegistrationPageTest extends TestCase
|
|||||||
self::assertSame('invite', $this->submit(new Invite(email: '[email protected]', token: 'hash'), false));
|
self::assertSame('invite', $this->submit(new Invite(email: '[email protected]', token: 'hash'), false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Under "both" the account holder is a student too, so the studio's
|
||||||
|
* questions are asked of them as well as of each student they add. Before
|
||||||
|
* this they were asked per student only, and the account holder's own
|
||||||
|
* answers were never collected or stored.
|
||||||
|
*/
|
||||||
|
public function testBothRecordsAnswersForTheAccountHolderAndEachStudent(): void
|
||||||
|
{
|
||||||
|
$_POST = [
|
||||||
|
'password' => 'thistle-marrow-42',
|
||||||
|
'display_name' => 'Grace',
|
||||||
|
'us_registering_for' => RegistrationPage::FOR_BOTH,
|
||||||
|
'us_answers' => ['7' => 'Cello'],
|
||||||
|
'children' => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']]],
|
||||||
|
];
|
||||||
|
|
||||||
|
$question = new Question(null, 'Instrument', isRequired: true, scope: Question::SCOPE_ACCOUNT, 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));
|
||||||
|
|
||||||
|
// The student's answer against the student, the account holder's against
|
||||||
|
// themselves — not one answer shared between them.
|
||||||
|
self::assertEqualsCanonicalizing([[101, 'Piano'], [42, 'Cello']], $recorded);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testBothRejectsAnUnansweredQuestionForTheAccountHolder(): void
|
||||||
|
{
|
||||||
|
$_POST = [
|
||||||
|
'password' => 'thistle-marrow-42',
|
||||||
|
'display_name' => 'Grace',
|
||||||
|
'us_registering_for' => RegistrationPage::FOR_BOTH,
|
||||||
|
'us_answers' => ['7' => ' '],
|
||||||
|
'children' => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']]],
|
||||||
|
];
|
||||||
|
|
||||||
|
$question = new Question(null, 'Instrument', isRequired: true, scope: Question::SCOPE_ACCOUNT, 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 message names nobody else — the student's answer was fine.
|
||||||
|
self::assertStringContainsString('Please answer all required registration questions.', $result);
|
||||||
|
self::assertStringNotContainsString('for each student', $result);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A pure guardian is not a student, so the questions are theirs to answer
|
||||||
|
* per student and never about them. Anything posted for them is ignored.
|
||||||
|
*/
|
||||||
|
public function testRegisteringOnlyForStudentsStoresNoAnswersForTheAccountHolder(): void
|
||||||
|
{
|
||||||
|
$_POST = [
|
||||||
|
'password' => 'thistle-marrow-42',
|
||||||
|
'display_name' => 'Grace',
|
||||||
|
'us_registering_for' => RegistrationPage::FOR_STUDENTS,
|
||||||
|
'us_answers' => ['7' => 'Should be ignored'],
|
||||||
|
'children' => [['name' => 'Ada', 'birth_year' => '2015', 'answers' => [7 => 'Piano']]],
|
||||||
|
];
|
||||||
|
|
||||||
|
$question = new Question(null, 'Instrument', isRequired: true, scope: Question::SCOPE_ACCOUNT, 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));
|
||||||
|
self::assertSame([101], $students);
|
||||||
|
}
|
||||||
|
|
||||||
public function testGuardianSignupWithNoChildrenIsRejected(): void
|
public function testGuardianSignupWithNoChildrenIsRejected(): void
|
||||||
{
|
{
|
||||||
$_POST = [
|
$_POST = [
|
||||||
|
|||||||
Reference in New Issue
Block a user