-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Description
Current Problem:
The IContentDefinitionService interface is currently part of the Module project. This setup is suboptimal because any other project that wants to use this service (e.g., to manage content definitions such as deleting a content type) would be forced to reference the Module project directly. This introduces unnecessary coupling and goes against clean architecture principles.
Solution
The IContentDefinitionService interface relies on view models and is only used in the Module project. This shows that this is not meant to be shared.
After looking at what is does it's clear that the issue is that its logic is necessary to drive the event handlers.
What I suggest is:
- Extract all the things that create view models in this interface by moving it to the controller that uses it. The controller should use the
IContentDefinitionManagerdirectly to create the view models. - Create a new interface (same name maybe) in the content project (not the Module project) that contains all the methods unrelated to view models that need to be reused across modules. It's acting like a coordinator. One example of why it's still necessary and not move event handlers triggers in the
IContentDefinitionManageris that the service can currently invoke 3 methods in theIContentDefinitionManagerand still invoke an event handler once.
TL;DR: Extract the interface so it's reusable, but only the things not related to view models, and triggering events.
Validation
If we want to move the event handlers to the IContentDefinitionManager implementation (see #16926 and #18184) then we need to check:
- the events are currently only invoked by
ContentDefinitionService. For instanceAlterTypePartsOrderAsyncinvokesContentTypeUpdated. This could be moved inIContentDefinitionManager. - the methods in
IContentDefinitionManagerare currently only invoked byContentDefinitionService. Right now many migrations, but alsoDefaultDefinitionDisplayManager.
Example:
AlterTypePartsOrderAsync invokes ContentTypeUpdated. If this was moved in AlterTypeDefinitionAsync, the only listener is DynamicContentFieldsIndexAliasProvider to invalidate a cache so it would not break anything to move it.