Problem found with upgrade step 2015021002

[polothy]

The code in question from db/upgrade.php:

foreach ($records as $record) {
    $DB->execute("UPDATE {assign_plugin_config} SET value = '2' WHERE plugin = 'mahara' AND subtype = 'assignsubmission' AND name = 'lock' AND value = '1'");
}

The SQL statement doesn't take into account $record - so it executes the same query over and over again. Most likely need to add AND assignment = ? to the SQL and pass [$record->assignment] as the query parameters.

[polothy]

This is the fix we used and tested:

$DB->execute("UPDATE {assign_plugin_config} SET value = '2' WHERE plugin = 'mahara' AND subtype = 'assignsubmission' AND name = 'lock' AND value = '1' AND assignment = ?", [$record->assignment]);

[kabalin]

Thanks @mrmark for bug report, well spotted! Feel free to submit a pull request if you like.

[kabalin]

@agwells shall we make an unplanned release given the seriousness of the issue?

[agwells]

@kabalin Oh, I just noticed this is actually a data loss issue, isn't it? It's supposed to identify all the assignments that have enabled the old feedback plugin, and replace that record with one that says that assignment should turn on "unlock after grading" in the submission plugin.

But instead, it changes every Mahara assignment submission instance to use "unlock after grading", regardless of its current locking settings.

Yeah, let's push out an update as soon as we can. Do we know if the "assign_plugin_config" records from the feedback plugin are still around after it's uninstalled? Or does Moodle automatically delete those? If they're not deleted, we could push out a correcting upgrade script that re-syncs the assignment submission settings with the ones from the assignment feedback plugin. Or, at least tell people how to do that manually.

[kabalin]

@agwells a partial data loss, it does not change every submission instance, only those that had "locking" enabled now became "unlock after grading". Interesting that no-one spotted yet. Records from feedback plugin are deleted, so no way to reverse.

[kabalin]

Fixed in https://github.com/MaharaProject/moodle-assignsubmission_mahara/pull/22, testing is required before merging to master.

[agwells]

@kabalin Right, the query is UPDATE {assign_plugin_config} SET value = '2' WHERE plugin = 'mahara' AND subtype = 'assignsubmission' AND name = 'lock' AND value = '1'.

So as you say, all assignments that were set to "Locking" move to "unlock after grading". So specifically, the data loss here is that if an assignment was set to permanent locking, it moves to temporary locking.

I'll try to take a look at your fix today. Today's my last day in the office before I head to the US for Christmas, though, so if you don't see anything from me today, it'll probably be most efficient if you just merge it and update the Moodle Plugin Database yourself. (I think you and @upats have access to the listing.)

[kabalin]

So as you say, all assignments that were set to "Locking" move to "unlock after grading". So specifically, the data loss here is that if an assignment was set to permanent locking, it moves to temporary locking.

And only if assignfeedback_mahara has been in use, those who has not used feedback plugin unlocking (assignfeedback_mahara) are not affected.

[kabalin]

Release in progress.

[kabalin]

Released. Closing the bug.