Add (type, post_id) index to sensei_lms_progress#7922
Draft
Add (type, post_id) index to sensei_lms_progress#7922
Conversation
…ata provider Extend Progress_Clauses_Service_Interface with add_last_activity_to_lessons_clauses() and add_days_to_completion_to_lessons_clauses() methods, mirroring the existing courses pattern. The comments-based implementation uses correlated subqueries on wp_comments, while the tables-based implementation queries sensei_lms_progress with quiz COALESCE logic for days-to-completion. Refactor Sensei_Reports_Overview_Data_Provider_Lessons to delegate to the clauses service via constructor injection instead of building SQL inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t table Extend Progress_Aggregation_Service_Interface with get_lesson_totals() for fetching aggregate lesson report header data. Replace the raw $wpdb query in get_totals_for_lesson_report_column_headers() and the sensei_check_for_activity() calls in get_row_data() with the aggregation service. Add Grading_Item::COMPLETED_STATUSES constant to centralize the set of statuses that count as completed, eliminating duplication across services and consumers. Add deprecation notices for removed sensei_analysis_lesson_learners and sensei_analysis_lesson_completions filter hooks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce Grading_Listing_Service_Interface with comments-based and tables-based implementations to replace direct sensei_check_for_activity() calls in the grading list table. The tables-based implementation queries sensei_lms_progress with quiz status COALESCE and grade retrieval via quiz submissions. Add Grading_Item value object with constructor and __get() magic method for backward compatibility with third-party code that accesses legacy WP_Comment property names through the sensei_grading_main_column_data filter. Add factory method create_grading_listing_service() to Progress_Query_Service_Factory. Refactor Sensei_Grading_Main to use the grading listing service for prepare_items() and Grading_Item properties in get_row_data(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add baseline entries for new grading listing service and aggregation service files. Update existing entries for modified grading-main and list-table-lessons. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Expand inline associative arrays to multi-line format, add missing short descriptions to docblock comments, extract get_default_args helper to reduce duplication, and fix inline closure formatting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Only let quiz status override lesson status when a quiz submission exists, matching the pattern used in count_lesson_statuses_with_quiz(). Without this check, quiz progress rows without submissions could produce phantom quiz-derived statuses. Add test verifying that quiz progress without a submission falls back to the lesson status. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The days-to-complete reporting metric was inconsistent between comments-based and tables-based (HPPS) storage. In comments-based storage, comment_date is always present (even for ungraded/failed statuses), so DATEDIFF always produced a value. In tables-based storage, completed_at is null for these statuses, producing NULL and skewing averages lower. Rather than adding a completion date to ungraded/failed quiz progress (which would introduce a model-level inconsistency), this aligns both systems by excluding statuses that lack a valid completion date from the DATEDIFF calculation entirely. The original decision to include ungraded/failed in completion calculations (PR #4897, specifically #4897 (comment)) was a side effect of comment_date always being present on WordPress comments — not an intentional design choice. From a teacher's perspective in reports, lessons with ungraded or failed quizzes are not truly "completed." Adds STATUSES_WITH_COMPLETION_DATE constant ('complete', 'graded', 'passed') for use in date calculations, separate from COMPLETED_STATUSES which continues to include all five statuses for completion counting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lete Align lesson_completed_count with days_to_complete_sum by using the same STATUSES_WITH_COMPLETION_DATE set for both. Previously, lesson_completed_count included ungraded/failed while days_to_complete_sum excluded them, producing incorrect averages when dividing days_to_complete_sum by lesson_completed_count. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No longer referenced in production code after aligning all lesson completion calculations to use STATUSES_WITH_COMPLETION_DATE. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Combine testGetLessonProgressItems_WithNoGrade_ReturnsNullGrade into testGetLessonProgressItems_WithLessonStatus_ReturnsGradingItems since they use identical setup. Add descriptive messages to all assertions to distinguish them in failure output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_Query The status filter and pagination tests were verifying WP_Comment_Query behavior, not our service logic. Our service passes these params straight through to sensei_check_for_activity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add tests to both aggregation services verifying: - Ungraded quiz status does not count as completed - Failed quiz status does not count as completed - Passed status includes correct days_to_complete_sum (exact value assertion using controlled start/completion dates) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ssertions Remove testGetLessonTotals_WithQuizStatus_UsesCoalesced — fully covered by WithPassedQuiz_IncludesDaysToComplete with stricter assertions. Replace assertGreaterThanOrEqual(1) with exact days_to_complete_sum assertions using controlled start/completion dates in both aggregation service test classes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tables-based tests: use fixed literal dates instead of mixing
gmdate() (UTC) with current_time('mysql') (site timezone).
Comments-based tests: use wp_date() instead of gmdate() so the
start date uses the same timezone as comment_date, preventing
off-by-one DATEDIFF results on non-UTC sites.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When grade_val is null (e.g., legacy data missing grade meta or HPPS submissions with NULL final_grade), the display showed just "%" instead of "N/A". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The HPPS tables store dates in UTC, but the UI expects local time (matching the comments-based behavior which uses comment_date). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lessons where a quiz exists but the student never submitted it and the lesson is already complete have nothing to grade. Filter these out from both the listing query and the status counts in the tables-based code. Note: this filter is only applied to the HPPS tables-based path. The comments-based path still includes these rows because filtering them would require per-row meta lookups that are too expensive for legacy code that will eventually be removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Change grade type from int to float in Grading_Item to match trunk behavior where grades like 60.98% are displayed with full precision. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When both course and lesson filters are active, count_args has both post__in (all course lessons) and post_id (specific lesson). The post__in check ran first, so post_id was ignored and the counts reflected the entire course instead of the filtered lesson. Fix by checking post_id before post__in in both comments-based and tables-based aggregation services. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Match the tables-based fix: use (float) instead of (int) so grades like 60.98% are not truncated to 60%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The "Completed" column should count all non-in-progress statuses (complete, graded, passed, failed, ungraded) to match trunk behavior, while "Days to Completion" should only divide by statuses that have a valid completion date (complete, graded, passed). Adds COMPLETED_STATUSES constant alongside STATUSES_WITH_COMPLETION_DATE and returns days_to_complete_count from get_lesson_totals() so each column uses the appropriate divisor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The exclusion filter for completed lessons with no quiz submission was unconditionally applied in count_lesson_statuses_with_quiz(), affecting both the Grading page (where it's correct) and the Reports page (where it incorrectly hid completed students). Extract the exclusion into build_unsubmitted_quiz_exclusion_clause() controlled by the exclude_unsubmitted_quiz_completions arg, defaulting to false. Only the Grading page sets this flag to true. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HPPS tables store dates in UTC while comments store local time. DATEDIFF on UTC dates can cross day boundaries differently than local time, causing a 1-day discrepancy in Days to Completion. Use CONVERT_TZ with a numeric offset from gmt_offset to convert UTC dates to local time before the DATEDIFF calculation. Numeric offsets avoid requiring MySQL timezone tables to be populated. Applied to all 3 DATEDIFF locations in the tables-based services: - get_lesson_totals() in the aggregation service - add_days_to_completion_to_courses_clauses() in the clauses service - add_days_to_completion_to_lessons_clauses() in the clauses service Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the duplicated UTC offset helper from both tables-based services to a single static method on Grading_Item, which is already the shared value object used by both services. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apply the same exclusion logic to comments-based storage that tables-based already has: hide rows where a quiz exists but the student has no quiz_answers commentmeta. This covers both 'complete' (never submitted) and orphaned 'passed'/'graded'/'failed' records with no answer data. In-progress students are kept. The grading listing service always applies the filter. The aggregation service count_statuses applies it only when the caller passes exclude_unsubmitted_quiz_completions => true (Grading page only). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the correlated EXISTS subquery with a CASE expression in COALESCE to determine effective status: only use quiz status when a quiz submission exists (qs.id IS NOT NULL). Remove the INNER JOIN on wp_posts for trash checking to match comments-based behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update grade assertions from int to float to match the decimal precision change. Update tests affected by the quiz answer exclusion filter: completed lessons with no quiz submission are now excluded from the grading listing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add get_status_counts() to Grading_Listing_Service_Interface. The tables-based implementation computes per-status counts via a GROUP BY query during get_lesson_progress_items(), replacing the separate COUNT(*) wrapper. This eliminates one full-table scan (~2.5s) per Grading page load. The comments-based implementation returns null (not applicable). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
In Sensei_Grading_Main::get_views(), read per-status counts from the listing service (populated during prepare_items) instead of calling count_statuses() which runs a separate full-table scan. Falls back to count_statuses() for the comments-based path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests verify: counts returned after query, null before query, and counts include all statuses even when a status filter is active. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing indexes have type as the last column, so queries that filter by type (e.g. WHERE type = 'lesson') require a full table scan. This index lets MySQL jump directly to the matching rows. Speeds up the Grading page queries and the admin menu badge counter which both filter by type = 'lesson' and join on post_id. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new composite index to the HPPS sensei_lms_progress table to improve performance of queries that filter by type (and optionally post_id), aligning with how several services query progress data (e.g. WHERE p.type = 'lesson').
Changes:
- Add
KEY type_post_id (type, post_id)to thesensei_lms_progresstable schema used bydbDelta. - Adjust the existing
statuskey line to include a trailing comma (to support adding the new key).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
109
to
+111
| UNIQUE KEY user_progress (post_id, user_id, type), | ||
| KEY status (status) | ||
| KEY status (status), | ||
| KEY type_post_id (type, post_id) |
There was a problem hiding this comment.
This schema change adds a new index, but the existing unit tests for Schema::create_tables() only assert table creation. Please extend the schema tests (e.g. tests/unit-tests/internal/installer/test-class-schema.php) to also assert that the type_post_id index exists after create_tables() runs (via SHOW INDEX / INFORMATION_SCHEMA.STATISTICS) so regressions in dbDelta/index definitions are caught.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
donnapep
force-pushed
the
add/grading-query-index
branch
from
6e9b6ff to
c435e87
Compare
donnapep
force-pushed
the
add/hpps-reports-grading-migration
branch
2 times, most recently
from
0a44259 to
4984ead
Compare
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
(type, post_id)tosensei_lms_progressto speed up queries that filter bytype(e.g.WHERE type = 'lesson')(post_id, user_id, type)hastypelast, so MySQL can't use it for type-filtered queries and does a full table scan insteaddbDeltahandles both new installs (index created with table) and existing installs (index added on next plugin update)Test plan
SHOW INDEX FROM wp_sensei_lms_progress;count_lesson_statuses_with_quizquery should be faster🤖 Generated with Claude Code