modelsbuilder / ModelsBuilder.Original

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

Composed properties are always generated ignoring "ImplementPropertyType". #128

Open SayTen opened 7 years ago

SayTen commented 7 years ago

The generated class would be something like:

public partial class Page: PublishedContentModel, ISeoComposition
{
    ...
    ///<summary>
    /// Meta Description
    ///</summary>
    [ImplementPropertyType("metaDescription")]
    public string MetaDescription
    {
        get { return SeoComposition.GetMetaDescription(this); }
    }
    ...
}

And my custom half is something like:

    public partial class Page
    {
        [ImplementPropertyType("metaDescription")]
        public string MetaDescription
        {
            get
            {
                // if metaDescription is set return that, otherwise return another processed property.
            }
        }

My issue is that with the ImplementPropertyType on the concrete class for a composed property it is generated when running the tool again. I've seen similar issues both open and closed but I'm not looking to change the interface signature here. Is this replicatable by someone else?

jbreuer commented 7 years ago

You probably need to override the property on the doctype Model that is used as the composition. Not sure how that works though.

zpqrtbnk commented 7 years ago

Hey, As soon as you declare that you implement a property in a local partial, we should not generate it. Will look into it.

aDotNetDeveloper commented 7 years ago

Ive also have this occurring.

Also the scenario of IgnorePropertyType on the overriding partial is not stopping the property from being generated - assuming this is the expected behaviour.

[IgnorePropertyType("metaDescription")]
public partial class Page
{
...
callumbwhyte commented 6 years ago

Did this ever get resolved? I'm running the latest ModelsBuilder (v3.0.10) with an almost identical situation to @SayTen above and can't seem to get my head around it.

daniel-chenery commented 6 years ago

@callumbwhyte I've run into this problem before, turns out I didn't put my partial in App_Data/Models, as soon as I popped it in there it worked.

zpqrtbnk commented 5 years ago

Better late than never... I am currently looking into this issue/question. What happens is this: because the MetaDescription property is a composition property, I assumed that if you wanted to change its behavior, you would want to do it at composition level.

The SeoComposition class provides a static getter for the property, which is then used by all classes using the composition. If you provide that getter (see below) we'll detect it, we won't generate it, and it will be used by all class using the composition:

public partial class SeoComposition
{
  public static string GetMetaDescription(ISeoComposition that) => ""..."";
}

That should be the proper way to provide your own implementation of a composition property.

Note: you probably want to avoid providing the entire property implementation (via [ImplementPropertyType] on a SeoComposition.MetaDescription property) as that would disable everything we may generate for the property, in all classes using the composition. Just work with the static getter.

That being said.

Once could argue that your usage of the ImplementPropertyType is perfectly valid. Hey, you may want to provide the implementation of the property for that class only. I did not think of that scenario. Going to look at how hard it would be to support it.

zpqrtbnk commented 5 years ago

Not too hard, going to fix.