Integrate HPPS in Reports > Lessons backend#7920
Integrate HPPS in Reports > Lessons backend#7920donnapep wants to merge 4 commits intoadd/hpps-full-migrationfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the HPPS progress-query service abstraction by migrating remaining Reports (Lessons) and Grading backend data access to storage-agnostic services, so reports/grading behave consistently under both comments-based and tables-based (HPPS) progress storage.
Changes:
- Extends the progress clauses + aggregation interfaces to support lesson last-activity/days-to-completion fields and lesson-header totals.
- Introduces a grading listing service (comments-based + tables-based) and a
Grading_Itemvalue object to unify grading row data across storage backends. - Refactors lesson reports and grading listing codepaths to use the new services and adds unit tests for the new service methods.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
includes/internal/services/class-progress-clauses-service-interface.php |
Adds lesson clause extension points to the clauses service interface. |
includes/internal/services/class-comments-based-progress-clauses-service.php |
Implements lesson last-activity/days-to-completion clauses for comments storage. |
includes/internal/services/class-tables-based-progress-clauses-service.php |
Implements lesson last-activity/days-to-completion clauses for HPPS tables storage. |
includes/internal/services/class-progress-aggregation-service-interface.php |
Adds get_lesson_totals() to support report header aggregation. |
includes/internal/services/class-comments-based-progress-aggregation-service.php |
Implements lesson totals aggregation for comments storage. |
includes/internal/services/class-tables-based-progress-aggregation-service.php |
Implements lesson totals aggregation for tables storage. |
includes/internal/services/class-grading-listing-service-interface.php |
New interface for grading listing retrieval. |
includes/internal/services/class-comments-based-grading-listing-service.php |
Comments-based grading listing implementation returning Grading_Item objects. |
includes/internal/services/class-tables-based-grading-listing-service.php |
Tables-based grading listing implementation with SQL-based pagination + coalesced statuses. |
includes/internal/services/class-grading-item.php |
New value object for grading rows + legacy WP_Comment property compatibility. |
includes/internal/services/class-progress-query-service-factory.php |
Adds factory creation for the grading listing service based on HPPS settings. |
includes/reports/overview/data-provider/class-sensei-reports-overview-data-provider-lessons.php |
Delegates lesson clause building to the clauses service. |
includes/reports/overview/list-table/class-sensei-reports-overview-list-table-lessons.php |
Uses aggregation service for per-row counts and header totals; deprecates legacy lesson filters. |
includes/class-sensei-grading-main.php |
Refactors grading list table to fetch/paginate items via the grading listing service. |
tests/unit-tests/internal/services/test-class-*-progress-clauses-service.php |
Adds unit tests for the new lesson clauses methods. |
tests/unit-tests/internal/services/test-class-*-progress-aggregation-service.php |
Adds unit tests for get_lesson_totals() behavior. |
tests/unit-tests/internal/services/test-class-*-grading-listing-service.php |
Adds unit tests for new grading listing services. |
tests/unit-tests/internal/services/test-class-grading-item.php |
Adds unit tests for Grading_Item legacy access and constants. |
config/psalm/psalm-baseline.xml |
Updates Psalm baseline for the refactors/new types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php
Outdated
Show resolved
Hide resolved
includes/internal/services/class-tables-based-progress-aggregation-service.php
Show resolved
Hide resolved
includes/internal/services/class-tables-based-progress-clauses-service.php
Outdated
Show resolved
Hide resolved
includes/internal/services/class-tables-based-grading-listing-service.php
Outdated
Show resolved
Hide resolved
includes/internal/services/class-tables-based-grading-listing-service.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Migrates remaining Reports → Lessons and Grading backend queries onto the HPPS-aware query service layer, so progress/reporting behaves consistently across comments-based vs tables-based storage.
Changes:
- Extends progress clauses + aggregation service interfaces to support lesson report fields (last activity, days-to-complete) and lesson header totals aggregation.
- Introduces grading listing service (comments + tables implementations) and a
Grading_Itemvalue object to unify the grading UI inputs across storage backends. - Refactors lesson reports + grading list table to use the new services; adds tests and changelog entries, and deprecates removed report filter hooks.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-tests/internal/services/test-class-tables-based-progress-clauses-service.php | Adds unit coverage for lesson clauses fields on tables backend. |
| tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php | Adds unit coverage for get_lesson_totals() and completion-status parity on tables backend. |
| tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php | New unit tests for tables-based grading listing pagination/filters/status coalescing. |
| tests/unit-tests/internal/services/test-class-grading-item.php | New unit tests for Grading_Item constant and legacy property compatibility. |
| tests/unit-tests/internal/services/test-class-comments-based-progress-clauses-service.php | Adds unit coverage for lesson clauses fields on comments backend. |
| tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php | Adds unit coverage for get_lesson_totals() on comments backend. |
| tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php | New unit tests for comments-based grading listing behavior. |
| includes/reports/overview/list-table/class-sensei-reports-overview-list-table-lessons.php | Refactors lesson report row + header totals to use aggregation service; adds deprecated hook notices. |
| includes/reports/overview/data-provider/class-sensei-reports-overview-data-provider-lessons.php | Refactors lesson report query clause building to use clauses service. |
| includes/internal/services/class-tables-based-progress-clauses-service.php | Adds lesson last-activity and days-to-complete clauses for HPPS tables. |
| includes/internal/services/class-tables-based-progress-aggregation-service.php | Adds get_lesson_totals() aggregation query for HPPS tables. |
| includes/internal/services/class-tables-based-grading-listing-service.php | New HPPS tables implementation for grading listing queries (items + count + ordering). |
| includes/internal/services/class-progress-query-service-factory.php | Adds factory method for grading listing service selection by storage backend. |
| includes/internal/services/class-progress-clauses-service-interface.php | Extends clauses interface with lesson-specific clause helpers. |
| includes/internal/services/class-progress-aggregation-service-interface.php | Extends aggregation interface with get_lesson_totals(). |
| includes/internal/services/class-grading-listing-service-interface.php | New interface for grading listing services. |
| includes/internal/services/class-grading-item.php | New value object for grading rows + legacy WP_Comment property shims. |
| includes/internal/services/class-comments-based-progress-clauses-service.php | Adds lesson last-activity and days-to-complete clauses for comments backend. |
| includes/internal/services/class-comments-based-progress-aggregation-service.php | Adds get_lesson_totals() aggregation query for comments backend. |
| includes/internal/services/class-comments-based-grading-listing-service.php | New comments implementation for grading listing via sensei_check_for_activity(). |
| includes/class-sensei-grading-main.php | Refactors grading list table to use grading listing service + Grading_Item. |
| config/psalm/psalm-baseline.xml | Updates Psalm baseline for newly introduced/static-analysis findings. |
| changelog/fix-days-to-complete-ungraded-failed | Patch changelog entry for days-to-complete fix (exclude ungraded/failed). |
| changelog/add-hpps-reports-grading-migration | Minor changelog entry for HPPS integration work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php
Outdated
Show resolved
Hide resolved
tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php
Outdated
Show resolved
Hide resolved
includes/reports/overview/list-table/class-sensei-reports-overview-list-table-lessons.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Integrates HPPS-aware service abstractions into the remaining Lessons Reports and Grading backend queries so progress/grading data access routes through the correct implementation (comments vs custom tables).
Changes:
- Extend progress clauses/aggregation service interfaces (lesson last-activity, days-to-complete clauses; lesson header totals aggregation).
- Introduce grading listing service interface + implementations and a
Grading_Itemvalue object for unified listing rows. - Refactor Lessons report list table/data provider and Grading list table to use the new services; add/expand unit coverage and changelog entries.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-tests/test-class-grading.php | Adds regression test ensuring prepare_items() respects sensei_count_statuses_args restrictions for tables storage. |
| tests/unit-tests/internal/services/test-class-tables-based-progress-clauses-service.php | Adds tests for new lesson clauses (last activity, days-to-complete, TZ conversion). |
| tests/unit-tests/internal/services/test-class-tables-based-progress-aggregation-service.php | Adds tests for get_lesson_totals() and new “exclude unsubmitted quiz completions” behavior. |
| tests/unit-tests/internal/services/test-class-tables-based-grading-listing-service.php | New test suite covering HPPS grading listing behavior (filters, pagination, counts, exclusion rules). |
| tests/unit-tests/internal/services/test-class-grading-item.php | New tests for Grading_Item constants/legacy getters and Utils::get_utc_offset_string(). |
| tests/unit-tests/internal/services/test-class-comments-based-progress-clauses-service.php | Adds tests for new lesson clauses in comments backend. |
| tests/unit-tests/internal/services/test-class-comments-based-progress-aggregation-service.php | Adds tests for get_lesson_totals() and “exclude unsubmitted quiz completions” behavior. |
| tests/unit-tests/internal/services/test-class-comments-based-grading-listing-service.php | New tests for comments-based grading listing wrapper behavior and exclusion rules. |
| includes/reports/overview/list-table/class-sensei-reports-overview-list-table-lessons.php | Uses aggregation service for header totals and status counts; updates days-to-completion divisor; deprecates old lesson filters. |
| includes/reports/overview/data-provider/class-sensei-reports-overview-data-provider-lessons.php | Delegates last-activity and days-to-complete field clauses to the clauses service. |
| includes/internal/services/class-utils.php | Adds shared helpers (UTC offset formatting; user exclusion clause builder). |
| includes/internal/services/class-tables-based-progress-clauses-service.php | Implements new lesson clauses and converts course/lesson day diffs to local time for DATEDIFF. |
| includes/internal/services/class-tables-based-progress-aggregation-service.php | Adds get_lesson_totals(), improves quiz-status handling, adds optional exclusion of unsubmitted quiz completions, refactors user exclusion to Utils. |
| includes/internal/services/class-tables-based-grading-listing-service.php | New HPPS grading listing implementation with cached per-status counts and grading row projection to Grading_Item. |
| includes/internal/services/class-progress-query-service-factory.php | Adds factory method to create the correct grading listing service. |
| includes/internal/services/class-progress-clauses-service-interface.php | Extends interface with lesson last-activity and days-to-completion clause methods. |
| includes/internal/services/class-progress-aggregation-service-interface.php | Extends interface with get_lesson_totals() and new count args flag. |
| includes/internal/services/class-grading-listing-service-interface.php | New interface for grading listing queries + optional cached status counts. |
| includes/internal/services/class-grading-item.php | New value object for grading rows with deprecated legacy WP_Comment-style property access. |
| includes/internal/services/class-comments-based-progress-clauses-service.php | Implements new lesson clause methods for comments backend. |
| includes/internal/services/class-comments-based-progress-aggregation-service.php | Adds get_lesson_totals(), adds optional exclusion of unsubmitted quiz completions, adjusts post filtering precedence. |
| includes/internal/services/class-comments-based-grading-listing-service.php | New comments-based grading listing wrapper returning Grading_Item rows and excluding “no answers” rows. |
| includes/class-sensei-grading-main.php | Refactors grading listing to use listing service, unifies row rendering via Grading_Item, and optionally uses cached status counts. |
| config/psalm/psalm-baseline.xml | Updates Psalm baseline for new services/value object usage and new property fetch patterns. |
| changelog/fix-grading-teacher-restrictions | Changelog entry for teacher grading listing restriction fix. |
| changelog/fix-grading-tab-counts-lesson-filter | Changelog entry for grading tab counts updating correctly with lesson filter. |
| changelog/fix-grading-hide-lessons-without-quiz-answers | Changelog entry for hiding grading rows without quiz answers/submissions. |
| changelog/fix-days-to-complete-ungraded-failed | Changelog entry for days-to-completion excluding statuses without completion dates. |
| changelog/fix-course-last-activity-date | Changelog entry for course last activity using correct aggregation. |
| changelog/add-hpps-reports-grading-migration | Changelog entry for the internal HPPS integration work. |
| AGENTS.md | Adds guidance for running Psalm locally and pre-push lint expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
867f59c to
0a44259
Compare
Extend clauses service with lesson-specific clause builders. Extend aggregation service with count_statuses() and get_lesson_totals(). Add Grading_Item value object with legacy WP_Comment backward compatibility. Add Utils class with shared helpers. Include tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Delegate clause building in the lessons data provider to the clauses service. Use the aggregation service for lesson row counts and header totals. Deprecate sensei_analysis_lesson_learners and sensei_analysis_lesson_completions filter hooks. Separate days_to_complete_count from lesson_completed_count for correct averaging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0a44259 to
4984ead
Compare
|
@merkushin This one is built on top of #7915 (still need to review your feedback there). Thanks! |
mdawaffe
left a comment
There was a problem hiding this comment.
One naive question inline, but looks good otherwise.
|
|
||
| // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table names from wpdb prefix. | ||
| $query = "SELECT COALESCE( q.status, p.status ) AS effective_status, COUNT( * ) AS total FROM {$table} p"; | ||
| $query .= " INNER JOIN {$wpdb->posts} post ON post.ID = p.post_id AND post.post_status != 'trash'"; |
There was a problem hiding this comment.
This removes the trash condition, which looks intentional since testCountStatuses_TrashedLesson_ExcludedFromCounts is also removed. Why do we want to include trashed posts?
Summary
Continues the service abstraction pattern from #7915 by migrating the remaining Reports (Lessons) backend queries so that all progress data access goes through the appropriate service based on HPPS settings.
Changes
Progress_Clauses_Service_Interfacewithadd_last_activity_to_lessons_clauses()andadd_days_to_completion_to_lessons_clauses()for both comments-based and tables-based implementationsProgress_Aggregation_Service_Interfacewithcount_statuses()andget_lesson_totals()for lesson report header aggregationGrading_Itemvalue object with__get()backward compatibility for legacyWP_Commentproperty namesGrading_Item::COMPLETED_STATUSES(all non-in-progress statuses) for lesson completion counts andGrading_Item::STATUSES_WITH_COMPLETION_DATE(excludes failed/ungraded) for days-to-complete calculationsUtilsclass withget_utc_offset_string()andbuild_user_exclusion_clause()shared helpersSensei_Reports_Overview_Data_Provider_Lessonsto delegate clause building to the clauses serviceSensei_Reports_Overview_List_Table_Lessonsto use the aggregation service for row counts and header totalssensei_analysis_lesson_learnersandsensei_analysis_lesson_completionsfilter hookslesson_completed_count(broad, includes failed/ungraded) fromdays_to_complete_count(narrow, only statuses with completion dates) inget_lesson_totals()so the Days to Completion average uses the correct divisorBug Fixes
Known discrepancies
Course Last Activity dates differ from trunk (bug fix)
Trunk's query for course Last Activity selected a non-aggregated
comment_date_gmtin aGROUP BYclause, which returns an arbitrary lesson's date rather than the most recent one. This branch replaces it withMAX()to correctly return the latest activity date across all lessons in the course. The dates on this branch are more accurate than trunk.Days to Completion differs from trunk for lessons with failed/ungraded students
Trunk includes
failedandungradedstatuses in both the days-to-complete sum and divisor. This branch excludes them because in HPPS tables, failed/ungraded students don't have acompleted_atdate, so including them would deflate the average (dividing by a larger count while those students contribute 0 days). For consistency, both the comments-based and tables-based paths use the same narrower set (STATUSES_WITH_COMPLETION_DATE: complete, graded, passed). This only affects lessons that have students with failed or ungraded quiz statuses — all other lessons match trunk exactly.Setup
After checking out this branch, run
composer dump-autoloadto register the new service classes with the autoloader.Test plan
Testing approach
There are two comparisons to make:
Regression check — On
trunkwith the "Store the progress of your students in separate tables" checkbox unchecked (default state), note the values. Then switch to this branch with comments-based storage selected. Results should match, except: Days to Completion may differ for lessons with failed/ungraded students (see known discrepancies above), and Course Last Activity dates may differ as trunk was showing an incorrect arbitrary date (now fixed withMAX()).Parity check — On this branch, compare comments-based storage to custom tables based storage. Results should be similar, with the known discrepancies noted above.
1. Reports > Lessons
🤖 Generated with Claude Code