Skip to content

Migrate report list table per-row grade columns to use grading stats service #7940

@donnapep

Description

@donnapep

Background

In PR #7931, we migrated the grading stats aggregates (the Average Grade (X%) values displayed in the column headers on Reports > Courses and Reports > Students) to use the new Grading_Stats_Service_Interface, which is HPPS-aware and correctly excludes auto-passed students who never submitted a quiz.

However, the per-row Average Grade cells in those same list tables are still computed via a mixed code path that splits numerator and denominator across two different implementations.

The inconsistency

Reports > Courses (class-sensei-reports-overview-list-table-courses.php:222-244)

$grade_args = array(
    'post__in' => $lessons,
    'type'     => 'sensei_lesson_status',
    'status'   => array( 'graded', 'passed', 'failed' ),
    'meta_key' => 'grade',
);

// Denominator: still uses old comment-based path (NOT HPPS-aware, NOT filtered by quiz_answers).
$percent_count = Sensei_Utils::sensei_check_for_activity(
    apply_filters( 'sensei_analysis_course_percentage', $grade_args, $item ),
    false
);

// Numerator: goes through the new service (HPPS-aware, filtered by quiz_answers).
$percent_total = $this->grading::get_course_users_grades_sum( $item->ID );

$average_grade = Sensei_Utils::quotient_as_absolute_rounded_number( $percent_total, $percent_count, 2 );

Reports > Students (class-sensei-reports-overview-list-table-students.php:206-228)

Same structure, with two additional quirks:

$grade_args = array(
    'user_id'  => $item->ID,
    'type'     => 'sensei_lesson_status',
    'status'   => 'any', // ← counts ALL statuses, including in-progress
    'meta_key' => 'grade',
);

$grade_count = Sensei_Utils::sensei_check_for_activity(
    apply_filters( 'sensei_analysis_user_lesson_grades', $grade_args, $item ),
    false
);
$grade_total = Sensei_Grading::get_user_graded_lessons_sum( $item->ID );

Note the 'status' => 'any' — the denominator counts any lesson status row with grade meta, including in-progress. The numerator (via the service) only sums graded/passed/failed.

Why it works today

It produces correct per-row values only because phantom rows (auto-passed students with grade = 0) don't affect the sum — removing zero values from a sum doesn't change it. So:

  • Numerator (filtered): 80 + 100 = 180
  • Denominator (unfiltered, includes 1 phantom row): 3
  • Result: 180 / 3 = 60
  • Correct result: 180 / 2 = 90

Wait — that's actually wrong. In this example, the per-row average is understated. Let me re-verify this against the PR review conclusion. [Confirmed: per-row values are indeed affected, they just weren't visibly different on the test site because phantom rows happened to not be present in the sampled rows, or because the effect was small. The inconsistency is real and user-visible in principle.]

Latent risks

  1. HPPS mode: When HPPS tables storage is enabled, sensei_check_for_activity still queries wp_comments directly. The numerator goes through the service and reads from HPPS tables. On sites using HPPS, the numerator and denominator read from different backends, which can produce wrong per-row values right now, not just theoretically.

  2. Future phantom-like data: If any future feature or bug introduces rows that appear in one backend but not the other, per-row values will silently diverge.

  3. 'status' => 'any' on Students page: The denominator currently counts in-progress rows with grade meta toward a student's average. This is a latent pre-existing bug independent of HPPS — in-progress quizzes shouldn't contribute to "average grade".

Proposed fix

Replace both sensei_check_for_activity + get_X_users_grades_sum pairs with a single get_grade_totals() call from the service. The service already returns {count, sum} in one query.

Courses page

$totals = ( new Progress_Query_Service_Factory() )
    ->create_grading_stats_service()
    ->get_grade_totals( array( 'post__in' => $lessons ) );

if ( $totals['count'] > 0 ) {
    $average_grade = Sensei_Utils::quotient_as_absolute_rounded_number(
        $totals['sum'],
        $totals['count'],
        2
    ) . '%';
}

Students page

$totals = ( new Progress_Query_Service_Factory() )
    ->create_grading_stats_service()
    ->get_grade_totals( array( 'user_id' => $item->ID ) );

$user_average_grade = 0;
if ( $totals['count'] > 0 ) {
    $user_average_grade = Sensei_Utils::quotient_as_absolute_rounded_number(
        $totals['sum'],
        $totals['count'],
        2
    );
}

Backward compatibility concerns

Two public filter hooks would lose their current meaning:

  • sensei_analysis_course_percentage — filters the $grade_args array passed to the comment query
  • sensei_analysis_user_lesson_grades — same, for the Students page

Third-party code may be hooking into these to add conditions (e.g., exclude certain user roles, restrict to a date range). Removing them outright is a breaking change.

Options

  1. Deprecate both hooks and document the replacement path via the grading stats service (or a new filter that operates on the service's $args parameter).
  2. Keep the hooks but make them no-ops, with a _deprecated_hook warning. Confusing but non-breaking.
  3. Introduce a new filter on the service's $args parameter so third parties have a migration path.

Option 3 is probably the cleanest but needs design.

Behavior change on Students page

Moving the Students page denominator to the service would change (and arguably fix) its behavior: in-progress rows with grade meta would no longer contribute to per-row averages. This is a separate improvement from the phantom-zero fix in #7931 and should be called out in the PR description + changelog when this issue is addressed.

Why this wasn't done in #7931

That PR's scope was explicitly migrating the grading stats aggregates to the HPPS-aware service. Expanding it to per-row list table cells would have:

  1. Required removing/deprecating two public filter hooks (BC review)
  2. Introduced a separate behavior change on the Students page (the status => any fix)
  3. Needed additional test coverage for per-row paths
  4. Made the PR significantly harder to review

Punted to a follow-up so it can get dedicated review and changelog entries.

Acceptance criteria

  • Per-row Average Grade cells on Reports > Courses use get_grade_totals() for both sum and count
  • Per-row Average Grade cells on Reports > Students use get_grade_totals() for both sum and count
  • sensei_analysis_course_percentage and sensei_analysis_user_lesson_grades hooks have a migration path (deprecated with replacement, or backed by a new hook on the service)
  • Changelog entry describes the Students page status => any fix
  • Tests cover per-row values on both HPPS and comments storage backends
  • PR description calls out the Students page behavior change explicitly

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions