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

Find all the places were section number is misused as a section id


    • Icon: Task Task
    • Resolution: Done
    • Icon: Minor Minor
    • None
    • 4.4
    • Course
    • HQ 2023 Sprint I4.3 Moppies

      A bit of context

      This issue is part of the course hierarchy project. The goal is to allow one-level subsections inside the course content. This will require deprecating many course global functions, especially the ones using section numbers to identify a section (instead of using the section id or the section_info instance) and abusing primitive parameters.

      What is this task about?

      Detect all bad uses of section numbers and propose alternative API methods to replace them.

      Once this is done, several issues will be created to deprecate the old global functions and replace them by new namespaced API methods  (on the location determined by MDL-80000).

      Deprecation plan

      The deprecation plan details are described in this document:



      Methods detected

      I added all the detected methods in the description because it is more editable than a tracker comment.

      Methods in course/lib.php

      set_section_visible($courseid, $sectionnumber, $visibility) ---> formatactions::section($courseid)->set_visible($sectionid, $visibility)
      course_create_section($courseorid, $position, $skipcheck) ---> formatactions::section($courseid)->create($position, $skipcheck)
      course_create_sections_if_missing($courseorid, $sections) ---> formatactions::section($courseid)->create_if_missing($sectionnums)
      move_section_to($course, $section, $destination, $ignorenumsections) ---> formatactions::section($courseid)->move_after($section, $aftersection);
      course_delete_section($course, $section, $forcedeleteifnotempty, $async) ---> formatactions::section($courseid)->delete($course, $section, $forcedeleteifnotempty, $async)course_update_section($course, $section, $data) ---> formatactions::section($courseid)->update($section, $data)course_add_cm_to_section($courseorid, $cmid, $sectionum, $beforemod): int ---> formatactions::section($courseid)->add_cm($section, $data)moveto_module($mod, $section, $beforemod) ---> formatactions::section($courseid)->move_cm($sectionorid, $cm, $beforecm)
      set_downloadcontent($id, bool $downloadcontent): bool ---> formatactions::course($courseid)->set_download_content($downloadcontent);
      course_change_visibility($courseid, $show) ---> formatactions::course($courseid)->set_visibility($show = true);
      course_delete_module($cmid, $async) ---> formatactions::cm($courseid)->delete($cmid, $async = false);
      delete_mod_from_section($modid, $sectionid) ---> formatactions::section($courseid)->dettach_cm($sectionid, $modid)
      set_coursemodule_visible($id, $visible, $visibleoncoursepage = 1, bool $rebuildcache = true) ---> formatactions::cm($courseid)->set_visible($cmid, $visible, $visibleoncoursepage, $rebuildcache)
      set_coursemodule_name($id, $name) ---> formatactions::cm($courseid)->set_name($cmdi, $name)
      set_coursemodule_groupmode($id, $groupmode) ---> formatactions::cm($courseid)->set_groupmode($cmid, $groupmode)
      set_coursemodule_idnumber($id, $idnumber) ---> formatactions::cm($courseid)->set_idnumber($cmid, $idnumber)
      add_course_module($mod) ---> formatactions::cm($courseid)->add($cmdata);create_module($moduleinfo) ---> formatactions::cm($courseid)->create($cmdata)
      update_module($moduleinfo) ---> formatactions::cm($courseid)->update($cmdata)
      mod_duplicate_activity($course, $cm, $unsued) ---> formatactions::cm($courseid)->duplicate($course, $cm) course_view($context, $sectionnumber) --> this method will not be compatible with delegated sections. The method should be deprecated and create a new "course_viewed($context, $sectionorid)". The method should store both sectionnum and sectionid in the event.get_section_name($courseorid, $section): string ---> this $section is the section num. The method should be deprecated and only use the format->get_section_name_by_numbercourse_can_delete_section($course, $section) ---> this method must be deprecated and replaced by a new core_courseformat\base::can_delete_section_content($sectionid)


      This list was created with the modinfolib after applying MDL-79999. Apart from the ambiguous section variables, the analysis also includes methods that should change their names due to ambiguous meaning. It also takes into consideration that delegate sections will exist soon.

      Methods from course_modinfo:

      get_sections(): array --> the meaning is unclear as it returns an array of arrays: sectionnum => cm_info[]. --> rename to get_listed_section_modules(): array
      get_section_info_all(): section_info[] --> with delegate sections this method has an ambiguous name --> rename to get_section_list(): section_info[]
      get_section_info($sectionnumber, $strictness = IGNORE_MISSING): ?section_info --> not critical but it is an ambiguous name --> it is optional but it could be renamed to get_section_info_by_number


      prepare_new_moduleinfo_data($course, $modulename, $section, string $suffix = ''): array ---> should be deprecated and create a new one using section id.
      get_moduleinfo_data($cm, $course) ---> internally is using section number but it sould also include the section id.


      Moodle net has some methods using section numbers to identify course content. This way of generating section URLs will stop working when indexing delegate section URLs:


      generate_mnet_endpoint(string $profileurl, int $course, int $section): string ---> should use $sectionid and the new seciton urls from MDL-79986
      verify_webfinger(string $profuleurl, int $course, int $section): array ---> same as the preivous one, should use $sectionid instead of section number.
      import_processor::__construct(...) ---> uses $section number to identify the section.
      import_strategy_file::import(...) ---> the $section variable is not used. Most probably it can be renamed to $unused.
      import_strategy_link::import(...) ---> the $section variable is not used. Most probably it can be renamed to $unused.


      convert_legacy_fields($rec, $section, $modgroupmembersonlyignored = false): ?string --> the $section param is a boolean. It should be renamed to $issection.


      dndupload_ajax_processor::__construct($courseid, $section, $type, $modulename) ---> this class shoud use $sectionid not $sectionnum. However, it is not easy because it is the constructor. We must evaluate the possibility of deprecating the full class (which will be overkill). Alternatively, $sectionnum must be nullable, add a new param ?int $sectionid, then evaluate adding a new static method dndupload_ajax_processor::instance($courseid, $sectionid, $type, $modulename)



      view_course($courseid, $sectionnumber = 0) --> cannot mark a delegated section as viewed
      edit_module($action, $id, $sectionreturn = null)
      get_module($id, $sectionreturn = null)
      edit_section($action, $id, $sectionreturn)


      moodleform_mod::__construct($current, $section, $cm, $course) ---> this method does not allow to create activities using sectionid. This fiull class should be refactored to use $sectionid instead of $sectionnum. However, this will be difficult as the class is extended by all activity modules.


      course_section_add_cm_control($course, $section, $sectionreturn = null, $displayoptions = array()) ---> the phpdoc is wrong and must be fixed, the $section should be renamed to $sectionid. However, the methods must be deprecated and replaced by one using $sectionreturnid, otherwise it won't work with delegated sections.



      This class has many methods using section number instead of section id.

      get_section($section, $strictness) --> this method is just a proxy to the course_modinfo methods. It must be deprecated.
      get_section_name($section): string ---> this method uses section number. The param must be renamed to $sectionornum. It is unclear if this method is needed for delegate sections (it can be provided for the delegate plugin instead). In that case, the method will be deprecated and a two new methods will appear: get_section_name_by_number and get_section_name_by_id.
      get_default_section_name($section): string ---> same as get_section_name. The param should be renamed to $sectionornum. If needed for the delegate sections, it will be deprecated and will be replaced by one using $sectionorid.
      set_section_number(int $singlesection) ---> it won't works with delegate sections. The method may remain for legacy uses but a new set_section_id must be created. Internally, the single section must be stored as a section id
      get_section_number() ---> a new get_seciton_id must be created and all rendering logic using get_section_number must be replaced for get_section_id.
      get_view_url($section, $options) ---> the use of section number must be deprecated in front of using seciton_info. Internally, section id must be used.
      get_format_options($section) ---> this $section param could be almost anything. Using the section number must be deprecated and only allow null|stdClass|section_info. can_delete_section($section) ---> the $section param is not documented. However, checking where is called it is $sectionnum. As comented in course/lib.php, a new core_courseformat\base::can_delete_section_content($sectionid) must be implemented based on the course_can_delete_section. And the can_delete_section must be deprecated and a new can_delete_section_id must be created.delete_section($section, $forcedeleteifnotempty = false) ---> the method must be deprecated and must be replaced by a format section action.move_section_after(section_info $section, section_info $destination) ---> should be replaced by formatactions::section($courseid)->move_after($section, $aftersection)allow_stealth_module_visibility


      get_coursemodule_from_id($modulename, $cmid, $courseid, $sectionnum...) ---> this method is not compatible with delegated sections. The use of the $sectionnum must be investigate to see if it can be deprecated with an $unsused.
      get_coursemodule_from_instance(...) ---> same as get_coursemodule_from_id. It is unclear why the section number is needed to get the coursemodule instance. The param should be deprecated.


      This library has two methods (load_course_sections and load_section_activities) that use section numbers. However, it is still too soon to know if they will be affected by the delegated sections.

            tusefomal Ferran Recio
            tusefomal Ferran Recio
            0 Vote for this issue
            7 Start watching this issue


                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.