charitylearning / moodle-mod_clearlesson Goto Github PK

your backup process doesn't seem complete - it doesn't include all tables in your plugin and the table it does include doesn't seem to include all fields in the backup.

when including and distributing an external library you need to be careful to retain the licensing information and attribute the original authors.

you should also be adding a thirdpartylibs.xml file:
https://docs.moodle.org/dev/Plugin_files#thirdpartylibs.xml

Moodle supports several databases - unix_timestamp is a mysql function and will throw an error when your plugin is used on other databases:
https://github.com/CharityLearning/moodle-mod_clearlessons/blob/7e24b926067683af6a7bbd7ff2ee457bf0d8efb4/lib.php#L450

you should review your other sql statements in the plugin to make sure you don't do this anywhere else.

you might want to remove the "s" from the github repo name - I just downloaded and tried to install into a folder called "clearlessons" and was wondering why I was getting all the failure messages :-)

no login/capability checks - although it does make a call to global $USER (but not used) which suggest that the user should be logged in.

If this file is supposed to be available anonymously you should include some code comments in the file to allow other reviewers to understand the role of this file and why login checks are not performed etc.

Compliance with the coding guidelines makes it much easier to review your code - this helps the plugin review process but compliance with the guidelines increases the number of sites willing to use your plugin. The plugins db has generated this report on your uploaded plugin which would be good to review:
https://moodle.org/plugins/pluginversion.php?id=15868&smurf=html#phpcs

you could also install the local_moodlecheck and local_codechecker plugins and integrate your github repo with travis which would help with this:
https://moodle.org/mod/forum/discuss.php?d=323384

https://github.com/CharityLearning/moodle-mod_clearlessons/blob/master/lib.php#L219

can probably just remove the $ ?

https://github.com/CharityLearning/moodle-mod_clearlessons/blob/7e24b926067683af6a7bbd7ff2ee457bf0d8efb4/lib.php#L433

you should really put this into a file inside a "classes" directory within your plugin - also would be worth taking a look at the docs here:
https://docs.moodle.org/dev/Automatic_class_loading

https://github.com/CharityLearning/moodle-mod_clearlessons/blob/7e24b926067683af6a7bbd7ff2ee457bf0d8efb4/lib.php#L473

You should be using the core Moodle functions/curl class to make these calls - many sites will have a proxy config or other settings that will be ignored by your code.

Since the sync code changed location the required files are no longer correct.

probably good to remove the ds store file from the github repo

on fresh install:
XMLDB has detected one TEXT/BINARY column (type) with some DEFAULT defined. This type of columns cannot have any default value. Please fix it in source (XML and/or upgrade script) to avoid this message to be displayed.
line 1727 of /lib/datalib.php: call to debugging()
line 352 of /lib/xmldb/xmldb_object.php: call to xmldb_debug()
line 261 of /lib/xmldb/xmldb_field.php: call to xmldb_object->debug()
line 359 of /lib/xmldb/xmldb_field.php: call to xmldb_field->setDefault()