Closed BCLN closed 3 years ago
@BCLN Ok:
@BCLN Also this is the restore: https://github.com/gjb2048/moodle-format_grid/blob/master/backup/moodle2/restore_format_grid_plugin.class.php and backup code: https://github.com/gjb2048/moodle-format_grid/blob/master/backup/moodle2/backup_format_grid_plugin.class.php - please look at it and let me know if you spot anything logically wrong with it.
Yes, sorry. It has been an ongoing issue with various versions, but let me spell out most recent release data:
The above data represents a recent backup and restore where the problem occurred on 5 of 12 restores.
This has been a regular occurrence with users over the last few years (and versions of both) - ie. some get restored correctly and some don't. Yes, I am happy to share an example.
@BCLN Ok, please could I have the MBZ so I can make progress with this.
Just done a five section M3.8 to M3.9 and works as expected. Cannot replicate. I need evidence please!
It took a few restores before I got another miss-match. https://wcln.ca/Math_10,FPC-20201109-0057-nu.mbz
Notes:
This has been common enough over the last few years, that I made a video for teachers to check their courses: WCLN.ca > Teacher Support > Check for orphaned units https://wcln.ca/mod/page/view.php?id=31845
Thank you!
@BCLN Went to see what it was like on M3.8 first and getting:
Please try again.
Extracted with 7Zip and made a new file half the size that appears to work.
When restoring on M3.8 then that file stops at 4 and there is:
Error: mdb->get_record() found more than one record!
line 1599 of \lib\dml\moodle_database.php: call to debugging()
line 1559 of \lib\dml\moodle_database.php: call to moodle_database->get_record_sql()
line 1538 of \lib\dml\moodle_database.php: call to moodle_database->get_record_select()
line 2028 of \course\format\grid\lib.php: call to moodle_database->get_record()
line 86 of \course\format\grid\lib.php: call to format_grid->get_summary_visibility()
line 53 of \course\format\grid\renderer.php: call to format_grid->is_section0_attop()
line 417 of \lib\outputfactories.php: call to format_grid_renderer->__construct()
line 2304 of \lib\outputlib.php: call to theme_overridden_renderer_factory->get_renderer()
line 874 of \lib\pagelib.php: call to theme_config->get_renderer()
line 141 of \course\view.php: call to moodle_page->get_renderer()
Stack trace.
Ok, it is likely that the MBZ / course was created around / before this https://github.com/gjb2048/moodle-format_grid/commit/328e9d121eb622ab21d05bd0b29594940e13cfce sort of thing. Thus the MBZ in course.xml has a 'numsections' of '4' when there are actually '8' in the file.
@BCLN Ok, this is what I think has happened over time for the 'numsections' to be '4' in the database but the course show 8 (does it in the M3.8 version???). So the course was created on 'Fri, 29 Sep 2017 16:29:25 +0000' and last modified on 'Tue, 30 Jul 2019 18:29:41 +0000'. Therefore with a window of time where numsections existed, set to 4, then did not exist, course sections changed, numsections came back, course still ok as https://github.com/gjb2048/moodle-format_grid/blob/master/renderer.php#L354 does not rely on it, but rather uses https://github.com/gjb2048/moodle-format_grid/blob/master/renderer.php#L407 which uses 'numsections' only if it exists as a course setting - therefore different behaviour depending on its existance. Only there to speed up ability to add / remove sections.
So, I'm not sure there is a code solution here! As the code would 'guess' if a section was shown or intentionally orphaned. Therefore for you with old problem courses:
Check the value in the DB for 'numsections' is the true value and make sure there is only one entry per course in 'format_grid_summary'. And (if needed to change an existing MBZ), then extract (it's a zip file after all) and edit 'course/course.xml' to be accurate, then zip everything up again.
Also.... I don't know why you're not spotting the issue in M3.8? Or is it that you have backups from before the upgrade that you repeatedly use? So.... if they are an issue then after fixing the problem course, why did you not make a new corrected backup?
Thank you VERY much for the feedback. Definitely helpful for zeroing in on the issue.
@gjb2048 I looked into this on behalf of BCLN. You are correct the issue is that 'numsections' does not exist in the table 'course_format_options'. This can manually be corrected by editing the course settings and clicking "Save and display". This writes the missing 'numsections' value to the table.
I am guessing there are other users out there who will come across this issue as well. I would recommend the following solution:
Let me know if you are able to make a code change to fix this issue. If not, we will write a small script to correct the issue in our database.
@colinperepelken Ok, pertaining to "You are correct the issue is that 'numsections' does not exist in the table 'course_format_options'". You've not understood what I have written. It is the fact that 'numsections' does exist in the table, then Grid did not use it, then the number of sections changed and as it was not used it was not updated, then it came back and the code still works, however with any backup there will be an anomaly, so when the restore happens 'numsections' is used and will have the old value as when it came back in the code the course settings were not changed and thus the updated value when 'numsections' was not being used was not re-written to when it came back and was being used.
Therefore your solution is wrong because you don't understand the problem fully.
@gjb2048 Yes I misunderstood what you wrote about 'numsections' being the wrong value. However what you are describing is not quite the issue we are facing...
Our issue is that 'numsections' doesn't exist. The course was last edited when the course format was not using and not storing 'numsections'.
This means when the course is backed up, 'numsections' doesn't exist in the course_format_options table so Moodle uses the course default numsections setting found at "Site administration -> Courses -> Course default settings" (which is 4).
My solution of setting 'numsections' during an upgrade works, and likely should have been added to the upgrade script when 'numsections' was reintroduced.
So why does the backup file have a 'numsections' of 4 when there is the code:
if ($courseid == 1) { // New course.
$defaultnumsections = $courseconfig->numsections;
} else { // Existing course that may not have 'numsections' - see get_last_section().
global $DB;
$defaultnumsections = $DB->get_field_sql('SELECT max(section) from {course_sections}
WHERE course = ?', array($courseid));
}
in lib.php which would be utilised in a backup process (I imagine) to understand what course format fields to be backed up.
So if you change the number of default sections, the does that number change in the backup file?
@gjb2048 Good question, I just tested setting the default number of sections to "16" and you are correct it did not write that to the backup file.
I'll keep looking into this and let you know if I get anywhere. For now ensuring 'numsections' is set correctly in the database has fixed the issue. Though I'm unsure where the value of 4 was coming from.
I did run a query before correcting anything, and all courses that had a 'numsections' value had the correct value.
Appreciate your help looking into this.
@colinperepelken Any news on this please?
@gjb2048 We haven't had any more issues related to this after ensuring 'numsections' is set for each course. I think this issue can be closed, thank you again for your help troubleshooting.
@colinperepelken Thanks for getting back to me.
@colinperepelken Will be an update today that does sort this as found an anomily.
@gjb2048 Thanks for letting me know! Will be sure to pull in that update.
We're big fans of Grid and have used it for years. Thank you!
We distribute courses by backing-up the courses for others to restore onto their site. Those courses using Grid (most of them) seem to often have troubles with restoring with the wrong number of topics. For example, a Math courses with 8 units is backed-up and a portion of users will find that when they restore the course, it'll have 0, 2, 4..... units, causing orphaned topics. It's not every course, but a significant portion. We don't see this problem with non-grid courses, so I assume it's related to the plugin.
It's mildly annoying to have to check every restored course and fix a portion of them.
Is this something that could be looked at?
Thank you