Fix: tables-based lesson progress ignores pass_required for failed quiz status#8005
Conversation
…iz status The Tables_Based_Lesson_Progress::get_status() method was missing the pass_required check that Comments_Based_Lesson_Progress correctly implements. When a lesson quiz has pass_required disabled, a learner who submits a failed quiz should still have the lesson marked complete — they only need to have attempted the quiz, not passed it. Mirrors the logic in Comments_Based_Lesson_Progress::get_status(), including the handling of passed/graded statuses that should map to STATUS_COMPLETE. Resolves: Automattic#7943
…t_status() Two integration tests exercising the fix introduced in the prior commit: - failed quiz + pass_required disabled => STATUS_COMPLETE - failed quiz + pass_required enabled => STATUS_IN_PROGRESS
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds logic to Tables_Based_Lesson_Progress so a failed quiz can still count as lesson completion when the lesson quiz doesn’t require passing (matching the comments-based progress behavior).
Changes:
- Implemented
Tables_Based_Lesson_Progress::get_status()with special handling forfailed+_pass_requireddisabled. - Added unit tests covering
failedstatus behavior for both pass-required and not-pass-required quizzes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/unit-tests/internal/student-progress/lesson-progress/models/test-class-tables-based-lesson-progress.php | Adds unit tests asserting status mapping for failed quizzes depending on _pass_required. |
| includes/internal/student-progress/lesson-progress/models/class-tables-based-lesson-progress.php | Implements status normalization for tables-based lesson progress, including _pass_required lookup for failed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * @internal | ||
| * | ||
| * @since $next-version$ |
| * | ||
| * @return string|null | ||
| */ | ||
| public function get_status(): ?string { |
| case 'failed': | ||
| // This may be 'completed' depending on... | ||
| // Get Quiz ID, this won't be needed once all Quiz meta fields are stored on the Lesson. | ||
| $lesson_quiz_id = Sensei()->lesson->lesson_quizzes( $this->lesson_id ); | ||
| if ( $lesson_quiz_id ) { | ||
| // ...the quiz pass setting. | ||
| $pass_required = get_post_meta( $lesson_quiz_id, '_pass_required', true ); | ||
| if ( empty( $pass_required ) ) { | ||
| // We just require the user to have done the quiz, not to have passed. | ||
| return self::STATUS_COMPLETE; | ||
| } | ||
| } | ||
| return self::STATUS_IN_PROGRESS; |
| return self::STATUS_COMPLETE; | ||
|
|
||
| case 'failed': | ||
| // This may be 'completed' depending on... |
…S_COMPLETE constant
|
Thanks for the automated review — wanted to address each point:
Return type N+1 query concern — Noted. The implementation mirrors Comment wording ('completed' vs 'complete') — Good catch. I pushed a follow-up commit that corrects the comment to Let me know if anything else needs addressing. |
|
Added a changelog entry to resolve the Changelogger CI failure — pushed The other failing check ( (full disclosure: AI helped me identify the issue and verify my work) |
What this fixes
When a lesson's quiz has Pass Required disabled, a learner who submits a failed quiz should still have the lesson marked complete — they only need to have attempted the quiz, not passed it.
The
Comments_Based_Lesson_Progress::get_status()already handles this correctly. The HPPS equivalent,Tables_Based_Lesson_Progress, inherits a bareget_status()fromLesson_Progress_Abstractthat simply returns$this->statusverbatim — so a'failed'internal status is returned as'failed'(maps toSTATUS_IN_PROGRESS) regardless of the_pass_requiredsetting on the quiz.This is the root cause of #7943.
What changed
includes/internal/student-progress/lesson-progress/models/class-tables-based-lesson-progress.phpget_status()override that mirrorsComments_Based_Lesson_Progress::get_status(), including the_pass_requiredcheck for the'failed'casepassed/gradedstatuses correctly returnSTATUS_COMPLETEfailed+pass_requireddisabled →STATUS_COMPLETEfailed+pass_requiredenabled →STATUS_IN_PROGRESSSTATUS_IN_PROGRESStests/unit-tests/internal/student-progress/lesson-progress/models/test-class-tables-based-lesson-progress.phptestGetStatus_ConstructedWithFailedStatusAndPassNotRequired_ReturnsCompletetestGetStatus_ConstructedWithFailedStatusAndPassRequired_ReturnsInProgressTesting
Both new tests should pass. Existing tests in this file are unaffected.
Related
Resolves #7943
This PR was developed with AI assistance (research, implementation, and test generation). All code was reviewed and validated before submission.