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

      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

            Mayank Gupta created issue -
            Mayank Gupta made changes -
            Field Original Value New Value
            Fix Version/s DEV backlog [ 10464 ]
            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.
            Dan Marsden made changes -
            Labels triaged
            Assignee Eloy Lafuente (stronk7) [ stronk7 ] Dan Marsden [ danmarsden ]
            Component/s Database SQL/XMLDB [ 10131 ]
            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
            Dan Marsden made changes -
            Labels triaged SCORM_2004 triaged
            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!
            Dan Marsden made changes -
            Link This issue duplicates MDL-24475 [ MDL-24475 ]
            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.
            Dan Marsden made changes -
            Status Open [ 1 ] Waiting for integration review [ 10010 ]
            Pull Master Diff URL https://github.com/danmarsden/moodle/compare/master...master_MDL-28493
            Pull Master Branch master_MDL-28493
            Pull from Repository git://github.com/danmarsden/moodle.git
            Testing Instructions Create SCORM using one of the linked items in the description
            Dan Marsden made changes -
            Testing Instructions Create SCORM using one of the linked items in the description 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.
            Petr Skoda made changes -
            Currently in integration Yes
            Dan Marsden made changes -
            Link This issue blocks MDL-28295 [ MDL-28295 ]
            Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            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.
            Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Andrew Davis made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester andyjdavis
            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');
            Andrew Davis made changes -
            Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
            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!
            Petr Skoda made changes -
            Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
            Hide
            Petr Skoda added a comment -

            fix integrated, thanks

            Show
            Petr Skoda added a comment - fix integrated, thanks
            Petr Skoda made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Petr Skoda made changes -
            Fix Version/s 2.2 [ 10656 ]
            Fix Version/s DEV backlog [ 10464 ]
            David Mudrak made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester andyjdavis mudrd8mz
            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!
            David Mudrak made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Petr Skoda made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes
            Integration date 3/Aug/11
            Mayank Gupta made changes -
            Assignee Dan Marsden [ danmarsden ] Mayank Gupta [ mayank_gupta2005 ]
            Dan Marsden made changes -
            Assignee Mayank Gupta [ mayank_gupta2005 ] Dan Marsden [ danmarsden ]
            Dan Marsden made changes -
            Testing Instructions 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.
            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.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: