-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
refactor(forms): Enable extension of SLM configuration field strategies via plugins #22507
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
base: 11.0/bugfixes
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the SLM (Service Level Management) field strategy system to enable plugin extensibility. It introduces a new interface-based architecture that allows plugins to register custom strategies for SLA/OLA fields, making the forms system more flexible while keeping the core simple. The refactoring also consolidates duplicate deserialization logic across multiple field config classes.
- Introduced
SLMFieldStrategyInterfaceto standardize strategy implementations - Updated
SLMFieldStrategyenum to implement the interface, enabling both core and plugin strategies - Simplified four field config classes by consolidating logic into
SLMFieldConfigparent class
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Glpi/Form/Destination/CommonITILField/SLMFieldStrategyInterface.php | Defines the interface that all SLM field strategies must implement, specifying methods for rendering, applying logic, and metadata |
| src/Glpi/Form/Destination/CommonITILField/SLMFieldStrategy.php | Updates core enum to implement the new interface, moving logic from field classes into strategy methods |
| src/Glpi/Form/Destination/CommonITILField/SLMFieldConfig.php | Adds support for plugin strategies via string keys, extra data storage, and consolidated deserialization logic |
| src/Glpi/Form/Destination/CommonITILField/SLMField.php | Updates to render and apply strategies dynamically from registered sources |
| src/Glpi/Form/Destination/FormDestinationManager.php | Adds registration and retrieval methods for plugin-provided SLM field strategies |
| src/Glpi/Form/Destination/CommonITILField/SLATTOFieldConfig.php | Simplified to extend parent class without duplicate deserialization code |
| src/Glpi/Form/Destination/CommonITILField/SLATTRFieldConfig.php | Simplified to extend parent class without duplicate deserialization code |
| src/Glpi/Form/Destination/CommonITILField/OLATTOFieldConfig.php | Simplified to extend parent class without duplicate deserialization code |
| src/Glpi/Form/Destination/CommonITILField/OLATTRFieldConfig.php | Simplified to extend parent class without duplicate deserialization code |
| templates/pages/admin/form/form_destination_commonitil_config.html.twig | Adds ternary check to support both enum strategies (with .value) and plugin strategies (with .key) |
| tests/fixtures/plugins/tester/src/Form/SpecificAnswerSLMStrategy.php | Example plugin strategy implementation for testing the extension mechanism |
| tests/fixtures/plugins/tester/templates/specific_answer_slm_strategy.html.twig | Template for rendering the plugin strategy's configuration UI |
| tests/fixtures/plugins/tester/setup.php | Registers the test strategy during plugin initialization |
| tests/functional/Glpi/Form/Destination/CommonITILField/SLMFieldStrategyExtensionTest.php | Comprehensive test suite validating strategy registration, serialization, and end-to-end functionality |
| .phpstan-baseline.missingType.iterableValue.php | Removes now-unnecessary PHPStan baseline entries for deleted deserialization methods |
AdrienClairembault
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, since this require a lot of changes in the core and it only allow customisation for a few specific fields, wouldn't it be simpler to just add the new strategies in the core?
I know this isn't what we said originally (sorry) but I think it make more sense like that :/
Checklist before requesting a review
Please delete options that are not relevant.
Description
This pull request refactors and extends the SLM (Service Level Management) field strategy system to support plugin-defined strategies, making it more flexible and extensible. The main changes include introducing a new
SLMFieldStrategyInterface, updating the configuration and deserialization logic to support both core and plugin strategies, and simplifying several field config classes. It also improves how extra configuration data is handled and displayed for these strategies.This extension option is intended to be used by the
advancedformsplugin in order to add more sophisticated and complex strategies. (This change is in line with our desire to keep the form system as simple as possible and extend its functionality with an additional plugin).Here is the relevant PR: pluginsGLPI/advancedforms#12