Fix issue #326

[kjac]

This PR solves issue #326.

By reusing the existing hacks for the color picker and upload datatypes, we can support tags and probably also other (built-in or custom) datatypes.

NOTE: This only adds support for tags stored in JSON.

Issue #232 is probably also solved with this PR.

[kgiszewski]

@kjac Thanks for looking at this, but I can't seem to get this PR to fix the issue. Despite implementing this on 7.4.2 and clearing all the caches and what not, I'm still getting:

2016-04-02 18:44:13,415 [P6772/D5/T12] ERROR Archetype.PropertyEditors.ArchetypePropertyEditor+ArchetypePropertyValueEditor - Type validation failed. The value type: 'JArray' does not match the DataType in PropertyType with alias: 'bar' System.Exception: Type validation failed. The value type: 'JArray' does not match the DataType in PropertyType with alias: 'bar' at Umbraco.Core.Models.Property.set_Value(Object value) at Archetype.PropertyEditors.ArchetypePropertyEditor.ArchetypePropertyValueEditor.ConvertDbToEditor(Property property, PropertyType propertyType, IDataTypeService dataTypeService)

Here is the JSON that it is saving which is valid JSON:

{"fieldsets":[{"properties":[{"alias":"bar","value":["boy","girl"],"propertyEditorAlias":"Umbraco.Tags","dataTypeId":1041,"dataTypeGuid":"b6b73142-b9c1-4bf8-a16d-e1c23320b549","editorState":null,"hostContentType":null}],"alias":"fo","disabled":false,"id":"2686ac64-6ce9-4b5f-9cc6-edae390354c0","collapse":false,"isValid":true}]}

Where bar is the tags field.

[kgiszewski]

Tracing code and it goes through here:

https://github.com/umbraco/Umbraco-CMS/blob/d50e49ad37fd5ca7bad2fd6e8fc994f3408ae70c/src/Umbraco.Core/Models/Property.cs#L124

and here:

https://github.com/umbraco/Umbraco-CMS/blob/fc7cfea4ada8ccc510c8e3c1180e27360a2e7d34/src/Umbraco.Core/Models/PropertyType.cs#L373-L426

The latter is returning false. I'm confused by the form scope fixing his issue. Can you help me understand this a bit?

I think the type is JArray and since that method doesn't know anything about that type, it's falling all the way to the bottom and returning false.

If that is all true, I'm not sure where the JArray is actually created. For this to work I suppose we need it to be a string.

FWIW, the tags works but just has the annoying log entry at the moment.

[Nicholas-Westby]

I don't have an IDE open at the moment, but I'll pull a random idea out of my *** in case it happens to be right. Umbraco has some detection of data on publish operations (or maybe it was when fetching property values... I forget). It checks if it looks like JSON, and if so it does something different with that data. Maybe having a tags on a normal Umbraco property allows for this process to work, but maybe with it nested within an Archetype this process gets bypassed?

If you are curious about the JSON inference, it is actually causing a core bug. I wrote down the details here a while back: http://issues.umbraco.org/issue/U4-7382

Again, this is a long shot, but thought I'd mention it just in case. Might be worth comparing those parts of the code you linked with a tags inside and a tags outside of Archetype.

[kgiszewski]

Hmmm good thoughts. Thanks for posting that.

[kjac]

Oh hey! I've just fixed the client side so the tags datatype works and doesn't throw exceptions to the JS console. Honestly I didn't think to check the Umbraco server logs for errors after the datatype started working client side... so it's quite possible something shows in the logs. I'll dig into this - hopefully this week, depending on how swamped I get with work :)

[kgiszewski]

My suspicion is that it might be an oddity with the core but I have no proof yet. If I get some time this week I might try to add JArray to the core method returning false. Of course this might just fix a symptom so I'd like to understand where in the process a JArray is passed instead of string which a non-wrapped property editor seems to do.

[kgiszewski]

Fixed it with this: 715a2bf

I'm on my own branch now for this, will review further after lunch :)

[kgiszewski]

So this issue is two-fold.

1. The JS errors
2. The core errors

I think @kjac has a fix for the JS in this PR. I've submitted a core PR to fix the logging issue:

http://issues.umbraco.org/issue/U4-8345
umbraco/Umbraco-CMS#1233

The problem with the core side of the house is it'll only get resolved in later versions of Umbraco. i.e. someone using 7.2.x or 7.3.x will have to have their log spammed unless the core can backport this fix.

@Nicholas-Westby I know I already released 1.12.4 today but if you want to run a custom build for a bit until the next patch release; at least your JS will not freak out. I'm doing some follow up testing right after this to make sure the JS part is at least working.

[kgiszewski]

Ok, this checked out. Thanks @kjac. I've created a separate issue for the core side here #354