Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            mayank_gupta2005 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_gupta2005 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
            danmarsden 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
            danmarsden 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_gupta2005 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_gupta2005 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
            danmarsden 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
            danmarsden 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_gupta2005 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_gupta2005 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys this has been integrated now.
            Hide
            andyjdavis 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
            andyjdavis 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
            danmarsden Dan Marsden added a comment -

            grrr - sorry have pushed through updated code!

            Show
            danmarsden Dan Marsden added a comment - grrr - sorry have pushed through updated code!
            Hide
            skodak Petr Skoda added a comment -

            fix integrated, thanks

            Show
            skodak Petr Skoda added a comment - fix integrated, thanks
            Hide
            mudrd8mz 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
            mudrd8mz 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:
                  Fix Release Date:
                  5/Dec/11