ndunand / moodle-mod_choicegroup

Moodle "Group Choice" plugin
https://moodle.org/plugins/mod_choicegroup
35 stars 63 forks source link

BUG: Link between original and restored choicegroup module after course is backed up and restored to new course #185

Closed cdipe closed 1 year ago

cdipe commented 1 year ago

Using Moodle 3.11.10 and Choicegroup 1.39 for Moodle 3.9-4.1 (Build: 2023010300)

When having backed up a course using mod_choicegroup, without backing up users and user data, the new course's choicegroup still contains the names of students from the backed up course. There is a link between the choicegroup modules when copied, making the copy show the members of the original.

To replicate:

  1. Create a course with students, groups and a choicegroup module
  2. Have the students select groups in the choicegroup module
  3. Backup the course without users (uncheck Include enrolled users)
  4. Restore to a new course
  5. Verify that the new course has no students, and that the new course's groups have no members
  6. Open the choicegroup module in the new course and verify: Link says "View 0 responses" but there are numbers in the members column
  7. Select Show group members and confirm that the names are from the original course, in the same groups as in the original course
  8. Go to the original course's choicegroup module and remove one of the students from a group
  9. Go to the new course and verify that the same students name is removed from the same group there as well

Extended testing

  1. Go to the original course's choicegroup module, log in as the student that was removed from the group (9) and select a group
  2. Go to the new course's choicegroup module and verify that the same students name is added to the same group there as well
  3. Finally: Go to the settings of the new course's choicegroup module and verify that there are actually no groups added to be selectable.
chiaravicere commented 1 year ago

I tried to replicate this in Moodle 3.11.1+ with Choicegroup 1.33 for Moodle 3.5-3.11 (Build: 2021083100). In my case, users are not linked from the original course, the "Members" column is empty.

cdipe commented 1 year ago

@chiaravicere it's likely that the issue is in only in later versions of Choicegroup. We've used Choicegroup for several years and only discovered this bug recently. We upgraded all plugins in late september 2022, Choicegroup to 1.34. (We then upgraded to 1.39 to see if that solved the problem which it didn't). Do you have the possibility to upgrade to a later version?

cdipe commented 1 year ago

@chiaravicere We have now tested in a "clean" local computer installation, with Moodle 3.11.12 and Choicegroup 1.39 (Build 2023010300) and the problem is exactly the same: The Choicegroup in the copied course mirrors the members in the original course.

cdipe commented 1 year ago

@NicoAlexH @chiaravicere Today, to make absolutely sure, I installed the latest Moodle MAMP package for OS X on a local computer and ran the same tests. The result is the same as described above, Choicegroup instances in copied courses are linked to the Choicegroup in the copied course.

Moodle 4.1.1 (Build: 20230116), Group Choice 1.39 for Moodle 3.9-4.1 (Build: 2023010300)

miotto commented 1 year ago

Ok I can reproduce the error. It occurs when the course is restored. In the following file in line 75 backup/moodle2/restore_choicegroup_stepslib.php

The return value of get_recordset_sql is the problem. If nothing is found, it returns an object, following a var_dump.

object(mysqli_native_moodle_recordset)#1338 (2) { ["result":protected]=> NULL ["current":protected]=> bool(false) }

It is an object and therefore true, then if (!$group) does not work and the mapping is skipped.

The query get_recordset_sqlcan be replaced by record_exists_sql, then it works again.

cdipe commented 1 year ago

@miotto great, we will try this! Is this something that will fix also the instances that has been copied and restored while the bug has been there? Or is there another way to do that, except for trying to find, delete and create a new Group Choice?

miotto commented 1 year ago

@cdipe It does not fix the problem of already existing instances. I think a subsequent synchronization of two or more choicegroup settings could be difficult. Both the courses and the content may have changed since the course was restored. There could be many different variations. I have to think about it. Maybe someone else has an idea.

cdipe commented 1 year ago

@miotto we did the fix in our QA (moodle 3.11.10) and it works as expected. Thanks!

cdipe commented 1 year ago

@NicoAlexH can this be expected as a patch or a new release soon, since this is a bug that will cause for everyone copying a course where Group Choice is used?

NicoAlexH commented 1 year ago

@cdipe @miotto Thanks for reporting and spotting the issue ! A new version of the plugin will be available very soon.

cdipe commented 1 year ago

@miotto great, thank you!

lucaboesch commented 1 year ago

I think it would be great if there would be a hint here for a DELETE or UPDATE command which would allow to fix this (delete the non legit choices of now non-participants) on database level. I mean in the form of "DELETE FROM mdl_choicegroup_option …" or "UPDATE mdl_choicegroup_option SET …" (I'm just guessing.). Or any other instruction. Thanks in advance.

Best, Luca

ndunand commented 1 year ago

@NicoAlexH could you come up with such a query ?

miotto commented 1 year ago

A possible SQL query

SELECT cg_op.* FROM {choicegroup_options} cg_op JOIN {choicegroup} cg on cg_op.choicegroupid=cg.id WHERE cg_op.groupid NOT IN (SELECT id FROM {groups} gr WHERE cg.course = gr.courseid)

Or for deleting

DELETE FROM {choicegroup_options} WHERE id IN ( SELECT cg_op.id FROM {choicegroup_options} cg_op JOIN {choicegroup} cg on cg_op.choicegroupid=cg.id WHERE cg_op.groupid NOT IN (SELECT id FROM {groups} gr WHERE cg.course = gr.courseid) )

ndunand commented 1 year ago

Thanks @miotto ! Indeed this looks like it would clean up messy choicegroup_options that should not be existing.

@lucaboesch would this do the job for you?

miotto commented 1 year ago

Or with a new version in the update.php script.

    if ($oldversion < 2023013001) {
        // Delete wrong choicegroup options, from the old bad Recovery process.
        $sql = "SELECT cg_op.id FROM {choicegroup_options} cg_op
                JOIN {choicegroup} cg on cg_op.choicegroupid=cg.id
                WHERE cg_op.groupid NOT IN (SELECT id FROM {groups} gr WHERE cg.course = gr.courseid)";

        while ($records = $DB->get_records_sql($sql)) {
            if (!empty($records)) {
                foreach ($records as $id => $record) {
                    $DB->delete_records('choicegroup_options', array('id' => $id));
                }
            }
        }

        // Choicegroup savepoint reached.
        upgrade_mod_savepoint(true, 2023013001, 'choicegroup');
    }
lucaboesch commented 1 year ago

Okay, this deals with removing wrong group choices which then would lead to a choicegroup activity emptied of options. If you want to 'fix' an existing choicegroup with options wrongfully pointing to the copy's source course' groups, proceed as follows.

Find what ID the choicegroup has, e.g. by searching it for name and course.

SELECT id FROM mdl_choicegroup WHERE name LIKE '%My groupchoice%' and course = 5555;

Then, with that ID, e.g. 4758, look up the options.

SELECT * FROM mdl_choicegroup_options WHERE choicegroupid = 4758;

id | choicegroupid | groupid | maxanswers | timemodified -------+---------------+---------+------------+-------------- 46672 | 4758 | 87611 | 11 | 1652717753 46673 | 4758 | 87612 | 11 | 1652717753 46674 | 4758 | 87613 | 11 | 1652717753 46675 | 4758 | 87614 | 11 | 1652717753

The group id are those of the old course (the wrong ones, probably filled), so you have to go bend them straight again and update to the current group ids which you have to look up beforehand in the UI (I don't explain that group ID lookup step in detail here).

UPDATE mdl_choicegroup_options SET groupid = 100139 WHERE id=46672; UPDATE mdl_choicegroup_options SET groupid = 100140 WHERE id=46673; UPDATE mdl_choicegroup_options SET groupid = 100141 WHERE id=46674; UPDATE mdl_choicegroup_options SET groupid = 100142 WHERE id=46675;

This should lead to the correct options.