From 290bb23fec2dd42047e458ad4880289be5e01f24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Fri, 3 Jul 2020 13:41:23 +0200 Subject: [PATCH 01/10] update data structure to fit with data structure RFC v3 --- .../blockeditormodelobject.service.js | 89 ++++++++--- .../services/block-editor-service.spec.js | 144 ++++++++++++++++-- 2 files changed, 202 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index ec232b4914..5cf0bbf944 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -136,7 +136,7 @@ // We also like to watch our data model to be able to capture changes coming from other places. if (forSettings === true) { - blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + "layout.settings" + "." + prop.alias, createLayoutSettingsModelWatcher(blockObject, prop))); + blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + "settingsData" + "." + prop.alias, createLayoutSettingsModelWatcher(blockObject, prop))); } else { blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + "data" + "." + prop.alias, createDataModelWatcher(blockObject, prop))); } @@ -167,9 +167,9 @@ */ function createLayoutSettingsModelWatcher(blockObject, prop) { return function() { - if (prop.value !== blockObject.layout.settings[prop.alias]) { + if (prop.value !== blockObject.settingsData[prop.alias]) { // sync data: - prop.value = blockObject.layout.settings[prop.alias]; + prop.value = blockObject.settingsData[prop.alias]; } } } @@ -193,9 +193,9 @@ */ function createSettingsModelPropWatcher(blockObject, prop) { return function() { - if (blockObject.layout.settings[prop.alias] !== prop.value) { + if (blockObject.settingsData[prop.alias] !== prop.value) { // sync data: - blockObject.layout.settings[prop.alias] = prop.value; + blockObject.settingsData[prop.alias] = prop.value; } } } @@ -245,7 +245,8 @@ // ensure basic part of data-structure is in place: this.value = propertyModelValue; this.value.layout = this.value.layout || {}; - this.value.data = this.value.data || []; + this.value.contentData = this.value.contentData || []; + this.value.settingsData = this.value.settingsData || []; this.propertyEditorAlias = propertyEditorAlias; this.blockConfigurations = blockConfigurations; @@ -459,16 +460,25 @@ var settingsScaffold = this.getScaffoldFromKey(blockConfiguration.settingsElementTypeKey); if (settingsScaffold !== null) { - layoutEntry.settings = layoutEntry.settings || {}; - - blockObject.settingsData = layoutEntry.settings; + if (!layoutEntry.settingsUdi) { + // if this block does not have settings data, then create it. This could happen because settings model has been added later than this content was created. + layoutEntry.settingsUdi = this._createSettingsEntry(blockConfiguration.settingsElementTypeKey); + } + + var settingsUdi = layoutEntry.settingsUdi; + + var settingsData = this._getSettingsByUdi(settingsUdi); + if (settingsData === null) { + console.error("Couldnt find content settings data of " + settingsUdi) + return null; + } + + blockObject.settingsData = settingsData; // make basics from scaffold blockObject.settings = Utilities.copy(settingsScaffold); - layoutEntry.settings = layoutEntry.settings || {}; - if (!layoutEntry.settings.key) { layoutEntry.settings.key = String.CreateGuid(); } - if (!layoutEntry.settings.contentTypeKey) { layoutEntry.settings.contentTypeKey = blockConfiguration.settingsElementTypeKey; } - mapToElementModel(blockObject.settings, layoutEntry.settings); + blockObject.settings.udi = settingsUdi; + mapToElementModel(blockObject.settings, settingsData); } } @@ -487,7 +497,7 @@ mapToPropertyModel(this.content, this.data); } if (this.config.settingsElementTypeKey !== null) { - mapToPropertyModel(this.settings, this.layout.settings); + mapToPropertyModel(this.settings, this.settingsData); } } @@ -508,6 +518,7 @@ delete this.config; delete this.layout; delete this.data; + delete this.settingsData; delete this.content; delete this.settings; @@ -536,8 +547,15 @@ */ removeDataAndDestroyModel: function (blockObject) { var udi = blockObject.content.udi; + var settingsUdi = null; + if (blockObject.settings) { + settingsUdi = blockObject.settings.udi; + } this.destroyBlockObject(blockObject); this.removeDataByUdi(udi); + if(settingsUdi) { + this.removeSettingsByUdi(settingsUdi); + } }, /** @@ -586,7 +604,7 @@ } if (blockConfiguration.settingsElementTypeKey != null) { - entry.settings = { key: String.CreateGuid(), contentTypeKey: blockConfiguration.settingsElementTypeKey }; + entry.settingsUdi = this._createSettingsEntry(blockConfiguration.settingsElementTypeKey) } return entry; @@ -641,26 +659,55 @@ contentTypeKey: elementTypeKey, udi: udiService.create("element") }; - this.value.data.push(content); + this.value.contentData.push(content); return content.udi; }, // private _getDataByUdi: function(udi) { - return this.value.data.find(entry => entry.udi === udi) || null; + return this.value.contentData.find(entry => entry.udi === udi) || null; }, /** * @ngdoc method * @name removeDataByUdi * @methodOf umbraco.services.blockEditorModelObject - * @description Removes the data of a given UDI. + * @description Removes the content data of a given UDI. * Notice this method does not remove the block from your layout, this will need to be handlede by the Property Editor since this services donst know about your layout structure. - * @param {string} udi The UDI of the data to be removed. + * @param {string} udi The UDI of the content data to be removed. */ removeDataByUdi: function(udi) { - const index = this.value.data.findIndex(o => o.udi === udi); + const index = this.value.contentData.findIndex(o => o.udi === udi); if (index !== -1) { - this.value.data.splice(index, 1); + this.value.contentData.splice(index, 1); + } + }, + + // private + _createSettingsEntry: function(elementTypeKey) { + var settings = { + contentTypeKey: elementTypeKey, + udi: udiService.create("element") + }; + this.value.settingsData.push(settings); + return settings.udi; + }, + // private + _getSettingsByUdi: function(udi) { + return this.value.settingsData.find(entry => entry.udi === udi) || null; + }, + + /** + * @ngdoc method + * @name removeSettingsByUdi + * @methodOf umbraco.services.blockEditorModelObject + * @description Removes the settings data of a given UDI. + * Notice this method does not remove the settingsUdi from your layout, this will need to be handlede by the Property Editor since this services donst know about your layout structure. + * @param {string} udi The UDI of the settings data to be removed. + */ + removeSettingsByUdi: function(udi) { + const index = this.value.settingsData.findIndex(o => o.udi === udi); + if (index !== -1) { + this.value.settingsData.splice(index, 1); } }, diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js index e0cd2d0c93..8936f731bb 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js @@ -36,7 +36,7 @@ } ] }, - data: [ + contentData: [ { udi: 1234, contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", @@ -45,6 +45,32 @@ ] }; + var blockWithSettingsConfigurationMock = { contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", label:"Test label", settingsElementTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", view: "testview.html"}; + var propertyModelWithSettingsMock = { + layout: { + "Umbraco.TestBlockEditor": [ + { + udi: 1234, + settingsUdi: 4567 + } + ] + }, + contentData: [ + { + udi: 1234, + contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", + testproperty: "myTestValue" + } + ], + settingsData: [ + { + udi: 4567, + contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", + testproperty: "myTestValueForSettings" + } + ] + }; + describe('init blockEditorModelObject', function () { it('fail if no model value', function () { @@ -110,8 +136,8 @@ var blockObject = modelObject.getBlockObject(layout[0]); expect(blockObject).not.toBeUndefined(); - expect(blockObject.data.udi).toBe(propertyModelMock.data[0].udi); - expect(blockObject.content.variants[0].tabs[0].properties[0].value).toBe(propertyModelMock.data[0].testproperty); + expect(blockObject.data.udi).toBe(propertyModelMock.contentData[0].udi); + expect(blockObject.content.variants[0].tabs[0].properties[0].value).toBe(propertyModelMock.contentData[0].testproperty); done(); }); @@ -135,9 +161,9 @@ $rootScope.$digest();// invoke angularJS Store. - expect(blockObject.data).toBe(propertyModel.data[0]); + expect(blockObject.data).toEqual(propertyModel.contentData[0]); expect(blockObject.data.testproperty).toBe("anotherTestValue"); - expect(propertyModel.data[0].testproperty).toBe("anotherTestValue"); + expect(propertyModel.contentData[0].testproperty).toBe("anotherTestValue"); // @@ -152,7 +178,7 @@ var propertyModel = angular.copy(propertyModelMock); var complexValue = {"list": ["A", "B", "C"]}; - propertyModel.data[0].testproperty = complexValue; + propertyModel.contentData[0].testproperty = complexValue; var modelObject = blockEditorService.createModelObject(propertyModel, "Umbraco.TestBlockEditor", [blockConfigurationMock], $scope, $scope); @@ -168,8 +194,8 @@ $rootScope.$digest();// invoke angularJS Store. - expect(propertyModel.data[0].testproperty.list[0]).toBe("AA"); - expect(propertyModel.data[0].testproperty.list.length).toBe(4); + expect(propertyModel.contentData[0].testproperty.list[0]).toBe("AA"); + expect(propertyModel.contentData[0].testproperty.list.length).toBe(4); done(); }); @@ -218,8 +244,8 @@ // remove from data; modelObject.removeDataAndDestroyModel(blockObject); - expect(propertyModel.data.length).toBe(0); - expect(propertyModel.data[0]).toBeUndefined(); + expect(propertyModel.contentData.length).toBe(0); + expect(propertyModel.contentData[0]).toBeUndefined(); expect(propertyModel.layout["Umbraco.TestBlockEditor"].length).toBe(0); expect(propertyModel.layout["Umbraco.TestBlockEditor"][0]).toBeUndefined(); @@ -229,6 +255,104 @@ }); + + + + + + it('getBlockObject of block with settings has values', function (done) { + + var propertyModel = angular.copy(propertyModelWithSettingsMock); + + var modelObject = blockEditorService.createModelObject(propertyModel, "Umbraco.TestBlockEditor", [blockWithSettingsConfigurationMock], $scope, $scope); + + modelObject.load().then(() => { + + var layout = modelObject.getLayout(); + + var blockObject = modelObject.getBlockObject(layout[0]); + + expect(blockObject).not.toBeUndefined(); + expect(blockObject.data.udi).toBe(propertyModel.contentData[0].udi); + expect(blockObject.content.variants[0].tabs[0].properties[0].value).toBe(propertyModel.contentData[0].testproperty); + + done(); + }); + + }); + + + it('getBlockObject of block with settings syncs primative values', function (done) { + + var propertyModel = angular.copy(propertyModelWithSettingsMock); + + var modelObject = blockEditorService.createModelObject(propertyModel, "Umbraco.TestBlockEditor", [blockWithSettingsConfigurationMock], $scope, $scope); + + modelObject.load().then(() => { + + var layout = modelObject.getLayout(); + + var blockObject = modelObject.getBlockObject(layout[0]); + + blockObject.content.variants[0].tabs[0].properties[0].value = "anotherTestValue"; + blockObject.settings.variants[0].tabs[0].properties[0].value = "anotherTestValueForSettings"; + + $rootScope.$digest();// invoke angularJS Store. + + expect(blockObject.data).toEqual(propertyModel.contentData[0]); + expect(blockObject.data.testproperty).toBe("anotherTestValue"); + expect(propertyModel.contentData[0].testproperty).toBe("anotherTestValue"); + + expect(blockObject.settingsData).toEqual(propertyModel.settingsData[0]); + expect(blockObject.settingsData.testproperty).toBe("anotherTestValueForSettings"); + expect(propertyModel.settingsData[0].testproperty).toBe("anotherTestValueForSettings"); + + // + + done(); + }); + + }); + + + it('getBlockObject of block with settings syncs values of object', function (done) { + + var propertyModel = angular.copy(propertyModelWithSettingsMock); + + var complexValue = {"list": ["A", "B", "C"]}; + propertyModel.contentData[0].testproperty = complexValue; + + var complexSettingsValue = {"list": ["A", "B", "C"]}; + propertyModel.settingsData[0].testproperty = complexSettingsValue; + + var modelObject = blockEditorService.createModelObject(propertyModel, "Umbraco.TestBlockEditor", [blockWithSettingsConfigurationMock], $scope, $scope); + + modelObject.load().then(() => { + + var layout = modelObject.getLayout(); + + var blockObject = modelObject.getBlockObject(layout[0]); + + blockObject.content.variants[0].tabs[0].properties[0].value.list[0] = "AA"; + blockObject.content.variants[0].tabs[0].properties[0].value.list.push("D"); + + blockObject.settings.variants[0].tabs[0].properties[0].value.list[0] = "settingsValue"; + blockObject.settings.variants[0].tabs[0].properties[0].value.list.push("settingsNewValue"); + + $rootScope.$digest();// invoke angularJS Store. + + expect(propertyModel.contentData[0].testproperty.list[0]).toBe("AA"); + expect(propertyModel.contentData[0].testproperty.list.length).toBe(4); + + expect(propertyModel.settingsData[0].testproperty.list[0]).toBe("settingsValue"); + expect(propertyModel.settingsData[0].testproperty.list.length).toBe(4); + + done(); + }); + + }); + + }); }); From 809826420658691952a2fb28997d9fbe1106d848 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 21 Jul 2020 13:28:19 +1000 Subject: [PATCH 02/10] WIP --- .../Models/Blocks/BlockListLayoutReference.cs | 32 ++++++++++++------- .../Models/Blocks/IBlockReference.cs | 2 +- .../BlockListPropertyValueConverterTests.cs | 10 +++--- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs b/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs index 19b30e6ea6..ed3f46db70 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs @@ -10,32 +10,42 @@ namespace Umbraco.Core.Models.Blocks [DataContract(Name = "blockListLayout", Namespace = "")] public class BlockListLayoutReference : IBlockElement { - public BlockListLayoutReference(Udi udi, IPublishedElement data, IPublishedElement settings) + public BlockListLayoutReference(Udi contentUdi, IPublishedElement content, Udi settingsUdi, IPublishedElement settings) { - Udi = udi ?? throw new ArgumentNullException(nameof(udi)); - Data = data ?? throw new ArgumentNullException(nameof(data)); + ContentUdi = contentUdi ?? throw new ArgumentNullException(nameof(contentUdi)); + Content = content ?? throw new ArgumentNullException(nameof(content)); Settings = settings; // can be null + SettingsUdi = settingsUdi; // can be null } /// - /// The Id of the data item + /// The Id of the content data item /// - [DataMember(Name = "udi")] - public Udi Udi { get; } + [DataMember(Name = "contentUdi")] + public Udi ContentUdi { get; } /// - /// The settings for the layout item + /// The Id of the settings data item /// - [DataMember(Name = "settings")] - public IPublishedElement Settings { get; } + [DataMember(Name = "settingsUdi")] + public Udi SettingsUdi { get; } /// - /// The data item referenced + /// The content data item referenced /// /// /// This is ignored from serialization since it is just a reference to the actual data element /// [IgnoreDataMember] - public IPublishedElement Data { get; } + public IPublishedElement Content { get; } + + /// + /// The settings data item referenced + /// + /// + /// This is ignored from serialization since it is just a reference to the actual data element + /// + [IgnoreDataMember] + public IPublishedElement Settings { get; } } } diff --git a/src/Umbraco.Core/Models/Blocks/IBlockReference.cs b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs index d63e5e4ca9..d1596b10bb 100644 --- a/src/Umbraco.Core/Models/Blocks/IBlockReference.cs +++ b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs @@ -8,6 +8,6 @@ /// public interface IBlockReference { - Udi Udi { get; } + Udi ContentUdi { get; } } } diff --git a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs index d2bc34e633..483b1d38cf 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -278,7 +278,7 @@ data: []}"; Assert.AreEqual(1, converted.Layout.Count()); var layout0 = converted.Layout.ElementAt(0); Assert.IsNull(layout0.Settings); - Assert.AreEqual(Udi.Parse("umb://element/1304E1DDAC87439684FE8A399231CB3D"), layout0.Udi); + Assert.AreEqual(Udi.Parse("umb://element/1304E1DDAC87439684FE8A399231CB3D"), layout0.ContentUdi); } [Test] @@ -326,12 +326,12 @@ data: []}"; Assert.AreEqual(2, converted.Layout.Count()); var item0 = converted.Layout.ElementAt(0); - Assert.AreEqual(Guid.Parse("1304E1DD-AC87-4396-84FE-8A399231CB3D"), item0.Data.Key); - Assert.AreEqual("Test1", item0.Data.ContentType.Alias); + Assert.AreEqual(Guid.Parse("1304E1DD-AC87-4396-84FE-8A399231CB3D"), item0.Content.Key); + Assert.AreEqual("Test1", item0.Content.ContentType.Alias); var item1 = converted.Layout.ElementAt(1); - Assert.AreEqual(Guid.Parse("0A4A416E-547D-464F-ABCC-6F345C17809A"), item1.Data.Key); - Assert.AreEqual("Test2", item1.Data.ContentType.Alias); + Assert.AreEqual(Guid.Parse("0A4A416E-547D-464F-ABCC-6F345C17809A"), item1.Content.Key); + Assert.AreEqual("Test2", item1.Content.ContentType.Alias); } From a194d1db4aa24157160839779587924fd7d94022 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 21 Jul 2020 15:50:11 +1000 Subject: [PATCH 03/10] first phase of new data format done in c# --- .../Models/Blocks/BlockEditorData.cs | 22 ++++++--- .../Models/Blocks/BlockEditorDataConverter.cs | 8 +-- .../Models/Blocks/BlockEditorModel.cs | 17 +++++-- .../Blocks/BlockListEditorDataConverter.cs | 4 +- .../Models/Blocks/BlockListLayoutItem.cs | 12 ++--- .../Models/Blocks/BlockListLayoutReference.cs | 2 +- .../Models/Blocks/BlockListModel.cs | 12 +++-- .../Blocks/ContentAndSettingsReference.cs | 46 +++++++++++++++++ .../Models/Blocks/IBlockElement.cs | 17 ------- .../Models/Blocks/IBlockReference.cs | 13 +++++ src/Umbraco.Core/Umbraco.Core.csproj | 2 +- .../BlockListPropertyValueConverterTests.cs | 49 +++++++++---------- .../BlockEditorPropertyEditor.cs | 4 +- .../BlockListPropertyValueConverter.cs | 47 +++++++++++------- 14 files changed, 162 insertions(+), 93 deletions(-) create mode 100644 src/Umbraco.Core/Models/Blocks/ContentAndSettingsReference.cs delete mode 100644 src/Umbraco.Core/Models/Blocks/IBlockElement.cs diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs index 3fe832dc04..ba9f22d945 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs @@ -18,24 +18,32 @@ namespace Umbraco.Core.Models.Blocks { } - public BlockEditorData(JToken layout, IReadOnlyList layoutBlockReferences, IReadOnlyList blocks) + public BlockEditorData(JToken layout, + IReadOnlyList references, + IReadOnlyList contentData, + IReadOnlyList settingsData) { Layout = layout ?? throw new ArgumentNullException(nameof(layout)); - LayoutBlockReferences = layoutBlockReferences ?? throw new ArgumentNullException(nameof(layoutBlockReferences)); - Blocks = blocks ?? throw new ArgumentNullException(nameof(blocks)); + References = references ?? throw new ArgumentNullException(nameof(references)); + ContentData = contentData ?? throw new ArgumentNullException(nameof(contentData)); + SettingsData = settingsData ?? throw new ArgumentNullException(nameof(settingsData)); } public JToken Layout { get; } - public IReadOnlyList LayoutBlockReferences { get; } = new List(); - public IReadOnlyList Blocks { get; } = new List(); + public IReadOnlyList References { get; } = new List(); + public IReadOnlyList ContentData { get; } = new List(); + public IReadOnlyList SettingsData { get; } = new List(); internal class BlockValue { [JsonProperty("layout")] public IDictionary Layout { get; set; } - [JsonProperty("data")] - public IEnumerable Data { get; set; } + [JsonProperty("contentData")] + public IEnumerable ContentData { get; set; } = new List(); + + [JsonProperty("settingsData")] + public IEnumerable SettingsData { get; set; } = new List(); } /// diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs index cd1931fbb0..f045a00401 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs @@ -31,8 +31,10 @@ namespace Umbraco.Core.Models.Blocks return BlockEditorData.Empty; var references = GetBlockReferences(layout); - var blocks = value.Data.ToList(); - return new BlockEditorData(layout, references, blocks); + var contentData = value.ContentData.ToList(); + var settingsData = value.SettingsData.ToList(); + + return new BlockEditorData(layout, references, contentData, settingsData); } /// @@ -40,7 +42,7 @@ namespace Umbraco.Core.Models.Blocks /// /// /// - protected abstract IReadOnlyList GetBlockReferences(JToken jsonLayout); + protected abstract IReadOnlyList GetBlockReferences(JToken jsonLayout); } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorModel.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorModel.cs index 307b69f6e0..fa5a29fece 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorModel.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorModel.cs @@ -10,9 +10,10 @@ namespace Umbraco.Core.Models.Blocks /// public abstract class BlockEditorModel { - protected BlockEditorModel(IEnumerable data) + protected BlockEditorModel(IEnumerable contentData, IEnumerable settingsData) { - Data = data ?? throw new ArgumentNullException(nameof(data)); + ContentData = contentData ?? throw new ArgumentNullException(nameof(contentData)); + SettingsData = settingsData ?? new List(); } public BlockEditorModel() @@ -21,9 +22,15 @@ namespace Umbraco.Core.Models.Blocks /// - /// The data items of the Block List editor + /// The content data items of the Block List editor /// - [DataMember(Name = "data")] - public IEnumerable Data { get; set; } + [DataMember(Name = "contentData")] + public IEnumerable ContentData { get; set; } = new List(); + + /// + /// The settings data items of the Block List editor + /// + [DataMember(Name = "settingsData")] + public IEnumerable SettingsData { get; set; } = new List(); } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs index eea2dfafd0..0dec05bc54 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs @@ -13,10 +13,10 @@ namespace Umbraco.Core.Models.Blocks { } - protected override IReadOnlyList GetBlockReferences(JToken jsonLayout) + protected override IReadOnlyList GetBlockReferences(JToken jsonLayout) { var blockListLayout = jsonLayout.ToObject>(); - return blockListLayout.Select(x => x.Udi).ToList(); + return blockListLayout.Select(x => new ContentAndSettingsReference(x.ContentUdi, x.SettingsUdi)).ToList(); } } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs b/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs index d05906a6c4..3453ff2a78 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs @@ -1,6 +1,5 @@ using Newtonsoft.Json; using Umbraco.Core.Serialization; -using static Umbraco.Core.Models.Blocks.BlockEditorData; namespace Umbraco.Core.Models.Blocks { @@ -9,11 +8,12 @@ namespace Umbraco.Core.Models.Blocks /// public class BlockListLayoutItem { - [JsonProperty("settings")] - public BlockItemData Settings { get; set; } - - [JsonProperty("udi")] + [JsonProperty("contentUdi", Required = Required.Always)] [JsonConverter(typeof(UdiJsonConverter))] - public Udi Udi { get; set; } + public Udi ContentUdi { get; set; } + + [JsonProperty("settingsUdi")] + [JsonConverter(typeof(UdiJsonConverter))] + public Udi SettingsUdi { get; set; } } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs b/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs index ed3f46db70..f576bd927f 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListLayoutReference.cs @@ -8,7 +8,7 @@ namespace Umbraco.Core.Models.Blocks /// Represents a layout item for the Block List editor /// [DataContract(Name = "blockListLayout", Namespace = "")] - public class BlockListLayoutReference : IBlockElement + public class BlockListLayoutReference : IBlockReference { public BlockListLayoutReference(Udi contentUdi, IPublishedElement content, Udi settingsUdi, IPublishedElement settings) { diff --git a/src/Umbraco.Core/Models/Blocks/BlockListModel.cs b/src/Umbraco.Core/Models/Blocks/BlockListModel.cs index 089ca7e6a3..0492cf0d73 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListModel.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListModel.cs @@ -10,8 +10,14 @@ namespace Umbraco.Core.Models.Blocks [DataContract(Name = "blockList", Namespace = "")] public class BlockListModel : BlockEditorModel { - public BlockListModel(IEnumerable data, IEnumerable layout) - : base(data) + public static BlockListModel Empty { get; } = new BlockListModel(); + + private BlockListModel() + { + } + + public BlockListModel(IEnumerable contentData, IEnumerable settingsData, IEnumerable layout) + : base(contentData, settingsData) { Layout = layout; } @@ -20,7 +26,7 @@ namespace Umbraco.Core.Models.Blocks /// The layout items of the Block List editor /// [DataMember(Name = "layout")] - public IEnumerable Layout { get; } + public IEnumerable Layout { get; } = new List(); } diff --git a/src/Umbraco.Core/Models/Blocks/ContentAndSettingsReference.cs b/src/Umbraco.Core/Models/Blocks/ContentAndSettingsReference.cs new file mode 100644 index 0000000000..523a964c7b --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/ContentAndSettingsReference.cs @@ -0,0 +1,46 @@ +using System.Collections.Generic; +using System; + +namespace Umbraco.Core.Models.Blocks +{ + public struct ContentAndSettingsReference : IEquatable + { + public ContentAndSettingsReference(Udi contentUdi, Udi settingsUdi) + { + ContentUdi = contentUdi ?? throw new ArgumentNullException(nameof(contentUdi)); + SettingsUdi = settingsUdi; + } + + public Udi ContentUdi { get; } + public Udi SettingsUdi { get; } + + public override bool Equals(object obj) + { + return obj is ContentAndSettingsReference reference && Equals(reference); + } + + public bool Equals(ContentAndSettingsReference other) + { + return EqualityComparer.Default.Equals(ContentUdi, other.ContentUdi) && + EqualityComparer.Default.Equals(SettingsUdi, other.SettingsUdi); + } + + public override int GetHashCode() + { + var hashCode = 272556606; + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ContentUdi); + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(SettingsUdi); + return hashCode; + } + + public static bool operator ==(ContentAndSettingsReference left, ContentAndSettingsReference right) + { + return left.Equals(right); + } + + public static bool operator !=(ContentAndSettingsReference left, ContentAndSettingsReference right) + { + return !(left == right); + } + } +} diff --git a/src/Umbraco.Core/Models/Blocks/IBlockElement.cs b/src/Umbraco.Core/Models/Blocks/IBlockElement.cs deleted file mode 100644 index 7577a7bedc..0000000000 --- a/src/Umbraco.Core/Models/Blocks/IBlockElement.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace Umbraco.Core.Models.Blocks -{ - - // TODO: IBlockElement doesn't make sense, this is a reference to an actual element with some settings - // and always has to do with the "Layout", should possibly be called IBlockReference or IBlockLayout or IBlockLayoutReference - /// - /// Represents a data item for a Block editor implementation - /// - /// - /// - /// see: https://github.com/umbraco/rfcs/blob/907f3758cf59a7b6781296a60d57d537b3b60b8c/cms/0011-block-data-structure.md#strongly-typed - /// - public interface IBlockElement : IBlockReference - { - TSettings Settings { get; } - } -} diff --git a/src/Umbraco.Core/Models/Blocks/IBlockReference.cs b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs index d1596b10bb..8d8ddd47f0 100644 --- a/src/Umbraco.Core/Models/Blocks/IBlockReference.cs +++ b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs @@ -1,5 +1,18 @@ namespace Umbraco.Core.Models.Blocks { + + /// + /// Represents a data item reference for a Block editor implementation + /// + /// + /// + /// see: https://github.com/umbraco/rfcs/blob/907f3758cf59a7b6781296a60d57d537b3b60b8c/cms/0011-block-data-structure.md#strongly-typed + /// + public interface IBlockReference : IBlockReference + { + TSettings Settings { get; } + } + /// /// Represents a data item reference for a Block Editor implementation /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 1c5e14a6b8..d7a1251f20 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -136,6 +136,7 @@ + @@ -150,7 +151,6 @@ - diff --git a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs index 483b1d38cf..58c4d22c89 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -138,14 +138,14 @@ namespace Umbraco.Tests.PropertyEditors var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); json = string.Empty; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); } @@ -161,7 +161,7 @@ namespace Umbraco.Tests.PropertyEditors var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); json = @"{ @@ -170,7 +170,7 @@ data: []}"; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); // Even though there is a layout, there is no data, so the conversion will result in zero elements in total @@ -179,18 +179,17 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'udi': 'umb://element/e7dba547615b4e9ab4ab2a7674845bc9', - 'settings': {} + 'contentUdi': 'umb://element/e7dba547615b4e9ab4ab2a7674845bc9' } ] }, - data: [] + contentData: [] }"; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); // Even though there is a layout and data, the data is invalid (missing required keys) so the conversion will result in zero elements in total @@ -199,12 +198,11 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'udi': 'umb://element/e7dba547615b4e9ab4ab2a7674845bc9', - 'settings': {} + 'contentUdi': 'umb://element/e7dba547615b4e9ab4ab2a7674845bc9' } ] }, - data: [ + contentData: [ { 'udi': 'umb://element/e7dba547615b4e9ab4ab2a7674845bc9' } @@ -214,7 +212,7 @@ data: []}"; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(0, converted.Data.Count()); + Assert.AreEqual(0, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); // Everthing is ok except the udi reference in the layout doesn't match the data so it will be empty @@ -223,12 +221,11 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D', - 'settings': {} + 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' } ] }, - data: [ + contentData: [ { 'contentTypeKey': '" + Key1 + @"', 'key': '1304E1DD-0000-4396-84FE-8A399231CB3D' @@ -239,7 +236,7 @@ data: []}"; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(1, converted.Data.Count()); + Assert.AreEqual(1, converted.ContentData.Count()); Assert.AreEqual(0, converted.Layout.Count()); } @@ -256,12 +253,11 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D', - 'settings': {} + 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' } ] }, - data: [ + contentData: [ { 'contentTypeKey': '" + Key1 + @"', 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' @@ -271,8 +267,8 @@ data: []}"; var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(1, converted.Data.Count()); - var item0 = converted.Data.ElementAt(0); + Assert.AreEqual(1, converted.ContentData.Count()); + var item0 = converted.ContentData.ElementAt(0); Assert.AreEqual(Guid.Parse("1304E1DD-AC87-4396-84FE-8A399231CB3D"), item0.Key); Assert.AreEqual("Test1", item0.ContentType.Alias); Assert.AreEqual(1, converted.Layout.Count()); @@ -294,16 +290,15 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D', - 'settings': {} + 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' }, { - 'udi': 'umb://element/0A4A416E547D464FABCC6F345C17809A', - 'settings': {} + 'contentUdi': 'umb://element/0A4A416E547D464FABCC6F345C17809A', + 'settingsUdi': null } ] }, - data: [ + contentData: [ { 'contentTypeKey': '" + Key1 + @"', 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' @@ -322,7 +317,7 @@ data: []}"; var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); - Assert.AreEqual(3, converted.Data.Count()); + Assert.AreEqual(3, converted.ContentData.Count()); Assert.AreEqual(2, converted.Layout.Count()); var item0 = converted.Layout.ElementAt(0); diff --git a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs index d2d144272c..866bd38e05 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs @@ -133,13 +133,13 @@ namespace Umbraco.Web.PropertyEditors var converted = _dataConverter.Convert(propertyValue.ToString()); - if (converted.Blocks.Count == 0) + if (converted.ContentData.Count == 0) return new List(); var contentTypePropertyTypes = new Dictionary>(); var result = new List(); - foreach(var block in converted.Blocks) + foreach(var block in converted.ContentData) { var contentType = GetElementType(block); if (contentType == null) diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index a72ed4385b..b2822dada9 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -53,56 +53,65 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters var contentTypes = configuration.Blocks; var contentTypeMap = contentTypes.ToDictionary(x => x.Key); - var elements = new Dictionary(); + var contentPublishedElements = new Dictionary(); + var settingsPublishedElements = new Dictionary(); var layout = new List(); - var model = new BlockListModel(elements.Values, layout); var value = (string)inter; - if (string.IsNullOrWhiteSpace(value)) return model; + if (string.IsNullOrWhiteSpace(value)) return BlockListModel.Empty; var converter = new BlockListEditorDataConverter(); var converted = converter.Convert(value); - if (converted.Blocks.Count == 0) return model; + if (converted.ContentData.Count == 0) return BlockListModel.Empty; var blockListLayout = converted.Layout.ToObject>(); - // parse the data elements - foreach (var data in converted.Blocks) + // convert the content data + foreach (var data in converted.ContentData) { var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; - elements[element.Key] = element; + contentPublishedElements[element.Key] = element; + } + // convert the settings data + foreach (var data in converted.SettingsData) + { + var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); + if (element == null) continue; + settingsPublishedElements[element.Key] = element; } // if there's no elements just return since if there's no data it doesn't matter what is stored in layout - if (elements.Count == 0) return model; + if (contentPublishedElements.Count == 0) return BlockListModel.Empty; foreach (var layoutItem in blockListLayout) { - // the result of this can be null, that's ok - var element = layoutItem.Settings != null ? _blockConverter.ConvertToElement(layoutItem.Settings, referenceCacheLevel, preview) : null; - - var guidUdi = (GuidUdi)layoutItem.Udi; - - // get the data reference - if (!elements.TryGetValue(guidUdi.Guid, out var data)) + // get the content reference + var contentGuidUdi = (GuidUdi)layoutItem.ContentUdi; + if (!contentPublishedElements.TryGetValue(contentGuidUdi.Guid, out var contentData)) continue; - if (!data.ContentType.TryGetKey(out var contentTypeKey)) + // get the setting reference + IPublishedElement settingsData = null; + var settingGuidUdi = layoutItem.SettingsUdi != null ? (GuidUdi)layoutItem.SettingsUdi : null; + if (settingGuidUdi != null) settingsPublishedElements.TryGetValue(settingGuidUdi.Guid, out settingsData); + + if (!contentData.ContentType.TryGetKey(out var contentTypeKey)) throw new InvalidOperationException("The content type was not of type " + typeof(IPublishedContentType2)); if (!contentTypeMap.TryGetValue(contentTypeKey, out var blockConfig)) continue; // this can happen if they have a settings type, save content, remove the settings type, and display the front-end page before saving the content again - if (element != null && string.IsNullOrWhiteSpace(blockConfig.SettingsElementTypeKey)) - element = null; + if (settingsData != null && string.IsNullOrWhiteSpace(blockConfig.SettingsElementTypeKey)) + settingsData = null; - var layoutRef = new BlockListLayoutReference(layoutItem.Udi, data, element); + var layoutRef = new BlockListLayoutReference(contentGuidUdi, contentData, settingGuidUdi, settingsData); layout.Add(layoutRef); } + var model = new BlockListModel(contentPublishedElements.Values, settingsPublishedElements.Values, layout); return model; } } From 5bc80e2d8415c58b1fb2c776cd51e64ed26bfb2b Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 21 Jul 2020 16:24:48 +1000 Subject: [PATCH 04/10] Adds tests for settings and changes to strongly typed guid --- .../BlockListPropertyValueConverterTests.cs | 80 ++++++++++++++----- .../blockeditormodelobject.service.js | 3 + .../PropertyEditors/BlockListConfiguration.cs | 5 +- .../BlockListPropertyValueConverter.cs | 15 +++- 4 files changed, 75 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs index 58c4d22c89..7ff2077d77 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -18,10 +18,14 @@ namespace Umbraco.Tests.PropertyEditors [TestFixture] public class BlockListPropertyValueConverterTests { - private readonly Guid Key1 = Guid.NewGuid(); - private readonly Guid Key2 = Guid.NewGuid(); - private readonly string Alias1 = "Test1"; - private readonly string Alias2 = "Test2"; + private readonly Guid ContentKey1 = Guid.NewGuid(); + private readonly Guid ContentKey2 = Guid.NewGuid(); + private const string ContentAlias1 = "Test1"; + private const string ContentAlias2 = "Test2"; + private readonly Guid SettingKey1 = Guid.NewGuid(); + private readonly Guid SettingKey2 = Guid.NewGuid(); + private const string SettingAlias1 = "Setting1"; + private const string SettingAlias2 = "Setting2"; /// /// Setup mocks for IPublishedSnapshotAccessor @@ -31,15 +35,25 @@ namespace Umbraco.Tests.PropertyEditors { var test1ContentType = Mock.Of(x => x.IsElement == true - && x.Key == Key1 - && x.Alias == Alias1); + && x.Key == ContentKey1 + && x.Alias == ContentAlias1); var test2ContentType = Mock.Of(x => x.IsElement == true - && x.Key == Key2 - && x.Alias == Alias2); + && x.Key == ContentKey2 + && x.Alias == ContentAlias2); + var test3ContentType = Mock.Of(x => + x.IsElement == true + && x.Key == SettingKey1 + && x.Alias == SettingAlias1); + var test4ContentType = Mock.Of(x => + x.IsElement == true + && x.Key == SettingKey2 + && x.Alias == SettingAlias2); var contentCache = new Mock(); - contentCache.Setup(x => x.GetContentType(Key1)).Returns(test1ContentType); - contentCache.Setup(x => x.GetContentType(Key2)).Returns(test2ContentType); + contentCache.Setup(x => x.GetContentType(ContentKey1)).Returns(test1ContentType); + contentCache.Setup(x => x.GetContentType(ContentKey2)).Returns(test2ContentType); + contentCache.Setup(x => x.GetContentType(SettingKey1)).Returns(test3ContentType); + contentCache.Setup(x => x.GetContentType(SettingKey2)).Returns(test4ContentType); var publishedSnapshot = Mock.Of(x => x.Content == contentCache.Object); var publishedSnapshotAccessor = Mock.Of(x => x.PublishedSnapshot == publishedSnapshot); return publishedSnapshotAccessor; @@ -60,11 +74,13 @@ namespace Umbraco.Tests.PropertyEditors Blocks = new[] { new BlockListConfiguration.BlockConfiguration { - Key = Key1 + ContentElementTypeKey = ContentKey1, + SettingsElementTypeKey = SettingKey2 }, new BlockListConfiguration.BlockConfiguration { - Key = Key2 + ContentElementTypeKey = ContentKey2, + SettingsElementTypeKey = SettingKey1 } } }; @@ -74,7 +90,7 @@ namespace Umbraco.Tests.PropertyEditors Blocks = new[] { new BlockListConfiguration.BlockConfiguration { - Key = Key1 + ContentElementTypeKey = ContentKey1 } } }; @@ -227,7 +243,7 @@ data: []}"; }, contentData: [ { - 'contentTypeKey': '" + Key1 + @"', + 'contentTypeKey': '" + ContentKey1 + @"', 'key': '1304E1DD-0000-4396-84FE-8A399231CB3D' } ] @@ -259,7 +275,7 @@ data: []}"; }, contentData: [ { - 'contentTypeKey': '" + Key1 + @"', + 'contentTypeKey': '" + ContentKey1 + @"', 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' } ] @@ -290,43 +306,63 @@ data: []}"; layout: { '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ { - 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' + 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D', + 'settingsUdi': 'umb://element/1F613E26CE274898908A561437AF5100' }, { 'contentUdi': 'umb://element/0A4A416E547D464FABCC6F345C17809A', - 'settingsUdi': null + 'settingsUdi': 'umb://element/63027539B0DB45E7B70459762D4E83DD' } ] }, - contentData: [ + contentData: [ { - 'contentTypeKey': '" + Key1 + @"', + 'contentTypeKey': '" + ContentKey1 + @"', 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' }, { - 'contentTypeKey': '" + Key2 + @"', + 'contentTypeKey': '" + ContentKey2 + @"', 'udi': 'umb://element/E05A034704424AB3A520E048E6197E79' }, { - 'contentTypeKey': '" + Key2 + @"', + 'contentTypeKey': '" + ContentKey2 + @"', 'udi': 'umb://element/0A4A416E547D464FABCC6F345C17809A' } - ] + ], + settingsData: [ + { + 'contentTypeKey': '" + SettingKey1 + @"', + 'udi': 'umb://element/63027539B0DB45E7B70459762D4E83DD' + }, + { + 'contentTypeKey': '" + SettingKey2 + @"', + 'udi': 'umb://element/1F613E26CE274898908A561437AF5100' + }, + { + 'contentTypeKey': '" + SettingKey2 + @"', + 'udi': 'umb://element/BCF4BA3DA40C496C93EC58FAC85F18B9' + } + ], }"; var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; Assert.IsNotNull(converted); Assert.AreEqual(3, converted.ContentData.Count()); + Assert.AreEqual(3, converted.SettingsData.Count()); Assert.AreEqual(2, converted.Layout.Count()); var item0 = converted.Layout.ElementAt(0); Assert.AreEqual(Guid.Parse("1304E1DD-AC87-4396-84FE-8A399231CB3D"), item0.Content.Key); Assert.AreEqual("Test1", item0.Content.ContentType.Alias); + Assert.AreEqual(Guid.Parse("1F613E26CE274898908A561437AF5100"), item0.Settings.Key); + Assert.AreEqual("Setting2", item0.Settings.ContentType.Alias); var item1 = converted.Layout.ElementAt(1); Assert.AreEqual(Guid.Parse("0A4A416E-547D-464F-ABCC-6F345C17809A"), item1.Content.Key); Assert.AreEqual("Test2", item1.Content.ContentType.Alias); + Assert.AreEqual(Guid.Parse("63027539B0DB45E7B70459762D4E83DD"), item1.Settings.Key); + Assert.AreEqual("Setting1", item1.Settings.ContentType.Alias); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index 4d520c02d0..7496c15b52 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -710,6 +710,7 @@ }, // private + // TODO: Then this can just be a method in the outer scope _createSettingsEntry: function(elementTypeKey) { var settings = { contentTypeKey: elementTypeKey, @@ -718,7 +719,9 @@ this.value.settingsData.push(settings); return settings.udi; }, + // private + // TODO: Then this can just be a method in the outer scope _getSettingsByUdi: function(udi) { return this.value.settingsData.find(entry => entry.udi === udi) || null; }, diff --git a/src/Umbraco.Web/PropertyEditors/BlockListConfiguration.cs b/src/Umbraco.Web/PropertyEditors/BlockListConfiguration.cs index 0ea48512d1..bfbbc80179 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockListConfiguration.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockListConfiguration.cs @@ -27,11 +27,12 @@ namespace Umbraco.Web.PropertyEditors [JsonProperty("thumbnail")] public string Thumbnail { get; set; } + // TODO: This is named inconsistently in JS but renaming it needs to be done in quite a lot of places, this should be contentElementTypeKey [JsonProperty("contentTypeKey")] - public Guid Key { get; set; } + public Guid ContentElementTypeKey { get; set; } [JsonProperty("settingsElementTypeKey")] - public string SettingsElementTypeKey { get; set; } + public Guid? SettingsElementTypeKey { get; set; } [JsonProperty("view")] public string View { get; set; } diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index b2822dada9..2b04106288 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -51,7 +51,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters { var configuration = propertyType.DataType.ConfigurationAs(); var contentTypes = configuration.Blocks; - var contentTypeMap = contentTypes.ToDictionary(x => x.Key); + var contentElementTypeMap = contentTypes.ToDictionary(x => x.ContentElementTypeKey); var contentPublishedElements = new Dictionary(); var settingsPublishedElements = new Dictionary(); @@ -100,12 +100,19 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters if (!contentData.ContentType.TryGetKey(out var contentTypeKey)) throw new InvalidOperationException("The content type was not of type " + typeof(IPublishedContentType2)); - if (!contentTypeMap.TryGetValue(contentTypeKey, out var blockConfig)) + if (!contentElementTypeMap.TryGetValue(contentTypeKey, out var blockConfig)) continue; // this can happen if they have a settings type, save content, remove the settings type, and display the front-end page before saving the content again - if (settingsData != null && string.IsNullOrWhiteSpace(blockConfig.SettingsElementTypeKey)) - settingsData = null; + // we also ensure that the content type's match since maybe the settings type has been changed after this has been persisted. + if (settingsData != null) + { + if (!settingsData.ContentType.TryGetKey(out var settingsElementTypeKey)) + throw new InvalidOperationException("The settings element type was not of type " + typeof(IPublishedContentType2)); + + if (!blockConfig.SettingsElementTypeKey.HasValue || settingsElementTypeKey != blockConfig.SettingsElementTypeKey) + settingsData = null; + } var layoutRef = new BlockListLayoutReference(contentGuidUdi, contentData, settingGuidUdi, settingsData); layout.Add(layoutRef); From f7a831f05486d36e540870820c8175573d259b97 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Jul 2020 16:03:35 +1000 Subject: [PATCH 05/10] Fixes some JS issues with new data format, streamlines the blockeditor data format serialization in c#, implements To/FromEditor methods --- .../Models/Blocks/BlockEditorData.cs | 68 ++--- .../Models/Blocks/BlockEditorDataConverter.cs | 17 +- .../Models/Blocks/BlockItemData.cs | 56 ++++ .../Blocks/BlockListEditorDataConverter.cs | 2 +- src/Umbraco.Core/Models/Blocks/BlockValue.cs | 18 ++ src/Umbraco.Core/Umbraco.Core.csproj | 2 + .../components/content/edit.controller.js | 2 +- .../blockeditormodelobject.service.js | 125 ++++---- .../services/servervalidationmgr.service.js | 1 - .../umbBlockListPropertyEditor.component.js | 27 +- .../BlockEditorPropertyEditor.cs | 274 +++++++++++++----- .../NestedContentPropertyEditor.cs | 20 +- .../ValueConverters/BlockEditorConverter.cs | 2 +- .../BlockListPropertyValueConverter.cs | 8 +- 14 files changed, 415 insertions(+), 207 deletions(-) create mode 100644 src/Umbraco.Core/Models/Blocks/BlockItemData.cs create mode 100644 src/Umbraco.Core/Models/Blocks/BlockValue.cs diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs index ba9f22d945..5ee609b148 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs @@ -1,74 +1,44 @@ -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Linq; using System; using System.Collections.Generic; -using Umbraco.Core.Serialization; namespace Umbraco.Core.Models.Blocks { /// - /// Converted block data from json + /// Convertable block data from json /// public class BlockEditorData { + private readonly string _propertyEditorAlias; + public static BlockEditorData Empty { get; } = new BlockEditorData(); private BlockEditorData() { } - public BlockEditorData(JToken layout, - IReadOnlyList references, - IReadOnlyList contentData, - IReadOnlyList settingsData) + public BlockEditorData(string propertyEditorAlias, + IEnumerable references, + BlockValue blockValue) { - Layout = layout ?? throw new ArgumentNullException(nameof(layout)); - References = references ?? throw new ArgumentNullException(nameof(references)); - ContentData = contentData ?? throw new ArgumentNullException(nameof(contentData)); - SettingsData = settingsData ?? throw new ArgumentNullException(nameof(settingsData)); - } - - public JToken Layout { get; } - public IReadOnlyList References { get; } = new List(); - public IReadOnlyList ContentData { get; } = new List(); - public IReadOnlyList SettingsData { get; } = new List(); - - internal class BlockValue - { - [JsonProperty("layout")] - public IDictionary Layout { get; set; } - - [JsonProperty("contentData")] - public IEnumerable ContentData { get; set; } = new List(); - - [JsonProperty("settingsData")] - public IEnumerable SettingsData { get; set; } = new List(); + if (string.IsNullOrWhiteSpace(propertyEditorAlias)) + throw new ArgumentException($"'{nameof(propertyEditorAlias)}' cannot be null or whitespace", nameof(propertyEditorAlias)); + _propertyEditorAlias = propertyEditorAlias; + BlockValue = blockValue ?? throw new ArgumentNullException(nameof(blockValue)); + References = references != null ? new List(references) : throw new ArgumentNullException(nameof(references)); } /// - /// Represents a single block's data in raw form + /// Returns the layout for this specific property editor /// - public class BlockItemData - { - [JsonProperty("contentTypeKey")] - public Guid ContentTypeKey { get; set; } + public JToken Layout => BlockValue.Layout.TryGetValue(_propertyEditorAlias, out var layout) ? layout : null; - [JsonProperty("udi")] - [JsonConverter(typeof(UdiJsonConverter))] - public Udi Udi { get; set; } + /// + /// Returns the reference to the original BlockValue + /// + public BlockValue BlockValue { get; } - /// - /// The remaining properties will be serialized to a dictionary - /// - /// - /// The JsonExtensionDataAttribute is used to put the non-typed properties into a bucket - /// http://www.newtonsoft.com/json/help/html/DeserializeExtensionData.htm - /// NestedContent serializes to string, int, whatever eg - /// "stringValue":"Some String","numericValue":125,"otherNumeric":null - /// - [JsonExtensionData] - public Dictionary RawPropertyValues { get; set; } = new Dictionary(); - } + public List References { get; } = new List(); } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs index f045a00401..22e364c0f8 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs @@ -1,9 +1,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Linq; using System.Linq; -using System; using System.Collections.Generic; -using static Umbraco.Core.Models.Blocks.BlockEditorData; namespace Umbraco.Core.Models.Blocks { @@ -20,21 +18,18 @@ namespace Umbraco.Core.Models.Blocks _propertyEditorAlias = propertyEditorAlias; } - public BlockEditorData Convert(string json) + public BlockEditorData Deserialize(string json) { var value = JsonConvert.DeserializeObject(json); if (value.Layout == null) return BlockEditorData.Empty; - if (!value.Layout.TryGetValue(_propertyEditorAlias, out var layout)) - return BlockEditorData.Empty; + var references = value.Layout.TryGetValue(_propertyEditorAlias, out var layout) + ? GetBlockReferences(layout) + : Enumerable.Empty(); - var references = GetBlockReferences(layout); - var contentData = value.ContentData.ToList(); - var settingsData = value.SettingsData.ToList(); - - return new BlockEditorData(layout, references, contentData, settingsData); + return new BlockEditorData(_propertyEditorAlias, references, value); } /// @@ -42,7 +37,7 @@ namespace Umbraco.Core.Models.Blocks /// /// /// - protected abstract IReadOnlyList GetBlockReferences(JToken jsonLayout); + protected abstract IEnumerable GetBlockReferences(JToken jsonLayout); } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockItemData.cs b/src/Umbraco.Core/Models/Blocks/BlockItemData.cs new file mode 100644 index 0000000000..12a636771e --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockItemData.cs @@ -0,0 +1,56 @@ +using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using Umbraco.Core.Serialization; + +namespace Umbraco.Core.Models.Blocks +{ + /// + /// Represents a single block's data in raw form + /// + public class BlockItemData + { + [JsonProperty("contentTypeKey")] + public Guid ContentTypeKey { get; set; } + + /// + /// not serialized, manually set and used during internally + /// + [JsonIgnore] + public string ContentTypeAlias { get; set; } + + [JsonProperty("udi")] + [JsonConverter(typeof(UdiJsonConverter))] + public Udi Udi { get; set; } + + [JsonIgnore] + public Guid Key => Udi != null ? ((GuidUdi)Udi).Guid : throw new InvalidOperationException("No Udi assigned"); + + /// + /// The remaining properties will be serialized to a dictionary + /// + /// + /// The JsonExtensionDataAttribute is used to put the non-typed properties into a bucket + /// http://www.newtonsoft.com/json/help/html/DeserializeExtensionData.htm + /// NestedContent serializes to string, int, whatever eg + /// "stringValue":"Some String","numericValue":125,"otherNumeric":null + /// + [JsonExtensionData] + public Dictionary RawPropertyValues { get; set; } = new Dictionary(); + + /// + /// Used during deserialization to convert the raw property data into data with a property type context + /// + [JsonIgnore] + public IDictionary PropertyValues { get; set; } = new Dictionary(); + + /// + /// Used during deserialization to populate the property value/property type of a block item content property + /// + public class BlockPropertyValue + { + public object Value { get; set; } + public PropertyType PropertyType { get; set; } + } + } +} diff --git a/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs index 0dec05bc54..23f69922d9 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Models.Blocks { } - protected override IReadOnlyList GetBlockReferences(JToken jsonLayout) + protected override IEnumerable GetBlockReferences(JToken jsonLayout) { var blockListLayout = jsonLayout.ToObject>(); return blockListLayout.Select(x => new ContentAndSettingsReference(x.ContentUdi, x.SettingsUdi)).ToList(); diff --git a/src/Umbraco.Core/Models/Blocks/BlockValue.cs b/src/Umbraco.Core/Models/Blocks/BlockValue.cs new file mode 100644 index 0000000000..4700ddfd3b --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockValue.cs @@ -0,0 +1,18 @@ +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using System.Collections.Generic; + +namespace Umbraco.Core.Models.Blocks +{ + public class BlockValue + { + [JsonProperty("layout")] + public IDictionary Layout { get; set; } + + [JsonProperty("contentData")] + public List ContentData { get; set; } = new List(); + + [JsonProperty("settingsData")] + public List SettingsData { get; set; } = new List(); + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index d7a1251f20..73af567cbc 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -134,8 +134,10 @@ + + diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index 68b19abf08..e3c2913e4e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -517,7 +517,7 @@ } function handleHttpException(err) { - if (!err.status) { + if (err && !err.status) { $exceptionHandler(err); } } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index 7496c15b52..e3e1e31a53 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -25,7 +25,7 @@ if (!elementModel || !elementModel.variants || !elementModel.variants.length) { return; } var variant = elementModel.variants[0]; - + for (var t = 0; t < variant.tabs.length; t++) { var tab = variant.tabs[t]; @@ -36,7 +36,7 @@ } } } - + } /** @@ -44,11 +44,11 @@ * needs to stay simple to avoid deep watching. */ function mapToPropertyModel(elementModel, dataModel) { - + if (!elementModel || !elementModel.variants || !elementModel.variants.length) { return; } var variant = elementModel.variants[0]; - + for (var t = 0; t < variant.tabs.length; t++) { var tab = variant.tabs[t]; @@ -59,7 +59,7 @@ } } } - + } /** @@ -98,13 +98,13 @@ } } - + /** * Generate label for Block, uses either the labelInterpolator or falls back to the contentTypeName. * @param {Object} blockObject BlockObject to recive data values from. */ function getBlockLabel(blockObject) { - if(blockObject.labelInterpolator !== undefined) { + if (blockObject.labelInterpolator !== undefined) { // We are just using the data model, since its a key/value object that is live synced. (if we need to add additional vars, we could make a shallow copy and apply those.) return blockObject.labelInterpolator(blockObject.data); } @@ -133,7 +133,7 @@ // But we like to sync non-primitive values as well! Yes, and this does happen, just not through this code, but through the nature of JavaScript. // Non-primitive values act as references to the same data and are therefor synced. blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + field + ".variants[0].tabs[" + t + "].properties[" + p + "].value", watcherCreator(blockObject, prop))); - + // We also like to watch our data model to be able to capture changes coming from other places. if (forSettings === true) { blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + "settingsData" + "." + prop.alias, createLayoutSettingsModelWatcher(blockObject, prop))); @@ -151,8 +151,8 @@ /** * Used to create a prop watcher for the data in the property editor data model. */ - function createDataModelWatcher(blockObject, prop) { - return function() { + function createDataModelWatcher(blockObject, prop) { + return function () { if (prop.value !== blockObject.data[prop.alias]) { // sync data: @@ -165,8 +165,8 @@ /** * Used to create a prop watcher for the settings in the property editor data model. */ - function createLayoutSettingsModelWatcher(blockObject, prop) { - return function() { + function createLayoutSettingsModelWatcher(blockObject, prop) { + return function () { if (prop.value !== blockObject.settingsData[prop.alias]) { // sync data: prop.value = blockObject.settingsData[prop.alias]; @@ -177,8 +177,8 @@ /** * Used to create a scoped watcher for a content property on a blockObject. */ - function createContentModelPropWatcher(blockObject, prop) { - return function() { + function createContentModelPropWatcher(blockObject, prop) { + return function () { if (blockObject.data[prop.alias] !== prop.value) { // sync data: blockObject.data[prop.alias] = prop.value; @@ -191,8 +191,8 @@ /** * Used to create a scoped watcher for a settings property on a blockObject. */ - function createSettingsModelPropWatcher(blockObject, prop) { - return function() { + function createSettingsModelPropWatcher(blockObject, prop) { + return function () { if (blockObject.settingsData[prop.alias] !== prop.value) { // sync data: blockObject.settingsData[prop.alias] = prop.value; @@ -255,17 +255,17 @@ this.isolatedScope = scopeOfExistance.$new(true); this.isolatedScope.blockObjects = {}; - + this.__watchers.push(this.isolatedScope.$on("$destroy", this.destroy.bind(this))); this.__watchers.push(propertyEditorScope.$on("postFormSubmitting", this.sync.bind(this))); }; - + BlockEditorModelObject.prototype = { update: function (propertyModelValue, propertyEditorScope) { // clear watchers - this.__watchers.forEach(w => { w(); }); + this.__watchers.forEach(w => { w(); }); delete this.__watchers; // clear block objects @@ -293,7 +293,7 @@ * @param {string} key contentTypeKey to recive the configuration model for. * @returns {Object | null} Configuration model for the that specific block. Or ´null´ if the contentTypeKey isnt available in the current block configurations. */ - getBlockConfiguration: function(key) { + getBlockConfiguration: function (key) { return this.blockConfigurations.find(bc => bc.contentTypeKey === key) || null; }, @@ -305,7 +305,7 @@ * @param {Object} blockObject BlockObject to receive data values from. * @returns {Promise} A Promise object which resolves when all scaffold models are loaded. */ - load: function() { + load: function () { var tasks = []; var scaffoldKeys = []; @@ -339,7 +339,7 @@ * @description Retrive a list of aliases that are available for content of blocks in this property editor, does not contain aliases of block settings. * @return {Array} array of strings representing alias. */ - getAvailableAliasesForBlockContent: function() { + getAvailableAliasesForBlockContent: function () { return this.blockConfigurations.map(blockConfiguration => this.getScaffoldFromKey(blockConfiguration.contentTypeKey).contentTypeAlias); }, @@ -351,13 +351,13 @@ * The purpose of this data is to provide it for the Block Picker. * @return {Array} array of objects representing available blocks, each object containing properties blockConfigModel and elementTypeModel. */ - getAvailableBlocksForBlockPicker: function() { + getAvailableBlocksForBlockPicker: function () { var blocks = []; this.blockConfigurations.forEach(blockConfiguration => { var scaffold = this.getScaffoldFromKey(blockConfiguration.contentTypeKey); - if(scaffold) { + if (scaffold) { blocks.push({ blockConfigModel: blockConfiguration, elementTypeModel: scaffold.documentType @@ -376,7 +376,7 @@ * @param {string} key contentTypeKey to recive the scaffold model for. * @returns {Object | null} Scaffold model for the that content type. Or null if the scaffolding model dosnt exist in this context. */ - getScaffoldFromKey: function(contentTypeKey) { + getScaffoldFromKey: function (contentTypeKey) { return this.scaffolds.find(o => o.contentTypeKey === contentTypeKey); }, @@ -388,7 +388,7 @@ * @param {string} alias contentTypeAlias to recive the scaffold model for. * @returns {Object | null} Scaffold model for the that content type. Or null if the scaffolding model dosnt exist in this context. */ - getScaffoldFromAlias: function(contentTypeAlias) { + getScaffoldFromAlias: function (contentTypeAlias) { return this.scaffolds.find(o => o.contentTypeAlias === contentTypeAlias); }, @@ -413,14 +413,14 @@ * @param {Object} layoutEntry the layout entry object to build the block model from. * @return {Object | null} The BlockObject for the given layout entry. Or null if data or configuration wasnt found for this block. */ - getBlockObject: function(layoutEntry) { + getBlockObject: function (layoutEntry) { - var udi = layoutEntry.udi; + var udi = layoutEntry.contentUdi; var dataModel = this._getDataByUdi(udi); if (dataModel === null) { - console.error("Couldnt find content model of " + udi) + console.error("Couldn't find content model of " + udi) return null; } @@ -428,11 +428,12 @@ var contentScaffold; if (blockConfiguration === null) { - console.error("The block entry of "+udi+" is not begin initialized cause its contentTypeKey is not allowed for this PropertyEditor"); - } else { + console.error("The block entry of " + udi + " is not being initialized because its contentTypeKey is not allowed for this PropertyEditor"); + } + else { contentScaffold = this.getScaffoldFromKey(blockConfiguration.contentTypeKey); - if(contentScaffold === null) { - console.error("The block entry of "+udi+" is not begin initialized cause its Element Type was not loaded."); + if (contentScaffold === null) { + console.error("The block entry of " + udi + " is not begin initialized cause its Element Type was not loaded."); } } @@ -443,12 +444,12 @@ unsupported: true }; contentScaffold = {}; - + } var blockObject = {}; // Set an angularJS cloneNode method, to avoid this object begin cloned. - blockObject.cloneNode = function() { + blockObject.cloneNode = function () { return null;// angularJS accept this as a cloned value as long as the } blockObject.key = String.CreateGuid().replace(/-/g, ""); @@ -465,7 +466,7 @@ this.__scope.$evalAsync(); } }.bind(blockObject) - , 10); + , 10); // make basics from scaffold blockObject.content = Utilities.copy(contentScaffold); @@ -505,7 +506,7 @@ } } - blockObject.retrieveValuesFrom = function(content, settings) { + blockObject.retrieveValuesFrom = function (content, settings) { if (this.content !== null) { mapElementValues(content, this.content); } @@ -545,7 +546,7 @@ delete this.settingsData; delete this.content; delete this.settings; - + // remove model from isolatedScope. delete this.__scope.blockObjects["_" + this.key]; // NOTE: It seems like we should call this.__scope.$destroy(); since that is the only way to remove a scope from it's parent, @@ -555,7 +556,7 @@ // removes this method, making it impossible to destroy again. delete this.destroy; - + // lets remove the key to make things blow up if this is still referenced: delete this.key; } @@ -580,7 +581,7 @@ } this.destroyBlockObject(blockObject); this.removeDataByUdi(udi); - if(settingsUdi) { + if (settingsUdi) { this.removeSettingsByUdi(settingsUdi); } }, @@ -592,7 +593,7 @@ * @description Destroys the Block Model, but all data is kept. * @param {Object} blockObject The BlockObject to be destroyed. */ - destroyBlockObject: function(blockObject) { + destroyBlockObject: function (blockObject) { blockObject.destroy(); }, @@ -604,13 +605,13 @@ * @param {object} defaultStructure if no data exist the layout of your poerty editor will be set to this object. * @return {Object} Layout object, structure depends on the model of your property editor. */ - getLayout: function(defaultStructure) { + getLayout: function (defaultStructure) { if (!this.value.layout[this.propertyEditorAlias]) { this.value.layout[this.propertyEditorAlias] = defaultStructure; } return this.value.layout[this.propertyEditorAlias]; }, - + /** * @ngdoc method * @name create @@ -619,21 +620,21 @@ * @param {string} contentTypeKey the contentTypeKey of the block you wish to create, if contentTypeKey is not avaiable in the block configuration then ´null´ will be returned. * @return {Object | null} Layout entry object, to be inserted at a decired location in the layout object. Or null if contentTypeKey is unavaiaible. */ - create: function(contentTypeKey) { - + create: function (contentTypeKey) { + var blockConfiguration = this.getBlockConfiguration(contentTypeKey); - if(blockConfiguration === null) { + if (blockConfiguration === null) { return null; } var entry = { - udi: this._createDataEntry(contentTypeKey) + contentUdi: this._createDataEntry(contentTypeKey) } if (blockConfiguration.settingsElementTypeKey != null) { entry.settingsUdi = this._createSettingsEntry(blockConfiguration.settingsElementTypeKey) } - + return entry; }, @@ -644,19 +645,19 @@ * @description Insert data from ElementType Model * @return {Object | null} Layout entry object, to be inserted at a decired location in the layout object. Or ´null´ if the given ElementType isnt supported by the block configuration. */ - createFromElementType: function(elementTypeDataModel) { + createFromElementType: function (elementTypeDataModel) { elementTypeDataModel = Utilities.copy(elementTypeDataModel); var contentTypeKey = elementTypeDataModel.contentTypeKey; var layoutEntry = this.create(contentTypeKey); - if(layoutEntry === null) { + if (layoutEntry === null) { return null; } var dataModel = this._getDataByUdi(layoutEntry.udi); - if(dataModel === null) { + if (dataModel === null) { return null; } @@ -672,7 +673,7 @@ * @methodOf umbraco.services.blockEditorModelObject * @description Force immidiate update of the blockobject models to the property model. */ - sync: function() { + sync: function () { for (const key in this.isolatedScope.blockObjects) { this.isolatedScope.blockObjects[key].sync(); } @@ -680,7 +681,7 @@ // private // TODO: Then this can just be a method in the outer scope - _createDataEntry: function(elementTypeKey) { + _createDataEntry: function (elementTypeKey) { var content = { contentTypeKey: elementTypeKey, udi: udiService.create("element") @@ -690,7 +691,7 @@ }, // private // TODO: Then this can just be a method in the outer scope - _getDataByUdi: function(udi) { + _getDataByUdi: function (udi) { return this.value.contentData.find(entry => entry.udi === udi) || null; }, @@ -699,10 +700,10 @@ * @name removeDataByUdi * @methodOf umbraco.services.blockEditorModelObject * @description Removes the content data of a given UDI. - * Notice this method does not remove the block from your layout, this will need to be handlede by the Property Editor since this services donst know about your layout structure. + * Notice this method does not remove the block from your layout, this will need to be handled by the Property Editor since this services don't know about your layout structure. * @param {string} udi The UDI of the content data to be removed. */ - removeDataByUdi: function(udi) { + removeDataByUdi: function (udi) { const index = this.value.contentData.findIndex(o => o.udi === udi); if (index !== -1) { this.value.contentData.splice(index, 1); @@ -711,7 +712,7 @@ // private // TODO: Then this can just be a method in the outer scope - _createSettingsEntry: function(elementTypeKey) { + _createSettingsEntry: function (elementTypeKey) { var settings = { contentTypeKey: elementTypeKey, udi: udiService.create("element") @@ -722,7 +723,7 @@ // private // TODO: Then this can just be a method in the outer scope - _getSettingsByUdi: function(udi) { + _getSettingsByUdi: function (udi) { return this.value.settingsData.find(entry => entry.udi === udi) || null; }, @@ -731,10 +732,10 @@ * @name removeSettingsByUdi * @methodOf umbraco.services.blockEditorModelObject * @description Removes the settings data of a given UDI. - * Notice this method does not remove the settingsUdi from your layout, this will need to be handlede by the Property Editor since this services donst know about your layout structure. + * Notice this method does not remove the settingsUdi from your layout, this will need to be handled by the Property Editor since this services don't know about your layout structure. * @param {string} udi The UDI of the settings data to be removed. */ - removeSettingsByUdi: function(udi) { + removeSettingsByUdi: function (udi) { const index = this.value.settingsData.findIndex(o => o.udi === udi); if (index !== -1) { this.value.settingsData.splice(index, 1); @@ -747,13 +748,13 @@ * @methodOf umbraco.services.blockEditorModelObject * @description Notice you should not need to destroy the BlockEditorModelObject since it will automaticly be destroyed when the scope of existance gets destroyed. */ - destroy: function() { + destroy: function () { this.__watchers.forEach(w => { w(); }); for (const key in this.isolatedScope.blockObjects) { this.destroyBlockObject(this.isolatedScope.blockObjects[key]); } - + delete this.__watchers; delete this.value; delete this.propertyEditorAlias; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js index b2fdf37aa4..7f8212f2c6 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js @@ -504,7 +504,6 @@ function serverValidationManager($timeout) { // add a generic error for the property addPropertyError(propertyValidationKey, culture, htmlFieldReference, value && Array.isArray(value) && value.length > 0 ? value[0] : null, segment); - hasPropertyErrors = true; } else { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js index aa66472778..45ba6ddaa7 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js @@ -132,7 +132,13 @@ // Called when we save the value, the server may return an updated data and our value is re-synced // we need to deal with that here so that our model values are all in sync so we basically re-initialize. - function onServerValueChanged(newVal, oldVal) { + function onServerValueChanged(newVal, oldVal) { + + // We need to ensure that the property model value is an object, this is needed for modelObject to recive a reference and keep that updated. + if (typeof newVal !== 'object' || newVal === null) {// testing if we have null or undefined value or if the value is set to another type than Object. + newVal = {}; + } + modelObject.update(newVal, $scope); onLoaded(); } @@ -148,6 +154,8 @@ // Store a reference to the layout model, because we need to maintain this model. vm.layout = modelObject.getLayout([]); + var invalidLayoutItems = []; + // Append the blockObjects to our layout. vm.layout.forEach(entry => { // $block must have the data property to be a valid BlockObject, if not its considered as a destroyed blockObject. @@ -155,9 +163,22 @@ var block = getBlockObject(entry); // If this entry was not supported by our property-editor it would return 'null'. - if(block !== null) { + if (block !== null) { entry.$block = block; } + else { + // then we need to filter this out and also update the underlying model. This could happen if the data + // is invalid for some reason or the data structure has changed. + invalidLayoutItems.push(entry); + } + } + }); + + // remove the ones that are invalid + invalidLayoutItems.forEach(entry => { + var index = vm.layout.findIndex(x => x === entry); + if (index >= 0) { + vm.layout.splice(index, 1); } }); @@ -240,7 +261,7 @@ function deleteBlock(block) { - var layoutIndex = vm.layout.findIndex(entry => entry.udi === block.content.udi); + var layoutIndex = vm.layout.findIndex(entry => entry.contentUdi === block.content.udi); if (layoutIndex === -1) { throw new Error("Could not find layout entry of block with udi: "+block.content.udi) } diff --git a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs index 866bd38e05..c20bd08204 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using static Umbraco.Core.Models.Blocks.BlockEditorData; +using static Umbraco.Core.Models.Blocks.BlockItemData; namespace Umbraco.Web.PropertyEditors { @@ -63,8 +64,12 @@ namespace Umbraco.Web.PropertyEditors var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); var result = new List(); + var blockEditorData = _blockEditorValues.DeserializeAndClean(rawJson); + if (blockEditorData == null) + return Enumerable.Empty(); - foreach (var row in _blockEditorValues.GetPropertyValues(rawJson)) + // TODO: What about Settings? + foreach (var row in blockEditorData.BlockValue.ContentData) { foreach (var prop in row.PropertyValues) { @@ -83,6 +88,132 @@ namespace Umbraco.Web.PropertyEditors return result; } + + #region Convert database // editor + + // note: there is NO variant support here + + /// + /// Ensure that sub-editor values are translated through their ToEditor methods + /// + /// + /// + /// + /// + /// + public override object ToEditor(Property property, IDataTypeService dataTypeService, string culture = null, string segment = null) + { + var val = property.GetValue(culture, segment); + + BlockEditorData blockEditorData; + try + { + blockEditorData = _blockEditorValues.DeserializeAndClean(val); + } + catch (JsonSerializationException) + { + // if this occurs it means the data is invalid, shouldn't happen but has happened if we change the data format. + return string.Empty; + } + + if (blockEditorData == null || blockEditorData.BlockValue.ContentData.Count == 0) + return string.Empty; + + foreach (var row in blockEditorData.BlockValue.ContentData) + { + foreach (var prop in row.PropertyValues) + { + try + { + // create a temp property with the value + // - force it to be culture invariant as the block editor can't handle culture variant element properties + prop.Value.PropertyType.Variations = ContentVariation.Nothing; + var tempProp = new Property(prop.Value.PropertyType); + + tempProp.SetValue(prop.Value.Value); + + // convert that temp property, and store the converted value + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + if (propEditor == null) + { + // NOTE: This logic was borrowed from Nested Content and I'm unsure why it exists. + // if the property editor doesn't exist I think everything will break anyways? + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = tempProp.GetValue()?.ToString(); + continue; + } + + var tempConfig = dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeId).Configuration; + var valEditor = propEditor.GetValueEditor(tempConfig); + var convValue = valEditor.ToEditor(tempProp, dataTypeService); + + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = convValue; + } + catch (InvalidOperationException) + { + // deal with weird situations by ignoring them (no comment) + row.PropertyValues.Remove(prop.Key); + } + } + } + + // return json convertable object + return blockEditorData.BlockValue; + } + + /// + /// Ensure that sub-editor values are translated through their FromEditor methods + /// + /// + /// + /// + public override object FromEditor(ContentPropertyData editorValue, object currentValue) + { + if (editorValue.Value == null || string.IsNullOrWhiteSpace(editorValue.Value.ToString())) + return null; + + BlockEditorData blockEditorData; + try + { + blockEditorData = _blockEditorValues.DeserializeAndClean(editorValue.Value); + } + catch (JsonSerializationException) + { + // if this occurs it means the data is invalid, shouldn't happen but has happened if we change the data format. + return string.Empty; + } + + if (blockEditorData == null || blockEditorData.BlockValue.ContentData.Count == 0) + return string.Empty; + + foreach (var row in blockEditorData.BlockValue.ContentData) + { + foreach (var prop in row.PropertyValues) + { + // Fetch the property types prevalue + var propConfiguration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeId).Configuration; + + // Lookup the property editor + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + if (propEditor == null) continue; + + // Create a fake content property data object + var contentPropData = new ContentPropertyData(prop.Value.Value, propConfiguration); + + // Get the property editor to do it's conversion + var newValue = propEditor.GetValueEditor().FromEditor(contentPropData, prop.Value.Value); + + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = newValue; + } + } + + // return json + return JsonConvert.SerializeObject(blockEditorData.BlockValue); + } + + #endregion } internal class BlockEditorValidator : ComplexEditorValidator @@ -96,19 +227,26 @@ namespace Umbraco.Web.PropertyEditors protected override IEnumerable GetElementTypeValidation(object value) { - foreach (var row in _blockEditorValues.GetPropertyValues(value)) + var blockEditorData = _blockEditorValues.DeserializeAndClean(value); + if (blockEditorData != null) { - var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Id); - foreach (var prop in row.PropertyValues) + foreach (var row in blockEditorData.BlockValue.ContentData) { - elementValidation.AddPropertyTypeValidation( - new PropertyTypeValidationModel(prop.Value.PropertyType, prop.Value.Value)); + var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Key); + foreach (var prop in row.PropertyValues) + { + elementValidation.AddPropertyTypeValidation( + new PropertyTypeValidationModel(prop.Value.PropertyType, prop.Value.Value)); + } + yield return elementValidation; } - yield return elementValidation; } } } + /// + /// Used to deserialize json values and clean up any values based on the existence of element types and layout structure + /// internal class BlockEditorValues { private readonly Lazy> _contentTypes; @@ -126,81 +264,77 @@ namespace Umbraco.Web.PropertyEditors return contentType; } - public IReadOnlyList GetPropertyValues(object propertyValue) + public BlockEditorData DeserializeAndClean(object propertyValue) { if (propertyValue == null || string.IsNullOrWhiteSpace(propertyValue.ToString())) - return new List(); + return null; - var converted = _dataConverter.Convert(propertyValue.ToString()); + var blockEditorData = _dataConverter.Deserialize(propertyValue.ToString()); - if (converted.ContentData.Count == 0) - return new List(); - - var contentTypePropertyTypes = new Dictionary>(); - var result = new List(); - - foreach(var block in converted.ContentData) + if (blockEditorData.BlockValue.ContentData.Count == 0) { - var contentType = GetElementType(block); - if (contentType == null) - continue; - - // get the prop types for this content type but keep a dictionary of found ones so we don't have to keep re-looking and re-creating - // objects on each iteration. - if (!contentTypePropertyTypes.TryGetValue(contentType.Alias, out var propertyTypes)) - propertyTypes = contentTypePropertyTypes[contentType.Alias] = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); - - var propValues = new Dictionary(); - - // find any keys that are not real property types and remove them - foreach (var prop in block.RawPropertyValues.ToList()) - { - // doesn't exist so remove it - if (!propertyTypes.TryGetValue(prop.Key, out var propType)) - { - block.RawPropertyValues.Remove(prop.Key); - } - else - { - // set the value to include the resolved property type - propValues[prop.Key] = new BlockPropertyValue - { - PropertyType = propType, - Value = prop.Value - }; - } - } - - result.Add(new BlockValue - { - ContentTypeAlias = contentType.Alias, - PropertyValues = propValues, - Id = ((GuidUdi)block.Udi).Guid - }); + // if there's no content ensure there's no settings too + blockEditorData.BlockValue.SettingsData.Clear(); + return null; } - return result; + var contentTypePropertyTypes = new Dictionary>(); + + // filter out any content that isn't referenced in the layout references + foreach(var block in blockEditorData.BlockValue.ContentData.Where(x => blockEditorData.References.Any(r => r.ContentUdi == x.Udi))) + { + ResolveBlockItemData(block, contentTypePropertyTypes); + } + // filter out any settings that isn't referenced in the layout references + foreach (var block in blockEditorData.BlockValue.SettingsData.Where(x => blockEditorData.References.Any(r => r.SettingsUdi == x.Udi))) + { + ResolveBlockItemData(block, contentTypePropertyTypes); + } + + // remove blocks that couldn't be resolved + blockEditorData.BlockValue.ContentData.RemoveAll(x => x.ContentTypeAlias.IsNullOrWhiteSpace()); + blockEditorData.BlockValue.SettingsData.RemoveAll(x => x.ContentTypeAlias.IsNullOrWhiteSpace()); + + return blockEditorData; } - /// - /// Used during deserialization to populate the property value/property type of a nested content row property - /// - internal class BlockPropertyValue + private bool ResolveBlockItemData(BlockItemData block, Dictionary> contentTypePropertyTypes) { - public object Value { get; set; } - public PropertyType PropertyType { get; set; } - } + var contentType = GetElementType(block); + if (contentType == null) + return false; - /// - /// Used during deserialization to populate the content type alias and property values of a block - /// - internal class BlockValue - { - public Guid Id { get; set; } - public string ContentTypeAlias { get; set; } - public IDictionary PropertyValues { get; set; } = new Dictionary(); - } + // get the prop types for this content type but keep a dictionary of found ones so we don't have to keep re-looking and re-creating + // objects on each iteration. + if (!contentTypePropertyTypes.TryGetValue(contentType.Alias, out var propertyTypes)) + propertyTypes = contentTypePropertyTypes[contentType.Alias] = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); + var propValues = new Dictionary(); + + // find any keys that are not real property types and remove them + foreach (var prop in block.RawPropertyValues.ToList()) + { + // doesn't exist so remove it + if (!propertyTypes.TryGetValue(prop.Key, out var propType)) + { + block.RawPropertyValues.Remove(prop.Key); + } + else + { + // set the value to include the resolved property type + propValues[prop.Key] = new BlockPropertyValue + { + PropertyType = propType, + Value = prop.Value + }; + } + } + + block.ContentTypeAlias = contentType.Alias; + block.PropertyValues = propValues; + + return true; + } } #endregion diff --git a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs index 4767dc19cd..ac9e0624bb 100644 --- a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs @@ -133,13 +133,20 @@ namespace Umbraco.Web.PropertyEditors #endregion - + #region Convert database // editor // note: there is NO variant support here - // TODO: What does this do? + /// + /// Ensure that sub-editor values are translated through their ToEditor methods + /// + /// + /// + /// + /// + /// public override object ToEditor(Property property, IDataTypeService dataTypeService, string culture = null, string segment = null) { var val = property.GetValue(culture, segment); @@ -186,11 +193,16 @@ namespace Umbraco.Web.PropertyEditors } } - // return json + // return the object, there's a native json converter for this so it will serialize correctly return rows; } - // TODO: What does this do? + /// + /// Ensure that sub-editor values are translated through their FromEditor methods + /// + /// + /// + /// public override object FromEditor(ContentPropertyData editorValue, object currentValue) { if (editorValue.Value == null || string.IsNullOrWhiteSpace(editorValue.Value.ToString())) diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs index 83c612e9f7..f043c8e66e 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs @@ -24,7 +24,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters } public IPublishedElement ConvertToElement( - BlockEditorData.BlockItemData data, + BlockItemData data, PropertyCacheLevel referenceCacheLevel, bool preview) { // hack! we need to cast, we have no choice beacuse we cannot make breaking changes. diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index 2b04106288..4d972f7a33 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -62,20 +62,20 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters if (string.IsNullOrWhiteSpace(value)) return BlockListModel.Empty; var converter = new BlockListEditorDataConverter(); - var converted = converter.Convert(value); - if (converted.ContentData.Count == 0) return BlockListModel.Empty; + var converted = converter.Deserialize(value); + if (converted.BlockValue.ContentData.Count == 0) return BlockListModel.Empty; var blockListLayout = converted.Layout.ToObject>(); // convert the content data - foreach (var data in converted.ContentData) + foreach (var data in converted.BlockValue.ContentData) { var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; contentPublishedElements[element.Key] = element; } // convert the settings data - foreach (var data in converted.SettingsData) + foreach (var data in converted.BlockValue.SettingsData) { var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; From e01f36514b70c7bf4132ded5568834eafe025a75 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Jul 2020 22:14:06 +1000 Subject: [PATCH 06/10] fixes tests --- src/Umbraco.Core/Models/Blocks/BlockEditorData.cs | 1 + .../ValueConverters/BlockListPropertyValueConverter.cs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs index 5ee609b148..d8186f0dfa 100644 --- a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs @@ -16,6 +16,7 @@ namespace Umbraco.Core.Models.Blocks private BlockEditorData() { + BlockValue = new BlockValue(); } public BlockEditorData(string propertyEditorAlias, diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index 4d972f7a33..b04ef2d444 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -18,11 +18,13 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters { private readonly IProfilingLogger _proflog; private readonly BlockEditorConverter _blockConverter; + private readonly BlockListEditorDataConverter _blockListEditorDataConverter; public BlockListPropertyValueConverter(IProfilingLogger proflog, BlockEditorConverter blockConverter) { _proflog = proflog; _blockConverter = blockConverter; + _blockListEditorDataConverter = new BlockListEditorDataConverter(); } /// @@ -61,8 +63,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters var value = (string)inter; if (string.IsNullOrWhiteSpace(value)) return BlockListModel.Empty; - var converter = new BlockListEditorDataConverter(); - var converted = converter.Deserialize(value); + var converted = _blockListEditorDataConverter.Deserialize(value); if (converted.BlockValue.ContentData.Count == 0) return BlockListModel.Empty; var blockListLayout = converted.Layout.ToObject>(); From 84dddd18691e64c81c6bfc7e679668dfc77484a5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Jul 2020 23:23:39 +1000 Subject: [PATCH 07/10] fixes tests --- .../common/services/block-editor-service.spec.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js index 577cb1b3e8..33341c7cec 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js @@ -264,16 +264,12 @@ expect(propertyModel.layout["Umbraco.TestBlockEditor"][0]).toBeUndefined(); done(); - }); - + } catch (e) { + done.fail(e); + } + }); }); - - - - - - it('getBlockObject of block with settings has values', function (done) { var propertyModel = angular.copy(propertyModelWithSettingsMock); From 692b8b819f57d6b082cc357382878765bab826d2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Jul 2020 23:31:55 +1000 Subject: [PATCH 08/10] fixes tests --- .../services/block-editor-service.spec.js | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js index 33341c7cec..e4e1133bd6 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js @@ -1,7 +1,9 @@ describe('blockEditorService tests', function () { - var key = "6A1F5BDD-67EF-4173-B061-D6348ED07094"; - var udi = "umb://element/6A1F5BDD67EF4173B061D6348ED07094"; + var contentKey = "6A1F5BDD-67EF-4173-B061-D6348ED07094"; + var contentUdi = "umb://element/6A1F5BDD67EF4173B061D6348ED07094"; + var settingsKey = "2AF42343-C8A2-400D-BA43-4818C2B3CDC5"; + var settingsUdi = "umb://element/2AF42343C8A2400DBA434818C2B3CDC5"; var blockEditorService, contentResource, $rootScope, $scope; @@ -20,7 +22,7 @@ contentResource = $injector.get("contentResource"); spyOn(contentResource, "getScaffoldByKey").and.callFake( function () { - return Promise.resolve(mocksUtils.getMockVariantContent(1234, key, udi)) + return Promise.resolve(mocksUtils.getMockVariantContent(1234, contentKey, contentUdi)) } ); @@ -35,13 +37,13 @@ layout: { "Umbraco.TestBlockEditor": [ { - udi: udi + contentUdi: contentUdi } ] }, contentData: [ { - udi: 1234, + udi: contentUdi, contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", testproperty: "myTestValue" } @@ -53,21 +55,21 @@ layout: { "Umbraco.TestBlockEditor": [ { - udi: 1234, - settingsUdi: 4567 + contentUdi: contentUdi, + settingsUdi: settingsUdi } ] }, contentData: [ { - udi: udi, + udi: contentUdi, contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", testproperty: "myTestValue" } ], settingsData: [ { - udi: 4567, + udi: settingsUdi, contentTypeKey: "7C5B74D1-E2F9-45A3-AE4B-FC7A829BF8AB", testproperty: "myTestValueForSettings" } @@ -339,33 +341,34 @@ modelObject.load().then(() => { - var layout = modelObject.getLayout(); + try { + var layout = modelObject.getLayout(); - var blockObject = modelObject.getBlockObject(layout[0]); + var blockObject = modelObject.getBlockObject(layout[0]); - blockObject.content.variants[0].tabs[0].properties[0].value.list[0] = "AA"; - blockObject.content.variants[0].tabs[0].properties[0].value.list.push("D"); + blockObject.content.variants[0].tabs[0].properties[0].value.list[0] = "AA"; + blockObject.content.variants[0].tabs[0].properties[0].value.list.push("D"); - blockObject.settings.variants[0].tabs[0].properties[0].value.list[0] = "settingsValue"; - blockObject.settings.variants[0].tabs[0].properties[0].value.list.push("settingsNewValue"); + blockObject.settings.variants[0].tabs[0].properties[0].value.list[0] = "settingsValue"; + blockObject.settings.variants[0].tabs[0].properties[0].value.list.push("settingsNewValue"); - $rootScope.$digest();// invoke angularJS Store. + $rootScope.$digest();// invoke angularJS Store. - expect(propertyModel.contentData[0].testproperty.list[0]).toBe("AA"); - expect(propertyModel.contentData[0].testproperty.list.length).toBe(4); + expect(propertyModel.contentData[0].testproperty.list[0]).toBe("AA"); + expect(propertyModel.contentData[0].testproperty.list.length).toBe(4); - expect(propertyModel.settingsData[0].testproperty.list[0]).toBe("settingsValue"); - expect(propertyModel.settingsData[0].testproperty.list.length).toBe(4); + expect(propertyModel.settingsData[0].testproperty.list[0]).toBe("settingsValue"); + expect(propertyModel.settingsData[0].testproperty.list.length).toBe(4); + + done(); + } catch (e) { + done.fail(e); + } + }); - done(); - } catch (e) { - done.fail(e); - } }); + }); - -}); - }); From e52cd3e3597249510237d897e59b713e5f190f07 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Jul 2020 23:53:58 +1000 Subject: [PATCH 09/10] fixes block removal with new data structure --- .../blocklist/umbBlockListPropertyEditor.component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js index 45ba6ddaa7..3ad18d87c1 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js @@ -271,7 +271,7 @@ var removed = vm.layout.splice(layoutIndex, 1); removed.forEach(x => { // remove any server validation errors associated - var guid = udiService.getKey(x.udi); + var guid = udiService.getKey(x.contentUdi); serverValidationManager.removePropertyError(guid, vm.umbProperty.property.culture, vm.umbProperty.property.segment, "", { matchType: "contains" }); }); From cf442f78c029fa118eb2520e223b81231ed98f28 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 28 Jul 2020 15:33:54 +1000 Subject: [PATCH 10/10] updates prop value converter to filter out all content/settings blocks outside of the layout from reaching razor + test --- .../BlockListPropertyValueConverterTests.cs | 79 +++++++++++++++++++ .../BlockListPropertyValueConverter.cs | 13 ++- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs index 7ff2077d77..23cc782106 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -366,5 +366,84 @@ data: []}"; } + [Test] + public void Data_Item_Removed_If_Removed_From_Config() + { + var editor = CreateConverter(); + + // The data below expects that ContentKey1 + ContentKey2 + SettingsKey1 + SettingsKey2 exist but only ContentKey2 exists so + // the data should all be filtered. + var config = new BlockListConfiguration + { + Blocks = new[] { + new BlockListConfiguration.BlockConfiguration + { + ContentElementTypeKey = ContentKey2, + SettingsElementTypeKey = null + } + } + }; + + var propertyType = GetPropertyType(config); + var publishedElement = Mock.Of(); + + var json = @" +{ + layout: { + '" + Constants.PropertyEditors.Aliases.BlockList + @"': [ + { + 'contentUdi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D', + 'settingsUdi': 'umb://element/1F613E26CE274898908A561437AF5100' + }, + { + 'contentUdi': 'umb://element/0A4A416E547D464FABCC6F345C17809A', + 'settingsUdi': 'umb://element/63027539B0DB45E7B70459762D4E83DD' + } + ] + }, + contentData: [ + { + 'contentTypeKey': '" + ContentKey1 + @"', + 'udi': 'umb://element/1304E1DDAC87439684FE8A399231CB3D' + }, + { + 'contentTypeKey': '" + ContentKey2 + @"', + 'udi': 'umb://element/E05A034704424AB3A520E048E6197E79' + }, + { + 'contentTypeKey': '" + ContentKey2 + @"', + 'udi': 'umb://element/0A4A416E547D464FABCC6F345C17809A' + } + ], + settingsData: [ + { + 'contentTypeKey': '" + SettingKey1 + @"', + 'udi': 'umb://element/63027539B0DB45E7B70459762D4E83DD' + }, + { + 'contentTypeKey': '" + SettingKey2 + @"', + 'udi': 'umb://element/1F613E26CE274898908A561437AF5100' + }, + { + 'contentTypeKey': '" + SettingKey2 + @"', + 'udi': 'umb://element/BCF4BA3DA40C496C93EC58FAC85F18B9' + } + ], +}"; + + var converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; + + Assert.IsNotNull(converted); + Assert.AreEqual(2, converted.ContentData.Count()); + Assert.AreEqual(0, converted.SettingsData.Count()); + Assert.AreEqual(1, converted.Layout.Count()); + + var item0 = converted.Layout.ElementAt(0); + Assert.AreEqual(Guid.Parse("0A4A416E-547D-464F-ABCC-6F345C17809A"), item0.Content.Key); + Assert.AreEqual("Test2", item0.Content.ContentType.Alias); + Assert.IsNull(item0.Settings); + + } + } } diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index b04ef2d444..25b22e1a9c 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -52,8 +52,8 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters using (_proflog.DebugDuration($"ConvertPropertyToBlockList ({propertyType.DataType.Id})")) { var configuration = propertyType.DataType.ConfigurationAs(); - var contentTypes = configuration.Blocks; - var contentElementTypeMap = contentTypes.ToDictionary(x => x.ContentElementTypeKey); + var blockConfigMap = configuration.Blocks.ToDictionary(x => x.ContentElementTypeKey); + var validSettingElementTypes = blockConfigMap.Values.Select(x => x.SettingsElementTypeKey).Where(x => x.HasValue).Distinct().ToList(); var contentPublishedElements = new Dictionary(); var settingsPublishedElements = new Dictionary(); @@ -71,6 +71,8 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters // convert the content data foreach (var data in converted.BlockValue.ContentData) { + if (!blockConfigMap.ContainsKey(data.ContentTypeKey)) continue; + var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; contentPublishedElements[element.Key] = element; @@ -78,6 +80,8 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters // convert the settings data foreach (var data in converted.BlockValue.SettingsData) { + if (!validSettingElementTypes.Contains(data.ContentTypeKey)) continue; + var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; settingsPublishedElements[element.Key] = element; @@ -96,12 +100,13 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters // get the setting reference IPublishedElement settingsData = null; var settingGuidUdi = layoutItem.SettingsUdi != null ? (GuidUdi)layoutItem.SettingsUdi : null; - if (settingGuidUdi != null) settingsPublishedElements.TryGetValue(settingGuidUdi.Guid, out settingsData); + if (settingGuidUdi != null) + settingsPublishedElements.TryGetValue(settingGuidUdi.Guid, out settingsData); if (!contentData.ContentType.TryGetKey(out var contentTypeKey)) throw new InvalidOperationException("The content type was not of type " + typeof(IPublishedContentType2)); - if (!contentElementTypeMap.TryGetValue(contentTypeKey, out var blockConfig)) + if (!blockConfigMap.TryGetValue(contentTypeKey, out var blockConfig)) continue; // this can happen if they have a settings type, save content, remove the settings type, and display the front-end page before saving the content again