Feature Request: Add in Static Getters for Compositions

[hfloyd]

Copied from a comment on another issue:

For reference, I've attached here two .generated.cs files from a v8 project which used OMB, which shows how the static "Getters" were generated.

One file is a "Composition" type (I always name my Composition Content Types with the prefix "Comp"), and the other is a "standard" Content Type, which includes a bunch of Compositions (so you can see how the properties are referenced). What was interesting here is the use of what Stephan called "Mixins" for the Compositions. Rather than just grabbing the property via alias directly, Compositions added an extra "layer" of static functions that get the data, and the property calls the function. What was really cool about this was that if you customized a Composition - specifically how a property value was obtained - that customization would apply to all Content Types which also used that Composition/Property.

MB_DEMO_TextPage.generated.cs.txt MB_DEMO_CompHeroBanner.generated.cs.txt

For example, say you had a standard "CompPageSettings", with a string property of "MetaTitle", and if the property from the CMS data was blank, the Node Name was returned (so that property would always have a valid value). You would only have to make that update to the Composition code file, and all the Content Types, since they are calling that same static function, will automatically be getting the right data back.

Example:

 public partial class CompPageSettings
    {
        /// <summary>Static getter for Meta Title</summary>
        public static string GetMetaTitle(ICompPageSettings that)
        {
            var meta = that.GetPropertyValue("MetaTitle").ToString();
            return meta != "" ? meta : that.Name;
        }
    }

    /// <summary>Contact Us Page</summary>
    [PublishedContentModel("ContactUsPage")]
    public partial class ContactUsPage : PublishedContentModel, ICompPageSettings, 
    {
        ...
        public ContactUsPage(IPublishedContent content): base(content){ }
        ...

        ///<summary>
        /// Meta Title
        ///</summary>
        [ImplementPropertyType("MetaTitle")]
        public string MetaTitle
        {
            get { return MySite.Models.CompPageSettings.GetMetaTitle(this); }
        }
        ...
    }

Originally posted by @hfloyd in https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/1#issuecomment-1047212425

[hfloyd]

Ps. In the Original ModelsBuilder, enabling this was a configuration (see "Umbraco.ModelsBuilder.StaticMixinGetters" on https://github.com/modelsbuilder/ModelsBuilder.Original/wiki/Install-And-Configure)

Perhaps it makes sense for it to be configurable here as well?

[abjerner]

Hi @hfloyd

If I had had five extra minutes at work today, I would have pushed as new beta. So I just did that when seeing this issue. Try giving v1.0.0-beta002 a go regarding the mixins, missing media compositions etc. 😉

I don't remember for sure, but I think EMB generated these mixins by default, so maybe it should also just be default for this package? That's at least how it's working now.

[hfloyd]

I don't remember for sure, but I think EMB generated these mixins by default, so maybe it should also just be default for this package? That's at least how it's working now.

Okay, if it's the default behavior, that's fine with me. :-) I will give it a go and provide any feedback. Thanks!

[hfloyd]

Hi @abjerner I tested the new version today. I noticed the static functions are getting generated nicely, and if I had a property defined in a partial, it was NOT getting generated, so that's working great.

One thing I noticed was that the Compositions aren't getting Properties, though, just the static functions:

I was most used to having both on the Compositions. example:

Was that an oversight, or by design?

Also, It seems that the types which use the composition, aren't getting the composition properties:

This seems to be related to the previous issue of no properties being generated for comps (thus the derivative models aren't "missing" those properties).

Is the idea that those composition properties wouldn't be accessible except via calling the static class directly? var prevId = myModel.GetPreviousNodeId()

[abjerner]

@hfloyd it was probably just me not thinking through what I was actually doing 😆

Luckily it should be easy to remove my checks that are omitting the properties in beta002. I was about to work a bit on our packages anyways, so I'll have a look at this as well 😉

[abjerner]

@hfloyd on second thought, isn't this because you have the property in your custom partial? I've tried setting up a similar composition, and the property generates fine in MediaCompLegacyData.generated.cs.

[hfloyd]

Well I only had it in the custom because it wasn't getting auto- generated. Generally if they could be auto-generated that would be great and then we wouldn't need so many customized partials

[abjerner]

Hi @hfloyd

I made my reply before having another look at the code:

it was probably just me not thinking through what I was actually doing 😆

But with beta002, I get the following MediaCompLegacyData.generated.cs if I don't have the file in advance:

//------------------------------------------------------------------------------
// <auto-generated>
//
//    Limbo.Umbraco.ModelsBuilder v1.0.0-beta002
//
//   Changes to this file will be lost if the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------

using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Infrastructure.ModelsBuilder;
using Umbraco.Extensions;

#pragma warning disable 0109
#pragma warning disable 1591

namespace UmbracoNineTests.Models.Umbraco.Media {

    public partial interface IMediaCompLegacyData : IPublishedContent {

        int PreviousNodeId { get; }

    }

    public partial class MediaCompLegacyData : PublishedContentModel, IMediaCompLegacyData {

        #region Constants

        public new const string ModelTypeAlias = "mediaCompLegacyData";

        #endregion

        #region Constructors

        public MediaCompLegacyData(IPublishedContent content, IPublishedValueFallback publishedValueFallback) : base(content, publishedValueFallback) { }

        #endregion

        #region Properties

        [ImplementPropertyType("previousNodeId")]
        public int PreviousNodeId => this.Value<int>("previousNodeId");

        #endregion

        #region Static methods

        public static int GetPreviousNodeId(IMediaCompLegacyData that) => that.Value<int>("previousNodeId");

        #endregion

    }

}

How does the complete file look at your end?

[hfloyd]

So, I had previous commented-out the custom partial code, so just to be sure, I deleted it ... So my custom looks like this:

    using Umbraco.Cms.Core.Models.PublishedContent;
    using Umbraco.Cms.Infrastructure.ModelsBuilder;
    using Umbraco.Extensions;

    public partial class MediaCompLegacyData
    {
      }

I re-built and re-ran "/umbraco/backoffice/Limbo/ModelsBuilder/GenerateModels". The resulting file contents are this:

  public partial interface IMediaCompLegacyData : IPublishedContent {

    }

    public partial class MediaCompLegacyData : PublishedContentModel, IMediaCompLegacyData {

        #region Constants

        public new const string ModelTypeAlias = "MediaCompLegacyData";

        #endregion

        #region Constructors

        public MediaCompLegacyData(IPublishedContent content, IPublishedValueFallback publishedValueFallback) : base(content, publishedValueFallback) { }

        #endregion

        #region Properties

        #endregion

        #region Static methods

        public static int GetPreviousNodeId(IMediaCompLegacyData that) => that.Value<int>("PreviousNodeId");

        #endregion

    }

So, for some reason, I'm not getting the Interface property either. Do I have to somehow "clear" out any records of the custom code - separate from the rebuilding?

[hfloyd]

Doh! 😣🤦‍♀️

Because it was originally not working, I added this to the GetModelsNotificationHandler

                     if (property.Alias == "PreviousNodeId")
                    {
                        property.IsIgnored = true;
                    }

I removed that, and regenerated, and it now looks like the code you provided. Sorry for the false alarm there...

However, one thing...

this bit:

        #region Properties

        [ImplementPropertyType("PreviousNodeId")]
        public int PreviousNodeId => this.Value<int>("PreviousNodeId");

        #endregion

should really be like this (using the static function - so customizing that function will work everywhere):

        #region Properties

        [ImplementPropertyType("PreviousNodeId")]
        public int PreviousNodeId => GetPreviousNodeId(this);

        #endregion

[abjerner]

Yes, not sure why I overlooked that 👍

It's fixed in 8908742654aff5eb5a42939e2e9b7c6a60a01d8c and released in v1.0.0-beta004 😉

[hfloyd]

Brilliant! Working perfectly now. 👏🏻 Thanks for the fix. (I also noticed you are now retuning a bit more info on the success JSON - nice!

[abjerner]

@hfloyd then wait till you see the dashboard 🎉

(that's why I added the extra information in the API output)

Anyways, I'm assuming the issue with the static getter methods now has been resolved, so I'm closing the issue. If I've still missed something, feel free to open the issue again 😉

[hfloyd]

Sexy! I figured you were planning to use that info.

Ooh... I see I've got that now! It was a secret bonus in the release! Thanks for the present 🎁