Fix fatal error when quiz's parent lesson has been deleted - #3173
Closed
thisismyurl wants to merge 8 commits into
Closed
Fix fatal error when quiz's parent lesson has been deleted#3173thisismyurl wants to merge 8 commits into
thisismyurl wants to merge 8 commits into
Conversation
) AGENTS.md is the universal entry point for AI coding agents (Codex, Cursor, Aider, Claude Code, and others) pointed at this repo. It covers project shape, the bundled-package and add-on ecosystem, the contribution workflow (branch from dev, PR to dev, .changelogs entry), coding standards, public API discipline, verification rules, and a where-to-look quick reference. CLAUDE.md is a one-line import of AGENTS.md so Claude Code reads the same file without a duplicated source of truth. Pattern borrowed from WooCommerce. Includes a .changelogs entry per the contributing workflow. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
* Extra check when updating data via the course builder so content cannot be moved around different courses. * Additional checks to the course builder parent values. * Updating lifterlms-rest
When a quiz has a stored `lesson_id` in post meta but the referenced lesson post no longer exists, `llms_get_post( $lesson_id )` returns false. Two access-check functions didn't guard against this: `llms_is_post_restricted_by_prerequisite()`: called `$lesson->get_course()` immediately after `llms_get_post()` with no null-check, causing a fatal error / TypeError on any page-restriction check for that quiz. `llms_is_post_restricted_by_time_period()`: had a copy-paste bug where the guard read `if ( ! $lesson_id )` instead of `if ( ! $lesson )`. Since `$lesson_id` was already confirmed truthy one line above, the guard was dead code and `$lesson->get( 'parent_course' )` was unprotected. Both functions now return `false` (no restriction found) when `llms_get_post()` returns a falsy value for the lesson, consistent with how the function handles other not-found cases. Reproduction: assign a quiz to a lesson, delete the lesson post, then visit the quiz URL as an enrolled student.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Bumps LifterLMS to v10.0.4 and hardens builder/access logic against unauthorized updates and orphaned quiz edge-cases, with accompanying tests and documentation for AI agent workflows.
Changes:
- Add defensive guards in access checks to prevent fatals when a quiz’s parent lesson is missing.
- Harden Course Builder update handlers (lessons/quizzes/sections) with additional capability checks and relationship enforcement, plus new PHPUnit tests.
- Release housekeeping: version bumps, changelog entry, composer exclude tweak, and AI-agent context docs (AGENTS.md/CLAUDE.md).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/unit-tests/functions/class-llms-test-functions-certificates.php | Makes snapshots resilient across WP versions by conditionally accounting for paragraph render changes. |
| tests/phpunit/unit-tests/admin/class-llms-test-admin-builder.php | Adds regression tests asserting builder cannot write/move content into unauthorized courses and enforces capability checks. |
| includes/functions/llms.functions.access.php | Adds/repairs null-guards around llms_get_post() to prevent fatals when parent lesson is missing. |
| includes/admin/class.llms.admin.builder.php | Adds authorization checks and prevents trusting client-supplied relationship fields for builder saves. |
| package.json / lifterlms.php / class-lifterlms.php | Version bump to 10.0.4 in JS and PHP plugin metadata. |
| composer.json | Bumps lifterlms-rest dependency and excludes artifacts from composer archives. |
| CHANGELOG.md | Adds 10.0.4 release notes including security fixes. |
| AGENTS.md / CLAUDE.md | Adds repo orientation and AI-agent entrypoint for tooling that reads CLAUDE.md. |
| .changelogs/fix-orphaned-quiz-fatal-error.yml | Adds a patch-level security changelog entry describing the orphaned-quiz guard fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @since 3.0.0 | ||
| * @since 3.16.11 Unknown. | ||
| * @since 6.5.0 Improve code readability turning if-elseif into a switch-case. | ||
| * @since $$next-version$$ Guard against a fatal error when the quiz's parent lesson no longer exists. |
| * @since 3.16.11 Unknown. | ||
| * @since 5.7.0 Replaced the call to the deprecated `LLMS_Lesson::get_parent_course()` method with `LLMS_Lesson::get( 'parent_course' )`. | ||
| * @since 6.5.0 Improve code readability turning if-elseif into a switch-case. | ||
| * @since $$next-version$$ Fix copy-paste error: guard checked `$lesson_id` (already truthy) instead of `$lesson` after `llms_get_post()`. |
Comment on lines
+1094
to
+1095
| $authorized_course_id = $course_id ? absint( $course_id ) : absint( $section->get( 'parent_course' ) ); | ||
| $can_edit_course = ! $authorized_course_id || current_user_can( 'edit_course', $authorized_course_id ); |
Comment on lines
+1203
to
+1207
| // Force the lesson into the authorized course and section. | ||
| $lesson->set( 'parent_section', $section->get( 'id' ) ); | ||
| if ( $authorized_course_id ) { | ||
| $lesson->set( 'parent_course', $authorized_course_id ); | ||
| } |
Comment on lines
+1403
to
+1405
| // Confirm the current user can edit the course this quiz belongs to, independent of earlier checks. | ||
| $authorized_course_id = $course_id ? absint( $course_id ) : absint( $lesson->get( 'parent_course' ) ); | ||
| if ( $authorized_course_id && ! current_user_can( 'edit_course', $authorized_course_id ) ) { |
| $quiz->set( $prop, $quiz_data[ $prop ] ); | ||
| } | ||
| } | ||
| $quiz->set( 'lesson_id', $lesson->get( 'id' ) ); |
Comment on lines
398
to
+401
| $lesson = llms_get_post( $lesson_id ); | ||
| if ( ! $lesson ) { | ||
| return false; | ||
| } |
Comment on lines
+182
to
+185
| /** | ||
| * WordPress 7.0+ adds a `wp-block-paragraph` class to rendered paragraph blocks. | ||
| * Detect the rendered output so the expected snapshots match across WP versions. | ||
| */ |
Author
|
Closing this in favour of #3192, which is branched cleanly from |
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.
Brian, this is a noticable bug in the system that can be addressed with the attached fix.
Description
Fixes a fatal error / TypeError that occurs when the restriction-check pipeline runs against a quiz whose parent lesson no longer exists in the database (e.g., the lesson was deleted or trashed but the quiz's
lesson_idpost meta was not cleaned up).Two functions in
llms.functions.access.phpwere unprotected:llms_is_post_restricted_by_prerequisite()— called$lesson->get_course()immediately afterllms_get_post( $lesson_id )with no null-check. Whenllms_get_post()returnsfalse, the next line is a method call on a non-object.llms_is_post_restricted_by_time_period()— had a copy-paste bug: the guard afterllms_get_post()readif ( ! $lesson_id )instead ofif ( ! $lesson ). Since$lesson_idwas already confirmed truthy three lines earlier, the guard was dead code.$lesson->get( 'parent_course' )was entirely unprotected.Both functions now return
false(no restriction found, consistent with how the functions handle other not-found cases) whenllms_get_post()returns a falsy value.Reproduction: Assign a quiz to a lesson. Delete the lesson post. Visit the quiz URL as an enrolled student — white screen / fatal error on PHP 7, TypeError on PHP 8.
How has this been tested?
Manually: created a course with a lesson + attached quiz, deleted the lesson, visited the quiz as a logged-in enrolled student. Confirmed the fatal error before the fix and clean page load after.
Types of changes
Checklist:
Developed with AI assistance (Claude). Manually reviewed.