modelsbuilder / ModelsBuilder.Original

The Community Models Builder for Umbraco
MIT License
114 stars 49 forks source link

BaseModel attribute for PublishedElementModel #205

Open ViGiLnT opened 5 years ago

ViGiLnT commented 5 years ago

In Umbraco 8.0.2, using ModelsBuilder 8.0.4 I'm unable to define a custom BaseModel because now generated Models may inherit from PublishedElementModel and PublishedContentModel. I have a document type which is an Element for NestedContent.

Can't we have another attribute for ElementBaseModel? Like:

[assembly: ModelsBaseClass(typeof(BaseModel))] [assembly: ElementModelsBaseClass(typeof(ElementBaseModel))]

zpqrtbnk commented 5 years ago

Oups. Valid point.

pgregorynz commented 5 years ago

Pretty vital that this one gets fixed ASAP as it is negating some of the benefits of using Models Builder. The base class functionality reduces the amount of Helper methods and common properties we need to define. We are stalled at present due to this issue.

pgregorynz commented 5 years ago

Have created pull request to at least ignore the element type from gaining the custom IpublishedContent base class. https://github.com/zpqrtbnk/Zbu.ModelsBuilder/pull/211

zpqrtbnk commented 5 years ago

Thanks! Definitively going to review. Might take a few more days as I'm partially offline for 1 more week. Stay tuned!

pgregorynz commented 5 years ago

We have now also added in the attribute for base classing element types. Just needs a Unit test :)

zpqrtbnk commented 5 years ago

Took most of the code from the PR + tweaked things & implemented tests. Now we would have:

Patterns can be a plain type name (eg page) but also support leading and trailing wildcards (eg *page or page*) and I am not sure we want to support more fancy patterns.

Patterns are ordered by length, longest first, before being applied, and the first one to match wins. So if you have both a pageFoo* and a page* pattern, the first one would apply for pageFooBar.

Thoughts?

pgregorynz commented 5 years ago

The pattern stuff is great! We actually have a version close to this on our fork that we were considering submitting that allowed for selective models that had the pattern so you could have multiple base models... Does your version allow for this also?

EG a catch all sort of base model, and then selective base models for other types eg "page*"

If that is the case this is awesome and is perfect.

zpqrtbnk commented 5 years ago

The pattern-less version [ContentModelsBaseClass(type)] actually registers type with a * pattern, which acts as a catch-all and, being the shortest one, matches last. So yes, you can define a catch-all + selective for some patterns.

Here I am assuming that patterns will mostly be "page" or "thing" and that we don't want/need to support crazy regex patterns.

pgregorynz commented 5 years ago

In our instance we have a PageBaseModel and a BlockBaseModel in the same project.
The builder works out which one to use when it is parsing the doctypes based on a pattern.

Changes can be seen here:

https://github.com/pgregorynz/Zbu.ModelsBuilder/commit/f9124dc56ba103311c5fd973e0764923067f6565

The attribute on our class was:

[assembly: SelectiveModelsBaseClass(typeof(BlockBaseClass), "*Block")]

We will update to your version and make sure it does what I think it does and will let you know but it looks about right based on your description 👍

zpqrtbnk commented 5 years ago

[ContentModelsBaseClass("*Block", typeof(BlockBaseClass))] should do exactly this ;-)

pgregorynz commented 5 years ago

NICE! Perfect.. We will update our build of models builder and give it a whirl.

Overall I think this will make ModelsBuilder super flexible. Our version did, so having it official is awesome!

euanrae commented 4 years ago

was this feature ever released? I'm using Umbraco 8.1.3 and Umbraco.ModelsBuilder 8.1.0 and I can't see it?

Thanks

zpqrtbnk commented 4 years ago

Not released. A new version of MB is in-progress but, due to the september organization changes, I am only working on it when I have some time available. Back to full open-source mode: it will hopefully be ready, when it's ready ;)

euanrae commented 4 years ago

all good - thanks for letting me know (wanted to make sure it wasn't there and I was just being dense)!

Zweben commented 4 years ago

I'd really like to take advantage of base models on my project. Is there any way I can use it at the moment with Umbraco 8.6, such as a stable fork that contains the fix, or a temporary workaround?

dhymik commented 4 years ago

I am also very interested....

Andrei- commented 4 years ago

I would also love to use this, any updates?

bielu commented 4 years ago

@zpqrtbnk I checked that and I am using configuration, which replaces your orginal idea, and works perfectly fine.