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
    • Rank:
      26875

      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.

        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: