Skip to content

Fix: accept both 'course' and 'cm' keys in __construct() - for backwards compatibility#51

Open
michael-milette wants to merge 1 commit intoModernLMS:masterfrom
michael-milette:fix-51
Open

Fix: accept both 'course' and 'cm' keys in __construct() - for backwards compatibility#51
michael-milette wants to merge 1 commit intoModernLMS:masterfrom
michael-milette:fix-51

Conversation

@michael-milette
Copy link

Summary

Commit 69e5c26 (fix #38) for issue #22 (Fixed course condition when duplicating an activity) introduced a save/load key mismatch that silently breaks all conditions saved after that commit.

Root cause

In classes\condition.php, save() and get_json() were updated to output the JSON key 'course', but __construct() was never updated — it still reads $structure->cm:

// __construct() — reads 'cm'
if (isset($structure->cm) && is_number($structure->cm)) {
    $this->courseid = (int)$structure->cm;
} else {
    throw new \coding_exception('Missing or invalid ->cm for completion condition');
}

// save() — writes 'course'
return (object)array('type' => 'othercompleted',
    'course' => $this->courseid, 'e' => $this->expectedcompletion);

Effect

Any availability condition saved after commit 69e5c26 throws a coding_exception when Moodle reloads it from the database, making the condition permanently unresolvable.

Fix

In __construct(), change the key read from 'cm' to 'course' to match save(). A DB migration is NOT required if __construct() is updated to accept both keys:

if (isset($structure->course) && is_number($structure->course)) {
    $this->courseid = (int)$structure->course;
} else if (isset($structure->cm) && is_number($structure->cm)) {
    $this->courseid = (int)$structure->cm;  // backwards compat for pre-69e5c26 data
} else {
    throw new \coding_exception('Missing or invalid ->course for completion condition');
}

Data will now self-heal on the next save.

This also fixes related JavaScript and phpUnit test references, changing 'cm' to 'course'.

Affected versions

All releases from commit 69e5c26 (2024-02-02) onwards.

Let me know if you have any questions or need any changes.

Best regards,

Michael Milette

…s compatibility

Commit 69e5c26 updated save() to output 'course' but did not update __construct()
which still read 'cm', causing a coding_exception on every load after save.

The constructor now accepts 'course' (post-69e5c26 DB data) and falls back to
'cm' (pre-69e5c26 DB data). No database migration required — data self-heals
on next save.

Fixes ModernLMS#38
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.

1 participant