limbo-works / Limbo.Umbraco.ModelsBuilder

Custom models builder for Umbraco.
MIT License
4 stars 3 forks source link

Don't generate interface properties for customized interface properties #4

Open hfloyd opened 2 years ago

hfloyd commented 2 years ago

Hi!

So, related to https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/3... Sometimes I customize the property for an interface. For instance, Content Pickers or MNTP by default will be returning "IPublishedContent", but I might know that it will always be of a specific ContentType . In order to be able to call the property and get the strongly-typed model in a View, I customize the composition model and interface. Example:

Customized 'CompRelatedBlogPosts.cs'

partial interface ICompRelatedBlogPosts
    {
        IEnumerable<BlogPost> SelectedPosts { get; }
    }

    partial class CompRelatedBlogPosts
    {
        [ImplementPropertyType("SelectedPosts")]
        public IEnumerable<BlogPost> SelectedPosts => GetSelectedPosts(this);

        public static IEnumerable<BlogPost> GetSelectedPosts(ICompRelatedBlogPosts that)
        {
            return that.Value<IEnumerable<IPublishedContent>>("SelectedPosts").ToContentModels<BlogPost>(true);
        }
    }
}

Then I just add an override to set the correct type in the Models which use the Composition. Example:

partial class TextPage
    {
        #region Implementation of ICompRelatedBlogPosts
        IEnumerable<BlogPost> ICompRelatedBlogPosts.SelectedPosts => CompRelatedBlogPosts.GetSelectedPosts(this);
        #endregion
    }

This works fine. However, when I re-generate the Models, the generated composition file gets the "IPublishedContent" property again:

'CompRelatedBlogPosts.generated.cs'

 public partial interface ICompRelatedBlogPosts : IPublishedContent {

        System.Collections.Generic.IEnumerable<global::Umbraco.Cms.Core.Models.PublishedContent.IPublishedContent> SelectedPosts { get; }

    }

I can solve the issue by explicitly ignoring the property:

'GetModelsNotificationHandler.cs'

    public class GetModelsNotificationHandler : INotificationHandler<GetModelsNotification>
    {
        public void Handle(GetModelsNotification notification)
        {
            foreach (TypeModel model in notification.Models)
            {
            ...             
                //Content-Type specific processing

                if (model.Alias == CompRelatedBlogPosts.ModelTypeAlias)
                {
                    foreach (PropertyModel property in model.Properties)
                    {
                        if (property.Alias == "SelectedPosts")
                        {
                            property.IsIgnored = true;
                        }

                    }
                }
               ... 
            }
        }
    }

But I wonder if the generating code can check for the customized property defined in a custom interface implementation and skip it, like it does for the regular properties?

abjerner commented 2 years ago

Hi @hfloyd

I don't know how OMB handles this, but looking at the custom partial to determine property type for the interface sounds like a good idea 👍

abjerner commented 2 years ago

If the picker is a MNTP, it might also be worth looking at our custom MNTP package.

It let's you select your custom "item converter" in the data type, which could then return the selected blog posts as BlogPost instead of IPublishedContent. Then the property will return the correct type regardless of whether you're using ModelsBuilder or calling the .Value() method directly.

If I implement #5, it would also mean that property values are better cached compared to when handling the conversion directly in the model.


I still like your proposal, so the above is just an example on another approach with the same end result 😉

hfloyd commented 2 years ago

@abjerner Cool package! Right now I'm working with a migrated site with a lot of standard Content Pickers in use, so it wouldn't really help all my use-cases.

Also a question - it seems your package was specifically designed such that you would manually design the data models and set up the Converter. Does it handle a basic usage of just using the standard Model for a specific existing DocType?

Regarding OMB's handling... What WAS handled by OMB was that if I "redefined" a property in an interface, it would skip re-generating it in the Composition interface. What it never did support was automatically adding the derived-models customization.

In my example, this bit I always had to add myself to all the affected Models:

partial class TextPage
    {
        #region Implementation of ICompRelatedBlogPosts
        IEnumerable<BlogPost> ICompRelatedBlogPosts.SelectedPosts => CompRelatedBlogPosts.GetSelectedPosts(this);
        #endregion
    }

Sometimes that could get annoying - if I had a Comp which I used on a lot of Doctypes, I would have to create "Custom" partials for all those Doctypes - even if this was the only customized bit. But, it's a pretty simple copy/paste for the code snippet, so not a huge deal.

abjerner commented 2 years ago

Hey @hfloyd

Just to let you know that I haven't forgotten you, but I haven't had the time yet to look through your issues. I hope I will have that soon 😉

hfloyd commented 2 years ago

No problem, @abjerner I know you have other things to work on also. I like to add issues when I come across them, but certainly have no expectations for instant attention. 😊