Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      Testing Instructions

      Basic Functionality

      • Open a course;
      • turn editing on;
      • ensure that both the activities and resources dropdowns are shown; and
      • confirm that all expected modules appear in the lists.

      Disabling and re-enabling plugins

      • navigate to Settings -> Site Administration -> Plugins -> Activity modules;
      • disable the assignment plugin, another activity (e.g. Chat), and a resource (e.g. Page);
      • refresh the course page and confirm that the assignment activity and all subtypes have disappeared, and the other activity and resource you disabled; and
      • re-enable the plugins and refresh your course page to confirm that they're back.

      Correct Functionality

      • try clicking through to each plugin page in turn to ensure that the you are redirected to the module editing page for the correct plugins.
        This is particularly important for assignment subtypes.
      Show
      Testing Instructions Basic Functionality Open a course; turn editing on; ensure that both the activities and resources dropdowns are shown; and confirm that all expected modules appear in the lists. Disabling and re-enabling plugins navigate to Settings -> Site Administration -> Plugins -> Activity modules; disable the assignment plugin, another activity (e.g. Chat), and a resource (e.g. Page); refresh the course page and confirm that the assignment activity and all subtypes have disappeared, and the other activity and resource you disabled; and re-enable the plugins and refresh your course page to confirm that they're back. Correct Functionality try clicking through to each plugin page in turn to ensure that the you are redirected to the module editing page for the correct plugins. This is particularly important for assignment subtypes.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30616-master-7
    • Rank:
      33423

      Description

      In course/lib.php, the print_section_add_menus function does a lot of work to retrieve module metadata (e.g. name and a URL for creating a new instance of the module).

      To create the module chooser in the most efficient manner, we should split out the retrieval of this metadata into a separate function that other functions can use.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi Andrew,

          Looks like a nice clean abstraction thanks.
          The only thing I noted was the spacing in the foreach within get_module_metadata, totally trivial. http://docs.moodle.org/dev/Coding_style#Foreach

          Feel free to put this up for integration when you are ready.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Andrew, Looks like a nice clean abstraction thanks. The only thing I noted was the spacing in the foreach within get_module_metadata, totally trivial. http://docs.moodle.org/dev/Coding_style#Foreach Feel free to put this up for integration when you are ready. Cheers Sam
          Hide
          Andrew Nicols added a comment -

          Thanks Sam,

          I've corrected that foreach spacing - IIRC it was in the original and I preserved that section of code.

          I've also rebased on this weeks master.

          Show
          Andrew Nicols added a comment - Thanks Sam, I've corrected that foreach spacing - IIRC it was in the original and I preserved that section of code. I've also rebased on this weeks master.
          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
          Eloy Lafuente (stronk7) added a comment -

          Uhm... I bet this will conflict with some changes accepted last week about MOD_ARCHETYPE_SYSTEM... checking...

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm... I bet this will conflict with some changes accepted last week about MOD_ARCHETYPE_SYSTEM... checking...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yeah, confirmed.

          Although the conflicts are easily solvable, I think it's better to move this to next week, so you can review if the new archetype needs also to be handled by your new get_module_metadata() function.

          So reopening, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yeah, confirmed. Although the conflicts are easily solvable, I think it's better to move this to next week, so you can review if the new archetype needs also to be handled by your new get_module_metadata() function. So reopening, ciao
          Hide
          Andrew Nicols added a comment -

          I've updated my patch to resolve the merge conflict and tested against the existing test instructions for both bugs

          Show
          Andrew Nicols added a comment - I've updated my patch to resolve the merge conflict and tested against the existing test instructions for both bugs
          Hide
          Andrew Nicols added a comment -

          Updated on latest master

          Show
          Andrew Nicols added a comment - Updated on latest master
          Hide
          Andrew Nicols added a comment -

          Rebased on weekly master - ready for peer review again

          Show
          Andrew Nicols added a comment - Rebased on weekly master - ready for peer review again
          Hide
          Andrew Nicols added a comment -

          Updated on latest master

          Show
          Andrew Nicols added a comment - Updated on latest master
          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
          Andrew Nicols added a comment -

          Rebases cleanly onto latest master

          Show
          Andrew Nicols added a comment - Rebases cleanly onto latest master
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday, with point releases happening next week based on current status.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday, with point releases happening next week based on current status. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some hours ago...

          the main moodle.git repository has 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 - Some hours ago... the main moodle.git repository has 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
          Eloy Lafuente (stronk7) added a comment -

          Interesting one. Let's break master! Yay! I like it. (joking, but can imagine at least 2 ongoing developments that will need to perform 2 adjustments here). Luckily, you arrived first.

          Integrated, thanks! Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Interesting one. Let's break master! Yay! I like it. (joking, but can imagine at least 2 ongoing developments that will need to perform 2 adjustments here). Luckily, you arrived first. Integrated, thanks! Ciao
          Hide
          Sam Hemelryk added a comment -

          It appears there is a bug with this presently that is preventing a user from creating a new assignment.
          I'm looking into this now.

          Show
          Sam Hemelryk added a comment - It appears there is a bug with this presently that is preventing a user from creating a new assignment. I'm looking into this now.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          here "Let's break master" becomes reality. Better I keep my mouth closed next time.

          Show
          Eloy Lafuente (stronk7) added a comment - here "Let's break master" becomes reality. Better I keep my mouth closed next time.
          Hide
          Sam Hemelryk added a comment -

          Problem found, I've applied a patch now for it:

          commit 772685e9830abfeb8dc6c0a9fb4ee104804eb45d
          Author: Sam Hemelryk <sam@moodle.com>
          Date:   Wed Mar 14 13:52:18 2012 +1300
          
              MDl-30616 course: Fixed bug with module subtype links
          
          diff --git a/course/lib.php b/course/lib.php
          index 4a31b80..6282112 100644
          --- a/course/lib.php
          +++ b/course/lib.php
          @@ -1964,7 +1964,7 @@ function get_module_metadata($course, $modnames) {
                               if (get_string_manager()->string_exists('help' . $subtype->name, $modname)) {
                                   $subtype->help = get_string('help' . $subtype->name, $modname);
                               }
          -                    $subtype->link = $urlbase . $type->type;
          +                    $subtype->link = $urlbase . $subtype->type;
                               $group->types[] = $subtype;
                           }
                           $modlist[$course->id][$modname] = $group;
          

          Changes look real good btw Andrew!

          Show
          Sam Hemelryk added a comment - Problem found, I've applied a patch now for it: commit 772685e9830abfeb8dc6c0a9fb4ee104804eb45d Author: Sam Hemelryk <sam@moodle.com> Date: Wed Mar 14 13:52:18 2012 +1300 MDl-30616 course: Fixed bug with module subtype links diff --git a/course/lib.php b/course/lib.php index 4a31b80..6282112 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1964,7 +1964,7 @@ function get_module_metadata($course, $modnames) { if (get_string_manager()->string_exists('help' . $subtype->name, $modname)) { $subtype->help = get_string('help' . $subtype->name, $modname); } - $subtype->link = $urlbase . $type->type; + $subtype->link = $urlbase . $subtype->type; $group->types[] = $subtype; } $modlist[$course->id][$modname] = $group; Changes look real good btw Andrew!
          Hide
          Jason Fowler added a comment -

          All good

          Show
          Jason Fowler added a comment - All good
          Hide
          Eloy Lafuente (stronk7) added a comment -

          FCT (fixed, closing, thanks). Ciao

          "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
          ~ Benjamin Disraeli

          Show
          Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

            People

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

              Dates

              • Created:
                Updated:
                Resolved: