- Issue created by @prashant.c
- 🇩🇪Germany marcus_johansson
From Slack: this is the combination 1.1.x of AI module and OpenAI provider 1.0.2
- 🇮🇳India prashant.c Dharamshala
I tried multiple tweaks, but nothing worked. I think the operation types are not getting discovered because they are not plugins.
Can we go with the approach of
1.0.x:
where it is working finehttps://git.drupalcode.org/project/ai/-/blob/1.0.x/src/AiProviderPluginM...
- 🇩🇪Germany marcus_johansson
I've tried with
* Drupal 11.1.x-dev
* AI 1.1.x-de
* AI Provider OpenAI 1.0.2And I get the settings as normal.
Could you try to do some debugging in the method getOperationTypes in src/AiProviderPluginManager to see where the OperationTypes gets lost?
Do you have any module that uses the hook_ai_operationtype or that changes the DefaultPluginManager isntalled?
- 🇩🇪Germany marcus_johansson
Ok, finished debugging and I will bump up the priority of this, since it will break AI module in the next D11.2 release if we do not do anything about it.
First offset - this will only show up in D11.x release.
This is happening because of this change in core and this change in AI module:
* https://git.drupalcode.org/project/drupal/-/commit/e2c511e89be78c02e04de...
* https://git.drupalcode.org/project/ai/-/merge_requests/412/diffs#9adebf4...What is happening is that the core issue 📌 Add a fallback classloader that can handle missing traits Active introduces a fallback loader for loading traits that is needed for plugin discovery. Part of that means that it adds a check to see that an class actually exists.
At the same time Drupal AI is kind of missusing the plugin system, where we are loading interfaces as the discoverable plugins. And because the main file is an interface, this means that the class_exists call will not set true.
We should not be moving around the OperationTypes interfaces, inputs and outputs since they are being referenced everywhere,
The one fix we can do for 1.1.x that are none breaking that I can see that does not revert ✨ Programmatically add OperationType Active is that we add the requirement that you have a plugin/class file outside of the interface, that could even have a method for finding the interface or we do that in the discovery phase as well. I would opt for that.
Any other suggestions?
- 🇧🇬Bulgaria valthebald Sofia
Having a simple autoloading class for the sake of autoloading sounds fair
- @prashantc opened merge request.
- 🇮🇳India prashant.c Dharamshala
With the help of an AI code assistant attempted the solution, which does the following:
- Created a base plugin class (OperationTypePluginBase) that implements the OperationTypeInterface
- Created a custom discovery class (OperationTypeDiscovery) that:
- Discovers interfaces in the OperationType directory
- Creates corresponding plugin class names
- Maintains the existing interface structure
- Works with the new attribute discovery system
- Updates the AiProviderPluginManager to use the discovery system
Possible benefits of this approach:
- Maintains backward compatibility - existing code using interfaces continues to work
- Works with the new attribute discovery system
- Doesn't require moving or restructuring existing interfaces
- This could be replaced/rewritten when a proper plugin system for operation types is in place going forward
- Now discover operation types from interfaces
Needs a thorough review of the code.
- 🇧🇬Bulgaria valthebald Sofia
After applying the patch, I see full list of operations in the settings form, but have no translate with ai links in node//translations
- 🇧🇬Bulgaria valthebald Sofia
Actually, my bad, translations work. @prashant.c can you review the CI bot comments?
- 🇧🇬Bulgaria valthebald Sofia
Looks good to me code-wise.
I miss some explanation on why this changes is needed in the first place, but this can be added in the follow-up issue when it's created.
Pending creation of 2.0.x/2.x branch to do this - 🇧🇬Bulgaria valthebald Sofia
Linked 📌 Use Drupal plugin system with OperationTypes Active as a reference, marking RTBC
- 🇩🇪Germany marcus_johansson
Works for me as well! Thanks for finding the issue and fixing it prashant.c! Also thanks to everyone else in this thread. Will merge it now.