Skip to content

Set the page course module in the overridden feedback tests - #167

Merged
adrian-sarmas merged 1 commit into
elearningsoftware:masterfrom
bycs-lp:MBS-11624_Fix_Overridden_Feedback_Page_Setup
Oct 26, 2026
Merged

adrian-sarmas merged 1 commit into
elearningsoftware:masterfrom
bycs-lp:MBS-11624_Fix_Overridden_Feedback_Page_Setup

Conversation

@PM84

@PM84 PM84 commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Hey! The overridden feedback tests in lib_test.php blow up with "The course you passed to $PAGE->set_cm does not correspond to the $cm" as soon as a TinyMCE plugin builds the navigation while the editor renders (e.g. tiny_stash), because only the module context was set and the page course was still the site. This sets the course module and course on $PAGE and closes the output buffer in a finally, so the tests are green again and don't leave a dangling buffer. Cheers!

…dden grade in tests

Editor plugins may build the settings navigation while the editor is
rendered. With only the module context set, the navigation looks up the
course module and fails because the page course is still the site.
Setting the course module and course of the page fixes this, and the
output buffer is now closed even if rendering fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@adrian-sarmas
adrian-sarmas merged commit 3fd8627 into elearningsoftware:master Oct 26, 2026
38 of 54 checks passed
@adrian-sarmas

Copy link
Copy Markdown
Collaborator

Thanks for the fix, merged!

It still failed on MariaDB for Moodle 4.4+ with "The theme has already been set up for this page". Since 4.4, create_and_enrol() sends a course welcome message, and email_to_user() sets up the page theme while the page course is still the site. After that, set_cm() can't change the course. I couldn't push to your branch, so I added a follow-up commit (81ff763) that sets up the page before the users are enrolled. Your set_cm() and try/finally changes are unchanged, and CI is green on all versions now.

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