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()); + } +}