From d4dcece85e4ac06ea88f13d466e139ee3dfe595b Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Wed, 25 Mar 2026 13:49:52 -0400 Subject: [PATCH 1/7] Migrate Grading backend to HPPS service layer Add Grading_Listing_Service_Interface with comments-based and tables-based implementations. Refactor Sensei_Grading_Main to use the listing service for prepare_items() and get_row_data(). Add create_grading_listing_service() to factory. Apply teacher and temp-user restrictions to tables-based grading. Add per-status count caching to avoid duplicate queries for tab counts. Co-Authored-By: Claude Opus 4.6 (1M context) --- changelog/add-grading-service-migration | 4 + ...-grading-hide-lessons-without-quiz-answers | 4 + .../fix-grading-tab-counts-lesson-filter | 4 + changelog/fix-grading-teacher-restrictions | 4 + config/psalm/psalm-baseline.xml | 41 +- includes/class-sensei-grading-main.php | 145 +++-- ...comments-based-grading-listing-service.php | 121 ++++ ...lass-grading-listing-service-interface.php | 58 ++ .../class-progress-query-service-factory.php | 17 + ...s-tables-based-grading-listing-service.php | 295 +++++++++ ...comments-based-grading-listing-service.php | 209 +++++++ ...s-tables-based-grading-listing-service.php | 572 ++++++++++++++++++ tests/unit-tests/test-class-grading.php | 57 ++ 13 files changed, 1475 insertions(+), 56 deletions(-) create mode 100644 changelog/add-grading-service-migration create mode 100644 changelog/fix-grading-hide-lessons-without-quiz-answers create mode 100644 changelog/fix-grading-tab-counts-lesson-filter create mode 100644 changelog/fix-grading-teacher-restrictions create mode 100644 includes/internal/services/class-comments-based-grading-listing-service.php create mode 100644 includes/internal/services/class-grading-listing-service-interface.php create mode 100644 includes/internal/services/class-tables-based-grading-listing-service.php create mode 100644 tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php create mode 100644 tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php diff --git a/changelog/add-grading-service-migration b/changelog/add-grading-service-migration new file mode 100644 index 0000000000..e54dfa9673 --- /dev/null +++ b/changelog/add-grading-service-migration @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Internal: Integrate HPPS in grading backend. diff --git a/changelog/fix-grading-hide-lessons-without-quiz-answers b/changelog/fix-grading-hide-lessons-without-quiz-answers new file mode 100644 index 0000000000..d8a1132e10 --- /dev/null +++ b/changelog/fix-grading-hide-lessons-without-quiz-answers @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Hide lessons with no quiz answers from the Grading page, including completed lessons where the student never submitted the quiz and orphaned records with quiz-derived statuses but no answer data. diff --git a/changelog/fix-grading-tab-counts-lesson-filter b/changelog/fix-grading-tab-counts-lesson-filter new file mode 100644 index 0000000000..0399049cc3 --- /dev/null +++ b/changelog/fix-grading-tab-counts-lesson-filter @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fix Grading page status tab counts not updating when filtering by a specific lesson. diff --git a/changelog/fix-grading-teacher-restrictions b/changelog/fix-grading-teacher-restrictions new file mode 100644 index 0000000000..30954848dc --- /dev/null +++ b/changelog/fix-grading-teacher-restrictions @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fix teachers not seeing all grading rows for their courses due to post-filter pagination mismatch. diff --git a/config/psalm/psalm-baseline.xml b/config/psalm/psalm-baseline.xml index f6b625d566..69d4923e01 100644 --- a/config/psalm/psalm-baseline.xml +++ b/config/psalm/psalm-baseline.xml @@ -1620,17 +1620,13 @@ - - $counts['complete'] - $counts['failed'] - $counts['graded'] - $counts['in-progress'] - $counts['passed'] - $counts['ungraded'] - - - $args - + + $item->grade + $item->lesson_id + $item->status + $item->updated_at + $item->user_id + $course_id $lesson_id @@ -1642,6 +1638,9 @@ $search + + $args + Sensei_Grading_Main Sensei_Grading_Main @@ -4556,6 +4555,11 @@ $row->user_id + + + $comment->comment_ID + + $row->days_to_complete_count @@ -4568,6 +4572,21 @@ ARRAY_A + + + $column + + + $row->effective_status + $row->final_grade + $row->post_id + $row->updated_at + $row->user_id + + + ARRAY_A + + $row->days_to_complete_count diff --git a/includes/class-sensei-grading-main.php b/includes/class-sensei-grading-main.php index e45265dde1..20dfcc4340 100755 --- a/includes/class-sensei-grading-main.php +++ b/includes/class-sensei-grading-main.php @@ -3,6 +3,10 @@ exit; // Exit if accessed directly } +use Sensei\Internal\Services\Grading_Listing_Service_Interface; +use Sensei\Internal\Services\Grading_Item; +use Sensei\Internal\Services\Progress_Query_Service_Factory; + /** * Admin Grading Overview Data Table in Sensei. * @@ -19,12 +23,22 @@ class Sensei_Grading_Main extends Sensei_List_Table { public $user_ids = false; public $page_slug = 'sensei_grading'; + /** + * The grading listing service. + * + * @var Grading_Listing_Service_Interface + */ + private Grading_Listing_Service_Interface $grading_listing_service; + /** * Constructor * * @since 1.3.0 + * + * @param array|null $args Constructor arguments. + * @param Grading_Listing_Service_Interface|null $grading_listing_service The grading listing service. */ - public function __construct( $args = null ) { + public function __construct( $args = null, ?Grading_Listing_Service_Interface $grading_listing_service = null ) { $defaults = array( 'course_id' => 0, @@ -44,6 +58,9 @@ public function __construct( $args = null ) { $this->view = $args['view']; } + $this->grading_listing_service = $grading_listing_service + ?? ( new Progress_Query_Service_Factory() )->create_grading_listing_service(); + // Load Parent token into constructor parent::__construct( 'grading_main' ); @@ -237,30 +254,45 @@ public function prepare_items() { */ $activity_args = apply_filters( 'sensei_grading_filter_statuses', $activity_args ); - // WP_Comment_Query doesn't support SQL_CALC_FOUND_ROWS, so instead do this twice - $total_statuses = Sensei_Utils::sensei_check_for_activity( - array_merge( - $activity_args, - array( - 'count' => true, - 'offset' => 0, - 'number' => 0, - ) - ) - ); - - // Ensure we change our range to fit (in case a search threw off the pagination) - Should this be added to all views? - if ( $total_statuses < $activity_args['offset'] ) { - $new_paged = floor( $total_statuses / $activity_args['number'] ); - $activity_args['offset'] = $new_paged * $activity_args['number']; + // Apply teacher and temp-user restrictions so that both listing rows + // and cached per-status counts reflect these filters. For tables-based + // storage, these args are applied as SQL clauses. For comments-based, + // post__in flows through to WP_Comment_Query and the remaining args + // are handled by existing post-filters on sensei_check_for_activity. + $count_restrictions = apply_filters( 'sensei_count_statuses_args', array( 'type' => 'lesson' ) ); + + // Merge teacher's post__in restriction. + if ( ! empty( $count_restrictions['post__in'] ) ) { + if ( ! empty( $activity_args['post__in'] ) ) { + // Intersect: keep only lessons in both the course filter and teacher filter. + $intersected = array_values( + array_intersect( $activity_args['post__in'], $count_restrictions['post__in'] ) + ); + + // Force no-results when the intersection is empty (e.g. teacher + // does not own any lessons in the selected course). + $activity_args['post__in'] = empty( $intersected ) ? array( 0 ) : $intersected; + } elseif ( ! empty( $activity_args['post_id'] ) ) { + // Validate that the specific lesson belongs to this teacher's courses. + if ( ! in_array( (int) $activity_args['post_id'], array_map( 'intval', $count_restrictions['post__in'] ), true ) ) { + $activity_args['post__in'] = array( 0 ); + } + } else { + $activity_args['post__in'] = $count_restrictions['post__in']; + } } - $statuses = Sensei_Utils::sensei_check_for_activity( $activity_args, true ); - // Need to always return an array, even with only 1 item - if ( ! is_array( $statuses ) ) { - $statuses = array( $statuses ); + + // Pass through temp-user exclusion for the listing service. + if ( ! empty( $count_restrictions['exclude_user_login_prefixes'] ) ) { + $activity_args['exclude_user_login_prefixes'] = $count_restrictions['exclude_user_login_prefixes']; + if ( ! empty( $count_restrictions['include_statuses_override'] ) ) { + $activity_args['include_statuses_override'] = $count_restrictions['include_statuses_override']; + } } - $this->total_items = $total_statuses; - $this->items = $statuses; + + $result = $this->grading_listing_service->get_lesson_progress_items( $activity_args ); + $this->total_items = $result['total_count']; + $this->items = $result['items']; $total_items = $this->total_items; $total_pages = ceil( $total_items / $per_page ); @@ -277,25 +309,31 @@ public function prepare_items() { * Generates content for a single row of the table, overriding parent * * @since 1.7.0 - * @param object $item The current item + * @param Grading_Item $item The current item. */ protected function get_row_data( $item ) { - global $wp_version; + $status = $item->status; + $user_id = $item->user_id; + $lesson_id = $item->lesson_id; + $updated = $item->updated_at; + $grade_val = $item->grade; + + $grade_display = null !== $grade_val ? $grade_val . '%' : __( 'N/A', 'sensei-lms' ); $grade = ''; - if ( 'complete' == $item->comment_approved ) { + if ( 'complete' == $status ) { $status_html = '' . esc_html__( 'Completed', 'sensei-lms' ) . ''; $grade = __( 'No Grade', 'sensei-lms' ); - } elseif ( 'graded' == $item->comment_approved ) { + } elseif ( 'graded' == $status ) { $status_html = '' . esc_html__( 'Graded', 'sensei-lms' ) . ''; - $grade = get_comment_meta( $item->comment_ID, 'grade', true ) . '%'; - } elseif ( 'passed' == $item->comment_approved ) { + $grade = $grade_display; + } elseif ( 'passed' == $status ) { $status_html = '' . esc_html__( 'Passed', 'sensei-lms' ) . ''; - $grade = get_comment_meta( $item->comment_ID, 'grade', true ) . '%'; - } elseif ( 'failed' == $item->comment_approved ) { + $grade = $grade_display; + } elseif ( 'failed' == $status ) { $status_html = '' . esc_html__( 'Failed', 'sensei-lms' ) . ''; - $grade = get_comment_meta( $item->comment_ID, 'grade', true ) . '%'; - } elseif ( 'ungraded' == $item->comment_approved ) { + $grade = $grade_display; + } elseif ( 'ungraded' == $status ) { $status_html = '' . esc_html__( 'Ungraded', 'sensei-lms' ) . ''; $grade = __( 'N/A', 'sensei-lms' ); } else { @@ -303,20 +341,20 @@ protected function get_row_data( $item ) { $grade = __( 'N/A', 'sensei-lms' ); } - $title = Sensei_Learner::get_full_name( $item->user_id ); + $title = Sensei_Learner::get_full_name( $user_id ); - $quiz_id = Sensei()->lesson->lesson_quizzes( $item->comment_post_ID, 'any' ); + $quiz_id = Sensei()->lesson->lesson_quizzes( $lesson_id, 'any' ); $quiz_link = add_query_arg( array( 'page' => $this->page_slug, - 'user' => $item->user_id, + 'user' => $user_id, 'quiz_id' => $quiz_id, ), admin_url( 'admin.php' ) ); $grade_link = ''; - switch ( $item->comment_approved ) { + switch ( $status ) { case 'ungraded': $grade_link = '' . esc_html__( 'Grade quiz', 'sensei-lms' ) . ''; break; @@ -328,7 +366,7 @@ protected function get_row_data( $item ) { break; } - $course_id = get_post_meta( $item->comment_post_ID, '_lesson_course', true ); + $course_id = get_post_meta( $lesson_id, '_lesson_course', true ); $course_title = ''; if ( ! empty( $course_id ) ) { @@ -347,11 +385,11 @@ protected function get_row_data( $item ) { add_query_arg( array( 'page' => $this->page_slug, - 'lesson_id' => $item->comment_post_ID, + 'lesson_id' => $lesson_id, ), admin_url( 'admin.php' ) ) - ) . '">' . esc_html( get_the_title( $item->comment_post_ID ) ) . ''; + ) . '">' . esc_html( get_the_title( $lesson_id ) ) . ''; /** * Filter columns data for the Grading list table. @@ -359,7 +397,7 @@ protected function get_row_data( $item ) { * @hook sensei_grading_main_column_data * * @param {array} $column_data Column data for a row. - * @param {object} $item Activity comment object. + * @param {Grading_Item} $item Grading item for the row. * @param {int} $course_id The course ID. * @return {array} Filtered column data. */ @@ -370,14 +408,14 @@ protected function get_row_data( $item ) { add_query_arg( array( 'page' => $this->page_slug, - 'user_id' => $item->user_id, + 'user_id' => $user_id, ), admin_url( 'admin.php' ) ) ) . '">' . esc_html( $title ) . '', 'course' => $course_title, 'lesson' => $lesson_title, - 'updated' => $item->comment_date, + 'updated' => $updated, 'user_status' => $status_html, 'user_grade' => $grade, 'action' => $grade_link, @@ -500,7 +538,8 @@ public function get_views() { // Setup counters. $count_args = array( - 'type' => 'lesson', + 'type' => 'lesson', + 'exclude_unsubmitted_quiz_completions' => true, ); $query_args = array( 'page' => $this->page_slug, @@ -566,7 +605,23 @@ public function get_views() { */ $count_args = apply_filters( 'sensei_grading_count_statuses', $count_args ); - $counts = Sensei()->grading->count_statuses( $count_args ); + // Use cached per-status counts from prepare_items() when available, + // avoiding a second full-table scan with the same JOINs. + $cached_counts = $this->grading_listing_service->get_status_counts(); + if ( null !== $cached_counts ) { + // Ensure all expected statuses exist with 0 defaults, matching + // the shape that count_statuses() returns. + $defaults = array_fill_keys( + array( 'graded', 'ungraded', 'passed', 'failed', 'in-progress', 'complete' ), + 0 + ); + $counts = array_merge( $defaults, $cached_counts ); + + /** This filter is documented in includes/class-sensei-grading.php */ + $counts = apply_filters( 'sensei_count_statuses', $counts, 'sensei_lesson_status' ); + } else { + $counts = Sensei()->grading->count_statuses( $count_args ); + } $inprogress_lessons_count = $counts['in-progress']; $ungraded_lessons_count = $counts['ungraded']; diff --git a/includes/internal/services/class-comments-based-grading-listing-service.php b/includes/internal/services/class-comments-based-grading-listing-service.php new file mode 100644 index 0000000000..cf2c815314 --- /dev/null +++ b/includes/internal/services/class-comments-based-grading-listing-service.php @@ -0,0 +1,121 @@ +comments}.comment_approved != 'in-progress'" + . " AND EXISTS ( SELECT 1 FROM {$wpdb->postmeta} pm" + . " WHERE pm.post_id = {$wpdb->comments}.comment_post_ID" + . " AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0 )" + . " AND NOT EXISTS ( SELECT 1 FROM {$wpdb->commentmeta} cm" + . " WHERE cm.comment_id = {$wpdb->comments}.comment_ID" + . " AND cm.meta_key = 'quiz_answers' ) )"; + return $clauses; + }; + add_filter( 'comments_clauses', $exclusion_filter ); + + // WP_Comment_Query doesn't support SQL_CALC_FOUND_ROWS, so run + // a separate count query first with no limit/offset. + $total_count = \Sensei_Utils::sensei_check_for_activity( + array_merge( + $args, + [ + 'count' => true, + 'offset' => 0, + 'number' => 0, + ] + ) + ); + + // If the requested offset is beyond the total (e.g. in case a search + // threw off the pagination), snap back to the last valid page. + $offset = $args['offset'] ?? 0; + $number = $args['number'] ?? 10; + if ( $number > 0 && $total_count < $offset ) { + $new_paged = floor( $total_count / $number ); + $args['offset'] = $new_paged * $number; + } + + $statuses = \Sensei_Utils::sensei_check_for_activity( $args, true ); + + remove_filter( 'comments_clauses', $exclusion_filter ); + + // sensei_check_for_activity returns a single object when there is + // exactly one result — normalize to an array. + if ( ! is_array( $statuses ) ) { + $statuses = [ $statuses ]; + } + + $items = []; + foreach ( $statuses as $comment ) { + // sensei_check_for_activity can return false when no results are + // found; skip anything that isn't a real comment. + if ( ! $comment instanceof \WP_Comment ) { + continue; + } + + $grade_value = get_comment_meta( $comment->comment_ID, 'grade', true ); + + $items[] = new Grading_Item( + $comment->comment_approved, + (int) $comment->user_id, + (int) $comment->comment_post_ID, + $comment->comment_date, + '' !== $grade_value ? (float) $grade_value : null + ); + } + + return [ + 'items' => $items, + 'total_count' => (int) $total_count, + ]; + } + + /** + * Get cached per-status counts. + * + * Not supported by comments-based implementation. + * + * @since $$next-version$$ + * + * @return array|null Always null for comments-based storage. + */ + public function get_status_counts(): ?array { + return null; + } +} diff --git a/includes/internal/services/class-grading-listing-service-interface.php b/includes/internal/services/class-grading-listing-service-interface.php new file mode 100644 index 0000000000..d469a0a0f7 --- /dev/null +++ b/includes/internal/services/class-grading-listing-service-interface.php @@ -0,0 +1,58 @@ +|null Associative array of status => count, or null. + */ + public function get_status_counts(): ?array; +} diff --git a/includes/internal/services/class-progress-query-service-factory.php b/includes/internal/services/class-progress-query-service-factory.php index 17c25cec09..91b7b6db91 100644 --- a/includes/internal/services/class-progress-query-service-factory.php +++ b/includes/internal/services/class-progress-query-service-factory.php @@ -43,6 +43,23 @@ public function create_clauses_service(): Progress_Clauses_Service_Interface { return new Comments_Based_Progress_Clauses_Service( $wpdb ); } + /** + * Create a Grading_Listing_Service_Interface instance. + * + * @since $$next-version$$ + * + * @return Grading_Listing_Service_Interface The grading listing service. + */ + public function create_grading_listing_service(): Grading_Listing_Service_Interface { + global $wpdb; + + if ( Progress_Storage_Settings::is_hpps_enabled() && Progress_Storage_Settings::is_tables_repository() ) { + return new Tables_Based_Grading_Listing_Service( $wpdb ); + } + + return new Comments_Based_Grading_Listing_Service(); + } + /** * Create a Progress_Aggregation_Service_Interface instance. * diff --git a/includes/internal/services/class-tables-based-grading-listing-service.php b/includes/internal/services/class-tables-based-grading-listing-service.php new file mode 100644 index 0000000000..f45c021cba --- /dev/null +++ b/includes/internal/services/class-tables-based-grading-listing-service.php @@ -0,0 +1,295 @@ +|null + */ + private ?array $status_counts = null; + + /** + * Constructor. + * + * @since $$next-version$$ + * + * @param \wpdb $wpdb WordPress database object. + */ + public function __construct( \wpdb $wpdb ) { + $this->wpdb = $wpdb; + } + + /** + * Get the progress table name. + * + * @since $$next-version$$ + * + * @return string + */ + private function get_progress_table_name(): string { + return $this->wpdb->prefix . 'sensei_lms_progress'; + } + + /** + * Get lesson progress items for the grading listing. + * + * @since $$next-version$$ + * + * @param array $args Arguments for the query (see interface). + * @return array{ items: Grading_Item[], total_count: int } + */ + public function get_lesson_progress_items( array $args ): array { + $wpdb = $this->wpdb; + $table = $this->get_progress_table_name(); + $submissions_table = $wpdb->prefix . 'sensei_lms_quiz_submissions'; + + // Count all statuses in a single query for the All/Ungraded/Graded/In Progress tabs. + $count_args = $args; + $count_args['status'] = 'any'; + $count_base_query = $this->build_base_query( $table, $submissions_table, $count_args ); + + // Get per-status counts from a single GROUP BY query. + $status_count_query = "SELECT effective_status, COUNT(*) AS total FROM ( $count_base_query ) AS counted GROUP BY effective_status"; + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared via build_base_query. Caching handled by callers. + $status_rows = (array) $wpdb->get_results( $status_count_query, ARRAY_A ); + + $this->status_counts = []; + foreach ( $status_rows as $row ) { + $this->status_counts[ $row['effective_status'] ] = (int) $row['total']; + } + + // Derive total_count from status counts. + if ( empty( $args['status'] ) || 'any' === $args['status'] ) { + $total_count = array_sum( $this->status_counts ); + } else { + $total_count = 0; + foreach ( (array) $args['status'] as $s ) { + $total_count += $this->status_counts[ $s ] ?? 0; + } + } + + // Build the full base query WITH status filter for the paginated listing. + $base_query = $this->build_base_query( $table, $submissions_table, $args ); + + // If the requested offset is beyond the total (e.g. in case a search + // threw off the pagination), snap back to the last valid page. + $offset = $args['offset'] ?? 0; + $number = $args['number'] ?? 10; + if ( $number > 0 && $total_count < $offset ) { + $new_paged = floor( $total_count / $number ); + $offset = $new_paged * $number; + } + + // Append ordering and pagination to the base query for the items fetch. + $items_query = $base_query; + $items_query .= $this->build_order_clause( $args ); + $items_query .= $wpdb->prepare( ' LIMIT %d OFFSET %d', $number, $offset ); + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared via build_base_query. Caching handled by callers. + $rows = (array) $wpdb->get_results( $items_query ); + + $items = []; + foreach ( $rows as $row ) { + $items[] = new Grading_Item( + $row->effective_status, + (int) $row->user_id, + (int) $row->post_id, + get_date_from_gmt( $row->updated_at ), + null !== $row->final_grade ? (float) $row->final_grade : null + ); + } + + return [ + 'items' => $items, + 'total_count' => $total_count, + ]; + } + + /** + * Build the base SELECT query. + * + * @since $$next-version$$ + * + * @param string $table Progress table name. + * @param string $submissions_table Quiz submissions table name. + * @param array $args Query arguments. + * @return string SQL query. + */ + private function build_base_query( string $table, string $submissions_table, array $args ): string { + $wpdb = $this->wpdb; + + $query = 'SELECT p.post_id, p.user_id, p.updated_at, COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status ) AS effective_status, qs.final_grade'; + $query .= " FROM {$table} p"; + $query .= " LEFT JOIN {$wpdb->postmeta} pm ON pm.post_id = p.post_id AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0"; + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. + $query .= " LEFT JOIN {$submissions_table} qs ON qs.quiz_id = pm.meta_value AND qs.user_id = p.user_id"; + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. + $query .= " LEFT JOIN {$table} q ON q.post_id = pm.meta_value AND q.user_id = p.user_id AND q.type = 'quiz'"; + $query .= " WHERE p.type = 'lesson'"; + // Exclude lessons where a quiz exists but the student never submitted it + // and the lesson is already complete — there is nothing to grade. + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. + $query .= " AND NOT ( pm.meta_value IS NOT NULL AND qs.id IS NULL AND p.status = 'complete' )"; + + $query .= $this->build_post_filter( $args ); + $query .= $this->build_user_filter( $args ); + $query .= $this->build_user_exclusion_filter( $args ); + $query .= $this->build_status_filter( $args ); + + return $query; + } + + /** + * Build SQL clause for filtering by post ID(s). + * + * @since $$next-version$$ + * + * @param array $args Query arguments. + * @return string SQL clause. + */ + private function build_post_filter( array $args ): string { + $wpdb = $this->wpdb; + + if ( ! empty( $args['post__in'] ) && is_array( $args['post__in'] ) ) { + $placeholders = implode( ', ', array_fill( 0, count( $args['post__in'] ), '%d' ) ); + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + return $wpdb->prepare( " AND p.post_id IN ( $placeholders )", $args['post__in'] ); + } + + if ( ! empty( $args['post_id'] ) ) { + return $wpdb->prepare( ' AND p.post_id = %d', $args['post_id'] ); + } + + return ''; + } + + /** + * Build SQL clause for filtering by user ID(s). + * + * @since $$next-version$$ + * + * @param array $args Query arguments. + * @return string SQL clause. + */ + private function build_user_filter( array $args ): string { + $wpdb = $this->wpdb; + + if ( ! empty( $args['user_id'] ) && is_array( $args['user_id'] ) ) { + $placeholders = implode( ', ', array_fill( 0, count( $args['user_id'] ), '%d' ) ); + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + return $wpdb->prepare( " AND p.user_id IN ( $placeholders )", $args['user_id'] ); + } + + if ( ! empty( $args['user_id'] ) ) { + return $wpdb->prepare( ' AND p.user_id = %d', $args['user_id'] ); + } + + return ''; + } + + /** + * Build SQL clause for excluding users by login prefix. + * + * @since $$next-version$$ + * + * @param array $args Query arguments. + * @return string SQL clause. + */ + private function build_user_exclusion_filter( array $args ): string { + $status_column = 'COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status )'; + return Utils::build_user_exclusion_clause( $this->wpdb, $args, $status_column ); + } + + /** + * Build SQL clause for filtering by status. + * + * @since $$next-version$$ + * + * @param array $args Query arguments. + * @return string SQL clause. + */ + private function build_status_filter( array $args ): string { + if ( empty( $args['status'] ) || 'any' === $args['status'] ) { + return ''; + } + + $wpdb = $this->wpdb; + $statuses = (array) $args['status']; + + $placeholders = implode( ', ', array_fill( 0, count( $statuses ), '%s' ) ); + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + return $wpdb->prepare( " AND COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status ) IN ( $placeholders )", $statuses ); + } + + /** + * Get cached per-status counts from the most recent query. + * + * Returns null if counts are not available (e.g. if + * get_lesson_progress_items has not been called yet). + * + * @since $$next-version$$ + * + * @return array|null Associative array of status => count, or null. + */ + public function get_status_counts(): ?array { + return $this->status_counts; + } + + /** + * Build ORDER BY clause. + * + * @since $$next-version$$ + * + * @param array $args Query arguments. + * @return string SQL ORDER BY clause. + */ + private function build_order_clause( array $args ): string { + $order = isset( $args['order'] ) && 'ASC' === strtoupper( $args['order'] ) ? 'ASC' : 'DESC'; + $orderby = $args['orderby'] ?? ''; + + // Title, course, and lesson columns map to post_id as a simplified + // approximation — actual title/course name ordering would require + // additional JOINs that are not worth the performance cost here. + // This means rows are sorted by numeric ID rather than alphabetically. + $orderby_map = [ + 'title' => 'p.post_id', + 'course' => 'p.post_id', + 'lesson' => 'p.post_id', + 'updated' => 'p.updated_at', + 'user_status' => 'effective_status', + 'user_grade' => 'qs.final_grade', + ]; + + $column = esc_sql( $orderby_map[ $orderby ] ?? 'p.updated_at' ); + + return " ORDER BY $column $order"; + } +} diff --git a/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php b/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php new file mode 100644 index 0000000000..2cff7e0812 --- /dev/null +++ b/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php @@ -0,0 +1,209 @@ +sensei_factory = new \Sensei_Factory(); + } + + /** + * Build default query args for get_lesson_progress_items. + * + * @param array $overrides Args to override. + * @return array + */ + private function get_default_args( array $overrides = [] ): array { + return array_merge( + [ + 'type' => 'sensei_lesson_status', + 'number' => 10, + 'offset' => 0, + 'orderby' => '', + 'order' => 'DESC', + 'status' => 'any', + ], + $overrides + ); + } + + public function testGetLessonProgressItems_WithLessonStatus_ReturnsGradingItems(): void { + /* Arrange. */ + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'in-progress' ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected exactly one result.' ); + $this->assertCount( 1, $result['items'], 'Expected exactly one item.' ); + $this->assertInstanceOf( Grading_Item::class, $result['items'][0], 'Expected a Grading_Item instance.' ); + $this->assertSame( 'in-progress', $result['items'][0]->status, 'Expected in-progress status.' ); + $this->assertSame( $user_id, $result['items'][0]->user_id, 'Expected matching user ID.' ); + $this->assertSame( $lesson_id, $result['items'][0]->lesson_id, 'Expected matching lesson ID.' ); + $this->assertNull( $result['items'][0]->grade, 'Expected null grade for non-graded item.' ); + } + + public function testGetLessonProgressItems_WithGradedStatus_ReturnsGradeValue(): void { + /* Arrange. */ + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $comment_id = \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'graded' ); + update_comment_meta( $comment_id, 'grade', 85 ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 85.0, $result['items'][0]->grade ); + } + + public function testGetLessonProgressItems_WithQuizButNoAnswers_ExcludedFromResults(): void { + /* Arrange. */ + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + + // Complete lesson with quiz but no quiz_answers meta — nothing to grade. + \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'complete' ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 0, $result['total_count'], 'Completed lesson with quiz but no answers should be excluded.' ); + $this->assertEmpty( $result['items'], 'No items should be returned.' ); + } + + public function testGetLessonProgressItems_WithStatusFilter_FiltersResults(): void { + /* Arrange. */ + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + \Sensei_Utils::update_lesson_status( $user1, $lesson_id, 'in-progress' ); + \Sensei_Utils::update_lesson_status( $user2, $lesson_id, 'in-progress' ); + \Sensei_Utils::update_lesson_status( $user2, $lesson_id, 'complete' ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'status' => 'in-progress', + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected one in-progress result.' ); + $this->assertSame( 'in-progress', $result['items'][0]->status, 'Expected in-progress status.' ); + } + + public function testGetLessonProgressItems_WithUserIdFilter_RestrictsToUser(): void { + /* Arrange. */ + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + \Sensei_Utils::update_lesson_status( $user1, $lesson_id, 'in-progress' ); + \Sensei_Utils::update_lesson_status( $user2, $lesson_id, 'in-progress' ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'user_id' => $user1, + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected one result for filtered user.' ); + $this->assertSame( $user1, $result['items'][0]->user_id, 'Expected matching user ID.' ); + } + + public function testGetStatusCounts_ReturnsNull(): void { + /* Arrange. */ + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act & Assert. */ + $this->assertNull( $service->get_status_counts(), 'Comments-based service should always return null.' ); + } + + public function testGetLessonProgressItems_WithOffsetBeyondTotal_CorrectsPagination(): void { + /* Arrange. */ + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'in-progress' ); + + $service = new Comments_Based_Grading_Listing_Service(); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'offset' => 100, + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected one total item.' ); + $this->assertCount( 1, $result['items'], 'Expected offset correction to return items.' ); + } +} diff --git a/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php b/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php new file mode 100644 index 0000000000..cb401e5066 --- /dev/null +++ b/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php @@ -0,0 +1,572 @@ +sensei_factory = new \Sensei_Factory(); + } + + /** + * Insert a progress row directly into the HPPS progress table. + * + * @param int $post_id The post ID. + * @param int $user_id The user ID. + * @param string $type The progress type. + * @param string $status The progress status. + * @param int|null $parent_post_id The parent post ID. + */ + private function insert_progress( int $post_id, int $user_id, string $type, string $status, ?int $parent_post_id = null ): void { + $wpdb = $GLOBALS['wpdb']; + $table = $wpdb->prefix . 'sensei_lms_progress'; + $now = current_time( 'mysql' ); + $data = [ + 'post_id' => $post_id, + 'user_id' => $user_id, + 'type' => $type, + 'status' => $status, + 'started_at' => $now, + 'created_at' => $now, + 'updated_at' => $now, + ]; + $format = [ '%d', '%d', '%s', '%s', '%s', '%s', '%s' ]; + if ( null !== $parent_post_id ) { + $data['parent_post_id'] = $parent_post_id; + $format[] = '%d'; + } + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery -- Test helper. + $wpdb->insert( $table, $data, $format ); + } + + /** + * Insert a quiz submission row. + * + * @param int $quiz_id The quiz post ID. + * @param int $user_id The user ID. + * @param int|null $final_grade The final grade. + */ + private function insert_quiz_submission( int $quiz_id, int $user_id, ?int $final_grade = null ): void { + $wpdb = $GLOBALS['wpdb']; + $table = $wpdb->prefix . 'sensei_lms_quiz_submissions'; + $now = current_time( 'mysql' ); + $data = [ + 'quiz_id' => $quiz_id, + 'user_id' => $user_id, + 'created_at' => $now, + 'updated_at' => $now, + ]; + $format = [ '%d', '%d', '%s', '%s' ]; + if ( null !== $final_grade ) { + $data['final_grade'] = $final_grade; + $format[] = '%d'; + } + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery -- Test helper. + $wpdb->insert( $table, $data, $format ); + } + + /** + * Build default query args for get_lesson_progress_items. + * + * @param array $overrides Args to override. + * @return array + */ + private function get_default_args( array $overrides = [] ): array { + return array_merge( + [ + 'type' => 'sensei_lesson_status', + 'number' => 10, + 'offset' => 0, + 'orderby' => '', + 'order' => 'DESC', + 'status' => 'any', + ], + $overrides + ); + } + + public function testGetLessonProgressItems_WithLessonStatus_ReturnsGradingItems(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user_id, 'lesson', 'in-progress', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected exactly one result.' ); + $this->assertCount( 1, $result['items'], 'Expected exactly one item.' ); + $this->assertInstanceOf( Grading_Item::class, $result['items'][0], 'Expected a Grading_Item instance.' ); + $this->assertSame( 'in-progress', $result['items'][0]->status, 'Expected in-progress status.' ); + $this->assertSame( $user_id, $result['items'][0]->user_id, 'Expected matching user ID.' ); + $this->assertSame( $lesson_id, $result['items'][0]->lesson_id, 'Expected matching lesson ID.' ); + $this->assertNull( $result['items'][0]->grade, 'Expected null grade for non-graded item.' ); + } + + public function testGetLessonProgressItems_WithQuizStatus_UsesCoalescedStatus(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + $this->insert_progress( $lesson_id, $user_id, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user_id, 'quiz', 'passed', $lesson_id ); + $this->insert_quiz_submission( $quiz_id, $user_id, 90 ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 'passed', $result['items'][0]->status, 'Quiz status should override lesson status.' ); + $this->assertSame( 90.0, $result['items'][0]->grade, 'Expected grade from quiz submission.' ); + } + + public function testGetLessonProgressItems_WithQuizProgressButNoSubmission_ExcludedFromResults(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + + // Quiz progress exists but no quiz submission — nothing to grade. + $this->insert_progress( $lesson_id, $user_id, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user_id, 'quiz', 'passed', $lesson_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + + /* Assert. */ + $this->assertSame( 0, $result['total_count'], 'Completed lesson with quiz but no submission should be excluded.' ); + $this->assertEmpty( $result['items'], 'No items should be returned.' ); + } + + public function testGetLessonProgressItems_WithPostInFilter_ReturnsMatchingLessons(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson1 = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $lesson2 = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $lesson3 = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson1, $user_id, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson2, $user_id, 'lesson', 'complete', $course_id ); + $this->insert_progress( $lesson3, $user_id, 'lesson', 'in-progress', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'post__in' => [ $lesson1, $lesson2 ] ] ) + ); + + /* Assert. */ + $this->assertSame( 2, $result['total_count'], 'Expected two items for the two filtered lessons.' ); + $returned_lesson_ids = array_map( + function ( $item ) { + return $item->lesson_id; + }, + $result['items'] + ); + $this->assertContains( $lesson1, $returned_lesson_ids, 'Expected lesson1 in results.' ); + $this->assertContains( $lesson2, $returned_lesson_ids, 'Expected lesson2 in results.' ); + $this->assertNotContains( $lesson3, $returned_lesson_ids, 'Expected lesson3 excluded from results.' ); + } + + public function testGetLessonProgressItems_WithOffsetBeyondTotal_CorrectsPagination(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user_id, 'lesson', 'in-progress', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'offset' => 100, + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected one total item.' ); + $this->assertCount( 1, $result['items'], 'Expected offset correction to return items.' ); + } + + public function testGetLessonProgressItems_WithMultipleStatusArray_FiltersCorrectly(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $user3 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + // User2 completed and submitted the quiz so is not excluded. + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user2, 'quiz', 'complete', $lesson_id ); + $this->insert_quiz_submission( $quiz_id, $user2 ); + + $this->insert_progress( $lesson_id, $user3, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user3, 'quiz', 'graded', $lesson_id ); + $this->insert_quiz_submission( $quiz_id, $user3 ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'status' => [ 'in-progress', 'complete' ], + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 2, $result['total_count'], 'Expected two items matching in-progress or complete.' ); + $statuses = array_map( + function ( $item ) { + return $item->status; + }, + $result['items'] + ); + $this->assertNotContains( 'graded', $statuses, 'Graded status should be excluded.' ); + } + + public function testGetLessonProgressItems_WithStatusFilter_FiltersResults(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'status' => 'in-progress', + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'] ); + $this->assertSame( 'in-progress', $result['items'][0]->status ); + } + + public function testGetLessonProgressItems_WithPagination_RespectsLimitAndOffset(): void { + /* Arrange. */ + global $wpdb; + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + for ( $i = 0; $i < 3; $i++ ) { + $user_id = $this->sensei_factory->user->create(); + $this->insert_progress( $lesson_id, $user_id, 'lesson', 'in-progress', $course_id ); + } + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'number' => 2, + 'post_id' => $lesson_id, + ] + ) + ); + + /* Assert. */ + $this->assertSame( 3, $result['total_count'] ); + $this->assertCount( 2, $result['items'] ); + } + + public function testGetLessonProgressItems_WithUserIdFilter_RestrictsToUser(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( [ 'user_id' => $user1 ] ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'] ); + $this->assertSame( $user1, $result['items'][0]->user_id ); + } + + public function testGetStatusCounts_AfterGetLessonProgressItems_ReturnsPerStatusCounts(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + + // User1: in-progress lesson (no quiz interaction). + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + + // User2: completed lesson with passed quiz. + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user2, 'quiz', 'passed', $lesson_id ); + $this->insert_quiz_submission( $quiz_id, $user2, 85 ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $service->get_lesson_progress_items( + $this->get_default_args( [ 'post_id' => $lesson_id ] ) + ); + $counts = $service->get_status_counts(); + + /* Assert. */ + $this->assertIsArray( $counts, 'Expected an array of status counts.' ); + $this->assertSame( 1, $counts['in-progress'] ?? 0, 'Expected 1 in-progress.' ); + $this->assertSame( 1, $counts['passed'] ?? 0, 'Expected 1 passed (coalesced from quiz).' ); + } + + public function testGetStatusCounts_BeforeGetLessonProgressItems_ReturnsNull(): void { + /* Arrange. */ + global $wpdb; + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act & Assert. */ + $this->assertNull( $service->get_status_counts(), 'Expected null before any query.' ); + } + + public function testGetLessonProgressItems_WithExcludeUserLoginPrefixes_ExcludesMatchingUsers(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create( [ 'user_login' => 'real_student' ] ); + $user2 = $this->sensei_factory->user->create( [ 'user_login' => 'preview_guest_123' ] ); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson_id, $user2, 'lesson', 'in-progress', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'post_id' => $lesson_id, + 'exclude_user_login_prefixes' => [ 'preview_' ], + ] + ) + ); + + /* Assert. */ + $this->assertSame( 1, $result['total_count'], 'Expected excluded user to be filtered out.' ); + $this->assertSame( $user1, $result['items'][0]->user_id, 'Expected only the real student.' ); + } + + public function testGetLessonProgressItems_WithIncludeStatusesOverride_KeepsExcludedUserWithMatchingStatus(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create( [ 'user_login' => 'real_student' ] ); + $user2 = $this->sensei_factory->user->create( [ 'user_login' => 'preview_guest_456' ] ); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $quiz_id = $this->sensei_factory->quiz->create( + [ + 'post_parent' => $lesson_id, + 'meta_input' => [ '_quiz_lesson' => $lesson_id ], + ] + ); + update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); + + // Real student: in-progress. + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + + // Preview user: completed with ungraded quiz — should be kept because of override. + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + $this->insert_progress( $quiz_id, $user2, 'quiz', 'ungraded', $lesson_id ); + $this->insert_quiz_submission( $quiz_id, $user2 ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $result = $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'post_id' => $lesson_id, + 'exclude_user_login_prefixes' => [ 'preview_' ], + 'include_statuses_override' => [ 'ungraded' ], + ] + ) + ); + + /* Assert. */ + $this->assertSame( 2, $result['total_count'], 'Expected both users — preview user kept by status override.' ); + $returned_user_ids = array_map( + function ( $item ) { + return $item->user_id; + }, + $result['items'] + ); + $this->assertContains( $user1, $returned_user_ids, 'Expected real student in results.' ); + $this->assertContains( $user2, $returned_user_ids, 'Expected preview user kept by ungraded override.' ); + } + + public function testGetStatusCounts_WithExcludeUserLoginPrefixes_ExcludesFromCounts(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create( [ 'user_login' => 'real_student2' ] ); + $user2 = $this->sensei_factory->user->create( [ 'user_login' => 'preview_guest_789' ] ); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson_id, $user2, 'lesson', 'in-progress', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act. */ + $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'post_id' => $lesson_id, + 'exclude_user_login_prefixes' => [ 'preview_' ], + ] + ) + ); + $counts = $service->get_status_counts(); + + /* Assert. */ + $this->assertSame( 1, $counts['in-progress'] ?? 0, 'Expected cached counts to also exclude the preview user.' ); + } + + public function testGetStatusCounts_WithStatusFilter_ReturnsAllStatuses(): void { + /* Arrange. */ + global $wpdb; + $user1 = $this->sensei_factory->user->create(); + $user2 = $this->sensei_factory->user->create(); + $course_id = $this->sensei_factory->course->create(); + $lesson_id = $this->sensei_factory->lesson->create( + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + $this->insert_progress( $lesson_id, $user1, 'lesson', 'in-progress', $course_id ); + $this->insert_progress( $lesson_id, $user2, 'lesson', 'complete', $course_id ); + + $service = new Tables_Based_Grading_Listing_Service( $wpdb ); + + /* Act -- query with status filter for in-progress only. */ + $service->get_lesson_progress_items( + $this->get_default_args( + [ + 'status' => 'in-progress', + 'post_id' => $lesson_id, + ] + ) + ); + $counts = $service->get_status_counts(); + + /* Assert -- counts should include ALL statuses, not just in-progress. */ + $this->assertSame( 1, $counts['in-progress'] ?? 0, 'Expected 1 in-progress.' ); + $this->assertSame( 1, $counts['complete'] ?? 0, 'Expected 1 complete even though status filter was in-progress.' ); + } +} diff --git a/tests/unit-tests/test-class-grading.php b/tests/unit-tests/test-class-grading.php index 08e438651c..8cd91df13b 100644 --- a/tests/unit-tests/test-class-grading.php +++ b/tests/unit-tests/test-class-grading.php @@ -31,6 +31,63 @@ public function testClassInstance() { $this->assertTrue( isset( Sensei()->grading ), 'Sensei Grading class is not loaded' ); } + /** + * Tests that prepare_items() applies sensei_count_statuses_args + * restrictions to listing rows for tables-based storage. + * + * @covers Sensei_Grading_Main::prepare_items + */ + public function testPrepareItems_TablesBasedWithCountStatusesArgsRestriction_RestrictsListingRows(): void { + /* Arrange. */ + global $wpdb; + $user_id = $this->factory->user->create(); + $course_id = $this->factory->course->create(); + $lesson_ids = $this->factory->lesson->create_many( + 2, + [ 'meta_input' => [ '_lesson_course' => $course_id ] ] + ); + + $table = $wpdb->prefix . 'sensei_lms_progress'; + $now = current_time( 'mysql' ); + foreach ( $lesson_ids as $lid ) { + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery -- Test helper. + $wpdb->insert( + $table, + [ + 'post_id' => $lid, + 'user_id' => $user_id, + 'type' => 'lesson', + 'status' => 'in-progress', + 'started_at' => $now, + 'created_at' => $now, + 'updated_at' => $now, + ], + [ '%d', '%d', '%s', '%s', '%s', '%s', '%s' ] + ); + } + + // Simulate teacher restriction: only allow the first lesson. + $restrict_filter = function ( $args ) use ( $lesson_ids ) { + $args['post__in'] = [ $lesson_ids[0] ]; + return $args; + }; + add_filter( 'sensei_count_statuses_args', $restrict_filter ); + + $this->login_as_admin(); + $service = new \Sensei\Internal\Services\Tables_Based_Grading_Listing_Service( $wpdb ); + $grading_main = new Sensei_Grading_Main( [ 'view' => 'all' ], $service ); + + /* Act. */ + $grading_main->prepare_items(); + + /* Assert. */ + $this->assertCount( 1, $grading_main->items, 'Listing should only show items for the allowed lesson.' ); + $this->assertSame( $lesson_ids[0], $grading_main->items[0]->lesson_id, 'Listing item should be for the restricted lesson.' ); + + /* Clean up. */ + remove_filter( 'sensei_count_statuses_args', $restrict_filter ); + } + /** * Tests that the ungraded quiz count is not displayed in the Grading menu. * From 91e2064b22a5fde2417b606105673ef0d9a4320f Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 09:46:57 -0400 Subject: [PATCH 2/7] Remove quiz submission exclusion filter and simplify effective_status The Grading page exclusion filter that hid lessons without quiz submissions was too aggressive and caused count mismatches between storage backends. Remove it in favor of showing all lesson progress rows, and simplify the tables-based effective_status from COALESCE(CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status) to COALESCE(q.status, p.status) so quiz progress status always takes precedence when it exists. This aligns tables-based status display with comments-based storage for lessons with real quizzes. Filtering rows with nothing to grade is tracked in #7926. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...-grading-hide-lessons-without-quiz-answers | 4 -- includes/class-sensei-grading-main.php | 3 +- ...comments-based-grading-listing-service.php | 20 ------ ...nts-based-progress-aggregation-service.php | 19 ------ ...progress-aggregation-service-interface.php | 1 - ...s-tables-based-grading-listing-service.php | 10 +-- ...les-based-progress-aggregation-service.php | 19 ++---- ...comments-based-grading-listing-service.php | 30 --------- ...nts-based-progress-aggregation-service.php | 65 ------------------- ...s-tables-based-grading-listing-service.php | 32 --------- ...les-based-progress-aggregation-service.php | 43 +----------- 11 files changed, 11 insertions(+), 235 deletions(-) delete mode 100644 changelog/fix-grading-hide-lessons-without-quiz-answers diff --git a/changelog/fix-grading-hide-lessons-without-quiz-answers b/changelog/fix-grading-hide-lessons-without-quiz-answers deleted file mode 100644 index d8a1132e10..0000000000 --- a/changelog/fix-grading-hide-lessons-without-quiz-answers +++ /dev/null @@ -1,4 +0,0 @@ -Significance: patch -Type: fixed - -Hide lessons with no quiz answers from the Grading page, including completed lessons where the student never submitted the quiz and orphaned records with quiz-derived statuses but no answer data. diff --git a/includes/class-sensei-grading-main.php b/includes/class-sensei-grading-main.php index 20dfcc4340..4f714419f6 100755 --- a/includes/class-sensei-grading-main.php +++ b/includes/class-sensei-grading-main.php @@ -538,8 +538,7 @@ public function get_views() { // Setup counters. $count_args = array( - 'type' => 'lesson', - 'exclude_unsubmitted_quiz_completions' => true, + 'type' => 'lesson', ); $query_args = array( 'page' => $this->page_slug, diff --git a/includes/internal/services/class-comments-based-grading-listing-service.php b/includes/internal/services/class-comments-based-grading-listing-service.php index cf2c815314..4e823cc1a5 100644 --- a/includes/internal/services/class-comments-based-grading-listing-service.php +++ b/includes/internal/services/class-comments-based-grading-listing-service.php @@ -31,24 +31,6 @@ class Comments_Based_Grading_Listing_Service implements Grading_Listing_Service_ * @return array{ items: Grading_Item[], total_count: int } */ public function get_lesson_progress_items( array $args ): array { - // Add a SQL-level filter to exclude lessons where a quiz exists but - // the student has no quiz answers — there is nothing to grade. - // This covers both 'complete' (never submitted) and orphaned - // 'passed'/'graded'/'failed' records with no answer data. - // In-progress students are kept since they haven't submitted yet. - $exclusion_filter = function ( array $clauses ): array { - global $wpdb; - $clauses['where'] .= " AND NOT ( {$wpdb->comments}.comment_approved != 'in-progress'" - . " AND EXISTS ( SELECT 1 FROM {$wpdb->postmeta} pm" - . " WHERE pm.post_id = {$wpdb->comments}.comment_post_ID" - . " AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0 )" - . " AND NOT EXISTS ( SELECT 1 FROM {$wpdb->commentmeta} cm" - . " WHERE cm.comment_id = {$wpdb->comments}.comment_ID" - . " AND cm.meta_key = 'quiz_answers' ) )"; - return $clauses; - }; - add_filter( 'comments_clauses', $exclusion_filter ); - // WP_Comment_Query doesn't support SQL_CALC_FOUND_ROWS, so run // a separate count query first with no limit/offset. $total_count = \Sensei_Utils::sensei_check_for_activity( @@ -73,8 +55,6 @@ public function get_lesson_progress_items( array $args ): array { $statuses = \Sensei_Utils::sensei_check_for_activity( $args, true ); - remove_filter( 'comments_clauses', $exclusion_filter ); - // sensei_check_for_activity returns a single object when there is // exactly one result — normalize to an array. if ( ! is_array( $statuses ) ) { diff --git a/includes/internal/services/class-comments-based-progress-aggregation-service.php b/includes/internal/services/class-comments-based-progress-aggregation-service.php index a506cfa0df..c97b2f2dd6 100644 --- a/includes/internal/services/class-comments-based-progress-aggregation-service.php +++ b/includes/internal/services/class-comments-based-progress-aggregation-service.php @@ -78,7 +78,6 @@ public function count_statuses( array $args ): array { $query .= $this->build_post_filter_clause( $args ); $query .= $this->build_user_filter_clause( $args ); $query .= $this->build_user_exclusion_clause( $args ); - $query .= $this->build_unsubmitted_quiz_exclusion_clause( $args ); if ( isset( $args['query'] ) ) { $query .= $args['query']; @@ -253,22 +252,4 @@ private function build_user_exclusion_clause( array $args ): string { * @param array $args Query arguments. * @return string SQL clause. */ - private function build_unsubmitted_quiz_exclusion_clause( array $args ): string { - $exclude = $args['exclude_unsubmitted_quiz_completions'] ?? false; - - if ( ! $exclude ) { - return ''; - } - - $wpdb = $this->wpdb; - - // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb. - return " AND NOT ( comment_approved != 'in-progress'" - . " AND EXISTS ( SELECT 1 FROM {$wpdb->postmeta} pm" - . " WHERE pm.post_id = {$wpdb->comments}.comment_post_ID" - . " AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0 )" - . " AND NOT EXISTS ( SELECT 1 FROM {$wpdb->commentmeta} cm" - . " WHERE cm.comment_id = {$wpdb->comments}.comment_ID" - . " AND cm.meta_key = 'quiz_answers' ) )"; - } } diff --git a/includes/internal/services/class-progress-aggregation-service-interface.php b/includes/internal/services/class-progress-aggregation-service-interface.php index a5058c0943..4259f8cd57 100644 --- a/includes/internal/services/class-progress-aggregation-service-interface.php +++ b/includes/internal/services/class-progress-aggregation-service-interface.php @@ -37,7 +37,6 @@ interface Progress_Aggregation_Service_Interface { * @type int|array $user_id Restrict to specific user IDs. * @type string[] $exclude_user_login_prefixes User login prefixes to exclude. * @type string[] $include_statuses_override Statuses that bypass user exclusion. - * @type bool $exclude_unsubmitted_quiz_completions Exclude completed lessons with no quiz submission (default: false). * } * @return array Associative array of status => count. */ diff --git a/includes/internal/services/class-tables-based-grading-listing-service.php b/includes/internal/services/class-tables-based-grading-listing-service.php index f45c021cba..da2f7fb78f 100644 --- a/includes/internal/services/class-tables-based-grading-listing-service.php +++ b/includes/internal/services/class-tables-based-grading-listing-service.php @@ -146,7 +146,7 @@ public function get_lesson_progress_items( array $args ): array { private function build_base_query( string $table, string $submissions_table, array $args ): string { $wpdb = $this->wpdb; - $query = 'SELECT p.post_id, p.user_id, p.updated_at, COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status ) AS effective_status, qs.final_grade'; + $query = 'SELECT p.post_id, p.user_id, p.updated_at, COALESCE( q.status, p.status ) AS effective_status, qs.final_grade'; $query .= " FROM {$table} p"; $query .= " LEFT JOIN {$wpdb->postmeta} pm ON pm.post_id = p.post_id AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0"; // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. @@ -154,10 +154,6 @@ private function build_base_query( string $table, string $submissions_table, arr // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. $query .= " LEFT JOIN {$table} q ON q.post_id = pm.meta_value AND q.user_id = p.user_id AND q.type = 'quiz'"; $query .= " WHERE p.type = 'lesson'"; - // Exclude lessons where a quiz exists but the student never submitted it - // and the lesson is already complete — there is nothing to grade. - // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. - $query .= " AND NOT ( pm.meta_value IS NOT NULL AND qs.id IS NULL AND p.status = 'complete' )"; $query .= $this->build_post_filter( $args ); $query .= $this->build_user_filter( $args ); @@ -224,7 +220,7 @@ private function build_user_filter( array $args ): string { * @return string SQL clause. */ private function build_user_exclusion_filter( array $args ): string { - $status_column = 'COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status )'; + $status_column = 'COALESCE( q.status, p.status )'; return Utils::build_user_exclusion_clause( $this->wpdb, $args, $status_column ); } @@ -246,7 +242,7 @@ private function build_status_filter( array $args ): string { $placeholders = implode( ', ', array_fill( 0, count( $statuses ), '%s' ) ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare - return $wpdb->prepare( " AND COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status ) IN ( $placeholders )", $statuses ); + return $wpdb->prepare( " AND COALESCE( q.status, p.status ) IN ( $placeholders )", $statuses ); } /** diff --git a/includes/internal/services/class-tables-based-progress-aggregation-service.php b/includes/internal/services/class-tables-based-progress-aggregation-service.php index 14fc1035b1..c9b0810ceb 100644 --- a/includes/internal/services/class-tables-based-progress-aggregation-service.php +++ b/includes/internal/services/class-tables-based-progress-aggregation-service.php @@ -166,9 +166,8 @@ public function get_lesson_totals( array $lesson_ids ): array { * This mirrors the comments-based behavior where a single comment per lesson * stores the quiz-derived status directly. * - * Uses COALESCE(CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status) - * so quiz status takes precedence only when a quiz submission exists; - * otherwise falls back to lesson status. + * Uses COALESCE(q.status, p.status) so quiz progress status takes + * precedence when it exists; otherwise falls back to lesson status. * * @since $$next-version$$ * @@ -181,7 +180,7 @@ private function count_lesson_statuses_with_quiz( array $args ): array { $submissions_table = $wpdb->prefix . 'sensei_lms_quiz_submissions'; // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. - $query = "SELECT COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status ) AS effective_status, COUNT( * ) AS total FROM {$table} p"; + $query = "SELECT COALESCE( q.status, p.status ) AS effective_status, COUNT( * ) AS total FROM {$table} p"; $query .= " LEFT JOIN {$wpdb->postmeta} pm ON pm.post_id = p.post_id AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0"; // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. $query .= " LEFT JOIN {$submissions_table} qs ON qs.quiz_id = pm.meta_value AND qs.user_id = p.user_id"; @@ -189,11 +188,10 @@ private function count_lesson_statuses_with_quiz( array $args ): array { $query .= " LEFT JOIN {$table} q ON q.post_id = pm.meta_value AND q.user_id = p.user_id AND q.type = 'quiz'"; $query .= $wpdb->prepare( ' WHERE p.type = %s', 'lesson' ); - $query .= $this->build_unsubmitted_quiz_exclusion_clause( $args ); $query .= $this->build_post_filter_clause( $args ); $query .= $this->build_user_filter_clause( $args ); - $query .= $this->build_user_exclusion_clause( $args, 'COALESCE( CASE WHEN qs.id IS NOT NULL THEN q.status END, p.status )' ); + $query .= $this->build_user_exclusion_clause( $args, 'COALESCE( q.status, p.status )' ); $query .= ' GROUP BY effective_status'; @@ -317,13 +315,4 @@ private function build_user_exclusion_clause( array $args, string $status_column * @param array $args Query arguments. * @return string SQL clause. */ - private function build_unsubmitted_quiz_exclusion_clause( array $args ): string { - $exclude = $args['exclude_unsubmitted_quiz_completions'] ?? false; - - if ( ! $exclude ) { - return ''; - } - - return " AND NOT ( pm.meta_value IS NOT NULL AND qs.id IS NULL AND p.status = 'complete' )"; - } } diff --git a/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php b/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php index 2cff7e0812..cf4e25b047 100644 --- a/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php +++ b/tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php @@ -86,36 +86,6 @@ public function testGetLessonProgressItems_WithGradedStatus_ReturnsGradeValue(): $this->assertSame( 85.0, $result['items'][0]->grade ); } - public function testGetLessonProgressItems_WithQuizButNoAnswers_ExcludedFromResults(): void { - /* Arrange. */ - $user_id = $this->sensei_factory->user->create(); - $course_id = $this->sensei_factory->course->create(); - $lesson_id = $this->sensei_factory->lesson->create( - [ 'meta_input' => [ '_lesson_course' => $course_id ] ] - ); - $quiz_id = $this->sensei_factory->quiz->create( - [ - 'post_parent' => $lesson_id, - 'meta_input' => [ '_quiz_lesson' => $lesson_id ], - ] - ); - update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); - - // Complete lesson with quiz but no quiz_answers meta — nothing to grade. - \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'complete' ); - - $service = new Comments_Based_Grading_Listing_Service(); - - /* Act. */ - $result = $service->get_lesson_progress_items( - $this->get_default_args( [ 'post_id' => $lesson_id ] ) - ); - - /* Assert. */ - $this->assertSame( 0, $result['total_count'], 'Completed lesson with quiz but no answers should be excluded.' ); - $this->assertEmpty( $result['items'], 'No items should be returned.' ); - } - public function testGetLessonProgressItems_WithStatusFilter_FiltersResults(): void { /* Arrange. */ $user1 = $this->sensei_factory->user->create(); diff --git a/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php b/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php index 1e45d72d97..47d4b6ef49 100644 --- a/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php +++ b/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php @@ -376,69 +376,4 @@ public function testCountStatuses_LessonWithQuizButNoAnswers_IncludedByDefault() $this->assertSame( 1, $result['complete'], 'Completed lesson with quiz but no answers should be included by default.' ); } - public function testCountStatuses_LessonWithQuizButNoAnswers_ExcludedWhenFlagSet(): void { - /* Arrange. */ - global $wpdb; - - $user_id = $this->sensei_factory->user->create(); - $course_id = $this->sensei_factory->course->create(); - $lesson_id = $this->sensei_factory->lesson->create( - [ 'meta_input' => [ '_lesson_course' => $course_id ] ] - ); - $quiz_id = $this->sensei_factory->quiz->create( - [ - 'post_parent' => $lesson_id, - 'meta_input' => [ '_quiz_lesson' => $lesson_id ], - ] - ); - update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); - - \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'complete' ); - - $service = new Comments_Based_Progress_Aggregation_Service( $wpdb ); - - /* Act. */ - $result = $service->count_statuses( - [ - 'type' => 'lesson', - 'exclude_unsubmitted_quiz_completions' => true, - ] - ); - - /* Assert. */ - $this->assertArrayNotHasKey( 'complete', $result, 'Completed lesson with quiz but no answers should be excluded when flag is set.' ); - } - - public function testCountStatuses_InProgressWithQuizButNoAnswers_NotExcludedWhenFlagSet(): void { - /* Arrange. */ - global $wpdb; - - $user_id = $this->sensei_factory->user->create(); - $course_id = $this->sensei_factory->course->create(); - $lesson_id = $this->sensei_factory->lesson->create( - [ 'meta_input' => [ '_lesson_course' => $course_id ] ] - ); - $quiz_id = $this->sensei_factory->quiz->create( - [ - 'post_parent' => $lesson_id, - 'meta_input' => [ '_quiz_lesson' => $lesson_id ], - ] - ); - update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); - - \Sensei_Utils::update_lesson_status( $user_id, $lesson_id, 'in-progress' ); - - $service = new Comments_Based_Progress_Aggregation_Service( $wpdb ); - - /* Act. */ - $result = $service->count_statuses( - [ - 'type' => 'lesson', - 'exclude_unsubmitted_quiz_completions' => true, - ] - ); - - /* Assert. */ - $this->assertSame( 1, $result['in-progress'], 'In-progress lesson should not be excluded even when flag is set.' ); - } } diff --git a/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php b/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php index cb401e5066..6d167d2511 100644 --- a/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php +++ b/tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php @@ -154,38 +154,6 @@ public function testGetLessonProgressItems_WithQuizStatus_UsesCoalescedStatus(): $this->assertSame( 90.0, $result['items'][0]->grade, 'Expected grade from quiz submission.' ); } - public function testGetLessonProgressItems_WithQuizProgressButNoSubmission_ExcludedFromResults(): void { - /* Arrange. */ - global $wpdb; - $user_id = $this->sensei_factory->user->create(); - $course_id = $this->sensei_factory->course->create(); - $lesson_id = $this->sensei_factory->lesson->create( - [ 'meta_input' => [ '_lesson_course' => $course_id ] ] - ); - $quiz_id = $this->sensei_factory->quiz->create( - [ - 'post_parent' => $lesson_id, - 'meta_input' => [ '_quiz_lesson' => $lesson_id ], - ] - ); - update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); - - // Quiz progress exists but no quiz submission — nothing to grade. - $this->insert_progress( $lesson_id, $user_id, 'lesson', 'complete', $course_id ); - $this->insert_progress( $quiz_id, $user_id, 'quiz', 'passed', $lesson_id ); - - $service = new Tables_Based_Grading_Listing_Service( $wpdb ); - - /* Act. */ - $result = $service->get_lesson_progress_items( - $this->get_default_args( [ 'post_id' => $lesson_id ] ) - ); - - /* Assert. */ - $this->assertSame( 0, $result['total_count'], 'Completed lesson with quiz but no submission should be excluded.' ); - $this->assertEmpty( $result['items'], 'No items should be returned.' ); - } - public function testGetLessonProgressItems_WithPostInFilter_ReturnsMatchingLessons(): void { /* Arrange. */ global $wpdb; diff --git a/tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php b/tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php index 5367160f10..08abaa26be 100644 --- a/tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php +++ b/tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php @@ -616,7 +616,7 @@ public function testCountStatuses_WithIncludeStatusesOverride_KeepsExcludedUsers $this->assertSame( 1, $result['ungraded'], 'Excluded user with override status should still be counted.' ); } - public function testCountStatuses_LessonWithQuizButNoSubmission_IncludedByDefault(): void { + public function testCountStatuses_LessonWithQuizButNoSubmission_UsesQuizStatus(): void { /* Arrange. */ global $wpdb; @@ -648,45 +648,8 @@ public function testCountStatuses_LessonWithQuizButNoSubmission_IncludedByDefaul ); /* Assert. */ - $this->assertSame( 1, $result['complete'], 'Completed lesson with quiz but no submission should be included by default.' ); - $this->assertArrayNotHasKey( 'passed', $result, 'Quiz progress without submission should not count as graded.' ); - } - - public function testCountStatuses_LessonWithQuizButNoSubmission_ExcludedWhenFlagSet(): void { - /* Arrange. */ - global $wpdb; - - $user_id = $this->sensei_factory->user->create(); - $course_id = $this->sensei_factory->course->create(); - $lesson_id = $this->sensei_factory->lesson->create( - [ 'meta_input' => [ '_lesson_course' => $course_id ] ] - ); - $quiz_id = $this->sensei_factory->quiz->create( - [ - 'post_parent' => $lesson_id, - 'meta_input' => [ '_quiz_lesson' => $lesson_id ], - ] - ); - update_post_meta( $lesson_id, '_lesson_quiz', $quiz_id ); - update_post_meta( $lesson_id, '_quiz_has_questions', 1 ); - - // Quiz progress exists but no quiz submission (e.g. migration phantom or lost data). - $this->insert_progress( $lesson_id, $user_id, 'lesson', 'complete', $course_id ); - $this->insert_progress( $quiz_id, $user_id, 'quiz', 'passed', $lesson_id ); - - $service = new Tables_Based_Progress_Aggregation_Service( $wpdb ); - - /* Act. */ - $result = $service->count_statuses( - [ - 'type' => 'lesson', - 'exclude_unsubmitted_quiz_completions' => true, - ] - ); - - /* Assert. */ - $this->assertArrayNotHasKey( 'complete', $result, 'Completed lesson with quiz but no submission should be excluded when flag is set.' ); - $this->assertArrayNotHasKey( 'passed', $result, 'Quiz progress without submission should not count as graded.' ); + $this->assertSame( 1, $result['passed'], 'Quiz progress status should take precedence over lesson status.' ); + $this->assertArrayNotHasKey( 'complete', $result, 'Lesson status should not appear when quiz progress exists.' ); } public function testCountStatuses_CourseType_ReturnsStatusCounts(): void { From 7b5562cc0c0b2698f0bddc53a1847148146c6aad Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 09:51:35 -0400 Subject: [PATCH 3/7] Fix lint: remove orphaned docblocks and trailing blank lines Co-Authored-By: Claude Opus 4.6 (1M context) --- ...comments-based-progress-aggregation-service.php | 14 -------------- ...s-tables-based-progress-aggregation-service.php | 13 ------------- ...comments-based-progress-aggregation-service.php | 1 - 3 files changed, 28 deletions(-) diff --git a/includes/internal/services/class-comments-based-progress-aggregation-service.php b/includes/internal/services/class-comments-based-progress-aggregation-service.php index c97b2f2dd6..b88645d12b 100644 --- a/includes/internal/services/class-comments-based-progress-aggregation-service.php +++ b/includes/internal/services/class-comments-based-progress-aggregation-service.php @@ -238,18 +238,4 @@ private function build_user_exclusion_clause( array $args ): string { return " AND $exclusion_sql"; } - - /** - * Build SQL clause for excluding completed lessons with no quiz submission. - * - * When enabled, excludes lessons where a quiz exists but the student has - * no quiz answers — there is nothing to grade. This covers both 'complete' - * (never submitted) and orphaned 'passed'/'graded'/'failed' records with - * no answer data. - * - * @since $$next-version$$ - * - * @param array $args Query arguments. - * @return string SQL clause. - */ } diff --git a/includes/internal/services/class-tables-based-progress-aggregation-service.php b/includes/internal/services/class-tables-based-progress-aggregation-service.php index c9b0810ceb..824980ba25 100644 --- a/includes/internal/services/class-tables-based-progress-aggregation-service.php +++ b/includes/internal/services/class-tables-based-progress-aggregation-service.php @@ -302,17 +302,4 @@ private function build_user_filter_clause( array $args ): string { private function build_user_exclusion_clause( array $args, string $status_column = 'p.status' ): string { return Utils::build_user_exclusion_clause( $this->wpdb, $args, $status_column ); } - - /** - * Build SQL clause for excluding completed lessons with no quiz submission. - * - * When enabled, excludes lessons where a quiz exists but the student never - * submitted it and the lesson is already complete — there is nothing to grade. - * Used by the Grading page; the Reports page passes false to include all students. - * - * @since $$next-version$$ - * - * @param array $args Query arguments. - * @return string SQL clause. - */ } diff --git a/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php b/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php index 47d4b6ef49..a05a67a866 100644 --- a/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php +++ b/tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php @@ -375,5 +375,4 @@ public function testCountStatuses_LessonWithQuizButNoAnswers_IncludedByDefault() /* Assert. */ $this->assertSame( 1, $result['complete'], 'Completed lesson with quiz but no answers should be included by default.' ); } - } From cd8a184cf87095e8b701ad91dfb8ee8516233f61 Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 09:54:09 -0400 Subject: [PATCH 4/7] Address PR review: fix offset snap and cached count filter bypass - Fix offset correction in both listing services to use ceil-based last page calculation, avoiding an empty result when total_count is an exact multiple of per_page. - Skip cached status counts when sensei_grading_count_statuses or its deprecated alias has listeners, so third-party filters on $count_args are respected. Co-Authored-By: Claude Opus 4.6 (1M context) --- includes/class-sensei-grading-main.php | 7 +++++-- .../class-comments-based-grading-listing-service.php | 6 +++--- .../class-tables-based-grading-listing-service.php | 6 +++--- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/includes/class-sensei-grading-main.php b/includes/class-sensei-grading-main.php index 4f714419f6..0e0cba95f4 100755 --- a/includes/class-sensei-grading-main.php +++ b/includes/class-sensei-grading-main.php @@ -606,8 +606,11 @@ public function get_views() { // Use cached per-status counts from prepare_items() when available, // avoiding a second full-table scan with the same JOINs. - $cached_counts = $this->grading_listing_service->get_status_counts(); - if ( null !== $cached_counts ) { + // Skip the cache if a plugin is filtering $count_args, since the + // cached counts would not reflect those modifications. + $has_count_filter = has_filter( 'sensei_grading_count_statuses' ) || has_filter( 'sensei_grading_count_statues' ); + $cached_counts = $this->grading_listing_service->get_status_counts(); + if ( null !== $cached_counts && ! $has_count_filter ) { // Ensure all expected statuses exist with 0 defaults, matching // the shape that count_statuses() returns. $defaults = array_fill_keys( diff --git a/includes/internal/services/class-comments-based-grading-listing-service.php b/includes/internal/services/class-comments-based-grading-listing-service.php index 4e823cc1a5..b58bafa1dd 100644 --- a/includes/internal/services/class-comments-based-grading-listing-service.php +++ b/includes/internal/services/class-comments-based-grading-listing-service.php @@ -48,9 +48,9 @@ public function get_lesson_progress_items( array $args ): array { // threw off the pagination), snap back to the last valid page. $offset = $args['offset'] ?? 0; $number = $args['number'] ?? 10; - if ( $number > 0 && $total_count < $offset ) { - $new_paged = floor( $total_count / $number ); - $args['offset'] = $new_paged * $number; + if ( $number > 0 && $total_count > 0 && $offset >= $total_count ) { + $last_page = max( 0, (int) ceil( $total_count / $number ) - 1 ); + $args['offset'] = $last_page * $number; } $statuses = \Sensei_Utils::sensei_check_for_activity( $args, true ); diff --git a/includes/internal/services/class-tables-based-grading-listing-service.php b/includes/internal/services/class-tables-based-grading-listing-service.php index da2f7fb78f..64d929b791 100644 --- a/includes/internal/services/class-tables-based-grading-listing-service.php +++ b/includes/internal/services/class-tables-based-grading-listing-service.php @@ -103,9 +103,9 @@ public function get_lesson_progress_items( array $args ): array { // threw off the pagination), snap back to the last valid page. $offset = $args['offset'] ?? 0; $number = $args['number'] ?? 10; - if ( $number > 0 && $total_count < $offset ) { - $new_paged = floor( $total_count / $number ); - $offset = $new_paged * $number; + if ( $number > 0 && $total_count > 0 && $offset >= $total_count ) { + $last_page = max( 0, ceil( $total_count / $number ) - 1 ); + $offset = (int) ( $last_page * $number ); } // Append ordering and pagination to the base query for the items fetch. From a6ccdfa8d22e610884d72ff47b9623c772047327 Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 10:14:27 -0400 Subject: [PATCH 5/7] Address PR review findings: tests, error logging, and stale docblock - Add factory tests for create_grading_listing_service (both HPPS on/off) to ensure correct service type is returned. - Add Utils::log_query_error() and apply to all wpdb query call sites so database failures are logged instead of silently returning empty. - Fix stale sensei_count_statuses_args filter docblock that still referenced "comment query" and only "post_in". Co-Authored-By: Claude Opus 4.6 (1M context) --- includes/class-sensei-grading.php | 5 ++-- ...nts-based-progress-aggregation-service.php | 2 ++ ...s-tables-based-grading-listing-service.php | 2 ++ ...les-based-progress-aggregation-service.php | 3 ++ includes/internal/services/class-utils.php | 20 ++++++++++++- ...t-class-progress-query-service-factory.php | 28 +++++++++++++++++++ 6 files changed, 57 insertions(+), 3 deletions(-) diff --git a/includes/class-sensei-grading.php b/includes/class-sensei-grading.php index 4944115ebf..c38fd5958c 100755 --- a/includes/class-sensei-grading.php +++ b/includes/class-sensei-grading.php @@ -555,9 +555,10 @@ public function get_stati( $type ) { */ public function count_statuses( $args = array() ) { /** - * Filter fires inside Sensei_Grading::count_statuses + * Filter the arguments used to count progress statuses. * - * Alter the post_in array to determine which posts the comment query should be limited to. + * Alter the query arguments (post restrictions, user exclusions) used + * to count progress statuses on the Grading page. * * @since 1.8.0 * diff --git a/includes/internal/services/class-comments-based-progress-aggregation-service.php b/includes/internal/services/class-comments-based-progress-aggregation-service.php index b88645d12b..e7ebe51b94 100644 --- a/includes/internal/services/class-comments-based-progress-aggregation-service.php +++ b/includes/internal/services/class-comments-based-progress-aggregation-service.php @@ -87,6 +87,7 @@ public function count_statuses( array $args ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared in advance. Caching handled by callers. $results = (array) $wpdb->get_results( $query, ARRAY_A ); + Utils::log_query_error( $wpdb, 'Comments-based status counts' ); $counts = []; foreach ( $results as $row ) { @@ -138,6 +139,7 @@ public function get_lesson_totals( array $lesson_ids ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared in advance. Caching handled by callers. $row = $wpdb->get_row( $query ); + Utils::log_query_error( $wpdb, 'Comments-based lesson totals' ); if ( ! $row ) { return $defaults; diff --git a/includes/internal/services/class-tables-based-grading-listing-service.php b/includes/internal/services/class-tables-based-grading-listing-service.php index 64d929b791..b036f723eb 100644 --- a/includes/internal/services/class-tables-based-grading-listing-service.php +++ b/includes/internal/services/class-tables-based-grading-listing-service.php @@ -80,6 +80,7 @@ public function get_lesson_progress_items( array $args ): array { $status_count_query = "SELECT effective_status, COUNT(*) AS total FROM ( $count_base_query ) AS counted GROUP BY effective_status"; // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared via build_base_query. Caching handled by callers. $status_rows = (array) $wpdb->get_results( $status_count_query, ARRAY_A ); + Utils::log_query_error( $wpdb, 'Grading listing status counts' ); $this->status_counts = []; foreach ( $status_rows as $row ) { @@ -115,6 +116,7 @@ public function get_lesson_progress_items( array $args ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared via build_base_query. Caching handled by callers. $rows = (array) $wpdb->get_results( $items_query ); + Utils::log_query_error( $wpdb, 'Grading listing items' ); $items = []; foreach ( $rows as $row ) { diff --git a/includes/internal/services/class-tables-based-progress-aggregation-service.php b/includes/internal/services/class-tables-based-progress-aggregation-service.php index 824980ba25..dc752db9a4 100644 --- a/includes/internal/services/class-tables-based-progress-aggregation-service.php +++ b/includes/internal/services/class-tables-based-progress-aggregation-service.php @@ -144,6 +144,7 @@ public function get_lesson_totals( array $lesson_ids ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared in advance. Caching handled by callers. $row = $wpdb->get_row( $query ); + Utils::log_query_error( $wpdb, 'Tables-based lesson totals' ); if ( ! $row ) { return $defaults; @@ -197,6 +198,7 @@ private function count_lesson_statuses_with_quiz( array $args ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared in advance. Caching handled by callers. $results = (array) $wpdb->get_results( $query, ARRAY_A ); + Utils::log_query_error( $wpdb, 'Tables-based lesson status counts' ); $counts = []; foreach ( $results as $row ) { @@ -231,6 +233,7 @@ private function count_course_statuses( array $args ): array { // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- SQL prepared in advance. Caching handled by callers. $results = (array) $wpdb->get_results( $query, ARRAY_A ); + Utils::log_query_error( $wpdb, 'Tables-based course status counts' ); $counts = []; foreach ( $results as $row ) { diff --git a/includes/internal/services/class-utils.php b/includes/internal/services/class-utils.php index 78e8c04233..09c608fec1 100644 --- a/includes/internal/services/class-utils.php +++ b/includes/internal/services/class-utils.php @@ -74,6 +74,21 @@ public static function build_user_exclusion_clause( \wpdb $wpdb, array $args, st return $wpdb->prepare( " AND p.user_id NOT IN ( $id_placeholders )", $excluded_user_ids ); } + /** + * Log a database query error if one occurred. + * + * @since $$next-version$$ + * + * @param \wpdb $wpdb WordPress database object. + * @param string $context Description of the query for debugging. + */ + public static function log_query_error( \wpdb $wpdb, string $context ): void { + if ( ! empty( $wpdb->last_error ) ) { + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- Intentional debug logging for query failures. + error_log( 'Sensei: ' . $context . ' query failed: ' . $wpdb->last_error ); + } + } + /** * Get user IDs whose login matches any of the given prefixes. * @@ -101,6 +116,9 @@ private static function get_user_ids_by_login_prefixes( \wpdb $wpdb, array $pref $where = implode( ' OR ', $like_clauses ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Dynamic WHERE built from prepared clauses. Caching handled by callers. - return array_map( 'intval', $wpdb->get_col( "SELECT ID FROM {$wpdb->users} WHERE $where" ) ); + $result = $wpdb->get_col( "SELECT ID FROM {$wpdb->users} WHERE $where" ); + self::log_query_error( $wpdb, 'User ID lookup by login prefix' ); + + return array_map( 'intval', $result ); } } diff --git a/tests/unit-tests/internal/services/test-class-progress-query-service-factory.php b/tests/unit-tests/internal/services/test-class-progress-query-service-factory.php index 8c4a5a9cd4..7bd107fce9 100644 --- a/tests/unit-tests/internal/services/test-class-progress-query-service-factory.php +++ b/tests/unit-tests/internal/services/test-class-progress-query-service-factory.php @@ -2,9 +2,11 @@ namespace SenseiTest\Internal\Services; +use Sensei\Internal\Services\Comments_Based_Grading_Listing_Service; use Sensei\Internal\Services\Comments_Based_Progress_Aggregation_Service; use Sensei\Internal\Services\Comments_Based_Progress_Clauses_Service; use Sensei\Internal\Services\Progress_Query_Service_Factory; +use Sensei\Internal\Services\Tables_Based_Grading_Listing_Service; use Sensei\Internal\Services\Tables_Based_Progress_Aggregation_Service; use Sensei\Internal\Services\Tables_Based_Progress_Clauses_Service; @@ -66,4 +68,30 @@ public function testCreateAggregationService_WhenHppsEnabled_ReturnsTablesBased( /* Assert. */ $this->assertInstanceOf( Tables_Based_Progress_Aggregation_Service::class, $service ); } + + public function testCreateGradingListingService_WhenHppsDisabled_ReturnsCommentsBased(): void { + /* Arrange. */ + \Sensei()->settings->settings['experimental_progress_storage'] = false; + \Sensei()->settings->settings['experimental_progress_storage_repository'] = 'comments'; + $factory = new Progress_Query_Service_Factory(); + + /* Act. */ + $service = $factory->create_grading_listing_service(); + + /* Assert. */ + $this->assertInstanceOf( Comments_Based_Grading_Listing_Service::class, $service ); + } + + public function testCreateGradingListingService_WhenHppsEnabled_ReturnsTablesBased(): void { + /* Arrange. */ + \Sensei()->settings->settings['experimental_progress_storage'] = true; + \Sensei()->settings->settings['experimental_progress_storage_repository'] = 'custom_tables'; + $factory = new Progress_Query_Service_Factory(); + + /* Act. */ + $service = $factory->create_grading_listing_service(); + + /* Assert. */ + $this->assertInstanceOf( Tables_Based_Grading_Listing_Service::class, $service ); + } } From c60ee04d620cad770f0072fb95cd0f21e38f2cd4 Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 10:16:05 -0400 Subject: [PATCH 6/7] Fix minor comment inaccuracies from PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - "completion date" → "activity date" in clauses service (uses updated_at) - Clarify lesson_completed_count docblock with specific status list - Add comment explaining why quiz progress join has no submission guard Co-Authored-By: Claude Opus 4.6 (1M context) --- .../services/class-progress-aggregation-service-interface.php | 2 +- .../services/class-tables-based-grading-listing-service.php | 3 +++ .../services/class-tables-based-progress-clauses-service.php | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/includes/internal/services/class-progress-aggregation-service-interface.php b/includes/internal/services/class-progress-aggregation-service-interface.php index 4259f8cd57..ce6c503b5a 100644 --- a/includes/internal/services/class-progress-aggregation-service-interface.php +++ b/includes/internal/services/class-progress-aggregation-service-interface.php @@ -51,7 +51,7 @@ public function count_statuses( array $args ): array; * @return array { * @type int $unique_student_count Number of distinct students. * @type int $lesson_start_count Number of lesson starts. - * @type int $lesson_completed_count Number of completed lessons (all non-in-progress statuses). + * @type int $lesson_completed_count Number of lessons with a finished status (complete, graded, passed, failed, or ungraded). * @type int $days_to_complete_count Number of lessons with a valid completion date. * @type int $days_to_complete_sum Sum of days to complete. * } diff --git a/includes/internal/services/class-tables-based-grading-listing-service.php b/includes/internal/services/class-tables-based-grading-listing-service.php index b036f723eb..3e87ce2b1e 100644 --- a/includes/internal/services/class-tables-based-grading-listing-service.php +++ b/includes/internal/services/class-tables-based-grading-listing-service.php @@ -153,6 +153,9 @@ private function build_base_query( string $table, string $submissions_table, arr $query .= " LEFT JOIN {$wpdb->postmeta} pm ON pm.post_id = p.post_id AND pm.meta_key = '_lesson_quiz' AND pm.meta_value > 0"; // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. $query .= " LEFT JOIN {$submissions_table} qs ON qs.quiz_id = pm.meta_value AND qs.user_id = p.user_id"; + // Quiz progress is joined without requiring a submission to exist, + // so that the effective_status reflects the quiz result even when + // the quiz_submissions row is missing (e.g. migrated data). // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. $query .= " LEFT JOIN {$table} q ON q.post_id = pm.meta_value AND q.user_id = p.user_id AND q.type = 'quiz'"; $query .= " WHERE p.type = 'lesson'"; diff --git a/includes/internal/services/class-tables-based-progress-clauses-service.php b/includes/internal/services/class-tables-based-progress-clauses-service.php index 2fdad826e1..0cfcaa018a 100644 --- a/includes/internal/services/class-tables-based-progress-clauses-service.php +++ b/includes/internal/services/class-tables-based-progress-clauses-service.php @@ -81,7 +81,7 @@ public function add_last_activity_to_courses_clauses( array $clauses ): array { AND p.status = 'complete' GROUP BY p.post_id"; - // Map lessons to courses via postmeta, then take the most recent completion date across all lessons per course. + // Map lessons to courses via postmeta, then take the most recent activity date across all lessons per course. $course_query = "SELECT pm.meta_value AS course_id, MAX(lq.last_activity_date) AS last_activity_date FROM {$wpdb->postmeta} pm JOIN ({$lessons_query}) lq ON lq.lesson_id = pm.post_id From 8a216904a36558de97072f1881f981b5b47ffbc8 Mon Sep 17 00:00:00 2001 From: Donna Peplinskie Date: Thu, 26 Mar 2026 10:32:55 -0400 Subject: [PATCH 7/7] Cast get_col result and wrap test filter cleanup in try/finally - Cast $wpdb->get_col() to (array) to prevent TypeError on PHP 8+ if the query returns null/false. - Wrap test assertions in try/finally so the sensei_count_statuses_args filter is always cleaned up, even if an assertion fails. Co-Authored-By: Claude Opus 4.6 (1M context) --- includes/internal/services/class-utils.php | 2 +- tests/unit-tests/test-class-grading.php | 27 +++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/includes/internal/services/class-utils.php b/includes/internal/services/class-utils.php index 09c608fec1..43458c1670 100644 --- a/includes/internal/services/class-utils.php +++ b/includes/internal/services/class-utils.php @@ -116,7 +116,7 @@ private static function get_user_ids_by_login_prefixes( \wpdb $wpdb, array $pref $where = implode( ' OR ', $like_clauses ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Dynamic WHERE built from prepared clauses. Caching handled by callers. - $result = $wpdb->get_col( "SELECT ID FROM {$wpdb->users} WHERE $where" ); + $result = (array) $wpdb->get_col( "SELECT ID FROM {$wpdb->users} WHERE $where" ); self::log_query_error( $wpdb, 'User ID lookup by login prefix' ); return array_map( 'intval', $result ); diff --git a/tests/unit-tests/test-class-grading.php b/tests/unit-tests/test-class-grading.php index 8cd91df13b..b986c7f414 100644 --- a/tests/unit-tests/test-class-grading.php +++ b/tests/unit-tests/test-class-grading.php @@ -73,19 +73,20 @@ public function testPrepareItems_TablesBasedWithCountStatusesArgsRestriction_Res }; add_filter( 'sensei_count_statuses_args', $restrict_filter ); - $this->login_as_admin(); - $service = new \Sensei\Internal\Services\Tables_Based_Grading_Listing_Service( $wpdb ); - $grading_main = new Sensei_Grading_Main( [ 'view' => 'all' ], $service ); - - /* Act. */ - $grading_main->prepare_items(); - - /* Assert. */ - $this->assertCount( 1, $grading_main->items, 'Listing should only show items for the allowed lesson.' ); - $this->assertSame( $lesson_ids[0], $grading_main->items[0]->lesson_id, 'Listing item should be for the restricted lesson.' ); - - /* Clean up. */ - remove_filter( 'sensei_count_statuses_args', $restrict_filter ); + try { + $this->login_as_admin(); + $service = new \Sensei\Internal\Services\Tables_Based_Grading_Listing_Service( $wpdb ); + $grading_main = new Sensei_Grading_Main( [ 'view' => 'all' ], $service ); + + /* Act. */ + $grading_main->prepare_items(); + + /* Assert. */ + $this->assertCount( 1, $grading_main->items, 'Listing should only show items for the allowed lesson.' ); + $this->assertSame( $lesson_ids[0], $grading_main->items[0]->lesson_id, 'Listing item should be for the restricted lesson.' ); + } finally { + remove_filter( 'sensei_count_statuses_args', $restrict_filter ); + } } /**