Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-28949

Duplicated course completion entries in database not handled correctly

    Details

    • Testing Instructions:
      Hide

      To test the upgrade code you must create duplicate records in the tables this commit cleans up (course_completions, course_completion_aggr_method, course_completion_crit_compl).

      These are the indexes that an upgrade should create, or a fresh install should include:

      $table = new xmldb_table('course_completions');
      $index = new xmldb_index('useridcourse', XMLDB_INDEX_UNIQUE, array('userid', 'course'));
       
      $table = new xmldb_table('course_completion_crit_compl');
      $index = new xmldb_index('useridcoursecriteraid', XMLDB_INDEX_UNIQUE, array('userid', 'course', 'criteriaid'));
       
      $table = new xmldb_table('course_completion_aggr_methd');
      $index = new xmldb_index('coursecriteriatype', XMLDB_INDEX_UNIQUE, array('course', 'criteriatype'));

      So creating multiple records that share identical values in these columns, but differing (or indentical, it doesn't matter) data in other columns should be merged into single records allowing the indexes to be created.

      An example of duplicate data I created during testing:

      // Add some test records
      $obj = new stdClass();
      $obj->course = 2;
      $obj->userid = 2;
      $obj->timecompleted = NULL;
      $obj->timestarted = 100;
      $obj->timeenrolled = 100;
      $DB->insert_record('course_completions', $obj);
       
      $obj->timestarted = 105;
      $DB->insert_record('course_completions', $obj);
       
      $obj->timecompleted = 105;
      $DB->insert_record('course_completions', $obj);
       
      $obj->timestarted = 0;
      $DB->insert_record('course_completions', $obj);
       
      $obj->userid = 3;
      $DB->insert_record('course_completions', $obj);
       
      $obj->timecompleted = 102;
      $DB->insert_record('course_completions', $obj);
       
       
      // Add some test records
      $obj = new stdClass();
      $obj->course = 2;
      $obj->userid = 2;
      $obj->criteriaid = 12;
      $obj->timecompleted = NULL;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      $obj->timecompleted = 12;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      $obj->timecompleted = 18;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      $obj->timecompleted = 11;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      $obj->userid = 3;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      $obj->timecompleted = 21;
      $DB->insert_record('course_completion_crit_compl', $obj);
       
      // Add some test records
      $obj = new stdClass();
      $obj->course = 2;
      $obj->criteriatype = 12;
      $obj->method = 1;
      $DB->insert_record('course_completion_aggr_methd', $obj);
       
      $obj->method = 2;
      $DB->insert_record('course_completion_aggr_methd', $obj);

      The logic behind merging duplicates means that it takes the lowest integer value for the unique columns, but will take an integer over null.

      To test that these changes do not affect the use of course completion:

      • Enable course completion in site advanced settings
      • Create a new course, enabling completion
      • In the "completion" menu in course settings, enable some criteria
      • Add the completion status block to course
      • Enrol users in the course
      • Have them complete some of the criteria (cron run may be required to have their "completion" register in system)
      • View their progress in the completion status block (as the learner), or in the completion course report as a teacher/siteadmin

      No coding or critical errors should occur.

      Show
      To test the upgrade code you must create duplicate records in the tables this commit cleans up (course_completions, course_completion_aggr_method, course_completion_crit_compl). These are the indexes that an upgrade should create, or a fresh install should include: $table = new xmldb_table('course_completions'); $index = new xmldb_index('useridcourse', XMLDB_INDEX_UNIQUE, array('userid', 'course'));   $table = new xmldb_table('course_completion_crit_compl'); $index = new xmldb_index('useridcoursecriteraid', XMLDB_INDEX_UNIQUE, array('userid', 'course', 'criteriaid'));   $table = new xmldb_table('course_completion_aggr_methd'); $index = new xmldb_index('coursecriteriatype', XMLDB_INDEX_UNIQUE, array('course', 'criteriatype')); So creating multiple records that share identical values in these columns, but differing (or indentical, it doesn't matter) data in other columns should be merged into single records allowing the indexes to be created. An example of duplicate data I created during testing: // Add some test records $obj = new stdClass(); $obj->course = 2; $obj->userid = 2; $obj->timecompleted = NULL; $obj->timestarted = 100; $obj->timeenrolled = 100; $DB->insert_record('course_completions', $obj);   $obj->timestarted = 105; $DB->insert_record('course_completions', $obj);   $obj->timecompleted = 105; $DB->insert_record('course_completions', $obj);   $obj->timestarted = 0; $DB->insert_record('course_completions', $obj);   $obj->userid = 3; $DB->insert_record('course_completions', $obj);   $obj->timecompleted = 102; $DB->insert_record('course_completions', $obj);     // Add some test records $obj = new stdClass(); $obj->course = 2; $obj->userid = 2; $obj->criteriaid = 12; $obj->timecompleted = NULL; $DB->insert_record('course_completion_crit_compl', $obj);   $obj->timecompleted = 12; $DB->insert_record('course_completion_crit_compl', $obj);   $obj->timecompleted = 18; $DB->insert_record('course_completion_crit_compl', $obj);   $obj->timecompleted = 11; $DB->insert_record('course_completion_crit_compl', $obj);   $obj->userid = 3; $DB->insert_record('course_completion_crit_compl', $obj);   $obj->timecompleted = 21; $DB->insert_record('course_completion_crit_compl', $obj);   // Add some test records $obj = new stdClass(); $obj->course = 2; $obj->criteriatype = 12; $obj->method = 1; $DB->insert_record('course_completion_aggr_methd', $obj);   $obj->method = 2; $DB->insert_record('course_completion_aggr_methd', $obj); The logic behind merging duplicates means that it takes the lowest integer value for the unique columns, but will take an integer over null. To test that these changes do not affect the use of course completion: Enable course completion in site advanced settings Create a new course, enabling completion In the "completion" menu in course settings, enable some criteria Add the completion status block to course Enrol users in the course Have them complete some of the criteria (cron run may be required to have their "completion" register in system) View their progress in the completion status block (as the learner), or in the completion course report as a teacher/siteadmin No coding or critical errors should occur.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      This bug has been turned into a more generic bug than it once was (as the original report is actually caused by multiple issues). This issue and MDL-32203 fix the majority of issues.

      The problem is due to the bug MDL-32203 and the fact that the completion code in cron needs to run at the same time the site is being used a few of the course completion tables can get filled with duplicate records.

      This breaks completion in multiple places due to it's assuming there will be no duplicates.

      Adding unique indexes to the columns will prevent the duplicates being created by other race conditions. This bug will also remove some unused database fields/tables that confuse developers, and most importantly clean up any existing duplicate records (which needs to happen before the indexes can be created).

      OLD DESCRIPTION:

      This is nearly the same problem as in MDL-28021 - somehow there are duplicate entires for course completion landing in the table {$CFG->prefix}_course_completion_crit_compl.

      This causes a 'more than one record in fetch' error for some users in courses with a completion block, effectively blocking them from using moodle.

      I'm not sure of the origin of these duplicates yet, I suggest for a start we delete duplicates and put a unique index on the database; this prevents the access loss issue and should throw an error from the responsible location when an insert is made.

      I don't yet have a way to reproduce this issue but it is confirmed in about half a dozen courses and affects about 200 users in the site...

      Backtrace:
      [error] [client 10.229.5.75] Default exception handler: Found more than one record in fetch() ! Debug:

      • line 429 of /lib/setuplib.php: moodle_exception thrown
      • line 130 of /lib/completion/data_object.php: call to print_error()
      • line 119 of /lib/completion/completion_criteria_completion.php: call to data_object::fetch_helper()
      • line 65 of /lib/completion/data_object.php: call to completion_criteria_completion::fetch()
      • line 273 of /lib/completionlib.php: call to data_object->__construct()
      • line 58 of /blocks/completionstatus/block_completionstatus.php: call to completion_info->get_completions()
      • line 280 of /blocks/moodleblock.class.php: call to block_completionstatus->get_content()

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Votes:
                  7 Vote for this issue
                  Watchers:
                  8 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12