Getting Started Questions/Issues

[hfloyd]

Hi @abjerner Thanks for sharing this new package. 😊

While setting it up for my own project, I have come across a few things...

1. The ReadMe doc states that "The ModelsMode option how Umbraco's build-in ModelsBuilder should work. To avoid conflicts with this package, the value should always be set to None." (https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder#models-mode) However, I was wondering if it should actually be "Nothing" (as per the official Umbraco config docs: https://our.umbraco.com/documentation/Reference/V9-Config/ModelsBuilderSettings/#models-mode)?

2. I have a Media Composition defined and added to the three standard Media types (File, Folder, Image). After generating the models, they don't build successfully - I get a message for each of the three Media types like this: 'Image' does not implement interface member 'IMediaCompLegacyData.PreviousNodeId' Is there some way I can manually specify the implementation for those three items? In Umbraco 8 I used the VSIX ModelsBuilder and would have partial class files where I could include manual implementations as needed, but I'm not sure if that same sort of code will work with your package? Example:

   #region Implementation of IMediaCompLegacyData
   public int PreviousNodeId=> MediaCompLegacyData.GetPreviousNodeId(this);
   #endregion

[hfloyd]

So, I answered my own second question - I was able to implement the partial classes and add in that code, however, I also had to manually implement changes to MediaCompLegacyData:

 public partial class MediaCompLegacyData
    {
        [ImplementPropertyType("PreviousNodeId")]
        public int PreviousNodeId => GetPreviousNodeId(this);

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

Is this something you might be interested in adding to the library? I wouldn't mind assisting with the implementation, but wanted to check in about your plans...

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

[hfloyd]

I did notice that when I regenerated the models, my customized property was ignored, and the original code was put back into the .generated.cs file:

 public partial class MediaCompLegacyData : PublishedContentModel, IMediaCompLegacyData {

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

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

    }

So, I guess the "[ImplementPropertyType("PreviousNodeId")]" is ignored, which is a different behavior to the original MB...

I did figure out how to stop it from generating via the GetModelsNotificationHandler :

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

[abjerner]

Hi @hfloyd

Yes, the models mode should be Nothing. That's also what I have in my test solution, so not sure why I wrote that wrong. I've fixed it now in the readme 😉

As for the package it self, I don't really have a good overview of what is missing compared the original Models Builder. At least not at the moment.

The ImplementPropertyType attribute should work - IIRC I check for both the one on this package, and the one that now ships with the build-in Embedded Models Builder. Creating a custom partial should also work, as I've done that in my test setup.

But I'm in no way certain that the logic around compositions work like they should. It has worked fine to the extend that I have tested, but then again, we're just getting started with this package. I'll try to look into that when I'm working on the package again 😉

[abjerner]

Hi again @hfloyd

I had a look and this, and found out that the properties of the composition didn't get added to the model because I was using the PropertyTypes property instead of the CompositionPropertyTypes property. I was doing it correctly for content, but now for media and members. You can see the change here: https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/commit/bf9447bbca43b0f385c658b281ce0dfe4578cd4b

There is still an issue where a property gets added to the generated model even if the property has been manually added to a custom partial. I thought this might be related to compositions, but it appears to be happening also when not using compositions, so I'll look into why that is.

[hfloyd]

Thanks for looking into it, @abjerner . I figured it might have just been something accidentally omitted for Media, since the Compositions on Content did get rendered out correctly.

Regarding...

As for the package it self, I don't really have a good overview of what is missing compared the original Models Builder. At least not at the moment.

I used the OMB (Original Models Builder ;-) ) quite a bit, and always disabled the Embedded version (once it was added to Umbraco itself), in favor of using OMB, so I am very familiar with it (at least the stuff I used). I'm not sure how much you used OMB, or if your package is more in response to missing functionality based on your personal experience with EMB (Embedded Models Builder)...

Personally, I'd love to have the OMB functionality I relied upon available in Umbraco 9, and your package is the the closest to that possibility, hence my intense interest. That being said, I know you might have your own vision of the functionality desired for your own use, and I don't want to "step on your toes" or anything.

I'm willing to assist in adding in extra functionality to your package, if you'll have me, and think the proposed changes make sense. I'd be happy to create discussions or issues for each separate "feature" if you'd prefer. Just let me know.

For general reference, I've attached here two .generated.cs files from a v8 project which used OMB, so you can see what that package 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); }
        }
        ...
    }

[abjerner]

Hi again @hfloyd

I worked a bit more on the package after my last comment, so I'm close to having implemented the static methods. I'm not sure I'm working on this package tomorrow, but I'll try committing what I have 😉

Anyways, I don't think we ever used the embedded Models Builder. We've getting used to the features that OMB provided, which is one of the reasons we're now building this package. But there might also be features of OMB that we haven't used. My comment about not knowing what was missing was mostly just to indicate that this package is work in progress, and I'm putting the pieces together as I go a long.

In terms of help, issues and PRs are always more than welcome. But I'm working on this package among a lot of other stuff, so I might not be able reply, discuss or point you in the right direction right away.

[hfloyd]

@abjerner Of course, I'm sure you have plenty of other things to do, which is why I wanted to offer assistance, rather than feature demands ;-) I went ahead and split off the "static getters for Compositions" feature to a new issue. so any further conversation could be organized better, and it could be linked-to on PRs, etc.

I really appreciate your time and efforts.

[abjerner]

Hi @hfloyd - can this be closed now as well?