Lecoati / LeBlender

LeBlender is an open source Umbraco backoffice extension which made the Grid Canvas Editors management easier and flexible.
30 stars 63 forks source link

Issues when using core property value converters #80

Open bjarnef opened 7 years ago

bjarnef commented 7 years ago

When using core property value converters in Umbraco 7.6.x and 7.7.x and casting e.g. value from an Author Picker Umbraco.MultiNodeTreePicker2 to IEnumerable<IPublishedContent> it returns an exception:

Der opstod en fejl under udførelsen af den underordnede anmodning for handler 'System.Web.Mvc.HttpHandlerUtil+ServerExecuteHttpHandlerAsyncWrapper'.
foreach (BlogPost blogPost in blogPosts)
{
     Employee author = (blogPost.Author ?? Enumerable.Empty<IPublishedContent>()).FirstOrDefault() as Employee;
}

When I use same logic just in a regular partial view there are no issues.

bjarnef commented 7 years ago

I had switcher from Umbraco.MultiNodeTreePicker to Umbraco.MultiNodeTreePicker2, but I also needed to re-select the items and recycle the application pool.

Jeavon commented 7 years ago

Line 49 & 50 in the below file should be removed

https://github.com/Lecoati/LeBlender/blob/e11905f192a1cb733ec012404998ff74f0b56ca5/Src/Lecoati.LeBlender.Extension/Models/LeBlenderPropertyModel.cs#L49

Source should never be returned, it should always be Object that is returned

bjarnef commented 7 years ago

It would be awesome if you submit a PR for that @Jeavon

I also think it would be great to have this overload method as in Umbraco core 👍 https://github.com/Lecoati/LeBlender/issues/81

bjarnef commented 7 years ago

@soreng could you have a look at this one please? 😁

I think you should be able to reproduce if you add a LeBlender property using Multi Url Picker. https://github.com/rasmusjp/umbraco-multi-url-picker/issues/63

E.g. create two datatype instances of Multi Url Picker:

Single Url Picker returns Link, so Model.Items.First().GetValue<Link>("link") should work. Multi Url Picker returns IEnumerable<Link>, so Model.Items.First().GetValue<IEnumerable<Link>>("links") should work.

soreng commented 6 years ago

hi @bjarnef I can have a look at it later, but I can't really say when it will be :-/

I will be looking into a few thing (maybe a v2) in the not so distant future

bjarnef commented 6 years ago

Okay, hopefully you get some time to look at this issue. In a project I am working on, it works with Model.Items.First().GetValue<Link>("link") which use multiple LeBlender items, but not when item a single LeBlender item (default if no min/max it set).

I have also had some issues when using MNTP for some menu items in a megamenu, where it sometimes returned IPublishedContent and sometimes IEnumerable<IPublishedContent> or just when casting Model.Items.First().GetValue<string>("link"), where I had to add a bit of a hack to make it work 😁 https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/issues/24#issuecomment-339716515

bjarnef commented 6 years ago

@soreng have you tried LeBlender with ModelsBuilder and property value converters enabled.

I have often see then returned value not is they expected inside LeBlender, while it works correct when the properties are directly on document types. It happens for e.g. Multinode Tree Picker, RJP MultiUrlPicker and others.

soreng commented 6 years ago

hi @bjarnef

after reading the documentation, I can see the LeBlender impl. is wrong, as per @Jeavon's reply above.

this need a test and a fix :)

bjarnef commented 6 years ago

@soreng looking forward a fix for this since it has really bothered me now and then, where it was returning different than expected.

Hopefully in next release and both a release for Our and NuGet. https://our.umbraco.com/projects/backoffice-extensions/leblender/

At the moment I think the latest NuGet version is broken, and doesn't contain some of the fixes which was merged to master branch before the release of the latest NuGet package.

soreng commented 6 years ago

Maybe a 1.10 is comming out sometime soon - who knows

Just need access to upload stuff to the page on our. Will see it I can get that.