Special project for the University of Montreal

[andrewscaya]

Hello everyone,

I'm reaching out to you because we're currently working on a project for the University of Montreal, and we'll be using the course_templates local plugin. We'll be giving some love to this plugin in the process, and we're wondering if you would be interested in getting a few pull requests (PRs) from us over the next few weeks?

These PRs would include :

• Bringing the code up to Moodle coding standards (CodeChecker, PHPDoc Checker, etc.),
• Adding configuration settings, like allowing the user to change the name of the category that is used to designate course templates,
• Removing all hard-coded strings to allow for easy translation,
• Allowing for non-admin users to use the plugin's capabilities,
• New translations in Amos,
• And, a few more goodies... :)

Please let us know what you think, ok?

Many thanks for sharing this great plugin with the Moodle community!

Best regards,

Andrew

[Flottertotte]

Hey Andrew,

That sounds awesome, thank you!. I think your enhancements will be beneficial for the majority of users and we are happy to include them! What would be great is if you could divide your enhancements into different pull requests for easy review and accepting.

Thanks again and looking forward to working together with you!

Flotter (emeneo)

[andrewscaya]

Hey @Flottertotte,

That's great! :)

OK, so the first commit that I'll be sending to you today will only be code styling (no new functionality and no modifications). The reason for this is that we need CodeChecker (CodeSniffer) and PHPDoc Checker to work for our next commits.

Within the next few days, I'll be submitting another PR with new functionality. Essentially, it will add an administrator setting to allow users of the plugin to choose a different course category as their default template category. For now, when performing a fresh plugin installation or an upgrade, a new parameter page would ask the user to choose a course category as the new default course template category. For those that would simply merge the code (no version.php), they would default to the first Moodle category (Miscellaneous). Is this ok with you? And also, do you want us to change the version.php file in our commit, or do you want to take care of that yourselves?

Looking forward to working with you all! :)

Best,

Andrew

[Flottertotte]

Thanks for sharing your plans.

OK, so the first commit that I'll be sending to you today will only be code styling (no new functionality and no modifications). The reason for this is that we need CodeChecker (CodeSniffer) and PHPDoc Checker to work for our next commits.

Great, looking forward to :)

Within the next few days, I'll be submitting another PR with new functionality. Essentially, it will add an administrator setting to allow users of the plugin to choose a different course category as their default template category.

Nice new feature

For now, when performing a fresh plugin installation or an upgrade, a new parameter page would ask the user to choose a course category as the new default course template category.

Regarding "fresh plugin installation", this is fine, but I would reccomend to change the way for users that upgrade from a previous version. The reason is, many institutions are already using the plugin for a long time and they all have the hard coded "Course templates" as their template category. I think it would confuse them if during an upgrade they suddenly would need to select a default category. Could you change so that in the case of an upgrade "Course templates" is put as default category value? Or something similar.

And also, do you want us to change the version.php file in our commit, or do you want to take care of that yourselves?

Please change "$plugin->version = 2019120800;" in case db upgrade is neccessary etc. such as in the case of the new "default category" feature.

Thanks and all the best, Flotter

[andrewscaya]

First PR sent : Code styling (PR #15).

[Flottertotte]

Merged :)

[andrewscaya]

@Flottertotte

The next PR is almost ready!

In light of your previous answers, I would like to ask you a question: if we create a 'Course templates' category by default (if it doesn't already exist, of course) and designate it as the plugin's default category for templates when installing or upgrading the plugin, as of version 2021121500, would that be fine by you? This would render the entire upgrade process seamless for your current users, and of course, the new admin setting would allow your users to change this setting afterwards anyway. What do you think? Is this a viable solution?

Thanks!

[Flottertotte]

Creating a course category "Course templates" is fine I think. I understand that for existing users it would not be created, because it exists already. But what exactly will happen when existing usres upgrade? I mean when they upgrade, would they need to do anything or would everything happen in the background?

Maybe one concern regarding different template categories: it might make usage a bit complicate for example in case categories get deleted. Maybe as a future improvement there should be some kind of overview/management page in the plugin settings?

[andrewscaya]

@Flottertotte OK! Great. Thanks!

Yes, as of the latest dev version, everything happens in the background when upgrading.

And, when installing, we can bring up the new admin settings page with the 'Course templates' category already selected, or not, as you see fit. What would you like?

[Flottertotte]

And, when installing, we can bring up the new admin settings page with the 'Course templates' category already selected, or not, as you see fit. What would you like?

I think latter ist better ('Course templates' category already selected) Thank you!

[andrewscaya]

@Flottertotte

Sorry for deleting the previous message, but we thought that our question (and the code examples) were not very clear. So, here we go for take 2! :)

Everything is clear about the installation process, but we just wanted to make sure that we understand your position about the upgrade process. So, in order to do so, we made two versions of the upgrade process.

One uses the 'settings.php' file to create the 'Course templates' category and set the default category to use for the admin form. The only thing is that when upgrading, Moodle will show the new admin setting window to confirm the new admin setting, by asking you to confirm that you wish to use the 'Course templates' category. So, with this option, the current users of the plugin will have to confirm this setting when upgrading.

Please see the code here : https://github.com/andrewscaya/moodle-local_course_templates/tree/newadminsetting-1

The second version of the upgrade process abstracts away the logic of creating the 'Course templates' category and the configuration of the admin setting in the 'install.php' and 'upgrade.php' files. When performing an installation, the new admin setting window will ask the user to confirm the choice of the default category, but when upgrading it will not. When upgrading, it will upgrade the plugin WITHOUT any user interaction, because the admin setting is already recorded in the database before Moodle begins its upgrade process.

Please see the code here : https://github.com/andrewscaya/moodle-local_course_templates/tree/newadminsetting-2

Specifically, the logic of this second version can be found here : https://github.com/andrewscaya/moodle-local_course_templates/blob/83922d490d05745d2b8392ca3f5ce43fe90ec525/db/upgrade.php#L61

Please let me know which one you prefer (or any variation thereof!), and I'll send the PR to you.

Many thanks!

[Flottertotte]

I think for existing users who upgrade, the second process is better. So if there are now disadvantages with this process, we would prefer that (process 2). Thank you for your efforts!

[andrewscaya]

Second PR sent : new admin setting that allows users to choose the template category (PR #16).

[andrewscaya]

@Flottertotte

We will be taking some time off for the Holidays. We'll be back in early January for a few more PRs. :)

Happy Holidays!

Best regards,

Andrew

[Flottertotte]

We merged your recent PR. One question: you ware working with which moodle version?

[andrewscaya]

@Flottertotte

We are currently working with Moodle 3.10. If you have multiple versions you would like for us to test against (both Moodle and PHP), please let us know, because we can run multiple Docker environments on the same code base.

[Flottertotte]

Hi Andrew, thanks a lot for your reply. 3.8 onwards would be great. We then would publish a new release for 3.8, 3.9, 3.10 . The current version on moodle is for 3.7

Btw: there has been a bug reported for 3.11, not sure if this bug also appears on 3.10 (on 3.7 we could not reproduce it)

The bug has been reported in the comments section of the plugin: https://moodle.org/plugins/local_course_templates

Does not seem to work in Moodle 3.11. I get the following error in the php error logs:

Default exception handler: error/setting_invalid_ui_label Debug: Error code: setting_invalid_ui_label $a contents:

• line 153 of /backup/util/ui/backup_ui_setting.class.php: base_setting_ui_exception thrown
• line 156 of /backup/moodle2/backup_section_task.class.php: call to base_setting_ui->set_label()
• line 153 of /backup/util/plan/base_task.class.php: call to backup_section_task->define_settings()
• line 60 of /backup/util/plan/base_plan.class.php: call to base_task->set_plan()
• line 161 of /backup/moodle2/backup_plan_builder.class.php: call to base_plan->add_task()
• line 188 of /backup/moodle2/backup_plan_builder.class.php: call to backup_plan_builder::build_section_plan()
• line 101 of /backup/moodle2/backup_plan_builder.class.php: call to backup_plan_builder::build_course_plan()
• line 163 of /backup/util/factories/backup_factory.class.php: call to backup_plan_builder::build_plan()
• line 67 of /backup/util/plan/backup_plan.class.php: call to backup_factory::build_plan()
• line 463 of /backup/controller/backup_controller.class.php: call to backup_plan->build()
• line 149 of /backup/controller/backup_controller.class.php: call to backup_controller->load_plan()
• line 1417 of /course/externallib.php: call to backup_controller->__construct()
• line 68 of /local/course_templates/process.php: call to core_course_external::duplicate_course(), referer: https://example.com/local/course_templates/index.php?step=3&cid=30

[andrewscaya]

@Flottertotte

This seems to be related to a change they made in the way Moodle filters the labels associated with course sections. In Moodle 3.10, labels were filtered using PARAM_TEXT, but this is no longer the case in Moodle 3.11. Please see here :

In Moodle 3.10, the method's parameter was checked to see if it was empty (could be false with raw labels), and then checked using a filter with PARAM_TEXT (could now be empty, for example). In Moodle 3.11, it is now filtered with PARAM_CLEANHTML (could become empty), and then checked to see if it is empty (would then be true).

It therefore seems to be an issue with Moodle, not the Course templates plugin, because there hasn't been any changes to the duplicate_course() method's definition, which is being called in the plugin's process.php file (although it is true that it should be called statically instead of instantiating the core_course_external class). Please see here (no changes to line 1417 of course/externallib.php) :

SOME EXTRA NOTES ON THIS TOPIC :

/**
 * PARAM_CLEANHTML - cleans submitted HTML code. Note that you almost never want
 * to use this. The normal mode of operation is to use PARAM_RAW when receiving
 * the input (required/optional_param or formslib) and then sanitise the HTML
 * using format_text on output. This is for the rare cases when you want to
 * sanitise the HTML on input. This cleaning may also fix xhtml strictness.
 */

/**
 * PARAM_TEXT - general plain text compatible with multilang filter, no other html tags. Please note '<', or '>' are allowed here.
 */

When we are done with development (in a few short weeks), it would definitely be our pleasure to help you out with this issue. Would this be fine with you?

P.S. I didn't find a corresponding issue in Moodle's bug tracker database.

[andrewscaya]

Hello @Flottertotte,

Happy New Year to everyone at the Course Templates project! :)

We are beginning this first week of 2022 with a brand new PR (#17). We've removed all hard-coded strings, and replaced them with the 'get_string' function. Also, we've reordered all translation strings alphabetically. Finally, we'll be submitting all translated strings in French to the Amos database.

PLEASE NOTE: We had to remove three lines in the plugin's files, because Moodle's Code Checker now considers the 'defined('MOODLE_INTERNAL') || die;' lines as unwanted in library files (no executable code, no side effects, no artefacts, thus unwanted).

If you have any questions, please feel free to ask away! :)

Best,

Andrew

[Flottertotte]

Hi Andrew,

Happy new year and thanks for your comments on the bug related to using a filter with PARAM_TEXT and your latest pull request, especially for removing the hard coded stuff which is really useful!

Best, Flotter

[Nabsteur]

Hello,

Thank you very much for working on this plugin. I would like to ask when you think there will be a new version available? Because I use Moodle 3.9 and I really need this plugin. Thanks in advance!

[andrewscaya]

Hello @Nabsteur,

Many thanks for your kind words!

I can't really answer your question. I think @Flottertotte might be able to give you a better idea of what is to come for the Course Templates plugin.

Cheers!

Andrew

[Flottertotte]

Interesting place you have chosen for your question @Nabsteur :-) But as we are already in this thread: there are still some commits which will come from @andrewscaya in the near future (@andrewscaya , please correct me if I am wrong). We planned to put a new release on moodle as soon as this development round is finished.

Is it ok for you @Nabsteur to use the latest version of the plugin directly from github?

[andrewscaya]

@Flottertotte

Sorry for the late reply, but we've been busy bees over the past ten days! :)

The next PRs are almost ready!

Things to look forward to in the next couple of weeks:

1. Non-admin users (a Category Manager for example) will be able to create a course from a template,
2. Any data submitted through the plugin's forms will now be valided (both on server-side and client-side).

The last round of PRs will touch on eAccessibility and implementing a privacy policy.

Does this sound good to you, @Flottertotte ?

/cc @Nabsteur

[Flottertotte]

Happy Tiger Year @andrewscaya

Thanks a lot. Improvement "1." would be really great. I think that we tried that some (long) time ago but for some reason we did not succeed in finding a solution without manually changing some rights. I think it was due to the function which is used to duplicate courses, but not sure. The workaround with manually changing rights works actually without code changes. If that would not be required anymore, that would be great of course.

What do you mean with "2. Any data submitted through the plugin's forms will now be valided (both on server-side and client-side)"

Greetings, Flotter

[andrewscaya]

Thanks, @Flottertotte! Same to you! :)

Improvement 2 will validate input from users using Moodle validation rules (server-side). Also, client-side validation will make the Course templates plugin's forms give meaningful messages to the users directly from within the form, like so:

Finally, the 'Select a course template' form will no longer be a radio button list, but an autocomplete select, in case users have hundreds of course templates (like a university for example! ;) ). This is what this new form looks like:

Any comments or questions are welcome, of course! :)

Yours,

Andrew

[Flottertotte]

The validation is cool. I am not sure if you aware of a current problem in the plugin: if a course short name is entered which already exists, the user does not such error message. If validation could include this would be really cool I think.

Regarding

the 'Select a course template' form will no longer be a radio button list, but an autocomplete select, in case users have hundreds of course templates (like a university for example! ;) ).

This is problematic as there are many institutions out there which only have 1 or just some course templates. Usually they use the plugin to standardize their courses across their institution. So for them to just offer an input box will be confusing. Also, they are already used to the current method. Could you please find a way which includes both methods? It does not have to be radio buttons, but the plugin user should immediately see the list of available plugins and should be able to choose the right one with a click.

Greetings, Flotter

[andrewscaya]

Hi @Flottertotte,

Yes, we will also include course name validation on the last form. Many thanks for your kind words!

No, the second form of the 'Add course from template' procedure will not be an input box, but an auto-complete select, like so:

This way, we have the best of both worlds (a list, like the previous radio field, and an auto-complete field, with search capability, in case we have hundreds of categories). Please feel free to comment further on this topic.

Yours,

Andrew

[Flottertotte]

Hi @andrewscaya ah, ok ic, the change is related to the category selection. Currently not very sure but for existing users (with only some categories), important is that they still can see all their categories at once and can easily select the desired one. I think if this is ensured, all fine...

[andrewscaya]

Hi @Flottertotte,

We've just created the latest pull request (PR #18).

The next PR will add a Privacy Policy (will fix issue #9).

After that, we will send you a PR that will fix the issue with Moodle 3.11+ that was mentioned here: https://github.com/emeneo/moodle-local_course_templates/issues/14#issuecomment-998054535.

If you have any questions, please let me know.

Best,

Andrew

[Flottertotte]

Wow, really great enhancements @andrewscaya I will merge asap and also pack a new release and update the version on moodle.org. I think many will love your improvements!

[andrewscaya]

Many thanks for the kind words, @Flottertotte! 🙂

The Privacy Policy PR is now also ready. I'll post this other PR as soon as #18 is merged.

[andrewscaya]

Hi @Flottertotte,

I've just created PR #19, which fixes issue #9. If this PR is merged, I'll send another PR to fix some minor accessibility issues. Once this second one is merged, I'll send the PR that will fix the issue mentioned in this thread. This third and final PR from us should also fix issue #13.

After these three last PRs are completed, I think our job will be over for now. Once all of this is said and done, you should have only one outstanding issue left to fix (#12).

As usual, if you have any questions, please let me know.

Cheers,

Andrew

[Flottertotte]

Thank you @andrewscaya, it's merged. One question regarding:

Once all of this is said and done, you should have only one outstanding issue left to fix (https://github.com/emeneo/moodle-local_course_templates/issues/12).

If I see it correctly, this is actually a problem of the moodle duplication function, isn't it? What do you think?

[andrewscaya]

That's great, @Flottertotte!

Concerning issue #12, I can certainly have a quick look at it. It might indeed already be fixed, or it might be a "wontfix" issue. I'll get back to you on this, either today or tomorrow. 👍

[andrewscaya]

@Flottertotte PR #21 created. This one fixes accessibility issues found in the plugin's main stepper.

[Flottertotte]

Great, merged @andrewscaya

[andrewscaya]

@Flottertotte WOW! That was fast! :)

I think that we just broke the World record for the shortest delay between the moment an issue is reported, and the final merge of the bug fix. 😆 👍

The next PR will be coming shortly (probably early next week), because the final version of the patch to fix the issue concerning the course section labels is ready, but it has to go through final code review before I can send it to you.

In the meantime, I'll have a look at issue #12.

[andrewscaya]

@Flottertotte --

I can confirm that #12 is a duplicate of MDL-41652.

The final PR is ready, and I'll be sending it to you next week, once our code review is done.

Have a great rest of the week! :)

Cheers,

Andrew

[Flottertotte]

This is great news @andrewscaya ! Best wishes!

[Flottertotte]

@andrewscaya I just packed a 3.8 release and published on moodle for version 3.7 and 3.8. I will then next week pack another release and publish. Greetings, Flotter

[andrewscaya]

@Flottertotte PR #22 created. This one fixes issue #13. It also fixes a couple of code styling issues, and removes unused files.

This is our final PR (issue #14 can now be closed). Once the new release is done, we'll start adding new translation strings in the Amos database.

It was truly a pleasure working with you @Flottertotte! 😃

I wish you all the best, and please don't hesitate to reach out to me if I can be of any further assistance to this project (you can find me on Twitter, LinkedIn, and GitHub, or you can send me a good old-fashioned email 🤪 )!

Cheers, everyone! 🥳

Andrew

[Flottertotte]

Hi Andrew,

Thanks a lot for your great efforts and superb cooperation. I will merge asap and publish a new release on Moodle. Regarding "old-fashioned email" - what is yours? Looking forward to working with you together again in the future!

All the best, Flotter

[andrewscaya]

@Flottertotte

You'll find my email in the copyright.

For all the other ways to contact me: https://andrewscaya.net.

Cheers!

Andrew

[Flottertotte]

@andrewscaya ooohk :) looked everywhere except there :-)
Alright and Thank you! Flotter

[andrewscaya]

@Flottertotte

You're very welcome! 👍

Cheers!

Andrew

[andrewscaya]

@Flottertotte --

Just a quick message to let you know that the French translation strings for the latest release have been submitted to the Amos database for review.

Talk to you soon! :)

Yours,

Andrew