Property type is not changed into the correct type

[stefankip]

This sounds like an awesome package, exactly what I need to get rid of all those IPublishedContent properties and get rid of all my manual overrides!
But...
It doesn't seem to work for me?

I have this MNTP:

And this is the generated property:

The type should be Ambassador here, right?

[stefankip]

To be honest, stuff got worse after installing this package.
For example, before I had this generated code:
public IEnumerable<ShopHeroSliderSlide> SliderItems => this.Value<IEnumerable<ShopHeroSliderSlide>>("sliderItems");
Which now became:
public IEnumerable<IPublishedElement> SliderItems => this.Value<IEnumerable<IPublishedElement>>("sliderItems");

[callumbwhyte]

Hey Stefan,

Thanks for using SuperValueConverters!

There is a small issue where you have to rebuild and regenerate your models a second time in order for it to discover all the types needed to be returned - this might be what is happening here? Can you try rebuilding your solution and regenerating your models to see if that helps?

If that doesn't work, please let me know your Umbraco version and version of the package you are using.

Cheers,
Callum

[stefankip]

I've rebuild the solution 3 times and rebuild the models every time as well, the result is the same.
Where I expect a strongly typed model, it remains IPublishedContent and some strongly types stuff is reverted to IPublishedElement as described in #16 (comment)

Umbraco v8.6.3, super value converter v2.0.1

[callumbwhyte]

Thanks for clarifying!

Just to check also which Models Builder mode you are using? And to confirm you are using the Models Builder plugin rather than embedded?

Cheers,
Callum

[stefankip]

The API mode with visual studio plugin

[stefankip]

I could really use this strongly typed feature right now, can I help debug?

[callumbwhyte]

It would be good if you could try and replicate this on a clean Umbraco install. Blank VS solution > Install Umbraco + the SuperValueConverters from NuGet.

If you can replicate, you should then clone down the source - attach to the process when generating models and see what's going on in this method.

I will be able to have a look at it shortly too, but no guarantees about timeframes unfortunately.

[stefankip]

I tested this just now on a clean solution (with MB in PureLive mode I think) and it worked.
So now the question is, why doesn't it work in my solution (using generated models from the API)... Will investigate.

[stefankip]

Soooooo I think the problem is generating models via the API in Visual Studio.
I switched to the pre-generated models solution and now this is what gets generated:
public IPublishedContent Person => this.Value<IPublishedContent>("person");

[stefankip]

Right, the namespace is the issue!

{
    var value = ConfigurationManager.AppSettings[Prefix + "ModelsNamespace"];

    if (string.IsNullOrWhiteSpace(value) == false)
    {
        return value;
    }

    return "Umbraco.Web.PublishedContentModels";
}

I have my namespace configured via a C# class, as mentioned here:
https://github.com/modelsbuilder/ModelsBuilder.Original/wiki/Control-Generation#models-namespace

[callumbwhyte]

Great find @stefankip! I can confirm I can replicate this also on AppData mode.

This seems to be a bit of an oversight in the implementation, and honestly this namespace check and the dependency on Models Builder shouldn't actually be needed at all.

This namespace check is purely there to try and avoid clashes between type names in namespaces we don't care about. The safer solution is surely to only check for types that implement IPublishedElement, and this would actually improve extensibility too as packages could return their own types.

I've been investigating options of switching the logic determining types to Umbraco's IPublishedModelFactory and have a prototype of this working but need a little more time to work out how to replicate the "common interface" logic we have in SuperValueConverters.

For now is it a suitable workaround to set this app setting?

[stefankip]

Yeah works fine using the appSetting, so I'll keep that in place for now ;-)