Moodle
  1. Moodle
  2. MDL-21568

Attempts grading dropdown initialized incorrectly when updating SCORM/AICC

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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

          Activity

          Hide
          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
          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
          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
          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 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 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
          Dan Marsden added a comment -

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

          Show
          Dan Marsden added a comment - makes sense to me - just haven't had a chance to review the patch yet
          Hide
          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
          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
          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
          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
          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
          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
          Dan Marsden added a comment -

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

          Show
          Dan Marsden added a comment - modified db scripts - less efficient but are much more likely to work on all our supported dbs.
          Hide
          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
          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
          David Mudrak 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
          David Mudrak 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
          Dan Marsden added a comment -

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

          Show
          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: