Fixes for Moodle 4.1 compatibility #153
Conversation
Options are stored as a static class variable, so need to be reset after tests to prevent knock-on effects on tests run after.
| @@ -98,7 +98,7 @@ function safety_checks($dryrun) { | |||
| AND u.deleted = 0"; | |||
|
|
|||
| if ($DB->count_records_sql($csql, $params)) { | |||
| $namefields = "u." . implode(', u.', get_all_user_name_fields()); | |||
| $namefields = "u." . implode(', u.', \core_user\fields::get_name_fields()); | |||
There was a problem hiding this comment.
won't this break in older moodles where this doesn't exist yet? Either we need to detect the version and call the old function of pick a version and cut a new stable branch. It would be better have this in a unit test to expose that issue across versions too
There was a problem hiding this comment.
This function was introduced on Moodle 3.11, so you're right that it won't work on Moodle 3.10 which this branch supports (although it's no longer a supported release). Personally I prefer separate stable branches over version-detection code, as the latter feels like technical debt. Shall I create a new branch and change this PR's target?
There was a problem hiding this comment.
Yup please cut a new stable
There was a problem hiding this comment.
Actually, I think you'll have to do that for me. I don't have write access and GitHub doesn't have a way for me to submit a new branch from my repo to this one. I think it would make sense to call it MOODLE_311_STABLE as this and the removed user fields both apply from 3.11 onwards.
There was a problem hiding this comment.
I've made the new branch as a clone of the 310 one so you can make new prs against that now thanks
marxjohnson
changed the base branch from
MOODLE_310_STABLE
to
MOODLE_311_STABLE
These changes only support Moodle 3.11+
|
Hi @brendanheywood , I've also raised additional PRs to update the README on the other branches. |
marxjohnson
force-pushed
the
152-moodle-401-fixes
branch
from
3f968ab
to
75c7982
Compare
|
@brendanheywood There we go, all green now on 3.11-4.1. |
|
looks good to me - I've also updated the default branch here it github to "MOODLE_311_STABLE" - I didn't delete the old "master" branch as it will break AU pipelines using the older master branch. |
Resolves #152