search

kgiszewski / Archetype

Archetype is an Umbraco 7 property editor that wraps other installed property editors.
https://github.com/kgiszewski/ArchetypeManual
MIT License
89 stars 54 forks source link

TypeConverter for ArchetypeModel #379

Closed leekelleher closed 7 years ago

leekelleher commented 8 years ago

Added a TypeConverter for both ArchetypeModel and ArchetypeFieldsetModel, so that they can be converted to their IPublishedContent equivalent.

Example usage would be, when you would typically do this...

var items = Model.Content.GetPropertyValue<ArchetypeModel>myArchetypeAlias");

You can now do this...

var items = Model.Content.GetPropertyValue<IEnumerable<IPublishedContent>>("myArchetypeAlias");

I know it seems more verbose, but the idea is that this could be hot-swappable with other property-editors, such as Nested Content. The frontend/code would deal purely with IPublishedContent objects.

Also included unit-tests that run via Umbraco Core's TryConvertTo extension method, (which is what Content.GetPropertyValue<T> uses internally in Umbraco).

There's a small caveat with the TypeConverter for ArchetypeFieldsetModel, I had to explicitly reject typeof(string), otherwise the JSON deserializer fails (in ArchetypeHelper.DeserializeJsonToArchetype). All other unit-tests still pass.

kgiszewski commented 8 years ago

Thanks @leekelleher, feel free to merge it unless you want a sanity check ;)

leekelleher commented 8 years ago

@kgiszewski It's one of those itches that I'd been wanting to scratch since doing the IPublishedContent code, but never got around to it (until last night). I don't have a need for it myself (yet), all my code is currently based on ArchetypeModel, and as far as I'm aware no one has requested this. So yeah, just me scratching an itch.

I haven't done any end-to-end/integration testing with it.

I guess the main question is, do we think this is useful?

kgiszewski commented 8 years ago

It's hard for me to say. I haven't done much with Umbraco in quite some time. For my purposes, I've always only needed the var items = Model.Content.GetPropertyValue<ArchetypeModel>myArchetypeAlias"); form.

I'd say if you don't have a direct need for this yet, it may behoove us to wait to merge but keep this PR here unless it's pretty much a transparent set of changes.

We haven't really done much with the code in awhile (good thing IMHO), so I've grown rather indifferent on the current landscape of Umbraco Core vs Archetype.

Maybe @kjac has an opinion :)

kjac commented 8 years ago

@leekelleher @kgiszewski I had a look at the PR and it looks pretty clean. And yes, it makes sense. Property retrieval is always mentioned as the culprit of working with Archetype for non-coders that are used to (read: have been forced to use) IPublishedContent.

I will pull the PR and try it out locally, then I'll get back to you. Thanks, @leekelleher 👍

leekelleher commented 7 years ago

Woo! (I'd totally forgot about this one... oops! :relaxed:)