diff --git a/AGENTS.md b/AGENTS.md index e12b3678aa..f762de2bf9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,4 +5,3 @@ ## Conventions - **Changelogs**: Every user-facing change MUST have a changelog entry before opening a PR. Run `npm run changelog` (entries stored in `changelog/`). -- **Test assertions**: When a single test has multiple assertions, pass a message as the third argument to each assertion (e.g., `self::assertSame( $expected, $actual, 'Foo should be updated.' );`) so a failure points to the specific check that failed. diff --git a/changelog/fix-hpps-lesson-course-sync b/changelog/fix-hpps-lesson-course-sync deleted file mode 100644 index e92d848651..0000000000 --- a/changelog/fix-hpps-lesson-course-sync +++ /dev/null @@ -1,4 +0,0 @@ -Significance: minor -Type: added - -Sync the HPPS progress table's course association when a lesson is moved between courses. diff --git a/includes/class-sensei.php b/includes/class-sensei.php index 3536b29b80..05b657872e 100644 --- a/includes/class-sensei.php +++ b/includes/class-sensei.php @@ -30,7 +30,6 @@ use Sensei\Internal\Student_Progress\Quiz_Progress\Repositories\Quiz_Progress_Repository_Factory; use Sensei\Internal\Student_Progress\Quiz_Progress\Repositories\Quiz_Progress_Repository_Interface; use Sensei\Internal\Student_Progress\Services\Course_Deleted_Handler; -use Sensei\Internal\Student_Progress\Services\Lesson_Course_Sync_Handler; use Sensei\Internal\Student_Progress\Services\Lesson_Deleted_Handler; use Sensei\Internal\Student_Progress\Services\Quiz_Deleted_Handler; use Sensei\Internal\Student_Progress\Services\User_Deleted_Handler; @@ -820,11 +819,6 @@ public function initialize_global_objects() { ( new Quiz_Deleted_Handler( $this->quiz_progress_repository ) )->init(); ( new User_Deleted_Handler( $this->course_progress_repository, $this->lesson_progress_repository, $this->quiz_progress_repository ) )->init(); - // Keep parent_post_id on lesson progress rows in sync with the lesson's _lesson_course postmeta. - if ( $tables_feature_enabled ) { - ( new Lesson_Course_Sync_Handler( $GLOBALS['wpdb'] ) )->init(); - } - // Cron for periodically cleaning guest user related data. Sensei_Temporary_User_Cleaner::instance()->init(); diff --git a/includes/internal/student-progress/services/class-lesson-course-sync-handler.php b/includes/internal/student-progress/services/class-lesson-course-sync-handler.php deleted file mode 100644 index 590b9444d1..0000000000 --- a/includes/internal/student-progress/services/class-lesson-course-sync-handler.php +++ /dev/null @@ -1,133 +0,0 @@ -wpdb = $wpdb; - } - - /** - * Adds hooks. - */ - public function init(): void { - add_action( 'added_post_meta', [ $this, 'handle_meta_change' ], 10, 4 ); - add_action( 'updated_post_meta', [ $this, 'handle_meta_change' ], 10, 4 ); - add_action( 'deleted_post_meta', [ $this, 'handle_meta_delete' ], 10, 3 ); - } - - /** - * Handle a postmeta add/update event. - * - * @param int $meta_id Meta ID (unused). - * @param int $object_id Post ID the meta belongs to. - * @param string $meta_key Meta key. - * @param mixed $meta_value New meta value. - */ - public function handle_meta_change( $meta_id, int $object_id, $meta_key, $meta_value ): void { - if ( ! $this->is_relevant_meta( $meta_key, $object_id ) ) { - return; - } - - $course_id = is_scalar( $meta_value ) ? (int) $meta_value : 0; - $this->update_parent_post_id( $object_id, $course_id > 0 ? $course_id : null ); - } - - /** - * Handle a postmeta delete event. - * - * @param array $meta_ids Meta IDs (unused). - * @param int $object_id Post ID the meta belongs to. - * @param string $meta_key Meta key. - */ - public function handle_meta_delete( $meta_ids, int $object_id, $meta_key ): void { - if ( ! $this->is_relevant_meta( $meta_key, $object_id ) ) { - return; - } - - $this->update_parent_post_id( $object_id, null ); - } - - /** - * Whether the meta event applies to a lesson's _lesson_course postmeta. - * - * @param string $meta_key Meta key. - * @param int $object_id Post ID. - * @return bool - */ - private function is_relevant_meta( $meta_key, int $object_id ): bool { - if ( self::LESSON_COURSE_META_KEY !== $meta_key ) { - return false; - } - - return 'lesson' === get_post_type( $object_id ); - } - - /** - * Update parent_post_id on all lesson progress rows for the given lesson. - * - * @param int $lesson_id Lesson ID. - * @param int|null $parent_post_id New course ID, or null to clear. - */ - private function update_parent_post_id( int $lesson_id, ?int $parent_post_id ): void { - $result = $this->wpdb->update( - $this->wpdb->prefix . 'sensei_lms_progress', - array( 'parent_post_id' => $parent_post_id ), - array( - 'post_id' => $lesson_id, - 'type' => 'lesson', - ), - array( '%d' ), - array( '%d', '%s' ) - ); - - if ( false === $result ) { - Utils::log_query_error( - $this->wpdb, - sprintf( 'Lesson_Course_Sync_Handler::update_parent_post_id lesson_id=%d parent_post_id=%s', $lesson_id, null === $parent_post_id ? 'NULL' : (string) $parent_post_id ) - ); - } - } -} diff --git a/tests/unit-tests/internal/student-progress/services/test-class-lesson-course-sync-handler.php b/tests/unit-tests/internal/student-progress/services/test-class-lesson-course-sync-handler.php deleted file mode 100644 index c6fcd7a913..0000000000 --- a/tests/unit-tests/internal/student-progress/services/test-class-lesson-course-sync-handler.php +++ /dev/null @@ -1,370 +0,0 @@ -factory = new \Sensei_Factory(); - } - - /** - * Tear down after each test. - */ - public function tearDown(): void { - parent::tearDown(); - $this->factory->tearDown(); - } - - /** - * Tests that init registers the postmeta hooks. - */ - public function testInit_WhenCalled_AddsHooks(): void { - /* Arrange. */ - global $wpdb; - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->init(); - - /* Assert. */ - self::assertSame( 10, has_action( 'added_post_meta', array( $handler, 'handle_meta_change' ) ), 'added_post_meta hook should be registered.' ); - self::assertSame( 10, has_action( 'updated_post_meta', array( $handler, 'handle_meta_change' ) ), 'updated_post_meta hook should be registered.' ); - self::assertSame( 10, has_action( 'deleted_post_meta', array( $handler, 'handle_meta_delete' ) ), 'deleted_post_meta hook should be registered.' ); - - remove_action( 'added_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'updated_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'deleted_post_meta', array( $handler, 'handle_meta_delete' ), 10 ); - } - - /** - * Tests that updating _lesson_course updates parent_post_id on every matching row. - */ - public function testHandleMetaChange_LessonCourseUpdated_UpdatesParentPostIdOnAllRows(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $old_course = 100; - $new_course = 200; - - $row_one = $this->insert_progress_row( $lesson_id, 11, $old_course ); - $row_two = $this->insert_progress_row( $lesson_id, 22, $old_course ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $lesson_id, '_lesson_course', $new_course ); - - /* Assert. */ - self::assertSame( $new_course, $this->get_parent_post_id( $row_one ), 'First user row should be updated to the new course.' ); - self::assertSame( $new_course, $this->get_parent_post_id( $row_two ), 'Second user row should be updated to the new course.' ); - } - - /** - * Tests that updating one lesson's course leaves other lessons' progress rows alone, - * even when they currently share the same parent_post_id. - */ - public function testHandleMetaChange_OtherLessonRows_AreNotTouched(): void { - /* Arrange. */ - global $wpdb; - $lesson_a = $this->factory->lesson->create(); - $lesson_b = $this->factory->lesson->create(); - $row_a = $this->insert_progress_row( $lesson_a, 11, 100 ); - $row_b = $this->insert_progress_row( $lesson_b, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $lesson_a, '_lesson_course', 200 ); - - /* Assert. */ - self::assertSame( 200, $this->get_parent_post_id( $row_a ), 'Lesson A row should be updated.' ); - self::assertSame( 100, $this->get_parent_post_id( $row_b ), 'Lesson B row sharing the old course should be untouched.' ); - } - - /** - * Tests that quiz progress rows (which use parent_post_id for the lesson) are left alone. - */ - public function testHandleMetaChange_DoesNotTouchQuizRows(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - // Quiz row uses parent_post_id for the lesson — not the course — but we - // match on type='lesson', so it must be left alone. - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery - $wpdb->insert( - $wpdb->prefix . 'sensei_lms_progress', - array( - 'post_id' => 999, - 'user_id' => 11, - 'parent_post_id' => $lesson_id, - 'type' => 'quiz', - 'status' => 'in-progress', - 'created_at' => current_time( 'mysql' ), - 'updated_at' => current_time( 'mysql' ), - ) - ); - $quiz_row = (int) $wpdb->insert_id; - - // And a lesson row that should be updated. - $lesson_row = $this->insert_progress_row( $lesson_id, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $lesson_id, '_lesson_course', 200 ); - - /* Assert. */ - self::assertSame( 200, $this->get_parent_post_id( $lesson_row ), 'Lesson row should be updated to the new course.' ); - self::assertSame( $lesson_id, $this->get_parent_post_id( $quiz_row ), 'Quiz row should be left untouched.' ); - } - - /** - * Tests that a zero, non-numeric, or non-scalar course value clears parent_post_id to NULL. - * - * @dataProvider provideInvalidMetaValues - * - * @param mixed $meta_value Bogus meta value coming from the hook. - */ - public function testHandleMetaChange_InvalidValue_SetsParentPostIdToNull( $meta_value ): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $row = $this->insert_progress_row( $lesson_id, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $lesson_id, '_lesson_course', $meta_value ); - - /* Assert. */ - $this->assertParentPostIdIsSqlNull( $row ); - } - - /** - * Tests that updates to unrelated meta keys do not touch progress rows. - */ - public function testHandleMetaChange_UnrelatedMetaKey_DoesNotUpdateRows(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $old_course = 100; - $row = $this->insert_progress_row( $lesson_id, 11, $old_course ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $lesson_id, '_some_other_meta', 999 ); - - /* Assert. */ - self::assertSame( $old_course, $this->get_parent_post_id( $row ) ); - } - - /** - * Tests that updating _lesson_course on a non-lesson post does not touch progress rows. - */ - public function testHandleMetaChange_NonLessonPost_DoesNotUpdateRows(): void { - /* Arrange. */ - global $wpdb; - $course_id = $this->factory->course->create(); - $lesson_id = $this->factory->lesson->create(); - $old_course = 100; - $row = $this->insert_progress_row( $lesson_id, 11, $old_course ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_change( 1, $course_id, '_lesson_course', 200 ); - - /* Assert. */ - self::assertSame( $old_course, $this->get_parent_post_id( $row ) ); - } - - /** - * Tests that deleting _lesson_course clears parent_post_id to NULL. - */ - public function testHandleMetaDelete_LessonCourseDeleted_SetsParentPostIdToNull(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $row = $this->insert_progress_row( $lesson_id, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - - /* Act. */ - $handler->handle_meta_delete( array( 1 ), $lesson_id, '_lesson_course' ); - - /* Assert. */ - $this->assertParentPostIdIsSqlNull( $row ); - } - - /** - * Tests that calling update_post_meta() triggers the registered hook end-to-end. - */ - public function testHandleMetaChange_TriggersFromUpdatePostMeta_UpdatesRow(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $row = $this->insert_progress_row( $lesson_id, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - $handler->init(); - - /* Act. */ - update_post_meta( $lesson_id, '_lesson_course', 200 ); - - /* Assert. */ - self::assertSame( 200, $this->get_parent_post_id( $row ) ); - - remove_action( 'added_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'updated_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'deleted_post_meta', array( $handler, 'handle_meta_delete' ), 10 ); - } - - /** - * Tests that calling delete_post_meta() triggers the registered hook end-to-end. - */ - public function testHandleMetaDelete_TriggersFromDeletePostMeta_ClearsRow(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - update_post_meta( $lesson_id, '_lesson_course', 100 ); - $row = $this->insert_progress_row( $lesson_id, 11, 100 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - $handler->init(); - - /* Act. */ - delete_post_meta( $lesson_id, '_lesson_course' ); - - /* Assert. */ - $this->assertParentPostIdIsSqlNull( $row ); - - remove_action( 'added_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'updated_post_meta', array( $handler, 'handle_meta_change' ), 10 ); - remove_action( 'deleted_post_meta', array( $handler, 'handle_meta_delete' ), 10 ); - } - - /** - * Tests the full add → update → delete lifecycle end-to-end via the WP meta API, - * locking in that all three registered hooks are wired correctly. - */ - public function testHandleMeta_FullLifecycle_AddUpdateDelete(): void { - /* Arrange. */ - global $wpdb; - $lesson_id = $this->factory->lesson->create(); - $row = $this->insert_progress_row( $lesson_id, 11, 0 ); - - $handler = new Lesson_Course_Sync_Handler( $wpdb ); - $handler->init(); - - /* Act + Assert: add. */ - add_post_meta( $lesson_id, '_lesson_course', 100, true ); - self::assertSame( 100, $this->get_parent_post_id( $row ), 'add_post_meta should set parent_post_id to the new course.' ); - - /* Act + Assert: update. */ - update_post_meta( $lesson_id, '_lesson_course', 200 ); - self::assertSame( 200, $this->get_parent_post_id( $row ), 'update_post_meta should set parent_post_id to the new course.' ); - - /* Act + Assert: delete. */ - delete_post_meta( $lesson_id, '_lesson_course' ); - $this->assertParentPostIdIsSqlNull( $row ); - } - - /** - * Data provider for invalid meta values. - * - * @return array - */ - public function provideInvalidMetaValues(): array { - return array( - 'zero int' => array( 0 ), - 'negative int' => array( -1 ), - 'non-numeric str' => array( 'not-a-number' ), - 'array' => array( array( 100 ) ), - ); - } - - /** - * Insert a lesson progress row and return its ID. - * - * @param int $lesson_id Lesson ID (post_id column). - * @param int $user_id User ID. - * @param int $course_id Parent course ID. - * @return int Inserted row ID. - */ - private function insert_progress_row( int $lesson_id, int $user_id, int $course_id ): int { - global $wpdb; - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery - $wpdb->insert( - $wpdb->prefix . 'sensei_lms_progress', - array( - 'post_id' => $lesson_id, - 'user_id' => $user_id, - 'parent_post_id' => $course_id, - 'type' => 'lesson', - 'status' => 'in-progress', - 'created_at' => current_time( 'mysql' ), - 'updated_at' => current_time( 'mysql' ), - ) - ); - return (int) $wpdb->insert_id; - } - - /** - * Read the parent_post_id of a progress row by ID. - * - * @param int $row_id Row ID. - * @return int|null - */ - private function get_parent_post_id( int $row_id ): ?int { - global $wpdb; - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching - $value = $wpdb->get_var( - $wpdb->prepare( - "SELECT parent_post_id FROM {$wpdb->prefix}sensei_lms_progress WHERE id = %d", - $row_id - ) - ); - return null === $value ? null : (int) $value; - } - - /** - * Assert that a row's parent_post_id is SQL NULL (not 0 or empty string). - * - * @param int $row_id Row ID. - */ - private function assertParentPostIdIsSqlNull( int $row_id ): void { - global $wpdb; - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching - $matches = (int) $wpdb->get_var( - $wpdb->prepare( - "SELECT COUNT(*) FROM {$wpdb->prefix}sensei_lms_progress WHERE id = %d AND parent_post_id IS NULL", - $row_id - ) - ); - self::assertSame( 1, $matches ); - } -}