Concerns about the handling of the original wwwroot

[abias]

Hi,

I have some concerns about the handling of the original wwwroot setting in this plugin. These concerns are targeted at the latest version of the plugin published on https://moodle.org/plugins/pluginversion.php?id=12918 and may be superseded by recent commits here in Github.

• As Nico Declerck already wrote in the comments on https://moodle.org/plugins/local_datacleaner, it is not clear when / how / where the plugin stores the original wwwroot. The README tells us on https://github.com/catalyst/moodle-local_datacleaner#configuration:

[...] Note that you MUST visit the DataCleaner config page to save the current wwwroot, or the cleaner will not run later in the other environments. ** Well, what's the DataCleaner config page? It's /local/datacleaner/index.php, but you won't know without looking at the code. ** The DataCleaner config page stores the original wwwroot automatically when the page is visited. But this page does not output any information that the original wwwroot has now been stored when you visit the page.

• There is no possibility to check that the original wwwroot is set correctly and / or to check which value it has without looking at the database.

• The original wwwroot is stored in a Moodle core variable on $CFG->original_wwwroot in the mdl_config table. As far as I know, plugin settings should rather go to mdl_config_plugins.

• The original wwwroot is set everytime when you visit the DataCleaner config page. This also happens when you visit the DataCleaner config page in the washing box Moodle instance. I think this is unexpected.

I would propose to describe crystal clearly in the README and on the DataCleaner config page how the original wwwroot setting works and what the admin has to know and do to make it work.

Thanks, Alex

[brendanheywood]

+1 this is all sane, need to make sure it gets migrated when we change the config key. We can also move the original wwwroot setting to a cron task like the envbar code does:

https://github.com/catalyst/moodle-local_envbar/blob/master/classes/task/sync_lastrefresh.php#L53-L58