Moodle
  1. Moodle
  2. MDL-7068 META: SCORM 2004 compliance + issues
  3. MDL-28493

objectiveID in SCORM 2004 packages can be text/string type. Alter Database table prefix_scorm_seq_objective structure

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.2
    • Component/s: SCORM
    • Database:
      Any
    • Testing Instructions:
      Hide

      To reproduce this issue follow the steps(with debugging turned on) -
      1. Download the ADL SCORM Test packages CM-02a or CM-02b or CM-03a. You can download them from here - https://github.com/mayankgupta/moodle_scorm_test_harness/tree/master/ADL2004/LMSTestCourseZipPackages
      2. Add these SCORM package as activity inside a course.
      3. Fill in the instructions and save
      4. Check to make sure no debugging messages appear as mention in the description of this issue.

      NOTE: Many other errors are reported by this test package - this patch only addresses the issues mentioned.

      Show
      To reproduce this issue follow the steps(with debugging turned on) - 1. Download the ADL SCORM Test packages CM-02a or CM-02b or CM-03a. You can download them from here - https://github.com/mayankgupta/moodle_scorm_test_harness/tree/master/ADL2004/LMSTestCourseZipPackages 2. Add these SCORM package as activity inside a course. 3. Fill in the instructions and save 4. Check to make sure no debugging messages appear as mention in the description of this issue. NOTE: Many other errors are reported by this test package - this patch only addresses the issues mentioned.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:
      master_MDL-28493
    • Rank:
      1367

      Description

      SCORM 2004 Packages can have objectiveID's as text/string type and not necessary int.
      The ADL SCORM 2004 Test packages have objectiveID as a string value in the imsmanifest.xml file. The objectvieid field in prefix_scorm_seq_objective table has type bigint(10), due to which while adding objectiveid to the scorm_seq_objective table a dml_write_exception is thrown.
      The imsmanifest.xml file for test package CM-02a can be found here - https://github.com/mayankgupta/moodle_scorm_test_harness/blob/master/ADL2004/LMSTestCoursePackageSrc/LMSTestPackage_CM-02a/imsmanifest.xml

      To reproduce this issue follow the steps -
      1. Download the ADL SCORM Test packages CM-02a or CM-02b or CM-03a. You can download them from here - https://github.com/mayankgupta/moodle_scorm_test_harness/tree/master/ADL2004/LMSTestCourseZipPackages
      2. Add these SCORM package as activity inside a course.
      3. Fill in the instructions and save the
      4. You will come across the error - "Error writing to database"
      5. If you have debugging turned on you would get the following debug info

      Debug info: Incorrect integer value: 'obj1' for column 'objectiveid' at row 1
      INSERT INTO mdl_scorm_seq_objective (scoid,primaryobj,objectiveid,minnormalizedmeasure) VALUES(?,?,?,?)
      [array (
      0 => 88,
      1 => 0,
      2 => 'obj1',
      3 => 1,
      )]
      Stack trace:
      line 397 of \lib\dml\moodle_database.php: dml_write_exception thrown
      line 878 of \lib\dml\mysqli_native_moodle_database.php: call to moodle_database->query_end()
      line 920 of \lib\dml\mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
      line 590 of \mod\scorm\datamodels\scormlib.php: call to mysqli_native_moodle_database->insert_record()
      line 238 of \mod\scorm\locallib.php: call to scorm_parse_scorm()
      line 119 of \mod\scorm\lib.php: call to scorm_parse()
      line 410 of \course\modedit.php: call to scorm_add_instance()

        Issue Links

          Activity

          Hide
          Mayank Gupta added a comment -

          Here [1] is a quick patch that changes the objectiveid field in prefix_scorm_seq_objective from int(10) to char(25).

          [1] - https://github.com/mayankgupta/moodle/commit/6b9ad75bc490d697c648b51e776314d2d1e42a8c

          I will be happy to modify it, if required.

          Thanks,
          Mayank.

          Show
          Mayank Gupta added a comment - Here [1] is a quick patch that changes the objectiveid field in prefix_scorm_seq_objective from int(10) to char(25). [1] - https://github.com/mayankgupta/moodle/commit/6b9ad75bc490d697c648b51e776314d2d1e42a8c I will be happy to modify it, if required. Thanks, Mayank.
          Hide
          Dan Marsden added a comment -

          Thanks Mayank - could you please add the code to mod/scorm/db/upgrade.php to change the field type for existing sites too? (you'll need to bump the scorm/version.php too. - as this is a db level fix it will only go into the master branch!

          good work!

          thanks,

          Dan

          Show
          Dan Marsden added a comment - Thanks Mayank - could you please add the code to mod/scorm/db/upgrade.php to change the field type for existing sites too? (you'll need to bump the scorm/version.php too. - as this is a db level fix it will only go into the master branch! good work! thanks, Dan
          Hide
          Mayank Gupta added a comment -

          I considered adding the code to mod/db/upgrade.php, but I was unsure in which version will this patch be pulled. Thus, I did not know with which version should I put a conditional check on (in upgrade.php).
          For now, I will put a check on the current version, when this is pulled the version might need to be changed.

          Thanks,
          Mayank.

          Show
          Mayank Gupta added a comment - I considered adding the code to mod/db/upgrade.php, but I was unsure in which version will this patch be pulled. Thus, I did not know with which version should I put a conditional check on (in upgrade.php). For now, I will put a check on the current version, when this is pulled the version might need to be changed. Thanks, Mayank.
          Hide
          Dan Marsden added a comment -

          great! - don't worry too much about the dates - usually I'll modify it slightly when making an integration request or the integration reviewer may just change the date used to suit. - in the meantime just bump the last digit of the version.php page - thanks!

          Show
          Dan Marsden added a comment - great! - don't worry too much about the dates - usually I'll modify it slightly when making an integration request or the integration reviewer may just change the date used to suit. - in the meantime just bump the last digit of the version.php page - thanks!
          Hide
          Mayank Gupta added a comment -

          I have added the code to upgrade.php as well.
          Link to the commit - https://github.com/mayankgupta/moodle/commit/8607aa9074a53cb35f43b6e880d629651849ac7b

          Thanks,
          Mayank.

          Show
          Mayank Gupta added a comment - I have added the code to upgrade.php as well. Link to the commit - https://github.com/mayankgupta/moodle/commit/8607aa9074a53cb35f43b6e880d629651849ac7b Thanks, Mayank.
          Hide
          Sam Hemelryk added a comment -

          Thanks guys this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks guys this has been integrated now.
          Hide
          Andrew Davis added a comment -

          Hello. I'm afraid that your upgrade code isn't quite correct. Running it causes the site to report this error "ERROR!!! The code you are using is OLDER than the version that made these databases!"

          You are calling upgrade_main_savepoint() instead of upgrade_mod_savepoint(). That means you are setting the version of Moodle core code to 2011073100. Unfortunately the version of Moodle core is only 2011072900.00.

          Instead of this
          upgrade_main_savepoint(true, 2011073100, 'scorm');
          you want this
          upgrade_mod_savepoint(true, 2011073100, 'scorm');

          Show
          Andrew Davis added a comment - Hello. I'm afraid that your upgrade code isn't quite correct. Running it causes the site to report this error "ERROR!!! The code you are using is OLDER than the version that made these databases!" You are calling upgrade_main_savepoint() instead of upgrade_mod_savepoint(). That means you are setting the version of Moodle core code to 2011073100. Unfortunately the version of Moodle core is only 2011072900.00. Instead of this upgrade_main_savepoint(true, 2011073100, 'scorm'); you want this upgrade_mod_savepoint(true, 2011073100, 'scorm');
          Hide
          Dan Marsden added a comment -

          grrr - sorry have pushed through updated code!

          Show
          Dan Marsden added a comment - grrr - sorry have pushed through updated code!
          Hide
          Petr Škoda added a comment -

          fix integrated, thanks

          Show
          Petr Škoda added a comment - fix integrated, thanks
          Hide
          David Mudrak added a comment -

          Veni, vidi, confirmavi.

          I was able to reproduce this issue at PostgreSQL (throws fatal error). The patched version works. Thanks!

          Show
          David Mudrak added a comment - Veni, vidi, confirmavi. I was able to reproduce this issue at PostgreSQL (throws fatal error). The patched version works. Thanks!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: