donhinkelman / moodle-block_sharing_cart

Content sharing plug-in for Moodle LMS. Now at version 4.4, release 3 (actually this is for Moodle LMS version 4.3. Requires PHP 7.4.
7 stars 38 forks source link

Issues highlighted by admin/cli/check_database_schema.php (block_sharing_cart_log) #108

Closed abias closed 1 year ago

abias commented 3 years ago

Every now and then, we are running the admin/cli/check_database_schema.php script in our Moodle installation.

This time, this issue was highlighted:

-------------------------------------------------------------------------------
block_sharing_cart_log
 * table is not expected
-------------------------------------------------------------------------------

In our installation, this table contains some rows with timestamps and data from the year 2021.

Upon investigation, I saw that this table is and was never contained in https://github.com/donhinkelman/moodle-block_sharing_cart/blob/master/db/install.xml. Instead, it is created in on-the-fly if needed on https://github.com/donhinkelman/moodle-block_sharing_cart/blob/master/classes/mysql_logger.php#L26 and there is a short notice about the purpose in https://github.com/donhinkelman/moodle-block_sharing_cart/blob/master/README.md.

While there may be valid reasons to have this table for logging the unfixed problem, I would consider it bad practice to create it on the fly.

Would you mind to change the mechanism to create it on the fly to a situation where the table is defined in db/install.xml for new installation and created in db/upgrade.php for updated installations?

Thanks in advance, Ale

donhinkelman commented 3 years ago

Thank you, Alex, for reporting this issue. I have passed this on to Ulrik of Praxis to fix or assign.

frederikmillingpytlick commented 1 year ago

This issue has been fixed by @marxjohnson, closing issue