Skip to content

Separate report rows from report metrics - #8249

Merged
donnapep merged 11 commits into
trunkfrom
update/hpps-report-rows-metrics
Sep 14, 2026
Merged

donnapep merged 11 commits into
trunkfrom
update/hpps-report-rows-metrics

Conversation

@donnapep

@donnapep donnapep commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Why

Reports_Listing_Service_Interface mixed two responsibilities: returning report rows and calculating lesson metrics. This change keeps report rows in Reports Listing while progress and grade metrics come from their dedicated services.

Proposed Changes

  • Move the existing lesson student and completion count methods from Reports Listing to Progress Aggregation.
  • Move the existing lesson average-grade method from Reports Listing to Grading Stats.
  • Inject Progress Aggregation and Grading Stats into the course report list table.
  • Preserve the comments backend's Sensei_Utils::sensei_check_for_activity() count implementation and the HPPS backend's existing SQL implementation.
  • Preserve the existing filters, arguments, displayed values, rounding, completion-status semantics, row ordering, and CSV output.
  • Remove lesson student count, completion count, and average grade methods from Reports Listing after moving their callers.
  • Keep the four Reports Listing row-returning methods unchanged.

Testing Instructions

  • Open Reports for a course with multiple lessons and confirm each lesson's Students, Completed, and Average Grade values match comments storage.
  • Enable HPPS with synchronization, switch reads to tables, and confirm the same lesson metrics and ordering.
  • Export the course lessons report and confirm the metric values match the on-screen rows.
  • Run targeted normal-mode PHPUnit: 151 tests, 304 assertions.
  • Run targeted HPPS-mode PHPUnit: 151 tests, 303 assertions, 1 expected comments-fixture skip.
  • Run PHP 8.2 Psalm diff analysis: no errors.
  • Run PHPCS on the changed PHP files and compare diagnostics with trunk: no new issues.
  • Run PHP syntax checks and git diff --check: clean.

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch - Backward-compatible bug fixes
  • Minor - Add or deprecate functionality in a backward-compatible manner
  • Major - Incompatible or breaking API changes

Type

  • Added - Adds new functionality
  • Changed - Changes existing functionality
  • Fixed - Fixes a bug
  • Deprecated - Marks functionality as deprecated
  • Removed - Removes functionality
  • Security - Security-related change
  • Development - Development or internal task

Message

Refactored course report lesson metrics into dedicated progress aggregation and grading statistics services.

Copilot AI lite review requested due to automatic review settings September 11, 2026 16:46
@donnapep donnapep added the No Changelog No change log entry needed label Sep 11, 2026
@donnapep donnapep added this to the 4.26.4 milestone Sep 11, 2026
@donnapep
donnapep force-pushed the update/hpps-report-rows-metrics branch from 711136e to 6869c3c Compare September 11, 2026 16:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Preserve translation and legacy filter semantics before approval, and strengthen bulk metric assertions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR separates report-row retrieval from lesson metric aggregation, adding bulk status and grade calculations for comments and HPPS storage.

Changes:

  • Adds bulk lesson status and average-grade service APIs and implementations.
  • Loads metrics once per displayed lesson page.
  • Removes migrated Reports Listing methods and updates tests.
File summaries
File Reviewed changes
tests/unit-tests/test-class-sensei-analysis-course-list-table.php Tests bulk metric loading; coverage should assert distinct per-lesson values, including tables-backed behavior (Nit, 2 votes).
tests/unit-tests/internal/services/test-class-tables-based-reports-listing-service.php Removes obsolete metric tests.
tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php Tests HPPS bulk status aggregation.
tests/unit-tests/internal/services/test-class-tables-based-grading-stats-service.php Tests HPPS bulk grade aggregation.
tests/unit-tests/internal/services/test-class-comments-based-reports-listing-service.php Removes obsolete metric tests.
tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php Tests comments-backed bulk status aggregation.
tests/unit-tests/internal/services/test-class-comments-based-grading-stats-service.php Tests comments-backed bulk grade aggregation.
includes/internal/services/class-tables-based-reports-listing-service.php Removes migrated metric methods while retaining row retrieval.
includes/internal/services/class-tables-based-progress-aggregation-service.php Implements HPPS bulk status aggregation.
includes/internal/services/class-tables-based-grading-stats-service.php Implements HPPS bulk grade aggregation.
includes/internal/services/class-reports-listing-service-interface.php Removes migrated metric contracts.
includes/internal/services/class-progress-aggregation-service-interface.php Adds bulk lesson status contract.
includes/internal/services/class-grading-stats-service-interface.php Adds bulk lesson grade contract.
includes/internal/services/class-comments-based-reports-listing-service.php Removes migrated metric methods while retaining row retrieval.
includes/internal/services/class-comments-based-progress-aggregation-service.php Implements comments-backed bulk status aggregation.
includes/internal/services/class-comments-based-grading-stats-service.php Implements comments-backed bulk grade aggregation.
includes/class-sensei-analysis-course-list-table.php Integrates bulk metrics. Moderate findings: preserve WPML ID translation/filter behavior (3 votes), preserve grade filter status and meta_key semantics (3 votes), and preserve HPPS learner-filter semantics (1 vote).
Review details

Suppressed comments (1)

includes/class-sensei-analysis-course-list-table.php:851

  • This fallback changes the HPPS behavior of the legacy sensei_analysis_lesson_learners filter. The removed Tables_Based_Reports_Listing_Service::get_lesson_student_count() filtered directly on the lesson row's p.status, while count_statuses() uses the quiz-effective COALESCE(q.status, p.status). A filter requesting complete or in-progress can therefore return a different count for lessons with quiz progress; preserve the backend-specific legacy semantics when handling filtered arguments.
	private function get_filtered_lesson_status_count( array $args ): int {
  • Files reviewed: 17/17 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread includes/class-sensei-analysis-course-list-table.php Outdated
Comment thread includes/class-sensei-analysis-course-list-table.php Outdated
Comment thread tests/unit-tests/test-class-sensei-analysis-course-list-table.php Outdated
@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

WordPress Playground Preview

The changes in this pull request can previewed and tested using a WordPress Playground instance.

Open WordPress Playground Preview

@donnapep
donnapep force-pushed the update/hpps-report-rows-metrics branch from 6869c3c to 898e534 Compare September 11, 2026 17:21
@donnapep donnapep self-assigned this Sep 11, 2026
Base automatically changed from update/hpps-storage-configuration to trunk September 14, 2026 12:56
@donnapep
donnapep force-pushed the update/hpps-report-rows-metrics branch from bb5a580 to 4c24c96 Compare September 14, 2026 14:40
@donnapep
donnapep force-pushed the update/hpps-report-rows-metrics branch from 4c24c96 to 0c9618c Compare September 14, 2026 15:44
@donnapep donnapep removed the No Changelog No change log entry needed label Sep 14, 2026
@donnapep
donnapep requested a lite review from Copilot September 14, 2026 20:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking issues were identified.

Review details
  • Files reviewed: 19/19 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@donnapep
donnapep merged commit d223ef7 into trunk Sep 14, 2026
24 checks passed
@donnapep
donnapep deleted the update/hpps-report-rows-metrics branch September 14, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Sponsor
SponsoredKunjungi sekarang
Promo