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

SCORM dnd not respecting default for maxattempt

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.1
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      Upgrade test:

      BEFORE UPRGADE (e.g. 2.2 install)

      1. Go Admin > Plugins > activities > Scorm
      2. Set maxattempts to something memberable

      RUN UPGRADE

      1. Verify that maxattempts is set as it was before upgrade in interface

      After upgrade test:

      1. in Admin > plugins > activities > SCORM set the maxattempt setting to something.
      2. use the standard Add an activity dialog to add a SCORM and check to make sure the default used on the admin settings page is correct.
      3. Add a SCORM package using Dnd - check to make sure the created SCORM uses the correct maxattempt default as previously set in the admin page.
      Show
      Upgrade test: BEFORE UPRGADE (e.g. 2.2 install) Go Admin > Plugins > activities > Scorm Set maxattempts to something memberable RUN UPGRADE Verify that maxattempts is set as it was before upgrade in interface After upgrade test: in Admin > plugins > activities > SCORM set the maxattempt setting to something. use the standard Add an activity dialog to add a SCORM and check to make sure the default used on the admin settings page is correct. Add a SCORM package using Dnd - check to make sure the created SCORM uses the correct maxattempt default as previously set in the admin page.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      master_MDL-33938

      Description

      the SCORM maxattempt admin default uses the name "maxattempts" but the db field is "maxattempt" - this causes issues when using get_config('scorm') to set up a list of default values for creation of a SCORM package - especially when using dnd.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Dan,
            changes look good. But won't all existing install loose there config setting for maxattempt?

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Dan, changes look good. But won't all existing install loose there config setting for maxattempt? Thanks
            Hide
            danmarsden Dan Marsden added a comment -

            thanks Ankit - yeah I went to add some upgrade code to convert the old var but didn't quite finish it - will try to add it tonight and submit for integration - thanks.

            Show
            danmarsden Dan Marsden added a comment - thanks Ankit - yeah I went to add some upgrade code to convert the old var but didn't quite finish it - will try to add it tonight and submit for integration - thanks.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Dan,

            Looks like this needs to be backported to 2.3 at least.
            Can you please produce branches for the versions you wish this to land for.
            If its master only no probs, but will be delayed for a week as master and 23 are being kept in sync this week.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Dan, Looks like this needs to be backported to 2.3 at least. Can you please produce branches for the versions you wish this to land for. If its master only no probs, but will be delayed for a week as master and 23 are being kept in sync this week. Cheers Sam
            Hide
            danmarsden Dan Marsden added a comment -

            yeah - I think I wrote this before 2.3 was released - so if if could be added to 23 and master that would be good thanks. - it should cherry-pick ok to both branches.

            Show
            danmarsden Dan Marsden added a comment - yeah - I think I wrote this before 2.3 was released - so if if could be added to 23 and master that would be good thanks. - it should cherry-pick ok to both branches.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to 23 and master, thanks.

            Show
            poltawski Dan Poltawski added a comment - Integrated to 23 and master, thanks.
            Hide
            fred Frédéric Massart added a comment -

            Test successful!

            Show
            fred Frédéric Massart added a comment - Test successful!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Congratulations your code is upstream - gold star for you!

            This issue + 79 others made it in in time for the minor releases.
            Thank you everyone involved for your exuberant efforts.

            Show
            samhemelryk Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Jul/12