Moodle
  1. Moodle
  2. MDL-35218

META: Course formats refactoring for 2.4

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: None
    • Component/s: Course
    • Labels:
    • Rank:
      43859

      Description

      Use object-oriented model and allow more flexibility.
      See http://docs.moodle.org/dev/Course_formats_2 and child issues

      For more changes to course display functions see MDL-36320 (Moodle 2.5)

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          I assume this is part of the current Dev sprint.

          Show
          Michael de Raadt added a comment - I assume this is part of the current Dev sprint.
          Hide
          Mark Nielsen added a comment - - edited

          One challenge I have faced with Flexpage course format is hiding an activity based on custom criteria. I know this is currently possible with course sections and conditional availability, but this doesn't work for course formats that do not support sections.

          The way I solved it was to add an additional hook into cm_info::obtain_dynamic_data() that allowed the format add its say on whether or not the module as visible or not. In the proposal, it does look like there is a nice hook for when the $cm is set (EG: page_set_cm()) and that could be used to stop the page from being viewed by the format. I'm wondering though if that's good enough.

          Edit: page_set_cm() hook wouldn't work as it wouldn't help when trying to determine the user can view an activity when interacting with just an instance of cm_info.

          Show
          Mark Nielsen added a comment - - edited One challenge I have faced with Flexpage course format is hiding an activity based on custom criteria. I know this is currently possible with course sections and conditional availability, but this doesn't work for course formats that do not support sections. The way I solved it was to add an additional hook into cm_info::obtain_dynamic_data() that allowed the format add its say on whether or not the module as visible or not. In the proposal, it does look like there is a nice hook for when the $cm is set (EG: page_set_cm()) and that could be used to stop the page from being viewed by the format. I'm wondering though if that's good enough. Edit: page_set_cm() hook wouldn't work as it wouldn't help when trying to determine the user can view an activity when interacting with just an instance of cm_info.
          Hide
          Mark Nielsen added a comment -

          Another suggestion is a hook into the format to determine if the "Add block..." should be displayed or not when editing is turned on. Flexpage for example has its own UI for adding blocks so the default method for adding blocks would be redundant.

          Show
          Mark Nielsen added a comment - Another suggestion is a hook into the format to determine if the "Add block..." should be displayed or not when editing is turned on. Flexpage for example has its own UI for adding blocks so the default method for adding blocks would be redundant.
          Hide
          Derek Chirnside added a comment -

          I have had a look at MDL-10039 'Course of courses'. If this does indeed happen, this will have a major impact on the need for metacourses. Initially I was skeptical (Extreme feature creep) but I think I'd see this as being a simpler concept than metacourses, if the implmentation is right.

          Show
          Derek Chirnside added a comment - I have had a look at MDL-10039 'Course of courses'. If this does indeed happen, this will have a major impact on the need for metacourses. Initially I was skeptical (Extreme feature creep) but I think I'd see this as being a simpler concept than metacourses, if the implmentation is right.
          Hide
          Marina Glancy added a comment -

          Mark, regarding the hooks in cm_info::obtain_dynamic_data(), I commented on similar request in MDL-35699

          I will try to consider your add-blocks request

          Show
          Marina Glancy added a comment - Mark, regarding the hooks in cm_info::obtain_dynamic_data(), I commented on similar request in MDL-35699 I will try to consider your add-blocks request
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          I've started to migrate Collapsed Topics to using the new format_base in lib.php type structure and move my settings into the course settings. All going well so far after reading up on your code and a bit of deduction. This will certainly help me to reduce the amount of code I need to have .

          I have one question though, in my course restore code for supporting previous Moodle versions where I need to process the data that would have gone into my bespoke table, is there an easy way to write this to the course settings? I can see that the code can retrieve the settings easily and they are written on saving the course settings correctly as long as my 'course_format_options' method is written correctly, but not a way to write them without creating my own specific DB method.

          Thank you for this work, it has made my code simpler and puts the settings in a far better, logical and user friendly place.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, I've started to migrate Collapsed Topics to using the new format_base in lib.php type structure and move my settings into the course settings. All going well so far after reading up on your code and a bit of deduction. This will certainly help me to reduce the amount of code I need to have . I have one question though, in my course restore code for supporting previous Moodle versions where I need to process the data that would have gone into my bespoke table, is there an easy way to write this to the course settings? I can see that the code can retrieve the settings easily and they are written on saving the course settings correctly as long as my 'course_format_options' method is written correctly, but not a way to write them without creating my own specific DB method. Thank you for this work, it has made my code simpler and puts the settings in a far better, logical and user friendly place. Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          Would it be possible to add in some functionality in the class 'format_base' in '/course/format/lib.php' that facilitates:

          1. Adding of form elements to the formats settings form group in the course setting that are not actual format settings but rather check boxes indicating that something should be reset to a default setting.
          2. Have the method 'edit_form_validation' capable of correcting an invalid setting in the '$data' variable by having it parsed in by reference as you do with the '$mform' variable in the method 'create_edit_form_elements'. This would help with '1'.

          ??

          Currently I have implemented my own workaround by providing a separate form that appears whilst editing. I have managed to update the settings (I hope) and refresh the cache through copying some of the code as requested above, but would be really good if a method in 'format_base' was provided.

          My 'resets' need to reset the current course and all courses of the same format for a given format setting.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, Would it be possible to add in some functionality in the class 'format_base' in '/course/format/lib.php' that facilitates: 1. Adding of form elements to the formats settings form group in the course setting that are not actual format settings but rather check boxes indicating that something should be reset to a default setting. 2. Have the method 'edit_form_validation' capable of correcting an invalid setting in the '$data' variable by having it parsed in by reference as you do with the '$mform' variable in the method 'create_edit_form_elements'. This would help with '1'. ?? Currently I have implemented my own workaround by providing a separate form that appears whilst editing. I have managed to update the settings (I hope) and refresh the cache through copying some of the code as requested above, but would be really good if a method in 'format_base' was provided. My 'resets' need to reset the current course and all courses of the same format for a given format setting. Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          Also I've discovered the need for the 'put in course settings and rebuild cache' method in the base class as I need to place code in my 'db/update.php' that transfers the settings that were in my table in a previous incarnation of Moodle to course settings in 2.4 when a user performs an upgrade.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, Also I've discovered the need for the 'put in course settings and rebuild cache' method in the base class as I need to place code in my 'db/update.php' that transfers the settings that were in my table in a previous incarnation of Moodle to course settings in 2.4 when a user performs an upgrade. Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          I've found and made use of 'update_format_options' for new / updated settings that avoids duplication of code. However, I still require the functionality of additional checkboxes etc. on the course edit form.

          Thank you,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, I've found and made use of 'update_format_options' for new / updated settings that avoids duplication of code. However, I still require the functionality of additional checkboxes etc. on the course edit form. Thank you, Gareth
          Hide
          Marina Glancy added a comment -

          Hi Gareth,

          during restore:
          to update format-specific data for the course you usually use update_course_format_options() or update_section_format_options(). But you can also write directly to the table course_format_options if you know what you are doing

          To alter the course edit form in a complicated way:
          1) overwrite create_edit_form_elements() in your plugin to add the comprehensive form elements (don't forget that function must return the array of all created elements)
          2) overwrite edit_form_validation() in your plugin to validate your elements
          3) overwrite update_course_format_options() to process additional fields that you added to the form such as checkbox 'reset to default'.

          There is an example of calculated fields in format_legacy::update_course_format_options(), it automatically populates the 'numsections' field. This is used when format without 'numsections' field is converted to 'topics' or 'weeks' format.

          Show
          Marina Glancy added a comment - Hi Gareth, during restore: to update format-specific data for the course you usually use update_course_format_options() or update_section_format_options(). But you can also write directly to the table course_format_options if you know what you are doing To alter the course edit form in a complicated way: 1) overwrite create_edit_form_elements() in your plugin to add the comprehensive form elements (don't forget that function must return the array of all created elements) 2) overwrite edit_form_validation() in your plugin to validate your elements 3) overwrite update_course_format_options() to process additional fields that you added to the form such as checkbox 'reset to default'. There is an example of calculated fields in format_legacy::update_course_format_options(), it automatically populates the 'numsections' field. This is used when format without 'numsections' field is converted to 'topics' or 'weeks' format.
          Hide
          Marina Glancy added a comment -

          Please also note that the function format_base::update_format_options() is quite low level, it is not recommended to overwrite it. All it does is insert/update records in table 'course_format_options' and calls rebuild_course_cache() if there are any changes. It is protected so it's never called from outside of the class.

          If you want to pre-process data before updating the DB, overwrite update_course_format_options() or update_section_format_options()

          Show
          Marina Glancy added a comment - Please also note that the function format_base::update_format_options() is quite low level, it is not recommended to overwrite it. All it does is insert/update records in table 'course_format_options' and calls rebuild_course_cache() if there are any changes. It is protected so it's never called from outside of the class. If you want to pre-process data before updating the DB, overwrite update_course_format_options() or update_section_format_options()
          Show
          Aparup Banerjee added a comment - Thanks for https://moodle.org/plugins/view.php?plugin=format_flexsections
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          Thank you. I've changed my code to call 'update_course_format_options()' instead of 'update_format_options()'. When I overwrite 'create_edit_form_elements()' I do return the elements array, but add in my custom form elements with $mform->addElement() - and when that happens, I get a new section at the bottom of the course edit form underneath the submit / cancel buttons - I could not work out how adding to the elements array would actually add elements to the form when I looked at the 'format_base' version of the method - as when I added the additional fields in my overridden 'course_format_options' method in the editing section they ended up in the DB which is not what I wanted. My code with this test commented out can be seen at https://github.com/gjb2048/moodle-format_topcoll/blob/9d45edb9698eda8d63f1758a150c873be7b6a43f/lib.php.

          If I can crack this nut, then I'll be able to use 'update_course_format_options()' as you describe rather than processing the resets in ' edit_form_validation()' as I originally thought.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, Thank you. I've changed my code to call 'update_course_format_options()' instead of 'update_format_options()'. When I overwrite 'create_edit_form_elements()' I do return the elements array, but add in my custom form elements with $mform->addElement() - and when that happens, I get a new section at the bottom of the course edit form underneath the submit / cancel buttons - I could not work out how adding to the elements array would actually add elements to the form when I looked at the 'format_base' version of the method - as when I added the additional fields in my overridden 'course_format_options' method in the editing section they ended up in the DB which is not what I wanted. My code with this test commented out can be seen at https://github.com/gjb2048/moodle-format_topcoll/blob/9d45edb9698eda8d63f1758a150c873be7b6a43f/lib.php . If I can crack this nut, then I'll be able to use 'update_course_format_options()' as you describe rather than processing the resets in ' edit_form_validation()' as I originally thought. Cheers, Gareth
          Hide
          Marina Glancy added a comment -

          Hi Gareth,
          that's exactly what I meant by "don't forget that function must return the array of all created elements"

          The trick is that you add elements in the end of the form but calling function moves them later to the appropriate place inside the form.

          This is how you add elements in create_edit_form_elements():

          $elements = array();
          // ...
          $elements[] = $mform->addElement('checkbox', 'resetcolour', get_string('resetcolour', 'format_topcoll'), false);
          // ...
          return $elements;
          
          Show
          Marina Glancy added a comment - Hi Gareth, that's exactly what I meant by "don't forget that function must return the array of all created elements" The trick is that you add elements in the end of the form but calling function moves them later to the appropriate place inside the form. This is how you add elements in create_edit_form_elements(): $elements = array(); // ... $elements[] = $mform->addElement('checkbox', 'resetcolour', get_string('resetcolour', 'format_topcoll'), false ); // ... return $elements;
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          Brilliant, thank you - now that makes sense. I just could not figure out the nature of the $elements array from your base class. Shame that PHP does not have type statements with variables for enlightenment purposes.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, Brilliant, thank you - now that makes sense. I just could not figure out the nature of the $elements array from your base class. Shame that PHP does not have type statements with variables for enlightenment purposes. Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Dear Marina,

          RE: MDL-33037

          I also notice that all of the style sheets are included for all course format's in the header causing page load delay when there need not be. Looking at the 2.4 code, could the 'theme_config' class in '/lib/outputlib.php' which uses the 'css_files()' method for determining the files to include be told via the 'renderer_base' class in '/lib/outputrenderers.php' (i.e. $OUTPUT global) which format is being used, perhaps as a new mandatory call in the course format's 'format.php' file or as a part of the 'format_base' class / constructor in '/course/format/lib.php'.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Marina, RE: MDL-33037 I also notice that all of the style sheets are included for all course format's in the header causing page load delay when there need not be. Looking at the 2.4 code, could the 'theme_config' class in '/lib/outputlib.php' which uses the 'css_files()' method for determining the files to include be told via the 'renderer_base' class in '/lib/outputrenderers.php' (i.e. $OUTPUT global) which format is being used, perhaps as a new mandatory call in the course format's 'format.php' file or as a part of the 'format_base' class / constructor in '/course/format/lib.php'. Cheers, Gareth
          Hide
          Martin Dougiamas added a comment -

          Since all the subtasks of this are closed, I'm closing the parent. Thanks Marina!

          Show
          Martin Dougiamas added a comment - Since all the subtasks of this are closed, I'm closing the parent. Thanks Marina!

            People

            • Votes:
              5 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: