OrchardCMS
Cleanup markdown by moving models to the Abstractions project
Can anyone give me proper reason why we need this before we merge? https://github.com/OrchardCMS/OrchardCore/issues/11698#issuecomment-1179579984
If you are creating an abstraction or core project where you define your models, while the model is using MarkdownField, there is no need to add dependency on the OC.Markdown on the core or abstraction model .
for example, I want to create a custom model in my project called xyz. I created xyz.Core or xyz.Abstraction where I keep a model that uses MarkdownField. In this case I won’t have to depend on OC.Markdown, instead all I would add is a reference to the abstractions
Still I didn't understand, when you need a MarkdownField you should reference it's project. If that's the case HtmlField should have an abstractions
Again I didn't see a value unless the implementation should be different for a particular abstractions project
Let me try to explain it in code, I hope this helps. In CustomModule.Abstraction project I want to add the following modal
public class SomeModel
{
public MarkdownField Content { get; set; }
}
For this to work currently, I would have to add dependency to OrchardCore.Markdown on my CustomModule.Abstraction which isn't needed here because CustomModule.Abstraction may not need to depend on the CMS or for the simple fact that my abstraction project does not need OrchardCore.Markdown. If we move the MarkdownField to OrchardCore.Markdown.Abstractions then CustomModule.Abstraction would only depend on OrchardCore.Markdown.Abstractions.
I got what you want to explain, but this something special in your case, If we want plan to accept this, what about HtmlField and other fields?
IMO, I think all field ContentFields, Markdown, MediaField, HtmlField all should be moved to their abstraction project. All these models and their settings can be used in Abstraction and Core project.
If it's like that do we really treat fields as abstractions? Which I don't think so, let us hear @sebastienros thought about this one?
It is just about reusability/extensibility. If you want to use a NumericField in a different module right now you need to reference the entire OrchardCore.ContentField project which is wrong to me. And yes, ContentFields needs to be reusable.
It is just about reusability/extensibility.
Agree, but there many things we should think about. I remembered two or three times I required to reference a module in Orchard Core Contrib (OCC) because OC encapsulate many of the types and services inside some modules. Hope to find time I mention some cases
Yes, I agree with you @hishamco but it is part of the tasks that we need to do to make OC more like a Framework. And these tasks will take time. It is probably better to do the work progressively.
@CrestApps just one question if we use something like this
public MarkdownField Content { get; set; }
How the UI will be generated for such field in the referenced module?!!
@hishamco the field will be added to a model to a CustomModule.Abstraction project. So it will not need to render the field. This would just be part of abstraction/core project. The module that would consume this project would then need to depend on the Markdown feature. The idea here is structuring the code is much reusable way.
This means any custom markdown module need to do all the heavt lifting instead of reusing the work that done in OC.Markdown
I don’t think I follow you. We do this just to abstract away the code. The module will still be referenced by other modules when needed.
The module will still be referenced by other modules when needed.
This could be done outside OC but not inside ;)
I am not sure I follow you. I guess the question is, do you see an issue with abstracting the model and the setting?
I guess the question is, do you see an issue with abstracting the model and the setting?
Nope, but wondering while any field should has its own UI, so is that mean any implementation should has to redefine the UI. I never mind if all the guys are agreed with the idea
@hishamco another good reason for this is to prevent the use of internal classes.
For example, if you look at the Taxonomies migration you'll see internal classes AliasPartSettings and AutoroutePartSettings created as shadow to the actual settings class, just because AliasPartSettings and AutoroutePartSettings are not part of the OrchardCore.Alias.Abstractions or OrchardCore.Autoroute.Core. The problem with creating internal models like that is what if the actual setting model change? I see no reason for doing such an approach “creating shadows classes” where we can simply move the model and setting into the abstraction class and everything will just fall into place as it should.
The Markdown and other modules falls into the same problem as TaxonomiesMigration. In fact settings and models classes are already part of many abstraction projects in OC. So at the very least there is a consistency problem with not moving the model and the settings into the abstraction project.
I think abstraction project should contain, interfaces, core/domain models including setting models.
Core project will include implementations and services and will depend on abstraction project. Then, the CMS module project will have everything else and will depend on the core project.