Moodle
  1. Moodle
  2. MDL-36795

In the default course settings, numsections is not limited to maxsections

    Details

    • Testing Instructions:
      Hide

      Test steps

      1. Log in as the administrator.
      2. Go to [Settings ► Site administration ► Courses ► Course default settings].
      3. Set 'Maximum for number of weeks/topics' to 0 and save changes.
        • Check that 'Number of weeks/topics' is set to 0 and that this is the only option.
      4. Set 'Maximum for number of weeks/topics' to nothing and save changes.
        • Check that 'Number of weeks/topics' now has the option to go up to 52.
      5. Create a new course [Settings ► Site administration ► Courses ► Add/edit courses].
      6. Select 'Weekly format'.
        • In the section 'Formatting options for Weekly format' check that 'Number of weeks/topics' has the option to go up to 52.
      7. Change the format to 'Topics format'.
        • In the section 'Formatting options for Topics format' check that 'Number of weeks/topics' has the option to go up to 52.
      Show
      Test steps Log in as the administrator. Go to [Settings ► Site administration ► Courses ► Course default settings] . Set 'Maximum for number of weeks/topics' to 0 and save changes. Check that 'Number of weeks/topics' is set to 0 and that this is the only option. Set 'Maximum for number of weeks/topics' to nothing and save changes. Check that 'Number of weeks/topics' now has the option to go up to 52. Create a new course [Settings ► Site administration ► Courses ► Add/edit courses] . Select 'Weekly format'. In the section 'Formatting options for Weekly format' check that 'Number of weeks/topics' has the option to go up to 52. Change the format to 'Topics format'. In the section 'Formatting options for Topics format' check that 'Number of weeks/topics' has the option to go up to 52.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-36795-master
    • Rank:
      46310

      Description

      When controlling the maximum number of sections allowed in a course and the default number of sections in a new course, the number of sections should be limited by the maximum value.

      Reproduction steps:

      1. Log in as admin
      2. Navigate to "Course default settings" (Settings -> Site administration -> Courses -> Course default settings)
      3. Change "Maximum for number of weeks/topics moodlecourse | maxsections" to 0
      4. Click save changes
      5. Check "Number of weeks/topics moodlecourse | numsections" drop-down

      Expected result: The number of weeks/topics should only show 0

      Actual result: The number of weeks/topics shows numbers from 0 to 52

      Currently the function that controls this value is in lib/adminlib.php...

      lib/adminlib.php, lines 3813 to 3829
      class admin_settings_num_course_sections extends admin_setting_configselect {
          public function __construct($name, $visiblename, $description, $defaultsetting) {
              parent::__construct($name, $visiblename, $description, $defaultsetting, array());
          }
      
          /** Lazy-load the available choices for the select box */
          public function load_choices() {
              $max = get_config('moodlecourse', 'maxsections');
              if (empty($max)) {
                  $max = 52;
              }
              for ($i = 0; $i <= $max; $i++) {
                  $this->choices[$i] = "$i";
              }
              return true;
          }
      }
      

      The check to see if the maximum value is set fails to allow for a value of zero. Changing the check to use !isset() instead of empty resolves this.

      if (!isset($max)) {
      

        Issue Links

          Activity

          Hide
          Rajesh Taneja added a comment -

          Thanks Adrian,

          Patch makes sense, but it will be nice to check if maxsection is not a number rather then empty string https://github.com/abgreeve/moodle/compare/moodle:master...wip-MDL-36795-master#L0R217

          As config can save maxsection as string, it might be hard to debug for any user who has space stored there.

          Show
          Rajesh Taneja added a comment - Thanks Adrian, Patch makes sense, but it will be nice to check if maxsection is not a number rather then empty string https://github.com/abgreeve/moodle/compare/moodle:master...wip-MDL-36795-master#L0R217 As config can save maxsection as string, it might be hard to debug for any user who has space stored there.
          Hide
          Adrian Greeve added a comment -

          Thanks Raj,

          I've changed the check to just look at whether the value is numeric or not.

          Show
          Adrian Greeve added a comment - Thanks Raj, I've changed the check to just look at whether the value is numeric or not.
          Hide
          Adrian Greeve added a comment -

          After further talks with Raj, I've put back the check to see if the variable is set, just to be safe.

          Show
          Adrian Greeve added a comment - After further talks with Raj, I've put back the check to see if the variable is set, just to be safe.
          Hide
          Rajesh Taneja added a comment -

          Thanks Adrian,

          Patch looks good.

          Show
          Rajesh Taneja added a comment - Thanks Adrian, Patch looks good.
          Hide
          Adrian Greeve added a comment -

          I've created branches for 2.3 and 2.2.
          They are different from master so they must be tested if they are integrated.

          Show
          Adrian Greeve added a comment - I've created branches for 2.3 and 2.2. They are different from master so they must be tested if they are integrated.
          Hide
          Sam Hemelryk added a comment -

          Thanks Adrian, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Adrian, this has been integrated now.
          Hide
          Andrew Davis added a comment -

          Works as described. Passing.

          Show
          Andrew Davis added a comment - Works as described. Passing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Y E S !

          Closing as fixed, many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Y E S ! Closing as fixed, many thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: