modelsbuilder / ModelsBuilder.Original

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

Methods/parameters on partial classes not inherited #147

Open harvzor opened 7 years ago

harvzor commented 7 years ago

In Umbraco I have Page document type which inherit from an SEO document type.

My SEO document type has a field called Page Title which the user can use to overwrite the page's Name on the front end.

ModelsBuilder generates something like this:

// Autogenerated:
namespace Umbraco.Web.PublishedContentModels
{
    public partial interface ISEO : IPublishedContent
    {
        string PageTitle { get; }
    }

    public partial class SEO : PublishedContentModel, ISEO
    {
        public string PageTitle
        {
            get { return GetPageTitle(this); }
        }
    }
}

ModelsBuilder also generates some code for my Page:

// Autogenerated:
namespace Umbraco.Web.PublishedContentModels
{
    public partial class Page : PublishedContentModel, ISEO
    {
        ...
    }
}

I can use this property on a view:

<!-- Where page is of type ContentModels.Page. -->
<h1>@(page.PageTitle.IsNullOrWhiteSpace() ? page.Name : page.PageTitle)</h1>

However, I decide that doing this on multiple pages isn't super DRY. I decide to add to the SEO partial class:

// Custom:
namespace Umbraco.Web.PublishedContentModels
{
    public partial class SEO : PublishedContentModel, ISEO
    {
        public string GetPageTitle()
        {
            return PageTitle.IsNullOrWhiteSpace() ? Name : PageTitle;
        }
    }
}

However, this property isn't on the Page content model, because that model inherits from ISEO and not the SEO model.

This forces me to do this:

<!-- Where page is of type ContentModels.Page. -->
@{
    var seo = new ContentModels.SEO(page);
}
<h1>@(seo.GetPageTitle())</h1>

I'd much rather the GetPageTitle() method was by default available on my Page. I could put the method onto the ISEO interface and then create custom partial classes for all models which inherit from ISEO but this is still not very DRY.

Is there a solution for this?

I can see something similar here: https://github.com/zpqrtbnk/Zbu.ModelsBuilder/issues/40#issuecomment-77839706

But I tried implementing your example and the auto generated items did not automatically implement my method.

P.S. Thanks for the great work with this. ModelsBuilder improves my life!

zpqrtbnk commented 7 years ago

Just akn the (very valid) question here - will try to reply asap ;-)

skttl commented 5 years ago

Would love to have this.

I can see from previous issues that determining what methods from partial classes should be copyed can be hard. But can we help by marking those methods with attributes?

Something in the likes of


        [ImplementPropertyType("something")]
        [AllowInheritingFromComposition]
        public string Something
        {
zpqrtbnk commented 5 years ago

I know this is an old issue but... trying to close it. Experimented with v8. I have two content types, co1 and co2, and co2 is composed of co1. By default, models are generated as (with non-essential stuff removed):

public partial interface ICo1 : IPublishedContent
{
    string Text { get; }
}

public partial class Co1 : PublishedContentModel, ICo1
{
    public string Text => GetText(this);

    public static string GetText(ICo1 that) => that.Value<string>("text");
}

public partial class Co2 : PublishedContentModel, ICo1
{
    public string Text => Co1.GetText(this);
}

Now if I understand correctly, you want to change what the Text property returns, at composition level. I have dropped the following partial code in the models directory:

public partial class Co1
{
    public static string GetText(ICo1 that) => that.Value<string>("text") + "!!";
}

And re-generated models. Models Builder detects that GetText is implemented, and does not re-implement it. Now, Co1 generated code looks like:

public partial class Co1 : PublishedContentModel, ICo1
{
    public string Text => GetText(this);
}

Meaning that anything composed of Co1, which would be getting Text via

public string Text => Co1.GetText(this);

Will see the changed value.

Is that what you are trying to achieve?

skttl commented 5 years ago

Seems right.

Is that new to v8, or can I do the same in v7?

zpqrtbnk commented 5 years ago

Can you try on v7? The generation code hasn't changed much, I'm pretty sure it works on v7.

skttl commented 5 years ago

Tried last night.

Using AppData models and generating to another project for building a dll.

At first I tried adding the method with a different type (as this is what I typically want - think Nested Content as their MB models). That didn't work, after regenerating the project would not build, because the Get{0} method wasn't found (I had it in my partial class).

I then tried changing my own Get{0} method to the same type, but got the same problem.

zpqrtbnk commented 5 years ago

One thing I notice in your original question is... GetPageTitle should be a static method. Making sense?

Alkaersig commented 5 years ago

Will this also fix the "opposite" (https://github.com/zpqrtbnk/Zbu.ModelsBuilder/issues/128). Implemeting properties that inherit from compositions are ignored atm. which is an issue i've run into quite a few times.