Moodle
  1. Moodle
  2. MDL-36335

Orphaned activities display error message

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.3.5
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      1. Put your moodle 2.3 site in max debug mode (developer)
      2. Create a course with either Weekly or Topics format with only 2 sections.
      3. In section 1 create any activity, e.g. Page.
      4. In section 2 create any activity, e.g. Page.
      5. In Edit mode, at the bottom of Section 2, click on the minus icon "Reduce the number of sections"
      6. VERIFY there are no errors
      Show
      Put your moodle 2.3 site in max debug mode (developer) Create a course with either Weekly or Topics format with only 2 sections. In section 1 create any activity, e.g. Page. In section 2 create any activity, e.g. Page. In Edit mode, at the bottom of Section 2, click on the minus icon "Reduce the number of sections" VERIFY there are no errors
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Rank:
      45129

      Description

      Orphaned activities

      Notice: Undefined variable: displaysection in \moodle\course\format\renderer.php on line 719
      

      using Moodle 2.3.2+ (Build: 20121101)

      I get this Notice when turning Edit on in a course with "orphaned activities" in a hidden Topic/Section.

        Activity

        Hide
        Michael de Raadt added a comment -

        Hi, Joseph.

        I tried a basic replication by reducing the number of sections in a course, but I didn't see any error messages when I turned editing off then back on again.

        Could you provide some specific replication steps?

        Show
        Michael de Raadt added a comment - Hi, Joseph. I tried a basic replication by reducing the number of sections in a course, but I didn't see any error messages when I turned editing off then back on again. Could you provide some specific replication steps?
        Hide
        Joseph Rézeau added a comment -

        @Michael,
        There is obviously something wrong in function print_multiple_section_page($course, $sections, $mods, $modnames, $modnamesused)
        line 719 uses a non-existent $displaysection parameter
        I have attached a patch to remove this non-existent $displaysection parameter

        Show
        Joseph Rézeau added a comment - @Michael, There is obviously something wrong in function print_multiple_section_page($course, $sections, $mods, $modnames, $modnamesused) line 719 uses a non-existent $displaysection parameter I have attached a patch to remove this non-existent $displaysection parameter
        Hide
        Michael de Raadt added a comment -

        Hi, Joseph.

        Thanks for identifying the code problem and providing a patch.

        It would still be good to get a set of replication steps. Perhaps you could skip ahead and provide a set of testing instructions.

        Show
        Michael de Raadt added a comment - Hi, Joseph. Thanks for identifying the code problem and providing a patch. It would still be good to get a set of replication steps. Perhaps you could skip ahead and provide a set of testing instructions.
        Hide
        Joseph Rézeau added a comment -

        @Michael,
        There is no need to provide replication steps. The bug I spotted (in 2.3) is real, and it has already been fixed in 2.4.
        In Moodle 2.4dev (Build: 20121101), file moodle/course/format/rendered.php line 707 has been corrected to:
        print_section($course, $thissection, null, null, true, "100%", false, 0); (as suggested in my patch)
        The same fix should be applied to moodle 2.3.

        Show
        Joseph Rézeau added a comment - @Michael, There is no need to provide replication steps. The bug I spotted (in 2.3) is real, and it has already been fixed in 2.4. In Moodle 2.4dev (Build: 20121101), file moodle/course/format/rendered.php line 707 has been corrected to: print_section($course, $thissection, null, null, true, "100%", false, 0); (as suggested in my patch) The same fix should be applied to moodle 2.3.
        Hide
        Michael de Raadt added a comment -

        Hi, Joseph.

        We will still need to test this change at some stage, so testing instructions will be needed.

        Show
        Michael de Raadt added a comment - Hi, Joseph. We will still need to test this change at some stage, so testing instructions will be needed.
        Hide
        Joseph Rézeau added a comment -

        Testing instructions
        0.- Put your moodle 2.3 site in max debug mode (developer)
        1.- Create a course with either Weekly or Topics format with only 2 sections.
        2.- In section 1 create any activity, e.g. Page.
        3.- In section 2 create any activity, e.g. Page.
        4.- In Edit mode, at the bottom of Section 2, click on the minus icon "Reduce the number of sections"
        5.- In Section 2 check that you see:
        a) the heading "Orphaned activities"
        b) an error message:
        Notice: Undefined variable: displaysection in moodle\course\format\renderer.php on line 719"

        Show
        Joseph Rézeau added a comment - Testing instructions 0.- Put your moodle 2.3 site in max debug mode (developer) 1.- Create a course with either Weekly or Topics format with only 2 sections. 2.- In section 1 create any activity, e.g. Page. 3.- In section 2 create any activity, e.g. Page. 4.- In Edit mode, at the bottom of Section 2, click on the minus icon "Reduce the number of sections" 5.- In Section 2 check that you see: a) the heading "Orphaned activities" b) an error message: Notice: Undefined variable: displaysection in moodle\course\format\renderer.php on line 719"
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that and providing a patch.

        Show
        Michael de Raadt added a comment - Thanks for reporting that and providing a patch.
        Hide
        Michael de Raadt added a comment -

        I was able to reproduce this in 2.3+, but not in master (2.4).

        Show
        Michael de Raadt added a comment - I was able to reproduce this in 2.3+, but not in master (2.4).
        Hide
        Joseph Rézeau added a comment -

        @Michel, yes, the bug has been fixed in master, as I said in my comment dated 06/Nov/12 4:10 PM.

        Show
        Joseph Rézeau added a comment - @Michel, yes, the bug has been fixed in master, as I said in my comment dated 06/Nov/12 4:10 PM.
        Hide
        Adrian Greeve added a comment -

        I made a very tiny change to the patch to bring it in line with the same file in 2.4+

        Show
        Adrian Greeve added a comment - I made a very tiny change to the patch to bring it in line with the same file in 2.4+
        Hide
        Rajesh Taneja added a comment -

        Thanks Adrian,

        Patch looks good.
        [y] Syntax
        [y] Output
        [y] Whitespace
        [-] Language
        [-] Databases
        [y] Testing
        [-] Security
        [-] Documentation
        [y] Git
        [y] Sanity check

        FYI: Adding 0 as last parameter makes sense, as it is used as section return parameter in editing icons. Although if it's not provided it will be default to 0 on editing pages and hence Joseph's patch was also working.

        Show
        Rajesh Taneja added a comment - Thanks Adrian, Patch looks good. [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check FYI: Adding 0 as last parameter makes sense, as it is used as section return parameter in editing icons. Although if it's not provided it will be default to 0 on editing pages and hence Joseph's patch was also working.
        Hide
        Adrian Greeve added a comment -

        I've created a patch with both commits. The integrators can decide which one they want to use. Both work.

        Show
        Adrian Greeve added a comment - I've created a patch with both commits. The integrators can decide which one they want to use. Both work.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now.

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now.
        Hide
        Ankit Agarwal added a comment -

        Am getting:-

        Performance warning: condition_info constructor is faster if you pass in a $item from get_fast_modinfo or the equivalent for sections. [This warning can be disabled, see phpdoc.]
        
            line 417 of /lib/conditionlib.php: call to debugging()
            line 187 of /lib/conditionlib.php: call to condition_info_base->__construct()
            line 417 of /course/format/renderer.php: call to condition_info_section->__construct()
            line 179 of /course/format/renderer.php: call to format_section_renderer_base->section_availability_message()
            line 718 of /course/format/renderer.php: call to format_section_renderer_base->section_header()
            line 52 of /course/format/topics/format.php: call to format_section_renderer_base->print_multiple_section_page()
            line 281 of /course/view.php: call to require()
        

        That doesn't look related to this patch, is that correct?
        Thanks

        Show
        Ankit Agarwal added a comment - Am getting:- Performance warning: condition_info constructor is faster if you pass in a $item from get_fast_modinfo or the equivalent for sections. [This warning can be disabled, see phpdoc.] line 417 of /lib/conditionlib.php: call to debugging() line 187 of /lib/conditionlib.php: call to condition_info_base->__construct() line 417 of /course/format/renderer.php: call to condition_info_section->__construct() line 179 of /course/format/renderer.php: call to format_section_renderer_base->section_availability_message() line 718 of /course/format/renderer.php: call to format_section_renderer_base->section_header() line 52 of /course/format/topics/format.php: call to format_section_renderer_base->print_multiple_section_page() line 281 of /course/view.php: call to require() That doesn't look related to this patch, is that correct? Thanks
        Hide
        Ankit Agarwal added a comment -

        Rest works as described.
        Thanks

        Show
        Ankit Agarwal added a comment - Rest works as described. Thanks
        Hide
        Adrian Greeve added a comment -

        Hi Ankit,

        No that isn't related to this patch.

        Show
        Adrian Greeve added a comment - Hi Ankit, No that isn't related to this patch.
        Hide
        Ankit Agarwal added a comment -

        Great
        Passing Thanks

        Show
        Ankit Agarwal added a comment - Great Passing Thanks
        Hide
        Dan Poltawski added a comment -

        Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

        Show
        Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: