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

Attempts grading dropdown initialized incorrectly when updating SCORM/AICC

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.5
    • Fix Version/s: 1.9.10
    • Component/s: SCORM
    • Labels:
      None
    • Environment:
      Linux, Apache 2.2, php 5.2.10, Mysql 5.1.34
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      When using the update SCORM/AICC form, the "Attempts Grading" option is always initialized to "Highest Attempt", no matter option is actually stored in the database.

      The problem seems to be line 192 in mod/scorm/mod_form.php

      Changing

      if (isset($default_values['grademethod']))

      { $default_values['whatgrade'] = intval($default_values['grademethod'] / 10); $default_values['grademethod'] = $default_values['grademethod'] % 10; }

      to

      if (isset($default_values['grademethod']))

      { $default_values['grademethod'] = $default_values['grademethod'] % 10; }

      seems to fix the problem. It looks like 'whatgrade' and 'grademethod' were, at some point, combined into one database column and that now 'whatgrade' has its own database column, but not all the code was changed.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            danmarsden Dan Marsden added a comment -

            Hi Bruce,

            thanks for the great bug report - I've just returned from leave, but will try to have a look at this early next week!

            Show
            danmarsden Dan Marsden added a comment - Hi Bruce, thanks for the great bug report - I've just returned from leave, but will try to have a look at this early next week!
            Hide
            bcota@unicon.net Bruce Cota added a comment -

            We had additional related problems. I went through and tried to clean out all vestiges of using one database column to hold two values. We haven't had problems since. Attached are the differences (remember it's version 1.9.5).

            Show
            bcota@unicon.net Bruce Cota added a comment - We had additional related problems. I went through and tried to clean out all vestiges of using one database column to hold two values. We haven't had problems since. Attached are the differences (remember it's version 1.9.5).
            Hide
            dongsheng Dongsheng Cai added a comment -

            Hi Dan

            Are you planing to apply this to moodle? Or is there any other plan for this?

            Thanks!

            Regards,
            Dongsheng Cai

            Show
            dongsheng Dongsheng Cai added a comment - Hi Dan Are you planing to apply this to moodle? Or is there any other plan for this? Thanks! Regards, Dongsheng Cai
            Hide
            danmarsden Dan Marsden added a comment -

            makes sense to me - just haven't had a chance to review the patch yet

            Show
            danmarsden Dan Marsden added a comment - makes sense to me - just haven't had a chance to review the patch yet
            Hide
            danmarsden Dan Marsden added a comment -

            finally had a chance to review this... problem is that it may cause older scorm objects to fail.

            It's possible that sites who created/uploaded objects before they were split into 2 columns might have problems with these objects and the grading methods not working as originally set.

            Ideally we should write an upgrade script to search/find all these old objects and convert them to using 2 columns. which makes this something that will probably take a bit more time (unless someone else writes the upgrade script for us!)

            thanks,

            Dan

            Show
            danmarsden Dan Marsden added a comment - finally had a chance to review this... problem is that it may cause older scorm objects to fail. It's possible that sites who created/uploaded objects before they were split into 2 columns might have problems with these objects and the grading methods not working as originally set. Ideally we should write an upgrade script to search/find all these old objects and convert them to using 2 columns. which makes this something that will probably take a bit more time (unless someone else writes the upgrade script for us!) thanks, Dan
            Hide
            bcota@unicon.net Bruce Cota added a comment -

            Ahh, we did this:

            update mdl_scorm set grademethod=grademethod%10;

            I guess if there are scorms that were created before the whatgrade column existed, you'd need

            update mdl_scorm set whatgrade = grademethod/10;
            udpate mdl-scorm set grademethod = grademethod%10;

            Show
            bcota@unicon.net Bruce Cota added a comment - Ahh, we did this: update mdl_scorm set grademethod=grademethod%10; I guess if there are scorms that were created before the whatgrade column existed, you'd need update mdl_scorm set whatgrade = grademethod/10; udpate mdl-scorm set grademethod = grademethod%10;
            Hide
            danmarsden Dan Marsden added a comment -

            I've just pushed this fix into HEAD - will need to check with DB gurus to make sure the sql is works cross-db

            thanks for the report and patch!

            Show
            danmarsden Dan Marsden added a comment - I've just pushed this fix into HEAD - will need to check with DB gurus to make sure the sql is works cross-db thanks for the report and patch!
            Hide
            danmarsden Dan Marsden added a comment -

            modified db scripts - less efficient but are much more likely to work on all our supported dbs.

            Show
            danmarsden Dan Marsden added a comment - modified db scripts - less efficient but are much more likely to work on all our supported dbs.
            Hide
            danmarsden Dan Marsden added a comment -

            fix now in 1.9Stable and Head - using a config var to prevent the upgrade scripts in 1.9stable and 2.0 from clashing and running twice. Please test this and make sure I've got it all!

            Show
            danmarsden Dan Marsden added a comment - fix now in 1.9Stable and Head - using a config var to prevent the upgrade scripts in 1.9stable and 2.0 from clashing and running twice. Please test this and make sure I've got it all!
            Hide
            mudrd8mz David Mudrák added a comment -

            Hi Dan,

            small regression caused by http://github.com/moodle/moodle/commit/d29c03772555530b197753d2859b19348bc5f0fe

            Warning:  Invalid argument supplied for foreach() in /.../moodle19/mod/scorm/db/upgrade.php  on line 315

            You run foreach() over false returned by get_records() if there are no SCORMs

            Show
            mudrd8mz David Mudrák added a comment - Hi Dan, small regression caused by http://github.com/moodle/moodle/commit/d29c03772555530b197753d2859b19348bc5f0fe Warning: Invalid argument supplied for foreach() in /.../moodle19/mod/scorm/db/upgrade.php on line 315 You run foreach() over false returned by get_records() if there are no SCORMs
            Hide
            danmarsden Dan Marsden added a comment -

            doh - sloppy code - should know better than that! - thanks David, fixed.

            Show
            danmarsden Dan Marsden added a comment - doh - sloppy code - should know better than that! - thanks David, fixed.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Oct/10