Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust filter plugin validation requirements to comply with Moodle 4.5 #326

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kabalin
Copy link
Member

@kabalin kabalin commented Oct 11, 2024

This patch is addressing filter plugin validation compliance with 4.5 raised at #321. Filter API have been updated to use the standard Moodle Class Autoloading infrastructure (https://moodledev.io/docs/4.5/devupdate#filter-plugins), this introduced requirement for backward compatibility in addition to new filter class location.

Possible Moodle version compatibility options for plugins now become:

  • 4.4 and below → expect file filter.php containing class filter_[pluginname].
  • 4.5 → expect file classes/text_filter.php containing class text_filter.
  • 4.5 and below → expect file classes/text_filter.php containing class text_filter and file filter.php containing class_alias.

Possible Moodle versions test scenarios matrix:

  • Moodle 4.5, plugin compatibility 4.5 → pass
  • Moodle 4.5, plugin compatibility 4.4 and below → fail
  • Moodle 4.4, plugin compatibility 4.4 and below → pass
  • Moodle 4.4, plugin compatibility 4.5 and below → pass
  • Moodle 4.4, plugin compatibility 4.5 → fail

The patch implements validation logic listed above and introduces 2 new methods:

  • getRequiredFunctionCalls in plugin type specific Requirements class to validate that file contains function call.
  • FileTokens::notFoundHint can be used to give some context for validation error to improve developer experience.

Currently this tool does not support the determine version compatibility declared in version.php ($plugin->supported), for this simple scenario I use file existence check as indicator of backward compatibility implemented in plugin, in future the tool would possibly benefit from using information from $plugin->supported property.

Changeset has been tested locally with https://github.com/gjbarnard/moodle-filter_synhi plugin:

4.5, filter_synhi main:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
> Found required file: classes/text_filter.php
! Skipping validation of missing or optional file: db/upgrade.php
> In classes/text_filter.php, found class filter_synhi\text_filter
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml
> In filter.php, found function call class_alias

4.5, filter_synhi main, filter.php deleted:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
> Found required file: classes/text_filter.php
! Skipping validation of missing or optional file: db/upgrade.php
> In classes/text_filter.php, found class filter_synhi\text_filter
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml
! Skipping validation of missing or optional file: filter.php

4.5, filter_synhi V402.1.0:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
X Failed to find required file: classes/text_filter.php
! Skipping validation of missing or optional file: db/upgrade.php
! Skipping validation of missing or optional file: classes/text_filter.php
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml
X In filter.php, failed to find function call class_alias
X Hint: https://moodledev.io/docs/4.5/devupdate#filter-plugins

4.4, filter_synhi V402.1.0:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
> Found required file: filter.php
! Skipping validation of missing or optional file: db/upgrade.php
> In filter.php, found class filter_synhi
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml

4.4, filter_synhi main:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
> Found required file: filter.php
! Skipping validation of missing or optional file: db/upgrade.php
> In classes/text_filter.php, found class filter_synhi\text_filter
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml
> In filter.php, found function call class_alias

4.4, filter_synhi main, filter.php deleted:

 RUN  Validating filter_synhi
> Found required file: version.php
> Found required file: lang/en/filter_synhi.php
X Failed to find required file: filter.php
! Skipping validation of missing or optional file: db/upgrade.php
> In classes/text_filter.php, found class filter_synhi\text_filter
> In lang/en/filter_synhi.php, found language filtername
! Skipping validation of missing or optional file: db/install.xml
! Skipping validation of missing or optional file: filter.php

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

Attention: Patch coverage is 87.80488% with 5 lines in your changes missing coverage. Please review.

Project coverage is 88.23%. Comparing base (f722b93) to head (ced63d2).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/PluginValidate/Finder/FunctionCallFinder.php 71.42% 2 Missing ⚠️
...uginValidate/Requirements/AbstractRequirements.php 0.00% 2 Missing ⚠️
src/PluginValidate/PluginValidate.php 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #326      +/-   ##
============================================
- Coverage     88.25%   88.23%   -0.03%     
- Complexity      737      756      +19     
============================================
  Files            76       77       +1     
  Lines          2274     2311      +37     
============================================
+ Hits           2007     2039      +32     
- Misses          267      272       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…validation.

This address scenario where file is supposed to contain certain function
call, such as `class_alias` in Filter plugin type backward compatibility
support per https://moodledev.io/docs/4.5/devupdate#filter-plugins

The patch makes possible for deleveloper to specify:
* getRequiredFunctionCalls to make sure file contains function call as
  name suggests.
* FileTokens::notFoundHint to give some context for requirement to improve developer experience. This works with FileTokens in any other validation methods.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MDL-82427 class alias usage for M4.5 filters fails validation.
1 participant