Ditto doesnt play nice with RelatedLinks about umbraco-ditto HOT 11 CLOSED

@bluefindigital Thanks for flagging it up.

The RelatedLinks type isn't from Umbraco core, it's from @Jeavon's PropertyConverter library (ref link).

The issue is that Ditto will try to make a new instance of the RelatedLinks object, but since the constructor requires a parameter, we have no idea what value to pass into it. If the type had an additional parameterless constructor, then it wouldn't be an issue.

https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/blob/v2/Our.Umbraco.PropertyConverters/Models/RelatedLinks.cs#L44

// pinging @Jeavon in on this, in case he has any thoughts on the topic.

from umbraco-ditto.

@leekelleher @bluefindigital Hmm, I guess the property is empty? I have just had a quick look at a recent project we have used RelatedLinks with Ditto

    public RelatedLinks RelatedPages { get; set; }

from umbraco-ditto.

@Jeavon Thanks for jumping in on this. Do you know if it's Ditto v0.8.0+?

@bluefindigital You mention a DittoValueResolver, do you have a custom one? or do you mean the default (Ditto) UmbracoProperty one?

from umbraco-ditto.

@leekelleher ah no, it's Ditto v0.7.0, am I going to need a empty constructor for v0.8.0+?

from umbraco-ditto.

I'll double check what we changed in v0.8.0. I vaguely recall making a decision with @mattbrailsford about constructor parameters of unknown type, that we'd throw an exception. I have a sneaky feeling that previous we were swallowing the exception. I'll check.

Otherwise, yeah, RelatedLinks would need an empty constructor.

from umbraco-ditto.

Ok, don't see why adding empty constructor would be a issue...?

from umbraco-ditto.

@Jeavon Sorry, I've misread your last reply 😕 - do you mean it's okay to add an empty constructor to RelatedLinks or that it shouldn't need it?

Re: Ditto - I'm a bit confused with what is going on, as the difference between Ditto 0.7 and 0.8 is minimal...

0.7.0 - https://github.com/leekelleher/umbraco-ditto/blob/0.7.0/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs#L277
0.8.0 - https://github.com/leekelleher/umbraco-ditto/blob/0.8.0/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs#L277

Following the logic, then trying to map a property (of type RelatedLinks) should have also thrown an exception with Ditto 0.7.0.

@bluefindigital Can you provide any code snippets? (Just for my sanity, more than anything else) 😃

from umbraco-ditto.

@leekelleher I just tried it out and added Ditto to the UCPVC test site and it "just worked" so I'm not sure...?

https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/blob/v3/TestSite/Views/umbHomePage.cshtml#L96

https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/blob/v3/TestSite.Logic/Models/Home.cs

from umbraco-ditto.

@Jeavon Oooo cool! 😎 (Thanks for testing it out)

I guess we need @bluefindigital to clarify the issue he experienced?

from umbraco-ditto.

Sorry guys this turned out to be my mistake. I am trying out Ditflo at the moment and the problem turned out to be how I was trying to resolve a recursive property in a DittoResolver. I was ending up trying to resolve the whole IPublishedContent as RelatedLinks, not the property itself. Apologies, false alarm!

from umbraco-ditto.

No worries @bluefindigital!

I wasn't 100% sure myself. I've added a unit-test to satisfy my curiosity 😃

from umbraco-ditto.