Talk:Course formats 2

Jump to: navigation, search

Some thoughts

output_... methods

I am not sure that these should exist. If I understand things correctly, the point is that the course format should be able to control aspects of the display for all pages within the course. So, for example, if I am in a quiz, and I do $OUTPUT->header(), then that might depend a bit on the course format. That is a good feature.

But, how will that be implemented? Surely, the header function will create the course-format renderer, and call its header() renderer. Typicaly developers will never need to do that, so I don't see how the output_... methods in the course format class help.--Tim Hunt 18:32, 30 August 2012 (WST)

We need to alter the themes as well so they include those headers. Usually themes will as well have layout options to skip them (i.e. in 'popup'). -- Marina Glancy 09:29, 4 September 2012 (WST)

Letting course formats add settings to the course form

There is a question of whether this is the best design. What happens when you try to change the course format using the course settings form? Suddenly the form will have a lot of irrelevant fields, and the user will need to refresh the form some-how, before they can set the relevant settings.

The alternative is to do what several contrib course formats do: have a separate form to edit the course-format-specific options. Of course, that is a less good UI in other ways.

Anyway, if you decide to go down the route of adding extra fields to the course form, then note that I did something similar in the quiz.

  1. https://github.com/moodle/moodle/blob/v2.3.1/mod/quiz/mod_form.php#L319 calls
  2. https://github.com/moodle/moodle/blob/v2.3.1/mod/quiz/accessmanager.php#L96 calls (for each plugin)
  3. https://github.com/moodle/moodle/blob/v2.3.1/mod/quiz/accessrule/accessrulebase.php#L233 an example of implementing this is
  4. https://github.com/timhunt/moodle-quizaccess_honestycheck/blob/master/rule.php#L71

--Tim Hunt 18:32, 30 August 2012 (WST)

Use plugin_supports()

That is a standard function defined in moodlelib.php: https://github.com/moodle/moodle/blob/v2.3.1/lib/moodlelib.php#L8174. I think you should use that rather than defining your own functions uses_sections(), supports_ajax(), etc. --Tim Hunt 18:32, 30 August 2012 (WST)

the page_set_cm, page_set_course callbacks are horrible

The moodle_page class shoudl not be linked to specific plugins like this. From what we discussed, the purpose of these callbacks is to that course formats can influence the navigation and navbar for all pages within the course. If that is the case, a much better to design is to make navigationlib.php call methods on the course format, to build the correct navigation.--Tim Hunt 18:32, 30 August 2012 (WST)

Tim, I will leave it till the end and look how else can I notify the course format which module it displays at the moment--Marina Glancy 09:32, 4 September 2012 (WST)

Followup from comment, I realized that this set_course hook doesn't make as much sense as it'll be called on things like mod/modname/view.php and the like where we wouldn't want formats doing anything to the page. Seems like a course format page init routine is more appropriate and is only called in the relevant locations. -- Mark Nielsen 02:46, 29 September 2012 (WST)

Please don't forget about unit tests

Just that! Actually, when I was hacking around with this ages ago and started playing with replacing get_all_mods() etc, I started trying to build unit tests to make sure I was keeping the output the same as it was before using expectOutputString(): https://github.com/danpoltawski/moodle/blob/4b9581fb376921ed0df2a38af6da5b705198c895/course/tests/course_section_controller_test.php I'm not suggesting that is a good idea at all, but I think this can be unit testable. --Dan Poltawski 12:17, 4 September 2012 (WST)