Fix factor token version #457 #458
Conversation
factor/token/version.php
Outdated
| $plugin->requires = 2022041908; // Support Moodle 4.0 and higher. | ||
| $plugin->component = 'factor_token'; | ||
| $plugin->release = 2022011700; | ||
| $plugin->maturity = MATURITY_STABLE; | ||
| $plugin->dependencies = ['tool_mfa' => 2019102400]; | ||
|
|
||
| // Fix version numbers that are higher than 4.3 core. | ||
| if (!during_initial_install() && get_config('factor_token', 'version') >= '2023100900') { |
There was a problem hiding this comment.
This string/int comparison doesnt feel quite right. Can you verify the type and do an explicit cast if required. Having it hardcoded as a string feels wrong.
There was a problem hiding this comment.
Thanks @Peterburnett
I copied from string part from the example as get_config('factor_token', 'version') is returning a string, but that's a good point.
This is running on every page so we'd want to go with the most efficient comparison. What do you think would be most efficient - leaving it as a string or casting with (int) or intval()?
There was a problem hiding this comment.
After some testing I ended up going with (int) get_config('factor_token', 'version') >= 2023100900 as the difference was almost negligible compared to the other check and getting the config.
bwalkerl
force-pushed
the
factor-token-version-457
branch
from
17438fa to
179fa1c
Compare
Based on the examples mentioned in #457
Doesn't feel ideal to do this in version.php, but appears to be the least disruptive option.