mdjnelson / moodle-mod_customcert

Enables the creation of dynamically generated certificates with complete customisation via the web browser.
https://moodle.org/plugins/mod_customcert
GNU General Public License v3.0
93 stars 159 forks source link

column 'verifyany' length is 2, expected at least 10 #597

Closed jbs1 closed 8 months ago

jbs1 commented 8 months ago

Hey,

so I noticed that /report/performance/index.php?detail=core_dbschema gives me a column 'verifyany' length is 2, expected at least 10 for the customcert mod.

This does not really hurt anything but it still shows up as an error in the report. install.xml defines the length to be 10 that's why it complains.

I assume this happened because I am running a fairly old instance of moodle and instead of having a fresh install I just continuously upgraded the versions since before this feature was introduced. And in upgrade.php the precision/length of the field is not 10.

I assume both install.xml and upgrade.php should probably be the same length, right?`

I assume most people don't run an instance that is that old so it probably won't show up as an error for most.

And I also assume that it should be the shorter length, because from what I can see, there is not need for a longer int, right?

mdjnelson commented 8 months ago

Thanks for reporting this. Yes, this a valid issue but it wont hurt your install for it to remain at 10.

mdjnelson commented 8 months ago

This is the culprit https://github.com/mdjnelson/moodle-mod_customcert/commit/1ac7c1ffb6f098e1e0d386689284391c891e0313. Going to leave this for now as it doesn't break anything but is worth keeping an eye on.

jbs1 commented 8 months ago

Hey,

that was certainly fast 😆 So I am not really familiar with the upgrade api for moodle so I am curious to understand it.

If I understand the doc correctly. one would just have to put this in the upgrade.php, since that is what the xmldb editor gives me and change the length in the install.xml to 1, right?

Or are more steps needed that I overlooked?

    if ($oldversion < XXXXXXXXXX) {

        // Changing precision of field verifyany on table customcert to (1).
        $table = new xmldb_table('customcert');
        $field = new xmldb_field('verifyany', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'requiredtime');

        // Launch change of precision for field verifyany.
        $dbman->change_field_precision($table, $field);

        // Customcert savepoint reached.
        upgrade_mod_savepoint(true, XXXXXXXXXX, 'customcert');
    }

Goes without saying that the XXX has to be replaced with the respective versions 😄

mdjnelson commented 8 months ago

Yep, bump the version number in version.php and that's what you put for XXXXXXXXXX. Note, be careful with this version numbering system and only bump it by one, not to today's date.

Hypothetical.

In 4.3 you want to add a new field just to that version and no others so you bump it to today's date. The next day you realise in 4.2 and 4.3 you missed a column name change, so you add that in and bump it to today's date (tomorrow from last bump). Then when you upgrade from 4.2 to 4.3 you miss the upgrade with the new field as the version number is now larger.

mdjnelson commented 8 months ago

Also, don't always expect it to be this quick. ;) I just happen to be in-between jobs and am doing some maintenance on this project in my spare time. Hopefully release a new version with much needed improvement and bug fixes. :)

mdjnelson commented 8 months ago

Addressed in all supported Moodle versions. :) Closing.