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

find_all_amd_modules() does not use proper frankenstyle for subsystems

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 3.0.4, 3.1, 3.2
    • Fix Version/s: 3.0.6, 3.1.2
    • Component/s: JavaScript
    • Labels:
    • Testing Instructions:
      Hide

      Run phpunit tests.

      1. Add an AMD module to a subsystem (e.g. group)
      2. Try and "require" it without the "core_" prefix on the component name :

        require(['group/mymodule'], function(g) { console.log(g); });
        

        It should not work.

      3. Try and "require" it with the "core_" prefix on the component name :

        require(['core_group/mymodule'], function(g) { console.log(g); });
        

        It should work.

      (Note: if you run grunt on the above example code it will complain about console.log - either use --force or invent your own test code).

      Show
      Run phpunit tests. Add an AMD module to a subsystem (e.g. group) Try and "require" it without the "core_" prefix on the component name : require(['group/mymodule'], function(g) { console.log(g); }); It should not work. Try and "require" it with the "core_" prefix on the component name : require(['core_group/mymodule'], function(g) { console.log(g); }); It should work. (Note: if you run grunt on the above example code it will complain about console.log - either use --force or invent your own test code).
    • Affected Branches:
      MOODLE_30_STABLE, MOODLE_31_STABLE, MOODLE_32_STABLE
    • Fixed Branches:
      MOODLE_30_STABLE, MOODLE_31_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-55133-master

      Description

      Was integrating MDL-55000, first amd module belonging to a subsystem, apparently, when I detected that it was not using proper component name but just the subsystem name.

      So things like this was working:

      $PAGE->requires->js_call_amd('grades/edittree_index', 'enhance');
      

      But this was not (and is the correct one):

      $PAGE->requires->js_call_amd('core_grades/edittree_index', 'enhance');
      

      Finally, curiously using it in a -lazy way, was working ok with correct component name:

      $PAGE->requires->js_call_amd('core_grades/edittree_index-lazy', 'enhance');
      

      Following the code, the problem seems to be @ lib/classes/requirejs.php where the directories for subsystems are added by name, not by component. I just applied ths quick and dirty hack and then 'core_grades' started to work:

      diff --git a/lib/classes/requirejs.php b/lib/classes/requirejs.php
      index 9894dc5..ebe4e6f 100644
      --- a/lib/classes/requirejs.php
      +++ b/lib/classes/requirejs.php
      @@ -90,7 +90,7 @@ class core_requirejs {
               $subsystems = core_component::get_core_subsystems();
               foreach ($subsystems as $subsystem => $dir) {
                   if (!empty($dir) && is_dir($dir . '/amd')) {
      -                $jsdirs[$subsystem] = $dir . '/amd';
      +                $jsdirs['core_' . $subsystem] = $dir . '/amd';
                   }
               }
               $plugintypes = core_component::get_plugin_types();
      

      Surely we should change it to always use correct component names, both for plugins (working already) and subsystems, covered with unit tests, before we start spreading amd modules over subsystems. That will make things work as documented (by component name) and will allow us to use correct @module, @package and js_call_amd() calls. And also will allow to move some current modules (tag....) to their corresponding subsystem.

      For your consideration, ciao

      PS: I'd consider this a bug at all effects.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              damyon Damyon Wiese
              Reporter:
              stronk7 Eloy Lafuente (stronk7)
              Peer reviewer:
              Ryan Wyllie Ryan Wyllie
              Integrator:
              David Monllaó David Monllaó
              Tester:
              Mark Nelson Mark Nelson
              Participants:
              Component watchers:
              Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                12/Sep/16