From c49171695a5e769f59adc3dd99f68d21e27be624 Mon Sep 17 00:00:00 2001 From: James Griffin Date: Sat, 18 Jul 2026 18:17:20 -0300 Subject: [PATCH 1/2] Add policy, intake, and payment history to the admin student detail view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The student-administration spec deferred three detail-view sections until Payments landed. Adds them now: policy-acceptance history (title, version, context, date), intake answers (label, answer, context), and — gated on manage_billing — payment history with HST breakdown and receipt numbers. New Auth\StudentHistory builds the display rows from per-student queries added to AcceptanceRepository, AnswerRepository, and PaymentRepository; the Payment model now carries created_at so unpaid rows still have a date. Closes #69 Co-Authored-By: Claude Fable 5 --- docs/features/student-administration.md | 25 ++- src/AdminMenu.php | 7 +- src/Auth/StudentController.php | 5 + src/Auth/StudentHistory.php | 112 +++++++++++ src/Payment/Payment.php | 3 + src/Payment/PaymentRepository.php | 17 ++ src/Plugin.php | 2 +- src/Policy/AcceptanceRepository.php | 17 ++ src/Registration/AnswerRepository.php | 17 ++ templates/admin/student-detail.php | 89 +++++++++ tests/Unit/Auth/StudentHistoryTest.php | 180 ++++++++++++++++++ tests/Unit/Payment/PaymentRepositoryTest.php | 17 ++ tests/Unit/Payment/PaymentTest.php | 2 + .../Unit/Policy/AcceptanceRepositoryTest.php | 25 +++ .../Registration/AnswerRepositoryTest.php | 28 +++ 15 files changed, 540 insertions(+), 6 deletions(-) create mode 100644 src/Auth/StudentHistory.php create mode 100644 tests/Unit/Auth/StudentHistoryTest.php diff --git a/docs/features/student-administration.md b/docs/features/student-administration.md index 05d3a10..e631db4 100644 --- a/docs/features/student-administration.md +++ b/docs/features/student-administration.md @@ -10,6 +10,11 @@ No new tables. The views are composed from existing data: - Students are WordPress users with the `us_student` role (`get_users`, `get_userdata`). - Lessons come from `{prefix}us_lessons` (with `{prefix}us_availability` for slot times). - Group-class enrolments come from `{prefix}us_group_enrollments`. +- Policy acceptances come from `{prefix}us_policy_acceptances` (with the policy + and version tables for titles/numbers). +- Intake answers come from `{prefix}us_question_answers` (with `{prefix}us_questions` + for labels). +- Payments come from `{prefix}us_payments`. ## Admin Interface **Students** in wp-admin (`manage_students`, studio admin only): @@ -22,10 +27,15 @@ No new tables. The views are composed from existing data: - **Upcoming lessons** and **Past lessons** — split by the linked availability slot's `start_dt`; each shows date/time, offering, instructor, and status. - **Group-class enrolments** — active/past, with offering title and status. - - *(Later)* policy-acceptance history, intake answers, and payment history once - Payments lands. + - **Policy acceptances** — every acceptance the student has recorded, newest + first: policy title, version, context (account signup / lesson / enrolment), + and when it was accepted. + - **Intake answers** — every registration-question answer, newest first: + question label, answer, and the registration it was given for. + - **Payment history** (`manage_billing` only) — every payment, newest first: + date, context, method, status, subtotal, HST, total, and receipt number. -Read-only in this iteration; cancel/edit actions are a possible follow-up. +Read-only in this iteration; cancel/edit actions are a possible follow-up (#70). ## Capabilities - `manage_students` — studio admin (administrators inherit it via the @@ -38,6 +48,11 @@ Read-only in this iteration; cancel/edit actions are a possible follow-up. `Availability\AvailabilityRepository::findById`, `Offering\OfferingRepository::findById`, `GroupClass\EnrollmentRepository::findByStudent` + `countActiveForStudent` +- History sections: `Auth\StudentHistory` builds the display rows from + `Policy\AcceptanceRepository::findByStudent`, + `Registration\AnswerRepository::findByStudent`, and + `Payment\PaymentRepository::findByStudent`, resolving policy/version titles and + question labels (unit-tested with mocked repositories). - Upcoming/past split: `Auth\StudentSchedule::partition()` (pure, unit-tested) - The upcoming/past split is extracted into a small pure helper so it is unit-testable (the controller itself follows the repo convention of not being @@ -45,3 +60,7 @@ Read-only in this iteration; cancel/edit actions are a possible follow-up. ## Tests - `tests/Unit/Auth/StudentScheduleTest.php` (the pure upcoming/past split helper) +- `tests/Unit/Auth/StudentHistoryTest.php` (history display rows + fallbacks) +- `findByStudent` coverage in `tests/Unit/Policy/AcceptanceRepositoryTest.php`, + `tests/Unit/Registration/AnswerRepositoryTest.php`, and + `tests/Unit/Payment/PaymentRepositoryTest.php` diff --git a/src/AdminMenu.php b/src/AdminMenu.php index 3907cf7..e71c9f2 100644 --- a/src/AdminMenu.php +++ b/src/AdminMenu.php @@ -13,6 +13,7 @@ use Unsupervised\Schedular\Auth\RegistrationController; use Unsupervised\Schedular\Auth\RegistrationMailer; use Unsupervised\Schedular\Auth\RoleManager; use Unsupervised\Schedular\Auth\StudentController; +use Unsupervised\Schedular\Auth\StudentHistory; use Unsupervised\Schedular\Booking\BookingRepository; use Unsupervised\Schedular\Booking\LessonController; use Unsupervised\Schedular\GroupClass\EnrollmentRepository; @@ -25,10 +26,12 @@ use Unsupervised\Schedular\Payment\PaymentReportController; use Unsupervised\Schedular\Payment\PaymentRepository; use Unsupervised\Schedular\Payment\PaymentService; use Unsupervised\Schedular\Payment\StudioSettings; +use Unsupervised\Schedular\Policy\AcceptanceRepository; use Unsupervised\Schedular\Policy\PolicyController; use Unsupervised\Schedular\Policy\PolicyRepository; use Unsupervised\Schedular\Policy\PolicyService; use Unsupervised\Schedular\Policy\PolicyVersionRepository; +use Unsupervised\Schedular\Registration\AnswerRepository; use Unsupervised\Schedular\Registration\QuestionController; use Unsupervised\Schedular\Registration\QuestionRepository; @@ -49,7 +52,7 @@ class AdminMenu { private PaymentController $paymentController; private PaymentReportController $paymentReportController; - public function __construct( AvailabilityRepository $availability, BookingRepository $bookings, OfferingRepository $offerings, QuestionRepository $questions, PolicyRepository $policies, PolicyVersionRepository $policyVersions, PolicyService $policyService, InviteRepository $invites, EnrollmentRepository $enrollments, StudioSettings $settings, PaymentRepository $payments, PaymentService $paymentService, BillingMethodResolver $resolver ) { + public function __construct( AvailabilityRepository $availability, BookingRepository $bookings, OfferingRepository $offerings, QuestionRepository $questions, AnswerRepository $answers, PolicyRepository $policies, PolicyVersionRepository $policyVersions, PolicyService $policyService, AcceptanceRepository $acceptances, InviteRepository $invites, EnrollmentRepository $enrollments, StudioSettings $settings, PaymentRepository $payments, PaymentService $paymentService, BillingMethodResolver $resolver ) { $this->availabilityController = new AvailabilityController( $availability, $offerings ); $this->lessonController = new LessonController( $bookings, $payments, $availability ); $this->offeringController = new OfferingController( $offerings ); @@ -58,7 +61,7 @@ class AdminMenu { $this->registrationController = new RegistrationController( $invites ); $this->registrationApprovalController = new RegistrationApprovalController( new RegistrationMailer() ); $this->groupClassController = new GroupClassController( $enrollments, $offerings ); - $this->studentController = new StudentController( $bookings, $availability, $offerings, $enrollments, $resolver ); + $this->studentController = new StudentController( $bookings, $availability, $offerings, $enrollments, $resolver, new StudentHistory( $acceptances, $policies, $policyVersions, $answers, $questions, $payments ) ); $this->instructorController = new InstructorController(); $this->settings = $settings; $this->accessSettings = new AccessSettings(); diff --git a/src/Auth/StudentController.php b/src/Auth/StudentController.php index 39ba93b..63c7f7c 100644 --- a/src/Auth/StudentController.php +++ b/src/Auth/StudentController.php @@ -21,6 +21,7 @@ class StudentController { private OfferingRepository $offerings, private EnrollmentRepository $enrollments, private BillingMethodResolver $resolver, + private StudentHistory $history, ) {} public function renderPage(): void { @@ -100,6 +101,10 @@ class StudentController { $this->enrollments->findByStudent( (int) $student->ID ) ); + $acceptances = $this->history->policyAcceptances( (int) $student->ID ); + $intake = $this->history->intakeAnswers( (int) $student->ID ); + $payments = $canBilling ? $this->history->payments( (int) $student->ID ) : []; + $backUrl = admin_url( 'admin.php?page=us-students' ); include USC_PLUGIN_DIR . 'templates/admin/student-detail.php'; } diff --git a/src/Auth/StudentHistory.php b/src/Auth/StudentHistory.php new file mode 100644 index 0000000..de058d2 --- /dev/null +++ b/src/Auth/StudentHistory.php @@ -0,0 +1,112 @@ + + */ + public function policyAcceptances( int $studentId ): array { + return array_map( + function ( PolicyAcceptance $acceptance ): array { + $version = $this->policyVersions->findById( $acceptance->policyVersionId ); + $policy = $version ? $this->policies->findById( $version->policyId ) : null; + + return [ + 'policy' => $policy ? $policy->title : sprintf( '#%d', $acceptance->policyVersionId ), + 'version' => $version ? sprintf( 'v%d', $version->versionNumber ) : '—', + 'context' => $this->contextLabel( $acceptance->registrationType, $acceptance->registrationId ), + 'accepted_at' => $acceptance->acceptedAt ?? '', + ]; + }, + $this->acceptances->findByStudent( $studentId ) + ); + } + + /** + * Every intake answer the student has submitted, newest registration first. + * + * @return list + */ + public function intakeAnswers( int $studentId ): array { + return array_map( + function ( Answer $answer ): array { + $question = $this->questions->findById( $answer->questionId ); + + return [ + 'question' => $question ? $question->label : sprintf( '#%d', $answer->questionId ), + 'answer' => $answer->answerValue ?? '—', + 'context' => $this->contextLabel( $answer->registrationType, $answer->registrationId ), + ]; + }, + $this->answers->findByStudent( $studentId ) + ); + } + + /** + * Every payment for the student, newest first. + * + * @return list + */ + public function payments( int $studentId ): array { + return array_map( + fn( Payment $payment ): array => [ + 'created_at' => $payment->createdAt ?? '', + 'context' => $this->contextLabel( $payment->registrationType, $payment->registrationId ), + 'method' => $payment->method, + 'status' => $payment->status, + 'amount' => $payment->amount, + 'tax_amount' => $payment->taxAmount, + 'total' => $payment->total(), + 'currency' => $payment->currency, + 'receipt' => $payment->receiptNumber ?? '—', + ], + $this->payments->findByStudent( $studentId ) + ); + } + + /** + * Human label for a polymorphic registration target. + */ + private function contextLabel( string $registrationType, int $registrationId ): string { + switch ( $registrationType ) { + case PolicyAcceptance::REG_ACCOUNT: + return __( 'Account signup', 'unsupervised-schedular' ); + case PolicyAcceptance::REG_LESSON: + /* translators: %d: the lesson id */ + return sprintf( __( 'Lesson #%d', 'unsupervised-schedular' ), $registrationId ); + case PolicyAcceptance::REG_ENROLLMENT: + /* translators: %d: the group-class enrolment id */ + return sprintf( __( 'Enrolment #%d', 'unsupervised-schedular' ), $registrationId ); + default: + return sprintf( '%s #%d', $registrationType, $registrationId ); + } + } +} diff --git a/src/Payment/Payment.php b/src/Payment/Payment.php index 7384b89..4f7ad4a 100644 --- a/src/Payment/Payment.php +++ b/src/Payment/Payment.php @@ -49,6 +49,7 @@ class Payment { public readonly ?string $receiptNumber = null, public readonly ?string $receiptSentAt = null, public readonly ?string $paidAt = null, + public readonly ?string $createdAt = null, public readonly ?int $id = null, ) {} @@ -69,6 +70,7 @@ class Payment { receiptNumber: Val::stringOrNull( $row->receipt_number ), receiptSentAt: Val::stringOrNull( $row->receipt_sent_at ), paidAt: Val::stringOrNull( $row->paid_at ), + createdAt: Val::stringOrNull( $row->created_at ), id: Val::int( $row->id ), ); } @@ -120,6 +122,7 @@ class Payment { 'status' => $this->status, 'receipt_number' => $this->receiptNumber, 'paid_at' => $this->paidAt, + 'created_at' => $this->createdAt, ]; } } diff --git a/src/Payment/PaymentRepository.php b/src/Payment/PaymentRepository.php index 4df4d9a..aced5b0 100644 --- a/src/Payment/PaymentRepository.php +++ b/src/Payment/PaymentRepository.php @@ -132,6 +132,23 @@ class PaymentRepository { return $row ? Payment::fromRow( $row ) : null; } + /** + * Every payment for a student, newest first (admin payment history). + * + * @return list + */ + public function findByStudent( int $studentId ): array { + $rows = $this->db->get_results( + $this->db->prepare( + 'SELECT * FROM %i WHERE student_id = %d ORDER BY created_at DESC, id DESC', + $this->table, + $studentId + ) + ); + + return array_map( Payment::fromRow( ... ), $rows ?? [] ); + } + /** * Pending payments, newest first (studio-admin confirmation queue). * diff --git a/src/Plugin.php b/src/Plugin.php index f32b7eb..82b5df3 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -79,7 +79,7 @@ class Plugin { ( new RoleManager() )->register(); ( new RegistrationLoginGate() )->register(); ( new EmailConfirmationHandler( $settings, $registrationMailer ) )->register(); - ( new AdminMenu( $availability, $bookings, $offerings, $questions, $policies, $policyVersions, $policyService, $invites, $enrollments, $settings, $paymentRepo, $paymentService, $resolver ) )->register(); + ( new AdminMenu( $availability, $bookings, $offerings, $questions, $answers, $policies, $policyVersions, $policyService, $acceptances, $invites, $enrollments, $settings, $paymentRepo, $paymentService, $resolver ) )->register(); ( new RestRegistrar( $availability, $bookings, $offerings, $questions, $policies, $policyVersions, $policyService, $registrationGate, $enrollments, $paymentService ) )->register(); ( new ShortcodeRegistrar( $bookingPage, $loginPage, $registrationPage, $groupClassPage ) )->register(); ( new BlockRegistrar( $bookingPage, $loginPage, $registrationPage, $groupClassPage ) )->register(); diff --git a/src/Policy/AcceptanceRepository.php b/src/Policy/AcceptanceRepository.php index 1407604..4aa0a2e 100644 --- a/src/Policy/AcceptanceRepository.php +++ b/src/Policy/AcceptanceRepository.php @@ -55,4 +55,21 @@ class AcceptanceRepository { return array_map( PolicyAcceptance::fromRow( ... ), $rows ?? [] ); } + + /** + * Find every acceptance a student has recorded, newest first. + * + * @return list + */ + public function findByStudent( int $studentId ): array { + $rows = $this->db->get_results( + $this->db->prepare( + 'SELECT * FROM %i WHERE student_id = %d ORDER BY accepted_at DESC, id DESC', + $this->table, + $studentId + ) + ); + + return array_map( PolicyAcceptance::fromRow( ... ), $rows ?? [] ); + } } diff --git a/src/Registration/AnswerRepository.php b/src/Registration/AnswerRepository.php index daa995b..a16b090 100644 --- a/src/Registration/AnswerRepository.php +++ b/src/Registration/AnswerRepository.php @@ -55,4 +55,21 @@ class AnswerRepository { return array_map( Answer::fromRow( ... ), $rows ?? [] ); } + + /** + * Find every answer a student has submitted, newest registration first. + * + * @return list + */ + public function findByStudent( int $studentId ): array { + $rows = $this->db->get_results( + $this->db->prepare( + 'SELECT * FROM %i WHERE student_id = %d ORDER BY id DESC', + $this->table, + $studentId + ) + ); + + return array_map( Answer::fromRow( ... ), $rows ?? [] ); + } } diff --git a/templates/admin/student-detail.php b/templates/admin/student-detail.php index 8a1415f..1ed89b9 100644 --- a/templates/admin/student-detail.php +++ b/templates/admin/student-detail.php @@ -10,6 +10,9 @@ if (! defined('ABSPATH')) { * @var list $upcoming * @var list $past * @var list $enrolments + * @var list $acceptances + * @var list $intake + * @var list $payments * @var string $backUrl * @var bool $canBilling * @var string $billingOverride @@ -104,4 +107,90 @@ $renderLessons = static function (array $rows): void { + +

+ +

+ + + + + + + + + + + + + + + + + + + + +
+ + +

+ +

+ + + + + + + + + + + + + + + + + + +
+ + + +

+ +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + diff --git a/tests/Unit/Auth/StudentHistoryTest.php b/tests/Unit/Auth/StudentHistoryTest.php new file mode 100644 index 0000000..eea4174 --- /dev/null +++ b/tests/Unit/Auth/StudentHistoryTest.php @@ -0,0 +1,180 @@ +acceptances = Mockery::mock(AcceptanceRepository::class); + $this->policies = Mockery::mock(PolicyRepository::class); + $this->policyVersions = Mockery::mock(PolicyVersionRepository::class); + $this->answers = Mockery::mock(AnswerRepository::class); + $this->questions = Mockery::mock(QuestionRepository::class); + $this->payments = Mockery::mock(PaymentRepository::class); + + $this->history = new StudentHistory( + $this->acceptances, + $this->policies, + $this->policyVersions, + $this->answers, + $this->questions, + $this->payments + ); + } + + public function testPolicyAcceptancesResolvePolicyTitleAndVersion(): void + { + $this->acceptances->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new PolicyAcceptance(9, 5, PolicyAcceptance::REG_ACCOUNT, 5, null, '2026-06-02 09:00:00', 1), + ]); + $this->policyVersions->shouldReceive('findById')->with(9) + ->andReturn(new PolicyVersion(2, 3, null, PolicyVersion::STATUS_PUBLISHED, id: 9)); + $this->policies->shouldReceive('findById')->with(2) + ->andReturn(new Policy('Waiver', 'waiver', 9, Policy::SCOPE_SIGNUP, 2)); + + $rows = $this->history->policyAcceptances(5); + + self::assertSame( + [ + [ + 'policy' => 'Waiver', + 'version' => 'v3', + 'context' => 'Account signup', + 'accepted_at' => '2026-06-02 09:00:00', + ], + ], + $rows + ); + } + + public function testPolicyAcceptancesFallBackWhenVersionIsGone(): void + { + $this->acceptances->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new PolicyAcceptance(9, 5, PolicyAcceptance::REG_LESSON, 12), + ]); + $this->policyVersions->shouldReceive('findById')->with(9)->andReturn(null); + + $rows = $this->history->policyAcceptances(5); + + self::assertSame('#9', $rows[0]['policy']); + self::assertSame('—', $rows[0]['version']); + self::assertSame('Lesson #12', $rows[0]['context']); + self::assertSame('', $rows[0]['accepted_at']); + } + + public function testIntakeAnswersResolveQuestionLabels(): void + { + $this->answers->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new Answer(4, Answer::REG_ENROLLMENT, 3, 5, 'Beginner', 1), + ]); + $this->questions->shouldReceive('findById')->with(4) + ->andReturn(new Question(1, 'Experience level', id: 4)); + + $rows = $this->history->intakeAnswers(5); + + self::assertSame( + [ + [ + 'question' => 'Experience level', + 'answer' => 'Beginner', + 'context' => 'Enrolment #3', + ], + ], + $rows + ); + } + + public function testIntakeAnswersFallBackWhenQuestionIsGone(): void + { + $this->answers->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new Answer(4, Answer::REG_LESSON, 12, 5, null, 1), + ]); + $this->questions->shouldReceive('findById')->with(4)->andReturn(null); + + $rows = $this->history->intakeAnswers(5); + + self::assertSame('#4', $rows[0]['question']); + self::assertSame('—', $rows[0]['answer']); + } + + public function testPaymentsBuildDisplayRows(): void + { + $this->payments->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new Payment( + 5, + 3, + Payment::REG_LESSON, + 12, + 100.00, + 'CAD', + Payment::METHOD_CARD, + Payment::STATUS_PAID, + taxRate: 13.0, + taxAmount: 13.00, + receiptNumber: 'USC-7', + createdAt: '2026-06-08 09:00:00', + id: 50 + ), + ]); + + $rows = $this->history->payments(5); + + self::assertSame( + [ + [ + 'created_at' => '2026-06-08 09:00:00', + 'context' => 'Lesson #12', + 'method' => Payment::METHOD_CARD, + 'status' => Payment::STATUS_PAID, + 'amount' => 100.00, + 'tax_amount' => 13.00, + 'total' => 113.00, + 'currency' => 'CAD', + 'receipt' => 'USC-7', + ], + ], + $rows + ); + } + + public function testPaymentsFallBackWhenUnpaidAndUndated(): void + { + $this->payments->shouldReceive('findByStudent')->once()->with(5)->andReturn([ + new Payment(5, 3, Payment::REG_ENROLLMENT, 3, 40.00, id: 51), + ]); + + $rows = $this->history->payments(5); + + self::assertSame('', $rows[0]['created_at']); + self::assertSame('Enrolment #3', $rows[0]['context']); + self::assertSame('—', $rows[0]['receipt']); + } +} diff --git a/tests/Unit/Payment/PaymentRepositoryTest.php b/tests/Unit/Payment/PaymentRepositoryTest.php index f1ca515..ce75011 100644 --- a/tests/Unit/Payment/PaymentRepositoryTest.php +++ b/tests/Unit/Payment/PaymentRepositoryTest.php @@ -158,6 +158,22 @@ class PaymentRepositoryTest extends TestCase self::assertCount(1, $this->repo->findPending()); } + public function testFindByStudentPreparesQueryAndMaps(): void + { + $this->db->shouldReceive('prepare') + ->once() + ->with(Mockery::pattern('/student_id = %d ORDER BY created_at DESC, id DESC/'), 'wp_us_payments', 5) + ->andReturn('SELECT ...'); + + $this->db->shouldReceive('get_results')->andReturn([$this->row()]); + + $payments = $this->repo->findByStudent(5); + + self::assertCount(1, $payments); + self::assertSame(5, $payments[0]->studentId); + self::assertSame('2026-06-08 09:00:00', $payments[0]->createdAt); + } + private function row(): object { return (object) [ @@ -177,6 +193,7 @@ class PaymentRepositoryTest extends TestCase 'receipt_number' => null, 'receipt_sent_at' => null, 'paid_at' => null, + 'created_at' => '2026-06-08 09:00:00', ]; } } diff --git a/tests/Unit/Payment/PaymentTest.php b/tests/Unit/Payment/PaymentTest.php index af56050..3548b71 100644 --- a/tests/Unit/Payment/PaymentTest.php +++ b/tests/Unit/Payment/PaymentTest.php @@ -46,6 +46,7 @@ class PaymentTest extends TestCase 'receipt_number' => 'USC-7', 'receipt_sent_at' => null, 'paid_at' => '2026-06-08 10:00:00', + 'created_at' => '2026-06-08 09:00:00', ]); self::assertSame(7, $payment->id); @@ -55,6 +56,7 @@ class PaymentTest extends TestCase self::assertSame(Payment::METHOD_COMP, $payment->method); self::assertTrue($payment->isPaid()); self::assertSame('USC-7', $payment->receiptNumber); + self::assertSame('2026-06-08 09:00:00', $payment->createdAt); } public function testTotalAddsTaxToAmount(): void diff --git a/tests/Unit/Policy/AcceptanceRepositoryTest.php b/tests/Unit/Policy/AcceptanceRepositoryTest.php index d6d791a..992fe4d 100644 --- a/tests/Unit/Policy/AcceptanceRepositoryTest.php +++ b/tests/Unit/Policy/AcceptanceRepositoryTest.php @@ -90,4 +90,29 @@ class AcceptanceRepositoryTest extends TestCase self::assertCount(1, $rows); self::assertInstanceOf(PolicyAcceptance::class, $rows[0]); } + + public function testFindByStudentMapsRowsNewestFirst(): void + { + $this->db->shouldReceive('prepare') + ->once() + ->with(Mockery::pattern('/student_id = %d ORDER BY accepted_at DESC, id DESC/'), 'wp_us_policy_acceptances', 5) + ->andReturn('SELECT ...'); + + $this->db->shouldReceive('get_results')->andReturn([ + (object) [ + 'id' => '2', + 'policy_version_id' => '9', + 'student_id' => '5', + 'registration_type' => PolicyAcceptance::REG_ACCOUNT, + 'registration_id' => '5', + 'accepted_at' => '2026-06-03 09:00:00', + 'ip_address' => null, + ], + ]); + + $rows = $this->repo->findByStudent(5); + + self::assertCount(1, $rows); + self::assertSame(5, $rows[0]->studentId); + } } diff --git a/tests/Unit/Registration/AnswerRepositoryTest.php b/tests/Unit/Registration/AnswerRepositoryTest.php index c00f021..7c6df95 100644 --- a/tests/Unit/Registration/AnswerRepositoryTest.php +++ b/tests/Unit/Registration/AnswerRepositoryTest.php @@ -98,4 +98,32 @@ class AnswerRepositoryTest extends TestCase self::assertInstanceOf(Answer::class, $answers[0]); self::assertSame(3, $answers[0]->questionId); } + + public function testFindByStudentPreparesQueryAndMaps(): void + { + $row = (object) [ + 'id' => '1', + 'question_id' => '3', + 'registration_type' => Answer::REG_LESSON, + 'registration_id' => '12', + 'student_id' => '5', + 'answer_value' => 'Beginner', + ]; + + $this->db->shouldReceive('prepare') + ->once() + ->with( + Mockery::pattern('/student_id = %d ORDER BY id DESC/'), + 'wp_us_question_answers', + 5 + ) + ->andReturn('SELECT ...'); + + $this->db->shouldReceive('get_results')->andReturn([$row]); + + $answers = $this->repo->findByStudent(5); + + self::assertCount(1, $answers); + self::assertSame(5, $answers[0]->studentId); + } } -- 2.54.0 From 5808523140d461962ad2d79e6853415ceed28e6b Mon Sep 17 00:00:00 2001 From: James Griffin Date: Sat, 18 Jul 2026 18:25:12 -0300 Subject: [PATCH 2/2] Add admin actions to the student detail view: cancel, withdraw, edit account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the #70 follow-up onto the student detail page: studio admins can now cancel an upcoming lesson (same path as student cancellation — slot freed, pending payment voided), withdraw an active group-class enrolment (seat freed, pending payment voided), and edit the student's display name and email with validation and uniqueness checks. Action logic lives in the new Auth\StudentActions (unit-tested with mocked repositories); the controller routes nonce-protected POSTs to it and shows success/error notices. Closes #70 Co-Authored-By: Claude Fable 5 --- docs/features/student-administration.md | 25 +++- src/AdminMenu.php | 3 +- src/Auth/StudentActions.php | 92 +++++++++++++ src/Auth/StudentController.php | 47 ++++++- templates/admin/student-detail.php | 72 ++++++++-- tests/Unit/Auth/StudentActionsTest.php | 169 ++++++++++++++++++++++++ 6 files changed, 393 insertions(+), 15 deletions(-) create mode 100644 src/Auth/StudentActions.php create mode 100644 tests/Unit/Auth/StudentActionsTest.php diff --git a/docs/features/student-administration.md b/docs/features/student-administration.md index e631db4..b4a19f8 100644 --- a/docs/features/student-administration.md +++ b/docs/features/student-administration.md @@ -1,9 +1,10 @@ # Feature: Student Administration ## Overview -A read-only studio-admin area to browse students and drill into one student's -history and upcoming activity — lessons and group-class enrolments — without -digging through individual records. +A studio-admin area to browse students, drill into one student's history and +upcoming activity — lessons and group-class enrolments — and act on their +behalf: cancel a lesson, withdraw them from a group class, or fix their account +details. ## Data Model No new tables. The views are composed from existing data: @@ -35,7 +36,17 @@ No new tables. The views are composed from existing data: - **Payment history** (`manage_billing` only) — every payment, newest first: date, context, method, status, subtotal, HST, total, and receipt number. -Read-only in this iteration; cancel/edit actions are a possible follow-up (#70). +### Admin actions (detail view) +All actions are nonce-protected POSTs handled on the detail page: + +- **Edit account** — display name and email. The email must be valid and not in + use by another account. +- **Cancel lesson** — on any non-cancelled upcoming lesson. Uses the same path + as student-initiated cancellation: the lesson is marked `cancelled`, the + availability slot is freed for rebooking, and a still-pending payment is + voided. Paid lessons keep their payment — refunds stay a manual decision (#72). +- **Withdraw** — on an active group-class enrolment: marked `cancelled` (freeing + its capacity seat), with the same pending-payment voiding. ## Capabilities - `manage_students` — studio admin (administrators inherit it via the @@ -53,6 +64,10 @@ Read-only in this iteration; cancel/edit actions are a possible follow-up (#70). `Registration\AnswerRepository::findByStudent`, and `Payment\PaymentRepository::findByStudent`, resolving policy/version titles and question labels (unit-tested with mocked repositories). +- Actions: `Auth\StudentActions` — cancel lesson / withdraw enrolment (both + refuse records that don't belong to the student, and reuse + `Payment\PaymentService::voidPending`) and account updates via + `wp_update_user` (unit-tested with mocked repositories). - Upcoming/past split: `Auth\StudentSchedule::partition()` (pure, unit-tested) - The upcoming/past split is extracted into a small pure helper so it is unit-testable (the controller itself follows the repo convention of not being @@ -61,6 +76,8 @@ Read-only in this iteration; cancel/edit actions are a possible follow-up (#70). ## Tests - `tests/Unit/Auth/StudentScheduleTest.php` (the pure upcoming/past split helper) - `tests/Unit/Auth/StudentHistoryTest.php` (history display rows + fallbacks) +- `tests/Unit/Auth/StudentActionsTest.php` (cancel/withdraw guards + side + effects, account validation) - `findByStudent` coverage in `tests/Unit/Policy/AcceptanceRepositoryTest.php`, `tests/Unit/Registration/AnswerRepositoryTest.php`, and `tests/Unit/Payment/PaymentRepositoryTest.php` diff --git a/src/AdminMenu.php b/src/AdminMenu.php index e71c9f2..36003c3 100644 --- a/src/AdminMenu.php +++ b/src/AdminMenu.php @@ -12,6 +12,7 @@ use Unsupervised\Schedular\Auth\RegistrationApprovalController; use Unsupervised\Schedular\Auth\RegistrationController; use Unsupervised\Schedular\Auth\RegistrationMailer; use Unsupervised\Schedular\Auth\RoleManager; +use Unsupervised\Schedular\Auth\StudentActions; use Unsupervised\Schedular\Auth\StudentController; use Unsupervised\Schedular\Auth\StudentHistory; use Unsupervised\Schedular\Booking\BookingRepository; @@ -61,7 +62,7 @@ class AdminMenu { $this->registrationController = new RegistrationController( $invites ); $this->registrationApprovalController = new RegistrationApprovalController( new RegistrationMailer() ); $this->groupClassController = new GroupClassController( $enrollments, $offerings ); - $this->studentController = new StudentController( $bookings, $availability, $offerings, $enrollments, $resolver, new StudentHistory( $acceptances, $policies, $policyVersions, $answers, $questions, $payments ) ); + $this->studentController = new StudentController( $bookings, $availability, $offerings, $enrollments, $resolver, new StudentHistory( $acceptances, $policies, $policyVersions, $answers, $questions, $payments ), new StudentActions( $bookings, $availability, $enrollments, $paymentService ) ); $this->instructorController = new InstructorController(); $this->settings = $settings; $this->accessSettings = new AccessSettings(); diff --git a/src/Auth/StudentActions.php b/src/Auth/StudentActions.php new file mode 100644 index 0000000..b3718ed --- /dev/null +++ b/src/Auth/StudentActions.php @@ -0,0 +1,92 @@ +bookings->findById( $lessonId ); + + if ( null === $lesson || $lesson->studentId !== $studentId || Lesson::STATUS_CANCELLED === $lesson->status ) { + return false; + } + + $this->bookings->updateStatus( $lessonId, Lesson::STATUS_CANCELLED ); + $this->availability->release( $lesson->slotId ); + $this->payments->voidPending( $lesson->paymentId ); + + return true; + } + + /** + * Withdraw the student from a group class: marks the active enrolment + * cancelled (freeing its capacity seat) and voids a still-pending payment. + */ + public function withdrawEnrollment( int $enrollmentId, int $studentId ): bool { + $enrollment = $this->enrollments->findById( $enrollmentId ); + + if ( null === $enrollment || $enrollment->studentId !== $studentId || Enrollment::STATUS_ACTIVE !== $enrollment->status ) { + return false; + } + + $this->enrollments->updateStatus( $enrollmentId, Enrollment::STATUS_CANCELLED ); + $this->payments->voidPending( $enrollment->paymentId ); + + return true; + } + + /** + * Update the student's display name and email. The email must be valid and + * not belong to another user. + */ + public function updateAccount( int $studentId, string $displayName, string $email ): bool|\WP_Error { + if ( '' === $displayName ) { + return new \WP_Error( 'empty_name', __( 'Display name cannot be empty.', 'unsupervised-schedular' ) ); + } + + if ( ! is_email( $email ) ) { + return new \WP_Error( 'invalid_email', __( 'Please enter a valid email address.', 'unsupervised-schedular' ) ); + } + + $existing = email_exists( $email ); + if ( false !== $existing && (int) $existing !== $studentId ) { + return new \WP_Error( 'email_taken', __( 'Another account already uses this email address.', 'unsupervised-schedular' ) ); + } + + $result = wp_update_user( + [ + 'ID' => $studentId, + 'display_name' => $displayName, + 'user_email' => $email, + ] + ); + + return $result instanceof \WP_Error ? $result : true; + } +} diff --git a/src/Auth/StudentController.php b/src/Auth/StudentController.php index 63c7f7c..479def2 100644 --- a/src/Auth/StudentController.php +++ b/src/Auth/StudentController.php @@ -22,6 +22,7 @@ class StudentController { private EnrollmentRepository $enrollments, private BillingMethodResolver $resolver, private StudentHistory $history, + private StudentActions $actions, ) {} public function renderPage(): void { @@ -66,7 +67,13 @@ class StudentController { private function renderDetail( \WP_User $student ): void { $canBilling = current_user_can( RoleManager::CAP_MANAGE_BILLING ); - if ( $canBilling && isset( $_POST['usc_action'] ) && check_admin_referer( 'usc_student_billing' ) ) { + // phpcs:ignore WordPress.Security.NonceVerification.Missing -- routing only; each action below verifies its own nonce. + $action = sanitize_key( Val::string( wp_unslash( $_POST['usc_action'] ?? '' ) ) ); + + $notice = ''; + $error = ''; + + if ( $canBilling && 'set_billing' === $action && check_admin_referer( 'usc_student_billing' ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce checked above. $method = sanitize_key( Val::string( wp_unslash( $_POST['payment_method'] ?? '' ) ) ); if ( in_array( $method, Payment::VALID_METHODS, true ) ) { @@ -76,6 +83,42 @@ class StudentController { } } + if ( 'update_account' === $action && check_admin_referer( 'usc_student_actions' ) ) { + // phpcs:disable WordPress.Security.NonceVerification.Missing -- nonce checked above. + $displayName = sanitize_text_field( Val::string( wp_unslash( $_POST['display_name'] ?? '' ) ) ); + $email = sanitize_email( Val::string( wp_unslash( $_POST['user_email'] ?? '' ) ) ); + // phpcs:enable WordPress.Security.NonceVerification.Missing + + $result = $this->actions->updateAccount( (int) $student->ID, $displayName, $email ); + if ( $result instanceof \WP_Error ) { + $error = $result->get_error_message(); + } else { + $notice = __( 'Account details updated.', 'unsupervised-schedular' ); + $fresh = get_userdata( (int) $student->ID ); + $student = $fresh instanceof \WP_User ? $fresh : $student; + } + } + + if ( 'cancel_lesson' === $action && check_admin_referer( 'usc_student_actions' ) ) { + // phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce checked above. + $lessonId = absint( Val::int( $_POST['lesson_id'] ?? 0 ) ); + if ( $this->actions->cancelLesson( $lessonId, (int) $student->ID ) ) { + $notice = __( 'Lesson cancelled.', 'unsupervised-schedular' ); + } else { + $error = __( 'This lesson could not be cancelled.', 'unsupervised-schedular' ); + } + } + + if ( 'withdraw_enrollment' === $action && check_admin_referer( 'usc_student_actions' ) ) { + // phpcs:ignore WordPress.Security.NonceVerification.Missing -- nonce checked above. + $enrollmentId = absint( Val::int( $_POST['enrollment_id'] ?? 0 ) ); + if ( $this->actions->withdrawEnrollment( $enrollmentId, (int) $student->ID ) ) { + $notice = __( 'Enrolment withdrawn.', 'unsupervised-schedular' ); + } else { + $error = __( 'This enrolment could not be withdrawn.', 'unsupervised-schedular' ); + } + } + $billingOverride = Val::string( get_user_meta( (int) $student->ID, BillingMethodResolver::META_METHOD, true ) ); $billingDefault = $this->resolver->defaultMethod(); @@ -94,6 +137,7 @@ class StudentController { $offering = $this->offerings->findById( $enrollment->offeringId ); return [ + 'id' => (int) $enrollment->id, 'offering' => $offering ? $offering->title : (string) $enrollment->offeringId, 'status' => $enrollment->status, ]; @@ -120,6 +164,7 @@ class StudentController { $instructor = get_userdata( $lesson->instructorId ); return [ + 'id' => (int) $lesson->id, 'start_dt' => $slot ? $slot->startDt : '', 'end_dt' => $slot ? $slot->endDt : '', 'offering' => $offering ? $offering->title : '—', diff --git a/templates/admin/student-detail.php b/templates/admin/student-detail.php index 1ed89b9..fe58041 100644 --- a/templates/admin/student-detail.php +++ b/templates/admin/student-detail.php @@ -7,9 +7,9 @@ if (! defined('ABSPATH')) { /** * @var \WP_User $student - * @var list $upcoming - * @var list $past - * @var list $enrolments + * @var list $upcoming + * @var list $past + * @var list $enrolments * @var list $acceptances * @var list $intake * @var list $payments @@ -17,9 +17,11 @@ if (! defined('ABSPATH')) { * @var bool $canBilling * @var string $billingOverride * @var string $billingDefault + * @var string $notice + * @var string $error */ -$renderLessons = static function (array $rows): void { +$renderLessons = static function (array $rows, bool $withActions = false): void { if (empty($rows)) { echo '

' . esc_html__('None.', 'unsupervised-schedular') . '

'; return; @@ -32,6 +34,9 @@ $renderLessons = static function (array $rows): void { + + + @@ -41,6 +46,20 @@ $renderLessons = static function (array $rows): void { + + + +
+ + + + +
+ + + @@ -54,11 +73,33 @@ $renderLessons = static function (array $rows): void { + +

+ + +

+ +

- - - -
user_email); ?>
user_registered); ?>
+
+ + + + + + + + + + + + + + + +
user_registered); ?>
+ +

@@ -81,7 +122,7 @@ $renderLessons = static function (array $rows): void {

- +

@@ -95,6 +136,7 @@ $renderLessons = static function (array $rows): void { + @@ -102,6 +144,18 @@ $renderLessons = static function (array $rows): void { + + +
+ + + + +
+ + diff --git a/tests/Unit/Auth/StudentActionsTest.php b/tests/Unit/Auth/StudentActionsTest.php new file mode 100644 index 0000000..ea55113 --- /dev/null +++ b/tests/Unit/Auth/StudentActionsTest.php @@ -0,0 +1,169 @@ +bookings = Mockery::mock(BookingRepository::class); + $this->availability = Mockery::mock(AvailabilityRepository::class); + $this->enrollments = Mockery::mock(EnrollmentRepository::class); + $this->payments = Mockery::mock(PaymentService::class); + + $this->actions = new StudentActions($this->bookings, $this->availability, $this->enrollments, $this->payments); + } + + public function testCancelLessonCancelsFreesSlotAndVoidsPayment(): void + { + $this->bookings->shouldReceive('findById')->with(12) + ->andReturn(new Lesson(7, 5, 3, status: Lesson::STATUS_PENDING, paymentId: 40, id: 12)); + + $this->bookings->shouldReceive('updateStatus')->once()->with(12, Lesson::STATUS_CANCELLED)->andReturn(true); + $this->availability->shouldReceive('release')->once()->with(7)->andReturn(true); + $this->payments->shouldReceive('voidPending')->once()->with(40); + + self::assertTrue($this->actions->cancelLesson(12, 5)); + } + + public function testCancelLessonRefusesAnotherStudentsLesson(): void + { + $this->bookings->shouldReceive('findById')->with(12) + ->andReturn(new Lesson(7, 6, 3, status: Lesson::STATUS_PENDING, id: 12)); + + $this->bookings->shouldNotReceive('updateStatus'); + $this->availability->shouldNotReceive('release'); + + self::assertFalse($this->actions->cancelLesson(12, 5)); + } + + public function testCancelLessonRefusesAlreadyCancelledLesson(): void + { + $this->bookings->shouldReceive('findById')->with(12) + ->andReturn(new Lesson(7, 5, 3, status: Lesson::STATUS_CANCELLED, id: 12)); + + $this->bookings->shouldNotReceive('updateStatus'); + + self::assertFalse($this->actions->cancelLesson(12, 5)); + } + + public function testCancelLessonRefusesMissingLesson(): void + { + $this->bookings->shouldReceive('findById')->with(99)->andReturn(null); + + self::assertFalse($this->actions->cancelLesson(99, 5)); + } + + public function testWithdrawEnrollmentCancelsAndVoidsPayment(): void + { + $this->enrollments->shouldReceive('findById')->with(3) + ->andReturn(new Enrollment(1, 5, 3, Enrollment::STATUS_ACTIVE, 41, 3)); + + $this->enrollments->shouldReceive('updateStatus')->once()->with(3, Enrollment::STATUS_CANCELLED)->andReturn(true); + $this->payments->shouldReceive('voidPending')->once()->with(41); + + self::assertTrue($this->actions->withdrawEnrollment(3, 5)); + } + + public function testWithdrawEnrollmentRefusesInactiveEnrolment(): void + { + $this->enrollments->shouldReceive('findById')->with(3) + ->andReturn(new Enrollment(1, 5, 3, Enrollment::STATUS_CANCELLED, null, 3)); + + $this->enrollments->shouldNotReceive('updateStatus'); + + self::assertFalse($this->actions->withdrawEnrollment(3, 5)); + } + + public function testWithdrawEnrollmentRefusesAnotherStudentsEnrolment(): void + { + $this->enrollments->shouldReceive('findById')->with(3) + ->andReturn(new Enrollment(1, 6, 3, Enrollment::STATUS_ACTIVE, null, 3)); + + $this->enrollments->shouldNotReceive('updateStatus'); + + self::assertFalse($this->actions->withdrawEnrollment(3, 5)); + } + + public function testUpdateAccountSavesNameAndEmail(): void + { + Functions\when('is_email')->justReturn(true); + Functions\when('email_exists')->justReturn(false); + Functions\expect('wp_update_user') + ->once() + ->with(['ID' => 5, 'display_name' => 'New Name', 'user_email' => 'new@example.com']) + ->andReturn(5); + + self::assertTrue($this->actions->updateAccount(5, 'New Name', 'new@example.com')); + } + + public function testUpdateAccountAllowsKeepingOwnEmail(): void + { + Functions\when('is_email')->justReturn(true); + Functions\when('email_exists')->justReturn(5); + Functions\when('wp_update_user')->justReturn(5); + + self::assertTrue($this->actions->updateAccount(5, 'Name', 'same@example.com')); + } + + public function testUpdateAccountRejectsEmptyName(): void + { + $result = $this->actions->updateAccount(5, '', 'new@example.com'); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('empty_name', $result->get_error_code()); + } + + public function testUpdateAccountRejectsInvalidEmail(): void + { + Functions\when('is_email')->justReturn(false); + + $result = $this->actions->updateAccount(5, 'Name', 'not-an-email'); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('invalid_email', $result->get_error_code()); + } + + public function testUpdateAccountRejectsEmailOwnedByAnotherUser(): void + { + Functions\when('is_email')->justReturn(true); + Functions\when('email_exists')->justReturn(9); + + $result = $this->actions->updateAccount(5, 'Name', 'taken@example.com'); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('email_taken', $result->get_error_code()); + } + + public function testUpdateAccountPassesThroughWpError(): void + { + Functions\when('is_email')->justReturn(true); + Functions\when('email_exists')->justReturn(false); + Functions\when('wp_update_user')->justReturn(new \WP_Error('update_failed', 'nope')); + + $result = $this->actions->updateAccount(5, 'Name', 'new@example.com'); + + self::assertInstanceOf(\WP_Error::class, $result); + self::assertSame('update_failed', $result->get_error_code()); + } +} -- 2.54.0