From dff90c6ec07d986f4b11b1f0e4c1f203b65d604d Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 10 Jan 2024 12:22:36 +0100 Subject: [PATCH 01/21] Run the same cleanup with scaffolding content as when copying. (#15541) * Run the same cleanup with scaffolding content as when copying. - Added a new ContentScaffoldedNotification - Published the notification when a new scaffold has been created from a blueprint (content template) - Linked up the ComplextPEContent handler to do the same cleanup for the new notification as when copying. - registered handlers to the event for blocklist, blockgrid and nested content * PR pattern matching suggestion Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- .../ContentScaffoldedNotification.cs | 18 +++++++++++ .../Notifications/ScaffoldedNotification.cs | 23 ++++++++++++++ .../UmbracoBuilder.CoreServices.cs | 3 ++ ...ropertyEditorContentNotificationHandler.cs | 15 ++++++++- .../Controllers/ContentController.cs | 31 ++++++++++++++----- 5 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs create mode 100644 src/Umbraco.Core/Notifications/ScaffoldedNotification.cs diff --git a/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs new file mode 100644 index 0000000000..47eda5468d --- /dev/null +++ b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs @@ -0,0 +1,18 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Notifications; + +/// +/// Notification that is send out when a Content item has been scaffolded from an original item and basic cleaning has been performed +/// +public sealed class ContentScaffoldedNotification : ScaffoldedNotification +{ + public ContentScaffoldedNotification(IContent original, IContent scaffold, int parentId, EventMessages messages) + : base(original, scaffold, parentId, messages) + { + } +} diff --git a/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs new file mode 100644 index 0000000000..f64bfd3933 --- /dev/null +++ b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs @@ -0,0 +1,23 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; + +namespace Umbraco.Cms.Core.Notifications; + +public abstract class ScaffoldedNotification : CancelableObjectNotification + where T : class +{ + protected ScaffoldedNotification(T original, T scaffold, int parentId, EventMessages messages) + : base(original, messages) + { + Scaffold = scaffold; + ParentId = parentId; + } + + public T Original => Target; + + public T Scaffold { get; } + + public int ParentId { get; } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index c65e50024c..bd6f2e9b38 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -358,10 +358,13 @@ public static partial class UmbracoBuilderExtensions builder .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs index 2b4ac75042..ce867d29e4 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs @@ -10,9 +10,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Handles nested Udi keys when +/// - saving: Empty keys get generated +/// - copy: keys get replaced by new ones while keeping references intact +/// - scaffolding: keys get replaced by new ones while keeping references intact +/// public abstract class ComplexPropertyEditorContentNotificationHandler : INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { protected abstract string EditorAlias { get; } @@ -31,6 +38,12 @@ public abstract class ComplexPropertyEditorContentNotificationHandler : } } + public void Handle(ContentScaffoldedNotification notification) + { + IEnumerable props = notification.Scaffold.GetPropertiesByEditor(EditorAlias); + UpdatePropertyValues(props, false); + } + protected abstract string FormatPropertyValue(string rawJson, bool onlyMissingKeys); private void UpdatePropertyValues(IEnumerable props, bool onlyMissingKeys) diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 8beec57812..0a1a82b3eb 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -16,6 +16,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Validation; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Routing; @@ -623,17 +624,33 @@ public class ContentController : ContentControllerBase [OutgoingEditorModelEvent] public ActionResult GetEmptyBlueprint(int blueprintId, int parentId) { - IContent? blueprint = _contentService.GetBlueprintById(blueprintId); - if (blueprint == null) + IContent? scaffold; + using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { - return NotFound(); + IContent? blueprint = _contentService.GetBlueprintById(blueprintId); + if (blueprint is null) + { + return NotFound(); + } + scaffold = (IContent)blueprint.DeepClone(); + + scaffold.Id = 0; + scaffold.Name = string.Empty; + scaffold.ParentId = parentId; + + var scaffoldedNotification = new ContentScaffoldedNotification(blueprint, scaffold, parentId, new EventMessages()); + if (scope.Notifications.PublishCancelable(scaffoldedNotification)) + { + scope.Complete(); + return Problem("Scaffolding was cancelled"); + } + + scope.Complete(); } - blueprint.Id = 0; - blueprint.Name = string.Empty; - blueprint.ParentId = parentId; - ContentItemDisplay? mapped = _umbracoMapper.Map(blueprint); + + ContentItemDisplay? mapped = _umbracoMapper.Map(scaffold); if (mapped is not null) { From 9da46462f7ce2777771d23974b3d7a9172843b92 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 10 Jan 2024 12:22:36 +0100 Subject: [PATCH 02/21] Run the same cleanup with scaffolding content as when copying. (#15541) * Run the same cleanup with scaffolding content as when copying. - Added a new ContentScaffoldedNotification - Published the notification when a new scaffold has been created from a blueprint (content template) - Linked up the ComplextPEContent handler to do the same cleanup for the new notification as when copying. - registered handlers to the event for blocklist, blockgrid and nested content * PR pattern matching suggestion Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> (cherry picked from commit dff90c6ec07d986f4b11b1f0e4c1f203b65d604d) --- .../ContentScaffoldedNotification.cs | 18 +++++++++++ .../Notifications/ScaffoldedNotification.cs | 23 ++++++++++++++ .../UmbracoBuilder.CoreServices.cs | 3 ++ ...ropertyEditorContentNotificationHandler.cs | 15 ++++++++- .../Controllers/ContentController.cs | 31 ++++++++++++++----- 5 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs create mode 100644 src/Umbraco.Core/Notifications/ScaffoldedNotification.cs diff --git a/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs new file mode 100644 index 0000000000..47eda5468d --- /dev/null +++ b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs @@ -0,0 +1,18 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Notifications; + +/// +/// Notification that is send out when a Content item has been scaffolded from an original item and basic cleaning has been performed +/// +public sealed class ContentScaffoldedNotification : ScaffoldedNotification +{ + public ContentScaffoldedNotification(IContent original, IContent scaffold, int parentId, EventMessages messages) + : base(original, scaffold, parentId, messages) + { + } +} diff --git a/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs new file mode 100644 index 0000000000..f64bfd3933 --- /dev/null +++ b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs @@ -0,0 +1,23 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; + +namespace Umbraco.Cms.Core.Notifications; + +public abstract class ScaffoldedNotification : CancelableObjectNotification + where T : class +{ + protected ScaffoldedNotification(T original, T scaffold, int parentId, EventMessages messages) + : base(original, messages) + { + Scaffold = scaffold; + ParentId = parentId; + } + + public T Original => Target; + + public T Scaffold { get; } + + public int ParentId { get; } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 754c0e349e..6f732e1a1e 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -357,10 +357,13 @@ public static partial class UmbracoBuilderExtensions builder .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs index 2b4ac75042..ce867d29e4 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs @@ -10,9 +10,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Handles nested Udi keys when +/// - saving: Empty keys get generated +/// - copy: keys get replaced by new ones while keeping references intact +/// - scaffolding: keys get replaced by new ones while keeping references intact +/// public abstract class ComplexPropertyEditorContentNotificationHandler : INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { protected abstract string EditorAlias { get; } @@ -31,6 +38,12 @@ public abstract class ComplexPropertyEditorContentNotificationHandler : } } + public void Handle(ContentScaffoldedNotification notification) + { + IEnumerable props = notification.Scaffold.GetPropertiesByEditor(EditorAlias); + UpdatePropertyValues(props, false); + } + protected abstract string FormatPropertyValue(string rawJson, bool onlyMissingKeys); private void UpdatePropertyValues(IEnumerable props, bool onlyMissingKeys) diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 6ba81172c8..a1ed75332b 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -19,6 +19,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Validation; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Routing; @@ -683,17 +684,33 @@ public class ContentController : ContentControllerBase [OutgoingEditorModelEvent] public ActionResult GetEmptyBlueprint(int blueprintId, int parentId) { - IContent? blueprint = _contentService.GetBlueprintById(blueprintId); - if (blueprint == null) + IContent? scaffold; + using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { - return NotFound(); + IContent? blueprint = _contentService.GetBlueprintById(blueprintId); + if (blueprint is null) + { + return NotFound(); + } + scaffold = (IContent)blueprint.DeepClone(); + + scaffold.Id = 0; + scaffold.Name = string.Empty; + scaffold.ParentId = parentId; + + var scaffoldedNotification = new ContentScaffoldedNotification(blueprint, scaffold, parentId, new EventMessages()); + if (scope.Notifications.PublishCancelable(scaffoldedNotification)) + { + scope.Complete(); + return Problem("Scaffolding was cancelled"); + } + + scope.Complete(); } - blueprint.Id = 0; - blueprint.Name = string.Empty; - blueprint.ParentId = parentId; - ContentItemDisplay? mapped = _umbracoMapper.Map(blueprint); + + ContentItemDisplay? mapped = _umbracoMapper.Map(scaffold); if (mapped is not null) { From 83a0bf4f4b8ad1f686869016ee2e2051a4810150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 10 Jan 2024 14:53:26 +0100 Subject: [PATCH 03/21] Fix: No Blocks Mode for RTE when required context for Blocks is not present (#15556) * noBlocksMode * another blockEditorApi check --- .../src/common/services/tinymce.service.js | 30 +++--- .../propertyeditors/rte/rte.component.js | 99 +++++++++++-------- 2 files changed, 74 insertions(+), 55 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js index 8f447e3f23..f8e87d2182 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js @@ -1439,6 +1439,10 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s function initBlocks() { + if(!args.blockEditorApi) { + return; + } + const blockEls = args.editor.contentDocument.querySelectorAll('umb-rte-block, umb-rte-block-inline'); for (var blockEl of blockEls) { if(!blockEl._isInitializedUmbBlock) { @@ -1743,19 +1747,21 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); - //Create the insert block plugin - self.createBlockPicker(args.editor, args.blockEditorApi, function (currentTarget, userData, imgDomElement) { - args.blockEditorApi.showCreateDialog(0, false, (newBlock) => { - // TODO: Handle if its an array: - if(Utilities.isArray(newBlock)) { - newBlock.forEach(block => { - self.insertBlockInEditor(args.editor, block.layout.contentUdi, block.config.displayInline); - }); - } else { - self.insertBlockInEditor(args.editor, newBlock.layout.contentUdi, newBlock.config.displayInline); - } + if(args.blockEditorApi) { + //Create the insert block plugin + self.createBlockPicker(args.editor, args.blockEditorApi, function (currentTarget, userData, imgDomElement) { + args.blockEditorApi.showCreateDialog(0, false, (newBlock) => { + // TODO: Handle if its an array: + if(Utilities.isArray(newBlock)) { + newBlock.forEach(block => { + self.insertBlockInEditor(args.editor, block.layout.contentUdi, block.config.displayInline); + }); + } else { + self.insertBlockInEditor(args.editor, newBlock.layout.contentUdi, newBlock.config.displayInline); + } + }); }); - }); + } //Create the embedded plugin self.createInsertEmbeddedMedia(args.editor, function (activeElement, modify) { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js index 2bb5b6c3a0..4e9cf7d014 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js @@ -43,6 +43,7 @@ var vm = this; vm.readonly = false; + vm.noBlocksMode = false; vm.tinyMceEditor = null; $attrs.$observe('readonly', (value) => { @@ -102,58 +103,69 @@ var found = angularHelper.traverseScopeChain($scope, s => s && s.vm && s.vm.constructor.name === "umbVariantContentController"); vm.umbVariantContent = found ? found.vm : null; if (!vm.umbVariantContent) { - throw "Could not find umbVariantContent in the $scope chain"; + //Could not find umbVariantContent in the $scope chain, lets go into no blocks mode: + vm.noBlocksMode = true; + vm.blocksLoading = false; + this.updateLoading(); } } + const config = vm.model.config || {}; + // set the onValueChanged callback, this will tell us if the block list model changed on the server // once the data is submitted. If so we need to re-initialize vm.model.onValueChanged = onServerValueChanged; - liveEditing = vm.model.config.useLiveEditing; + liveEditing = config.useLiveEditing; vm.listWrapperStyles = {}; - if (vm.model.config.maxPropertyWidth) { - vm.listWrapperStyles['max-width'] = vm.model.config.maxPropertyWidth; + if (config.maxPropertyWidth) { + vm.listWrapperStyles['max-width'] = config.maxPropertyWidth; } - // 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. + // We need to ensure that the property model value is an object, this is needed for modelObject to receive a reference and keep that updated. ensurePropertyValue(vm.model.value); - var scopeOfExistence = $scope; - if (vm.umbVariantContentEditors && vm.umbVariantContentEditors.getScope) { - scopeOfExistence = vm.umbVariantContentEditors.getScope(); - } else if(vm.umbElementEditorContent && vm.umbElementEditorContent.getScope) { - scopeOfExistence = vm.umbElementEditorContent.getScope(); + const assetPromises = []; + + if(vm.noBlocksMode !== true) { + + var scopeOfExistence = $scope; + if (vm.umbVariantContentEditors && vm.umbVariantContentEditors.getScope) { + scopeOfExistence = vm.umbVariantContentEditors.getScope(); + } else if(vm.umbElementEditorContent && vm.umbElementEditorContent.getScope) { + scopeOfExistence = vm.umbElementEditorContent.getScope(); + } + + /* + copyAllBlocksAction = { + labelKey: "clipboard_labelForCopyAllEntries", + labelTokens: [vm.model.label], + icon: "icon-documents", + method: requestCopyAllBlocks, + isDisabled: true, + useLegacyIcon: false + }; + + deleteAllBlocksAction = { + labelKey: "clipboard_labelForRemoveAllEntries", + labelTokens: [], + icon: "icon-trash", + method: requestDeleteAllBlocks, + isDisabled: true, + useLegacyIcon: false + }; + + var propertyActions = [copyAllBlocksAction, deleteAllBlocksAction]; + */ + + // Create Model Object, to manage our data for this Block Editor. + modelObject = blockEditorService.createModelObject(vm.model.value.blocks, vm.model.editor, config.blocks, scopeOfExistence, $scope); + const blockModelObjectLoading = modelObject.load(); + assetPromises.push(blockModelObjectLoading) + blockModelObjectLoading.then(onLoaded); } - /* - copyAllBlocksAction = { - labelKey: "clipboard_labelForCopyAllEntries", - labelTokens: [vm.model.label], - icon: "icon-documents", - method: requestCopyAllBlocks, - isDisabled: true, - useLegacyIcon: false - }; - - deleteAllBlocksAction = { - labelKey: "clipboard_labelForRemoveAllEntries", - labelTokens: [], - icon: "icon-trash", - method: requestDeleteAllBlocks, - isDisabled: true, - useLegacyIcon: false - }; - - var propertyActions = [copyAllBlocksAction, deleteAllBlocksAction]; - */ - - // Create Model Object, to manage our data for this Block Editor. - modelObject = blockEditorService.createModelObject(vm.model.value.blocks, vm.model.editor, vm.model.config.blocks, scopeOfExistence, $scope); - const blockModelObjectLoading = modelObject.load() - blockModelObjectLoading.then(onLoaded); - // ******************** // // RTE PART: @@ -165,19 +177,18 @@ // we have this mini content editor panel that can be launched with MNTP. vm.textAreaHtmlId = vm.model.alias + "_" + String.CreateGuid(); - var editorConfig = vm.model.config ? vm.model.config.editor : null; + var editorConfig = config.editor ?? null; if (!editorConfig || Utilities.isString(editorConfig)) { editorConfig = tinyMceService.defaultPrevalues(); } + var width = editorConfig.dimensions ? parseInt(editorConfig.dimensions.width, 10) || null : null; var height = editorConfig.dimensions ? parseInt(editorConfig.dimensions.height, 10) || null : null; vm.containerWidth = "auto"; vm.containerHeight = "auto"; - vm.containerOverflow = "inherit"; - - const assetPromises = [blockModelObjectLoading]; + vm.containerOverflow = "inherit" //queue file loading tinyMceAssets.forEach(function (tinyJsAsset) { @@ -256,7 +267,7 @@ }, culture: vm.umbProperty?.culture ?? null, segment: vm.umbProperty?.segment ?? null, - blockEditorApi: vm.blockEditorApi, + blockEditorApi: vm.noBlocksMode ? undefined : vm.blockEditorApi, parentForm: vm.propertyForm, valFormManager: vm.valFormManager, currentFormInput: $scope.rteForm.modelValue @@ -346,7 +357,9 @@ ensurePropertyValue(newVal); - modelObject.update(vm.model.value.blocks, $scope); + if(modelObject) { + modelObject.update(vm.model.value.blocks, $scope); + } onLoaded(); } From b916c85738f04d5ed026e19471fbc5dafd638daf Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:30:51 +0100 Subject: [PATCH 04/21] V13: TinyMCE does not toggle plugin buttons (#15558) * bump tinymce from 6.8.1 to 6.8.2 * replace stateSelector with appropriate onSetup() functionality and replace addButton() with addToggleButton() to support toggling --- src/Umbraco.Web.UI.Client/package-lock.json | 8 +-- src/Umbraco.Web.UI.Client/package.json | 2 +- .../src/common/services/tinymce.service.js | 72 +++++++++++++++---- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index 19f0e602af..043a0ec461 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -39,7 +39,7 @@ "ng-file-upload": "12.2.13", "nouislider": "15.7.1", "spectrum-colorpicker2": "2.0.10", - "tinymce": "6.8.1", + "tinymce": "6.8.2", "typeahead.js": "0.11.1", "underscore": "1.13.6", "wicg-inert": "3.1.2" @@ -16606,9 +16606,9 @@ "integrity": "sha512-NB6Dk1A9xgQPMoGqC5CVXn123gWyte215ONT5Pp5a0yt4nlEoO1ZWeCwpncaekPHXO60i47ihFnZPiRPjRMq4Q==" }, "node_modules/tinymce": { - "version": "6.8.1", - "resolved": "https://registry.npmjs.org/tinymce/-/tinymce-6.8.1.tgz", - "integrity": "sha512-WYPvMXIjBrXM/oBiqGCbT2a8ptiO3TWXm/xxPWDCl8SxRKMW7Rfp0Lk190E9fXmX6uh9lJMRCnmKHzvryz0ftA==" + "version": "6.8.2", + "resolved": "https://registry.npmjs.org/tinymce/-/tinymce-6.8.2.tgz", + "integrity": "sha512-Lho79o2Y1Yn+XdlTEkHTEkEmzwYWTXz7IUsvPwxJF3VTtgHUIAAuBab29kik+f2KED3rZvQavr9D7sHVMJ9x4A==" }, "node_modules/to-absolute-glob": { "version": "2.0.2", diff --git a/src/Umbraco.Web.UI.Client/package.json b/src/Umbraco.Web.UI.Client/package.json index f23a0ab750..ec8ec22307 100644 --- a/src/Umbraco.Web.UI.Client/package.json +++ b/src/Umbraco.Web.UI.Client/package.json @@ -51,7 +51,7 @@ "ng-file-upload": "12.2.13", "nouislider": "15.7.1", "spectrum-colorpicker2": "2.0.10", - "tinymce": "6.8.1", + "tinymce": "6.8.2", "typeahead.js": "0.11.1", "underscore": "1.13.6", "wicg-inert": "3.1.2" diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js index f8e87d2182..cfea1d5594 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js @@ -543,10 +543,15 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s * @param {Object} editor the TinyMCE editor instance */ createInsertEmbeddedMedia: function (editor, callback) { - editor.ui.registry.addButton('umbembeddialog', { + editor.ui.registry.addToggleButton('umbembeddialog', { icon: 'embed', tooltip: 'Embed', - stateSelector: 'div[data-embed-url]', + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('div[data-embed-url]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + }, onAction: function () { // Get the selected element @@ -555,6 +560,12 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s var nodeName = selectedElm.nodeName; var modify = null; + // If we have an iframe, we need to get the parent element + if (nodeName.toUpperCase() === "IFRAME") { + selectedElm = selectedElm.parentElement; + nodeName = selectedElm.nodeName; + } + if (nodeName.toUpperCase() === "DIV" && selectedElm.classList.contains("embeditem")) { // See if we can go and get the attributes var embedUrl = editor.dom.getAttrib(selectedElm, "data-embed-url"); @@ -630,10 +641,15 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s * @param {Object} editor the TinyMCE editor instance */ createMediaPicker: function (editor, callback) { - editor.ui.registry.addButton('umbmediapicker', { + editor.ui.registry.addToggleButton('umbmediapicker', { icon: 'image', tooltip: 'Image Picker', - stateSelector: 'img[data-udi]', + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('img[data-udi]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + }, onAction: function () { var selectedElm = editor.selection.getNode(), @@ -781,10 +797,15 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); }); - editor.ui.registry.addButton('umbblockpicker', { + editor.ui.registry.addToggleButton('umbblockpicker', { icon: 'visualblocks', tooltip: 'Insert Block', - stateSelector: 'umb-rte-block[data-content-udi], umb-rte-block-inline[data-content-udi]', + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('umb-rte-block[data-content-udi], umb-rte-block-inline[data-content-udi]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + }, onAction: function () { var blockEl = editor.selection.getNode(); @@ -900,9 +921,15 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s } /** Adds the button instance */ - editor.ui.registry.addButton('umbmacro', { + editor.ui.registry.addToggleButton('umbmacro', { icon: 'preferences', tooltip: 'Insert macro', + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('div.umb-macro-holder', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + }, /** The insert macro button click event handler */ onAction: function () { @@ -1207,32 +1234,47 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); } - editor.ui.registry.addButton('link', { + editor.ui.registry.addToggleButton('link', { icon: 'link', tooltip: 'Insert/edit link', shortcut: 'Ctrl+K', onAction: createLinkList(showDialog), - stateSelector: 'a[href]' + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('a[href]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + } }); - editor.ui.registry.addButton('unlink', { + editor.ui.registry.addToggleButton('unlink', { icon: 'unlink', tooltip: 'Remove link', onAction: () => { editor.execCommand('unlink'); }, - stateSelector: 'a[href]' + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('a[href]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + } }); editor.addShortcut('Ctrl+K', '', createLinkList(showDialog)); this.showDialog = showDialog; - editor.ui.registry.addMenuItem('link', { + editor.ui.registry.addToggleMenuItem('link', { icon: 'link', text: 'Insert link', shortcut: 'Ctrl+K', onAction: createLinkList(showDialog), - stateSelector: 'a[href]', + onSetup: function (api) { + const changed = editor.selection.selectorChangedWithUnbind('a[href]', (state) => + api.setActive(state) + ); + return () => changed.unbind(); + }, context: 'insert', prependToContext: true }); @@ -1712,7 +1754,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); //Create the insert media plugin - self.createMediaPicker(args.editor, function (currentTarget, userData, imgDomElement) { + self.createMediaPicker(args.editor, function (currentTarget, userData) { var startNodeId, startNodeIsVirtual; if (!args.model.config.startNodeId) { @@ -1736,7 +1778,7 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s startNodeIsVirtual: startNodeIsVirtual, dataTypeKey: args.model.dataTypeKey, submit: function (model) { - self.insertMediaInEditor(args.editor, model.selection[0], imgDomElement); + self.insertMediaInEditor(args.editor, model.selection[0]); editorService.close(); }, close: function () { From d3dc85f31fe632d7f8c64e7ccafba43f8287a850 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 11 Jan 2024 10:20:58 +0100 Subject: [PATCH 05/21] Only update security stamp once per request (#15562) * Add item in requestcache when security stamp is already updated in request * Propagate constructur obsoletion to implementing services and fix unit tests --------- Co-authored-by: kjac --- .../Security/BackOfficeSignInManager.cs | 35 +++++++++++++++- .../Security/MemberSignInManager.cs | 33 ++++++++++++++- .../Security/UmbracoSignInManager.cs | 40 +++++++++++++++++-- .../Security/MemberSignInManagerTests.cs | 4 +- 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs index 8af883135d..7448a303a0 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; @@ -37,8 +38,9 @@ public class BackOfficeSignInManager : UmbracoSignInManager confirmation, IEventAggregator eventAggregator, - IOptions securitySettings) - : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings) + IOptions securitySettings, + IRequestCache requestCache) + : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings, requestCache) { _userManager = userManager; _externalLogins = externalLogins; @@ -46,6 +48,35 @@ public class BackOfficeSignInManager : UmbracoSignInManager claimsFactory, + IOptions optionsAccessor, + IOptions globalSettings, + ILogger> logger, + IAuthenticationSchemeProvider schemes, + IUserConfirmation confirmation, + IEventAggregator eventAggregator, + IOptions securitySettings) + : this( + userManager, + contextAccessor, + externalLogins, + claimsFactory, + optionsAccessor, + globalSettings, + logger, + schemes, + confirmation, + eventAggregator, + securitySettings, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + [Obsolete("Use non-obsolete constructor. This is scheduled for removal in V14.")] public BackOfficeSignInManager( BackOfficeUserManager userManager, diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index 9a8aaa72f4..1139c04270 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; @@ -32,13 +33,41 @@ public class MemberSignInManager : UmbracoSignInManager, IMe IUserConfirmation confirmation, IMemberExternalLoginProviders memberExternalLoginProviders, IEventAggregator eventAggregator, - IOptions securitySettings) - : base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings) + IOptions securitySettings, + IRequestCache requestCache) + : base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings, requestCache) { _memberExternalLoginProviders = memberExternalLoginProviders; _eventAggregator = eventAggregator; } + [Obsolete("Use non-obsolete constructor. This is scheduled for removal in V15.")] + public MemberSignInManager( + UserManager memberManager, + IHttpContextAccessor contextAccessor, + IUserClaimsPrincipalFactory claimsFactory, + IOptions optionsAccessor, + ILogger> logger, + IAuthenticationSchemeProvider schemes, + IUserConfirmation confirmation, + IMemberExternalLoginProviders memberExternalLoginProviders, + IEventAggregator eventAggregator, + IOptions securitySettings) + : this( + memberManager, + contextAccessor, + claimsFactory, + optionsAccessor, + logger, + schemes, + confirmation, + memberExternalLoginProviders, + eventAggregator, + securitySettings, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + [Obsolete("Use non-obsolete constructor. This is scheduled for removal in V14.")] public MemberSignInManager( UserManager memberManager, diff --git a/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs index 84cbce6d8d..f52db46241 100644 --- a/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Security; @@ -19,6 +20,7 @@ namespace Umbraco.Cms.Web.Common.Security; public abstract class UmbracoSignInManager : SignInManager where TUser : UmbracoIdentityUser { + private readonly IRequestCache _requestCache; private SecuritySettings _securitySettings; // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs @@ -44,7 +46,31 @@ public abstract class UmbracoSignInManager : SignInManager logger, schemes, confirmation, - StaticServiceProvider.Instance.GetRequiredService>()) + StaticServiceProvider.Instance.GetRequiredService>(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Use non-obsolete constructor. This is scheduled for removal in V15.")] + public UmbracoSignInManager( + UserManager userManager, + IHttpContextAccessor contextAccessor, + IUserClaimsPrincipalFactory claimsFactory, + IOptions optionsAccessor, + ILogger> logger, + IAuthenticationSchemeProvider schemes, + IUserConfirmation confirmation, + IOptions securitySettingsOptions) + : this( + userManager, + contextAccessor, + claimsFactory, + optionsAccessor, + logger, + schemes, + confirmation, + securitySettingsOptions, + StaticServiceProvider.Instance.GetRequiredService()) { } @@ -56,9 +82,11 @@ public abstract class UmbracoSignInManager : SignInManager ILogger> logger, IAuthenticationSchemeProvider schemes, IUserConfirmation confirmation, - IOptions securitySettingsOptions) + IOptions securitySettingsOptions, + IRequestCache requestCache) : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) { + _requestCache = requestCache; _securitySettings = securitySettingsOptions.Value; } @@ -370,7 +398,13 @@ public abstract class UmbracoSignInManager : SignInManager if (_securitySettings.AllowConcurrentLogins is false) { - await UserManager.UpdateSecurityStampAsync(user); + + if (_requestCache.Get("SecurityStampUpdated") is null) + { + await UserManager.UpdateSecurityStampAsync(user); + _requestCache.Set("SecurityStampUpdated", true); + } + } Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 5394de5fd7..b80555bd43 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -10,6 +10,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Net; @@ -71,7 +72,8 @@ public class MemberSignInManagerTests Mock.Of>(), Mock.Of(), Mock.Of(), - Mock.Of>(x => x.Value == new SecuritySettings())); + Mock.Of>(x => x.Value == new SecuritySettings()), + new DictionaryAppCache()); } private static Mock MockMemberManager() From 57b3a196bf920e380b274746fbb4c17f10a55b30 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Thu, 11 Jan 2024 12:46:31 +0100 Subject: [PATCH 06/21] V10: Pass in variation context to published cache (#15563) * Make sure that we always have variation context * Fix references --- .../PublishedCache/PublishedCacheBase.cs | 22 ++++++++++++++++--- .../ContentCache.cs | 2 +- .../MediaCache.cs | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs index 3e961ce434..ffb2ef4278 100644 --- a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs +++ b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs @@ -1,6 +1,8 @@ using System.Xml.XPath; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Xml; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Core.PublishedCache; @@ -9,10 +11,24 @@ public abstract class PublishedCacheBase : IPublishedCache { private readonly IVariationContextAccessor? _variationContextAccessor; - public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) => _variationContextAccessor = - variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); - protected PublishedCacheBase(bool previewDefault) => PreviewDefault = previewDefault; + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) + : this(variationContextAccessor, false) + { + } + + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + protected PublishedCacheBase(bool previewDefault) + : this(StaticServiceProvider.Instance.GetRequiredService(), previewDefault) + { + } + + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor, bool previewDefault) + { + _variationContextAccessor = variationContextAccessor; + PreviewDefault = previewDefault; + } public bool PreviewDefault { get; } diff --git a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs index d8a5c0bc04..1686f6aacf 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs @@ -36,7 +36,7 @@ public class ContentCache : PublishedCacheBase, IPublishedContentCache, INavigab IDomainCache domainCache, IOptions globalSettings, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _snapshotCache = snapshotCache; diff --git a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs index 626e2fe36c..68c23c5a34 100644 --- a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs @@ -17,7 +17,7 @@ public class MediaCache : PublishedCacheBase, IPublishedMediaCache, INavigableDa #region Constructors public MediaCache(bool previewDefault, ContentStore.Snapshot snapshot, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _variationContextAccessor = variationContextAccessor; From 597d8553c0e113e7ee832b031ee500fe548a2f7a Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Thu, 11 Jan 2024 12:49:33 +0100 Subject: [PATCH 07/21] bump version.json --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index 73318d49b0..15469ada83 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "13.1.0-rc", + "version": "13.1.0-rc2", "assemblyVersion": { "precision": "build" }, From 49f5d2e2d42e71838ca6d055ced6d53a4df41ffe Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 12 Jan 2024 13:13:20 +0100 Subject: [PATCH 08/21] Updates JSON schema for Umbraco 10 to include details of additional configuration introduced in Forms and Deploy. (#15566) --- src/JsonSchema/JsonSchema.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/JsonSchema/JsonSchema.csproj b/src/JsonSchema/JsonSchema.csproj index 662c35ecb5..9ef072ccdf 100644 --- a/src/JsonSchema/JsonSchema.csproj +++ b/src/JsonSchema/JsonSchema.csproj @@ -13,7 +13,7 @@ - - + + From a70c606c270549a08b4b7493f27515a45cb4d36e Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 15 Jan 2024 14:14:27 +0100 Subject: [PATCH 09/21] Do not log direct query inputs (#15577) --- .../Services/ApiMediaQueryService.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiMediaQueryService.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiMediaQueryService.cs index 1fe1e92d9b..5969e0a788 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/ApiMediaQueryService.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiMediaQueryService.cs @@ -113,7 +113,7 @@ internal sealed class ApiMediaQueryService : IApiMediaQueryService if (parts.Length != 2) { // invalid filter - _logger.LogInformation($"The \"{nameof(filters)}\" query option \"{filter}\" is not valid"); + _logger.LogInformation("An invalid filter option was encountered. Please ensure that supplied filter options are two-part, separated by ':'."); return null; } @@ -127,7 +127,7 @@ internal sealed class ApiMediaQueryService : IApiMediaQueryService break; default: // unknown filter - _logger.LogInformation($"The \"{nameof(filters)}\" query option \"{filter}\" is not supported"); + _logger.LogInformation("An unsupported filter option was supplied for the query. Please use only valid filter options. See the documentation for details."); return null; } } @@ -143,7 +143,7 @@ internal sealed class ApiMediaQueryService : IApiMediaQueryService if (parts.Length != 2) { // invalid sort - _logger.LogInformation($"The \"{nameof(sorts)}\" query option \"{sort}\" is not valid"); + _logger.LogInformation("An invalid sort option was encountered. Please ensure that the supplied sort options are two-part, separated by ':'."); return null; } @@ -164,7 +164,7 @@ internal sealed class ApiMediaQueryService : IApiMediaQueryService break; default: // unknown sort - _logger.LogInformation($"The \"{nameof(sorts)}\" query option \"{sort}\" is not supported"); + _logger.LogInformation("An unsupported sort option was supplied for the query. Please use only valid sort options. See the documentation for details."); return null; } From 14299b66748a1bd2388c070714ba11124c6eac64 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:10:14 +0100 Subject: [PATCH 10/21] bump version to 13.2.0-rc --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index 15469ada83..43d283b98f 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "13.1.0-rc2", + "version": "13.2.0-rc", "assemblyVersion": { "precision": "build" }, From 9196d7ad00829ac7adc43502409c9a7c3dcb2dcf Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 18 Jan 2024 13:19:36 +0100 Subject: [PATCH 11/21] V13: Rich text editor does not show its toolbar on grid layout rte's (#15595) * synchronize normal rte with grid-rte * restore pinToolbar and unpinToolbar from v10 and update to tinymce v6 and apply to grid-rte * linting * Reverting `pinToolbar` from v8 * remove unused variable --------- Co-authored-by: leekelleher --- .../components/grid/grid.rte.directive.js | 141 ++++----- .../src/common/services/tinymce.service.js | 5 + .../propertyeditors/grid/editors/rte.html | 4 +- .../propertyeditors/rte/rte.component.js | 273 +++++++++--------- 4 files changed, 214 insertions(+), 209 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/grid/grid.rte.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/grid/grid.rte.directive.js index 812fec6e9c..7821c00e69 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/grid/grid.rte.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/grid/grid.rte.directive.js @@ -12,22 +12,18 @@ angular.module("umbraco.directives") replace: true, link: function (scope, element, attrs) { - // TODO: A lot of the code below should be shared between the grid rte and the normal rte - scope.isLoading = true; - var promises = []; - //To id the html textarea we need to use the datetime ticks because we can have multiple rte's per a single property alias // because now we have to support having 2x (maybe more at some stage) content editors being displayed at once. This is because // we have this mini content editor panel that can be launched with MNTP. scope.textAreaHtmlId = scope.uniqueId + "_" + String.CreateGuid(); - var editorConfig = scope.configuration ? scope.configuration : null; + let editorConfig = scope.configuration ? scope.configuration : null; if (!editorConfig || Utilities.isString(editorConfig)) { editorConfig = tinyMceService.defaultPrevalues(); //for the grid by default, we don't want to include the macro or the block-picker toolbar - editorConfig.toolbar = _.without(editorConfig, "umbmacro", "umbblockpicker"); + editorConfig.toolbar = _.without(editorConfig.toolbar, "umbmacro", "umbblockpicker"); } //ensure the grid's global config is being passed up to the RTE, these 2 properties need to be in this format @@ -39,46 +35,50 @@ angular.module("umbraco.directives") scope.dataTypeKey = scope.datatypeKey; //Yes - this casing is rediculous, but it's because the var starts with `data` so it can't be `data-type-id` :/ //stores a reference to the editor - var tinyMceEditor = null; + let tinyMceEditor = null; + + const assetPromises = []; //queue file loading tinyMceAssets.forEach(function (tinyJsAsset) { - promises.push(assetsService.loadJs(tinyJsAsset, scope)); + assetPromises.push(assetsService.loadJs(tinyJsAsset, scope)); }); - promises.push(tinyMceService.getTinyMceEditorConfig({ - htmlId: scope.textAreaHtmlId, - stylesheets: editorConfig.stylesheets, - toolbar: editorConfig.toolbar, - mode: editorConfig.mode - })); - $q.all(promises).then(function (result) { + //wait for assets to load before proceeding + $q.all(assetPromises) + .then(function () { + return tinyMceService.getTinyMceEditorConfig({ + htmlId: scope.textAreaHtmlId, + stylesheets: editorConfig.stylesheets, + toolbar: editorConfig.toolbar, + mode: editorConfig.mode + }) + }) - var standardConfig = result[promises.length - 1]; + // Handle additional assets loading depending on the configuration before initializing the editor + .then(function (tinyMceConfig) { + // Load the plugins.min.js file from the TinyMCE Cloud if a Cloud Api Key is specified + if (tinyMceConfig.cloudApiKey) { + return assetsService.loadJs(`https://cdn.tiny.cloud/1/${tinyMceConfig.cloudApiKey}/tinymce/${tinymce.majorVersion}.${tinymce.minorVersion}/plugins.min.js`) + .then(() => tinyMceConfig); + } + + return tinyMceConfig; + }) + + //wait for config to be ready after assets have loaded + .then(function (standardConfig) { //create a baseline Config to extend upon - var baseLineConfigObj = { - maxImageSize: editorConfig.maxImageSize, - toolbar_sticky: true + let baseLineConfigObj = { + maxImageSize: editorConfig.maxImageSize }; - Utilities.extend(baseLineConfigObj, standardConfig); - baseLineConfigObj.setup = function (editor) { //set the reference tinyMceEditor = editor; - //initialize the standard editor functionality for Umbraco - tinyMceService.initializeEditor({ - editor: editor, - toolbar: editorConfig.toolbar, - model: scope, - // Form is found in the scope of the grid controller above us, not in our isolated scope - // https://github.com/umbraco/Umbraco-CMS/issues/7461 - currentForm: angularHelper.getCurrentForm(scope.$parent) - }); - //custom initialization for this editor within the grid editor.on('init', function (e) { @@ -96,49 +96,52 @@ angular.module("umbraco.directives") }, 400); }); + + //initialize the standard editor functionality for Umbraco + tinyMceService.initializeEditor({ + editor: editor, + toolbar: editorConfig.toolbar, + model: scope, + // Form is found in the scope of the grid controller above us, not in our isolated scope + // https://github.com/umbraco/Umbraco-CMS/issues/7461 + currentForm: angularHelper.getCurrentForm(scope.$parent) + }); }; - /** Loads in the editor */ - function loadTinyMce() { - - //we need to add a timeout here, to force a redraw so TinyMCE can find - //the elements needed - $timeout(function () { - tinymce.init(baseLineConfigObj); - }, 150, false); - } - - loadTinyMce(); - - // TODO: This should probably be in place for all RTE, not just for the grid, which means - // this code can live in tinyMceService.initializeEditor - var tabShownListener = eventsService.on("app.tabChange", function (e, args) { - - var tabId = args.id; - var myTabId = element.closest(".umb-tab-pane").attr("rel"); - - if (String(tabId) === myTabId) { - //the tab has been shown, trigger the mceAutoResize (as it could have timed out before the tab was shown) - if (tinyMceEditor !== undefined && tinyMceEditor != null) { - tinyMceEditor.execCommand('mceAutoResize', false, null, null); - } - } - - }); - - //when the element is disposed we need to unsubscribe! - // NOTE: this is very important otherwise if this is part of a modal, the listener still exists because the dom - // element might still be there even after the modal has been hidden. - scope.$on('$destroy', function () { - eventsService.unsubscribe(tabShownListener); - //ensure we unbind this in case the blur doesn't fire above - if (tinyMceEditor !== undefined && tinyMceEditor != null) { - tinyMceEditor.destroy() - } - }); + Utilities.extend(baseLineConfigObj, standardConfig); + //we need to add a timeout here, to force a redraw so TinyMCE can find + //the elements needed + $timeout(function () { + tinymce.init(baseLineConfigObj); + }, 150); }); + const tabShownListener = eventsService.on("app.tabChange", function (e, args) { + + const tabId = String(args.id); + const myTabId = element.closest(".umb-tab-pane").attr("rel"); + + if (tabId === myTabId) { + //the tab has been shown, trigger the mceAutoResize (as it could have timed out before the tab was shown) + if (tinyMceEditor !== undefined && tinyMceEditor != null) { + tinyMceEditor.execCommand('mceAutoResize', false, null, null); + } + } + }); + + //when the element is disposed we need to unsubscribe! + // NOTE: this is very important otherwise if this is part of a modal, the listener still exists because the dom + // element might still be there even after the modal has been hidden. + scope.$on('$destroy', function () { + eventsService.unsubscribe(tabShownListener); + + //ensure we unbind this in case the blur doesn't fire above + if (tinyMceEditor) { + tinyMceEditor.destroy(); + tinyMceEditor = null; + } + }); } }; }); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js index cfea1d5594..ebf759c20c 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js @@ -797,6 +797,11 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s }); }); + // Do not add any further controls if the block editor is not available + if (!blockEditorApi) { + return; + } + editor.ui.registry.addToggleButton('umbblockpicker', { icon: 'visualblocks', tooltip: 'Insert Block', diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/rte.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/rte.html index fb6f3b2a4b..2fbd0b2fe3 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/rte.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/rte.html @@ -4,8 +4,8 @@ configuration="model.config.rte" value="control.value" unique-id="control.$uniqueId" - datatype-key="{{model.dataTypeKey}}" - ignore-user-start-nodes="{{model.config.ignoreUserStartNodes}}"> + datatype-key="{{model.dataTypeKey}}" + ignore-user-start-nodes="{{model.config.ignoreUserStartNodes}}"> diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js index 4e9cf7d014..7f06215148 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/rte/rte.component.js @@ -195,162 +195,148 @@ assetPromises.push(assetsService.loadJs(tinyJsAsset, $scope)); }); - const tinyMceConfigDeferred = $q.defer(); - //wait for assets to load before proceeding - $q.all(assetPromises).then(function () { - - tinyMceService.getTinyMceEditorConfig({ - htmlId: vm.textAreaHtmlId, - stylesheets: editorConfig.stylesheets, - toolbar: editorConfig.toolbar, - mode: editorConfig.mode + $q.all(assetPromises) + .then(function () { + return tinyMceService.getTinyMceEditorConfig({ + htmlId: vm.textAreaHtmlId, + stylesheets: editorConfig.stylesheets, + toolbar: editorConfig.toolbar, + mode: editorConfig.mode + }) }) - .then(function (tinyMceConfig) { - // Load the plugins.min.js file from the TinyMCE Cloud if a Cloud Api Key is specified - if (tinyMceConfig.cloudApiKey) { - return assetsService.loadJs(`https://cdn.tiny.cloud/1/${tinyMceConfig.cloudApiKey}/tinymce/${tinymce.majorVersion}.${tinymce.minorVersion}/plugins.min.js`) - .then(() => tinyMceConfig); + + // Handle additional assets loading depending on the configuration before initializing the editor + .then(function (tinyMceConfig) { + // Load the plugins.min.js file from the TinyMCE Cloud if a Cloud Api Key is specified + if (tinyMceConfig.cloudApiKey) { + return assetsService.loadJs(`https://cdn.tiny.cloud/1/${tinyMceConfig.cloudApiKey}/tinymce/${tinymce.majorVersion}.${tinymce.minorVersion}/plugins.min.js`) + .then(() => tinyMceConfig); + } + + return tinyMceConfig; + }) + + //wait for config to be ready after assets have loaded + .then(function (standardConfig) { + + if (height !== null) { + standardConfig.plugins.splice(standardConfig.plugins.indexOf("autoresize"), 1); } - return tinyMceConfig; - }) - .then(function (tinyMceConfig) { - tinyMceConfigDeferred.resolve(tinyMceConfig); - }); - }); + //create a baseline Config to extend upon + let baseLineConfigObj = { + maxImageSize: editorConfig.maxImageSize, + width: width, + height: height + }; - //wait for config to be ready after assets have loaded - tinyMceConfigDeferred.promise.then(function (standardConfig) { + baseLineConfigObj.setup = function (editor) { - if (height !== null) { - standardConfig.plugins.splice(standardConfig.plugins.indexOf("autoresize"), 1); - } + //set the reference + vm.tinyMceEditor = editor; - //create a baseline Config to extend upon - var baseLineConfigObj = { - maxImageSize: editorConfig.maxImageSize, - width: width, - height: height - }; - - baseLineConfigObj.setup = function (editor) { - - //set the reference - vm.tinyMceEditor = editor; - - vm.tinyMceEditor.on('init', function (e) { - $timeout(function () { - vm.rteLoading = false; - vm.updateLoading(); - }); - }); - vm.tinyMceEditor.on("focus", function () { - $element[0].dispatchEvent(new CustomEvent('umb-rte-focus', {composed: true, bubbles: true})); - }); - vm.tinyMceEditor.on("blur", function () { - $element[0].dispatchEvent(new CustomEvent('umb-rte-blur', {composed: true, bubbles: true})); - }); - - //initialize the standard editor functionality for Umbraco - tinyMceService.initializeEditor({ - //scope: $scope, - editor: editor, - toolbar: editorConfig.toolbar, - model: vm.model, - getValue: function () { - return vm.model.value.markup; - }, - setValue: function (newVal) { - vm.model.value.markup = newVal; - $scope.$evalAsync(); - }, - culture: vm.umbProperty?.culture ?? null, - segment: vm.umbProperty?.segment ?? null, - blockEditorApi: vm.noBlocksMode ? undefined : vm.blockEditorApi, - parentForm: vm.propertyForm, - valFormManager: vm.valFormManager, - currentFormInput: $scope.rteForm.modelValue - }); - - }; - - Utilities.extend(baseLineConfigObj, standardConfig); - - // Readonly mode - baseLineConfigObj.toolbar = vm.readonly ? false : baseLineConfigObj.toolbar; - baseLineConfigObj.readonly = vm.readonly ? 1 : baseLineConfigObj.readonly; - - // We need to wait for DOM to have rendered before we can find the element by ID. - $timeout(function () { - tinymce.init(baseLineConfigObj); - }, 50); - - //listen for formSubmitting event (the result is callback used to remove the event subscription) - unsubscribe.push($scope.$on("formSubmitting", function () { - if (vm.tinyMceEditor != null && !vm.rteLoading) { - - // Remove unused Blocks of Blocks Layout. Leaving only the Blocks that are present in Markup. - var blockElements = vm.tinyMceEditor.dom.select(`umb-rte-block, umb-rte-block-inline`); - const usedContentUdis = blockElements.map(blockElement => blockElement.getAttribute('data-content-udi')); - - const unusedBlocks = vm.layout.filter(x => usedContentUdis.indexOf(x.contentUdi) === -1); - unusedBlocks.forEach(blockLayout => { - deleteBlock(blockLayout.$block); - }); - - - // Remove Angular Classes from markup: - var parser = new DOMParser(); - var doc = parser.parseFromString(vm.model.value.markup, 'text/html'); - - // Get all elements in the parsed document - var elements = doc.querySelectorAll('*[class]'); - elements.forEach(element => { - var classAttribute = element.getAttribute("class"); - if (classAttribute) { - // Split the class attribute by spaces and remove "ng-scope" and "ng-isolate-scope" - var classes = classAttribute.split(" "); - var newClasses = classes.filter(function (className) { - return className !== "ng-scope" && className !== "ng-isolate-scope"; + vm.tinyMceEditor.on('init', function (e) { + $timeout(function () { + vm.rteLoading = false; + vm.updateLoading(); }); - - // Update the class attribute with the remaining classes - if (newClasses.length > 0) { - element.setAttribute('class', newClasses.join(' ')); - } else { - // If no remaining classes, remove the class attribute - element.removeAttribute('class'); - } - } + }); + vm.tinyMceEditor.on("focus", function () { + $element[0].dispatchEvent(new CustomEvent('umb-rte-focus', {composed: true, bubbles: true})); + }); + vm.tinyMceEditor.on("blur", function () { + $element[0].dispatchEvent(new CustomEvent('umb-rte-blur', {composed: true, bubbles: true})); }); - vm.model.value.markup = doc.body.innerHTML; + //initialize the standard editor functionality for Umbraco + tinyMceService.initializeEditor({ + //scope: $scope, + editor: editor, + toolbar: editorConfig.toolbar, + model: vm.model, + getValue: function () { + return vm.model.value.markup; + }, + setValue: function (newVal) { + vm.model.value.markup = newVal; + $scope.$evalAsync(); + }, + culture: vm.umbProperty?.culture ?? null, + segment: vm.umbProperty?.segment ?? null, + blockEditorApi: vm.noBlocksMode ? undefined : vm.blockEditorApi, + parentForm: vm.propertyForm, + valFormManager: vm.valFormManager, + currentFormInput: $scope.rteForm.modelValue + }); - } - })); + }; - vm.focusRTE = function () { - vm.tinyMceEditor.focus(); - } + Utilities.extend(baseLineConfigObj, standardConfig); + + // Readonly mode + baseLineConfigObj.toolbar = vm.readonly ? false : baseLineConfigObj.toolbar; + baseLineConfigObj.readonly = vm.readonly ? 1 : baseLineConfigObj.readonly; + + // We need to wait for DOM to have rendered before we can find the element by ID. + $timeout(function () { + tinymce.init(baseLineConfigObj); + }, 50); + + //listen for formSubmitting event (the result is callback used to remove the event subscription) + unsubscribe.push($scope.$on("formSubmitting", function () { + if (vm.tinyMceEditor != null && !vm.rteLoading) { + + // Remove unused Blocks of Blocks Layout. Leaving only the Blocks that are present in Markup. + var blockElements = vm.tinyMceEditor.dom.select(`umb-rte-block, umb-rte-block-inline`); + const usedContentUdis = blockElements.map(blockElement => blockElement.getAttribute('data-content-udi')); + + const unusedBlocks = vm.layout.filter(x => usedContentUdis.indexOf(x.contentUdi) === -1); + unusedBlocks.forEach(blockLayout => { + deleteBlock(blockLayout.$block); + }); + + + // Remove Angular Classes from markup: + var parser = new DOMParser(); + var doc = parser.parseFromString(vm.model.value.markup, 'text/html'); + + // Get all elements in the parsed document + var elements = doc.querySelectorAll('*[class]'); + elements.forEach(element => { + var classAttribute = element.getAttribute("class"); + if (classAttribute) { + // Split the class attribute by spaces and remove "ng-scope" and "ng-isolate-scope" + var classes = classAttribute.split(" "); + var newClasses = classes.filter(function (className) { + return className !== "ng-scope" && className !== "ng-isolate-scope"; + }); + + // Update the class attribute with the remaining classes + if (newClasses.length > 0) { + element.setAttribute('class', newClasses.join(' ')); + } else { + // If no remaining classes, remove the class attribute + element.removeAttribute('class'); + } + } + }); + + vm.model.value.markup = doc.body.innerHTML; - // When the element is disposed we need to unsubscribe! - // NOTE: this is very important otherwise if this is part of a modal, the listener still exists because the dom - // element might still be there even after the modal has been hidden. - $scope.$on('$destroy', function () { - if (vm.tinyMceEditor != null) { - if($element) { - $element[0]?.dispatchEvent(new CustomEvent('blur', {composed: true, bubbles: true})); } - vm.tinyMceEditor.destroy(); - vm.tinyMceEditor = null; - } - }); + })); - }); + }); }; + vm.focusRTE = function () { + if (vm.tinyMceEditor) { + vm.tinyMceEditor.focus(); + } + } + // 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) { @@ -978,6 +964,17 @@ for (const subscription of unsubscribe) { subscription(); } + + // When the element is disposed we need to unsubscribe! + // NOTE: this is very important otherwise if this is part of a modal, the listener still exists because the dom + // element might still be there even after the modal has been hidden. + if (vm.tinyMceEditor != null) { + if($element) { + $element[0]?.dispatchEvent(new CustomEvent('blur', {composed: true, bubbles: true})); + } + vm.tinyMceEditor.destroy(); + vm.tinyMceEditor = null; + } }); } From bc803ae78b4aa71b847adfdf4d88bf7c0fb824ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Kottal?= Date: Fri, 19 Jan 2024 13:21:17 +0100 Subject: [PATCH 12/21] Fix RTE block rendering failing (#15583) Adds optional class attribute to regex responsible for parsing RTE values to block models --- .../PropertyEditors/ValueConverters/RichTextParsingRegexes.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RichTextParsingRegexes.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RichTextParsingRegexes.cs index c7fa4e4592..75851ee856 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RichTextParsingRegexes.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RichTextParsingRegexes.cs @@ -4,6 +4,6 @@ namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; internal static partial class RichTextParsingRegexes { - [GeneratedRegex(".[^\"]*)\"><\\/umb-rte-block(?:-inline)?>")] + [GeneratedRegex(".[^\"]*)\"><\\/umb-rte-block(?:-inline)?>")] public static partial Regex BlockRegex(); } From 5198e7c52d2a9fafb321ee4b8e79783580c0635b Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Fri, 19 Jan 2024 20:02:57 +0100 Subject: [PATCH 13/21] Backport relation tracking fixes and get references from recursive (nested/block) properties (#15593) * Include automatic relation type aliases from factory and fix SQL parameter overflow (#15141) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations (#15160) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations * Always get all automatic relation type aliases * Do not set relation type alias for unknown entity types * Get references from recursive (nested/block) properties --- .../Composing/BuilderCollectionBase.cs | 18 +- .../Composing/IBuilderCollection.cs | 7 +- .../Models/Editors/UmbracoEntityReference.cs | 57 +++++- .../DataValueReferenceFactoryCollection.cs | 174 +++++++++++++----- .../Implement/ContentRepositoryBase.cs | 102 ++++------ .../BlockEditorPropertyValueEditor.cs | 85 ++++----- .../BlockGridPropertyEditorBase.cs | 3 +- .../BlockListPropertyEditorBase.cs | 3 +- .../NestedContentPropertyEditor.cs | 72 +++----- .../DataValueEditorReuseTests.cs | 8 +- ...ataValueReferenceFactoryCollectionTests.cs | 36 +++- 11 files changed, 337 insertions(+), 228 deletions(-) diff --git a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs index ffacd89cff..e92790ab4e 100644 --- a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs +++ b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs @@ -3,30 +3,26 @@ using System.Collections; namespace Umbraco.Cms.Core.Composing; /// -/// Provides a base class for builder collections. +/// Provides a base class for builder collections. /// /// The type of the items. public abstract class BuilderCollectionBase : IBuilderCollection { private readonly LazyReadOnlyCollection _items; - /// Initializes a new instance of the - /// - /// with items. + /// + /// Initializes a new instance of the with items. /// /// The items. - public BuilderCollectionBase(Func> items) => _items = new LazyReadOnlyCollection(items); + public BuilderCollectionBase(Func> items) + => _items = new LazyReadOnlyCollection(items); /// public int Count => _items.Count; - /// - /// Gets an enumerator. - /// + /// public IEnumerator GetEnumerator() => _items.GetEnumerator(); - /// - /// Gets an enumerator. - /// + /// IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/src/Umbraco.Core/Composing/IBuilderCollection.cs b/src/Umbraco.Core/Composing/IBuilderCollection.cs index 56036997bc..c8920149c5 100644 --- a/src/Umbraco.Core/Composing/IBuilderCollection.cs +++ b/src/Umbraco.Core/Composing/IBuilderCollection.cs @@ -1,13 +1,16 @@ namespace Umbraco.Cms.Core.Composing; /// -/// Represents a builder collection, ie an immutable enumeration of items. +/// Represents a builder collection, ie an immutable enumeration of items. /// /// The type of the items. public interface IBuilderCollection : IEnumerable { /// - /// Gets the number of items in the collection. + /// Gets the number of items in the collection. /// + /// + /// The count. + /// int Count { get; } } diff --git a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs index c093962408..3f02be10b1 100644 --- a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs +++ b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs @@ -1,49 +1,88 @@ namespace Umbraco.Cms.Core.Models.Editors; /// -/// Used to track reference to other entities in a property value +/// Used to track a reference to another entity in a property value. /// public struct UmbracoEntityReference : IEquatable { private static readonly UmbracoEntityReference _empty = new(UnknownTypeUdi.Instance, string.Empty); + /// + /// Initializes a new instance of the struct. + /// + /// The UDI. + /// The relation type alias. public UmbracoEntityReference(Udi udi, string relationTypeAlias) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); RelationTypeAlias = relationTypeAlias ?? throw new ArgumentNullException(nameof(relationTypeAlias)); } + /// + /// Initializes a new instance of the struct for a document or media item. + /// + /// The UDI. public UmbracoEntityReference(Udi udi) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); switch (udi.EntityType) { + case Constants.UdiEntityType.Document: + RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + break; case Constants.UdiEntityType.Media: RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedMediaAlias; break; default: - RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + // No relation type alias convention for this entity type, so leave it empty + RelationTypeAlias = string.Empty; break; } } + /// + /// Gets the UDI. + /// + /// + /// The UDI. + /// public Udi Udi { get; } - public static UmbracoEntityReference Empty() => _empty; - - public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); - + /// + /// Gets the relation type alias. + /// + /// + /// The relation type alias. + /// public string RelationTypeAlias { get; } - public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + /// + /// Gets an empty reference. + /// + /// + /// An empty reference. + /// + public static UmbracoEntityReference Empty() => _empty; + /// + /// Determines whether the specified reference is empty. + /// + /// The reference. + /// + /// true if the specified reference is empty; otherwise, false. + /// + public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); + + /// public override bool Equals(object? obj) => obj is UmbracoEntityReference reference && Equals(reference); + /// public bool Equals(UmbracoEntityReference other) => EqualityComparer.Default.Equals(Udi, other.Udi) && RelationTypeAlias == other.RelationTypeAlias; + /// public override int GetHashCode() { var hashCode = -487348478; @@ -52,5 +91,9 @@ public struct UmbracoEntityReference : IEquatable return hashCode; } + /// + public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + + /// public static bool operator !=(UmbracoEntityReference left, UmbracoEntityReference right) => !(left == right); } diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index 24d6f17eb0..c173e766e0 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -4,64 +4,148 @@ using Umbraco.Cms.Core.Models.Editors; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Provides a builder collection for items. +/// public class DataValueReferenceFactoryCollection : BuilderCollectionBase { - public DataValueReferenceFactoryCollection(Func> items) - : base(items) - { - } - // TODO: We could further reduce circular dependencies with PropertyEditorCollection by not having IDataValueReference implemented // by property editors and instead just use the already built in IDataValueReferenceFactory and/or refactor that into a more normal collection - public IEnumerable GetAllReferences( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var trackedRelations = new HashSet(); - foreach (IProperty p in properties) + /// + /// Initializes a new instance of the class. + /// + /// The items. + public DataValueReferenceFactoryCollection(Func> items) + : base(items) + { } + + /// + /// Gets all unique references from the specified properties. + /// + /// The properties. + /// The property editors. + /// + /// The unique references from the specified properties. + /// + public ISet GetAllReferences(IPropertyCollection properties, PropertyEditorCollection propertyEditors) + { + var references = new HashSet(); + + foreach (IProperty property in properties) { - if (!propertyEditors.TryGet(p.PropertyType.PropertyEditorAlias, out IDataEditor? editor)) + if (!propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { continue; } - // TODO: We will need to change this once we support tracking via variants/segments - // for now, we are tracking values from ALL variants - foreach (IPropertyValue propertyVal in p.Values) + // Only use edited value for now + references.UnionWith(GetReferences(dataEditor, property.Values.Select(x => x.EditedValue))); + } + + return references; + } + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, params object?[] values) + => GetReferences(dataEditor, (IEnumerable)values); + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, IEnumerable values) + { + // TODO: We will need to change this once we support tracking via variants/segments + // for now, we are tracking values from ALL variants + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + foreach (UmbracoEntityReference reference in values.SelectMany(dataValueReference.GetReferences)) { - var val = propertyVal.EditedValue; - - IDataValueEditor? valueEditor = editor?.GetValueEditor(); - if (valueEditor is IDataValueReference reference) - { - IEnumerable refs = reference.GetReferences(val); - foreach (UmbracoEntityReference r in refs) - { - trackedRelations.Add(r); - } - } - - // Loop over collection that may be add to existing property editors - // implementation of GetReferences in IDataValueReference. - // Allows developers to add support for references by a - // package /property editor that did not implement IDataValueReference themselves - foreach (IDataValueReferenceFactory item in this) - { - // Check if this value reference is for this datatype/editor - // Then call it's GetReferences method - to see if the value stored - // in the dataeditor/property has referecnes to media/content items - if (item.IsForEditor(editor)) - { - foreach (UmbracoEntityReference r in item.GetDataValueReference().GetReferences(val)) - { - trackedRelations.Add(r); - } - } - } + yield return reference; } } - return trackedRelations; + // Loop over collection that may be add to existing property editors + // implementation of GetReferences in IDataValueReference. + // Allows developers to add support for references by a + // package /property editor that did not implement IDataValueReference themselves + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + // Check if this value reference is for this datatype/editor + // Then call it's GetReferences method - to see if the value stored + // in the dataeditor/property has references to media/content items + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + IDataValueReference factoryDataValueReference = dataValueReferenceFactory.GetDataValueReference(); + foreach (UmbracoEntityReference reference in values.SelectMany(factoryDataValueReference.GetReferences)) + { + yield return reference; + } + } + } + } + + /// + /// Gets all relation type aliases that are automatically tracked. + /// + /// The property editors. + /// + /// All relation type aliases that are automatically tracked. + /// + public ISet GetAllAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) + { + // Always add default automatic relation types + var automaticRelationTypeAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); + + // Add relation types for all property editors + foreach (IDataEditor dataEditor in propertyEditors) + { + automaticRelationTypeAliases.UnionWith(GetAutomaticRelationTypesAliases(dataEditor)); + } + + return automaticRelationTypeAliases; + } + + /// + /// Gets the automatic relation types aliases. + /// + /// The data editor. + /// + /// The automatic relation types aliases. + /// + public IEnumerable GetAutomaticRelationTypesAliases(IDataEditor dataEditor) + { + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + // Return custom relation types from value editor implementation + foreach (var alias in dataValueReference.GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + // Return custom relation types from factory + foreach (var alias in dataValueReferenceFactory.GetDataValueReference().GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index de247a6120..6569ad4d89 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1083,81 +1083,59 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected void PersistRelations(TEntity entity) { // Get all references from our core built in DataEditors/Property Editors - // Along with seeing if deverlopers want to collect additional references from the DataValueReferenceFactories collection - var trackedRelations = new List(); - trackedRelations.AddRange(_dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors)); - - var relationTypeAliases = GetAutomaticRelationTypesAliases(entity.Properties, PropertyEditors).ToArray(); + // Along with seeing if developers want to collect additional references from the DataValueReferenceFactories collection + ISet references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors); // First delete all auto-relations for this entity - RelationRepository.DeleteByParent(entity.Id, relationTypeAliases); + ISet automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors); + RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); - if (trackedRelations.Count == 0) + if (references.Count == 0) { + // No need to add new references/relations return; } - trackedRelations = trackedRelations.Distinct().ToList(); - var udiToGuids = trackedRelations.Select(x => x.Udi as GuidUdi) - .ToDictionary(x => (Udi)x!, x => x!.Guid); - - // lookup in the DB all INT ids for the GUIDs and chuck into a dictionary - var keyToIds = Database.Fetch(Sql() - .Select(x => x.NodeId, x => x.UniqueId) - .From() - .WhereIn(x => x.UniqueId, udiToGuids.Values)) - .ToDictionary(x => x.UniqueId, x => x.NodeId); - - var allRelationTypes = RelationTypeRepository.GetMany(Array.Empty())? - .ToDictionary(x => x.Alias, x => x); - - IEnumerable toSave = trackedRelations.Select(rel => - { - if (allRelationTypes is null || !allRelationTypes.TryGetValue(rel.RelationTypeAlias, out IRelationType? relationType)) - { - throw new InvalidOperationException($"The relation type {rel.RelationTypeAlias} does not exist"); - } - - if (!udiToGuids.TryGetValue(rel.Udi, out Guid guid)) - { - return null; // This shouldn't happen! - } - - if (!keyToIds.TryGetValue(guid, out var id)) - { - return null; // This shouldn't happen! - } - - return new ReadOnlyRelation(entity.Id, id, relationType.Id); - }).WhereNotNull(); - - // Save bulk relations - RelationRepository.SaveBulk(toSave); - } - - private IEnumerable GetAutomaticRelationTypesAliases( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var automaticRelationTypesAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); - - foreach (IProperty property in properties) + // Lookup node IDs for all GUID based UDIs + IEnumerable keys = references.Select(x => x.Udi).OfType().Select(x => x.Guid); + var keysLookup = Database.FetchByGroups(keys, Constants.Sql.MaxParameterCount, guids => { - if (propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? editor) is false ) - { - continue; - } + return Sql() + .Select(x => x.NodeId, x => x.UniqueId) + .From() + .WhereIn(x => x.UniqueId, guids); + }).ToDictionary(x => x.UniqueId, x => x.NodeId); - if (editor.GetValueEditor() is IDataValueReference reference) + // Lookup all relation type IDs + var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()).ToDictionary(x => x.Alias, x => x.Id); + + // Get all valid relations + var relations = new List(references.Count); + foreach (UmbracoEntityReference reference in references) + { + if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) { - foreach (var alias in reference.GetAutomaticRelationTypesAliases()) - { - automaticRelationTypesAliases.Add(alias); - } + // Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code + Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias); + } + else if (!relationTypeLookup.TryGetValue(reference.RelationTypeAlias, out int relationTypeId)) + { + // A non-existent relation type could be caused by an environment issue (e.g. it was manually removed) + Logger.LogWarning("The reference to {Udi} uses a relation type {RelationTypeAlias} that does not exist.", reference.Udi, reference.RelationTypeAlias); + } + else if (reference.Udi is not GuidUdi udi || !keysLookup.TryGetValue(udi.Guid, out var id)) + { + // Relations only support references to items that are stored in the NodeDto table (because of foreign key constraints) + Logger.LogInformation("The reference to {Udi} can not be saved as relation, because doesn't have a node ID.", reference.Udi); + } + else + { + relations.Add(new ReadOnlyRelation(entity.Id, id, relationTypeId)); } } - return automaticRelationTypesAliases; + // Save bulk relations + RelationRepository.SaveBulk(relations); } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index fbf2239828..9739553b99 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -17,12 +17,14 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV { private BlockEditorValues? _blockEditorValues; private readonly IDataTypeService _dataTypeService; - private readonly ILogger _logger; private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; + private readonly ILogger _logger; protected BlockEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -32,6 +34,7 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV : base(textService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _dataTypeService = dataTypeService; _logger = logger; } @@ -42,73 +45,58 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV set => _blockEditorValues = value; } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) { - return Enumerable.Empty(); - } - - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) - { - foreach (KeyValuePair prop in row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) + { + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) + { + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) + { + continue; + } + + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; + } + } + } + + private IEnumerable GetAllPropertyValues(object? value) { var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + if (blockEditorData is null) { - return Enumerable.Empty(); + yield break; } - var result = new List(); - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) + // Return all property values from the content and settings data + IEnumerable data = blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData); + foreach (BlockItemData.BlockPropertyValue propertyValue in data.SelectMany(x => x.PropertyValues.Select(x => x.Value))) { - foreach (KeyValuePair prop in row.PropertyValues) - { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; - - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); - } + yield return propertyValue; } - - return result; } #region Convert database // editor @@ -119,7 +107,6 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV /// Ensure that sub-editor values are translated through their ToEditor methods /// /// - /// /// /// /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs index 73b767bd4f..7e67c2d8a7 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs @@ -50,6 +50,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor public BlockGridEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -58,7 +59,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor IIOHelper ioHelper, IContentTypeService contentTypeService, IPropertyValidationService propertyValidationService) - : base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + : base(attribute, propertyEditors, dataValueReferenceFactories, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockGridEditorDataConverter(jsonSerializer), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs index 194383560e..d59af3817c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs @@ -46,6 +46,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor public BlockListEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService textService, @@ -54,7 +55,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : - base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + base(attribute, propertyEditors, dataValueReferenceFactories,dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockListEditorDataConverter(), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs index d64df34aa4..e34de1a3ad 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs @@ -89,9 +89,10 @@ public class NestedContentPropertyEditor : DataEditor internal class NestedContentPropertyValueEditor : DataValueEditor, IDataValueReference, IDataValueTags { private readonly IDataTypeService _dataTypeService; + private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; private readonly ILogger _logger; private readonly NestedContentValues _nestedContentValues; - private readonly PropertyEditorCollection _propertyEditors; public NestedContentPropertyValueEditor( IDataTypeService dataTypeService, @@ -100,16 +101,19 @@ public class NestedContentPropertyEditor : DataEditor IShortStringHelper shortStringHelper, DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, ILogger logger, IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { - _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; + _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _logger = logger; _nestedContentValues = new NestedContentValues(contentTypeService); + Validators.Add(new NestedContentValidator(propertyValidationService, _nestedContentValues, contentTypeService)); } @@ -137,66 +141,45 @@ public class NestedContentPropertyEditor : DataEditor } } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in _nestedContentValues.GetPropertyValues(rawJson)) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in - row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) { - IReadOnlyList rows = - _nestedContentValues.GetPropertyValues(value); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in rows.ToList()) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in row.PropertyValues - .ToList()) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; } } - - return result; } + private IEnumerable GetAllPropertyValues(object? value) + => _nestedContentValues.GetPropertyValues(value).SelectMany(x => x.PropertyValues.Values); + #region DB to String public override string ConvertDbToString(IPropertyType propertyType, object? propertyValue) @@ -422,7 +405,8 @@ public class NestedContentPropertyEditor : DataEditor // set values to null row.PropertyValues[elementTypeProp.Alias] = new NestedContentValues.NestedContentPropertyValue { - PropertyType = elementTypeProp, Value = null, + PropertyType = elementTypeProp, + Value = null, }; row.RawPropertyValues[elementTypeProp.Alias] = null; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs index d88a9689ab..75d4f0d509 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs @@ -1,4 +1,3 @@ -using System.Linq; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -15,6 +14,7 @@ public class DataValueEditorReuseTests { private Mock _dataValueEditorFactoryMock; private PropertyEditorCollection _propertyEditorCollection; + private DataValueReferenceFactoryCollection _dataValueReferenceFactories; [SetUp] public void SetUp() @@ -31,6 +31,7 @@ public class DataValueEditorReuseTests Mock.Of())); _propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + _dataValueReferenceFactories = new DataValueReferenceFactoryCollection(Enumerable.Empty); _dataValueEditorFactoryMock .Setup(m => @@ -38,6 +39,7 @@ public class DataValueEditorReuseTests .Returns(() => new BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor( new DataEditorAttribute("a", "b", "c"), _propertyEditorCollection, + _dataValueReferenceFactories, Mock.Of(), Mock.Of(), Mock.Of(), @@ -93,7 +95,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); @@ -114,7 +116,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs index 46ed8af979..38fc5125dc 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs @@ -1,9 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; -using System.Collections.Generic; -using System.Linq; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; @@ -174,6 +171,33 @@ public class DataValueReferenceFactoryCollectionTests Assert.AreEqual(trackedUdi4, result.ElementAt(1).Udi.ToString()); } + [Test] + public void GetAutomaticRelationTypesAliases_ContainsDefault() + { + var collection = new DataValueReferenceFactoryCollection(Enumerable.Empty); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes; + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + + [Test] + public void GetAutomaticRelationTypesAliases_ContainsCustom() + { + var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield()); + + var labelPropertyEditor = new LabelPropertyEditor(DataValueEditorFactory, IOHelper, EditorConfigurationParser); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => labelPropertyEditor.Yield())); + var serializer = new ConfigurationEditorJsonSerializer(); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes.Append("umbTest"); + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + private class TestDataValueReferenceFactory : IDataValueReferenceFactory { public IDataValueReference GetDataValueReference() => new TestMediaDataValueReference(); @@ -197,6 +221,12 @@ public class DataValueReferenceFactoryCollectionTests yield return new UmbracoEntityReference(udi); } } + + public IEnumerable GetAutomaticRelationTypesAliases() => new[] + { + "umbTest", + "umbTest", // Duplicate on purpose to test distinct aliases + }; } } } From ce223155206fd4d8b1eab73a9ab73e1415fbb3ea Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Fri, 19 Jan 2024 20:02:57 +0100 Subject: [PATCH 14/21] Backport relation tracking fixes and get references from recursive (nested/block) properties (#15593) * Include automatic relation type aliases from factory and fix SQL parameter overflow (#15141) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations (#15160) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations * Always get all automatic relation type aliases * Do not set relation type alias for unknown entity types * Get references from recursive (nested/block) properties (cherry picked from commit 5198e7c52d2a9fafb321ee4b8e79783580c0635b) --- .../Composing/BuilderCollectionBase.cs | 15 +- .../Composing/IBuilderCollection.cs | 7 +- .../Models/Editors/UmbracoEntityReference.cs | 57 +++++- .../DataValueReferenceFactoryCollection.cs | 174 +++++++++++++----- .../Implement/ContentRepositoryBase.cs | 112 ++++------- .../BlockEditorPropertyValueEditor.cs | 84 ++++----- .../BlockGridPropertyEditorBase.cs | 3 +- .../BlockListPropertyEditorBase.cs | 3 +- .../NestedContentPropertyEditor.cs | 72 +++----- .../DataValueEditorReuseTests.cs | 8 +- ...ataValueReferenceFactoryCollectionTests.cs | 35 +++- 11 files changed, 336 insertions(+), 234 deletions(-) diff --git a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs index 722a6efca0..e92790ab4e 100644 --- a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs +++ b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs @@ -3,7 +3,7 @@ using System.Collections; namespace Umbraco.Cms.Core.Composing; /// -/// Provides a base class for builder collections. +/// Provides a base class for builder collections. /// /// The type of the items. public abstract class BuilderCollectionBase : IBuilderCollection @@ -11,21 +11,18 @@ public abstract class BuilderCollectionBase : IBuilderCollection private readonly LazyReadOnlyCollection _items; /// - /// Initializes a new instance of the with items. + /// Initializes a new instance of the with items. /// /// The items. - public BuilderCollectionBase(Func> items) => _items = new LazyReadOnlyCollection(items); + public BuilderCollectionBase(Func> items) + => _items = new LazyReadOnlyCollection(items); /// public int Count => _items.Count; - /// - /// Gets an enumerator. - /// + /// public IEnumerator GetEnumerator() => _items.GetEnumerator(); - /// - /// Gets an enumerator. - /// + /// IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/src/Umbraco.Core/Composing/IBuilderCollection.cs b/src/Umbraco.Core/Composing/IBuilderCollection.cs index 56036997bc..c8920149c5 100644 --- a/src/Umbraco.Core/Composing/IBuilderCollection.cs +++ b/src/Umbraco.Core/Composing/IBuilderCollection.cs @@ -1,13 +1,16 @@ namespace Umbraco.Cms.Core.Composing; /// -/// Represents a builder collection, ie an immutable enumeration of items. +/// Represents a builder collection, ie an immutable enumeration of items. /// /// The type of the items. public interface IBuilderCollection : IEnumerable { /// - /// Gets the number of items in the collection. + /// Gets the number of items in the collection. /// + /// + /// The count. + /// int Count { get; } } diff --git a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs index c093962408..3f02be10b1 100644 --- a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs +++ b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs @@ -1,49 +1,88 @@ namespace Umbraco.Cms.Core.Models.Editors; /// -/// Used to track reference to other entities in a property value +/// Used to track a reference to another entity in a property value. /// public struct UmbracoEntityReference : IEquatable { private static readonly UmbracoEntityReference _empty = new(UnknownTypeUdi.Instance, string.Empty); + /// + /// Initializes a new instance of the struct. + /// + /// The UDI. + /// The relation type alias. public UmbracoEntityReference(Udi udi, string relationTypeAlias) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); RelationTypeAlias = relationTypeAlias ?? throw new ArgumentNullException(nameof(relationTypeAlias)); } + /// + /// Initializes a new instance of the struct for a document or media item. + /// + /// The UDI. public UmbracoEntityReference(Udi udi) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); switch (udi.EntityType) { + case Constants.UdiEntityType.Document: + RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + break; case Constants.UdiEntityType.Media: RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedMediaAlias; break; default: - RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + // No relation type alias convention for this entity type, so leave it empty + RelationTypeAlias = string.Empty; break; } } + /// + /// Gets the UDI. + /// + /// + /// The UDI. + /// public Udi Udi { get; } - public static UmbracoEntityReference Empty() => _empty; - - public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); - + /// + /// Gets the relation type alias. + /// + /// + /// The relation type alias. + /// public string RelationTypeAlias { get; } - public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + /// + /// Gets an empty reference. + /// + /// + /// An empty reference. + /// + public static UmbracoEntityReference Empty() => _empty; + /// + /// Determines whether the specified reference is empty. + /// + /// The reference. + /// + /// true if the specified reference is empty; otherwise, false. + /// + public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); + + /// public override bool Equals(object? obj) => obj is UmbracoEntityReference reference && Equals(reference); + /// public bool Equals(UmbracoEntityReference other) => EqualityComparer.Default.Equals(Udi, other.Udi) && RelationTypeAlias == other.RelationTypeAlias; + /// public override int GetHashCode() { var hashCode = -487348478; @@ -52,5 +91,9 @@ public struct UmbracoEntityReference : IEquatable return hashCode; } + /// + public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + + /// public static bool operator !=(UmbracoEntityReference left, UmbracoEntityReference right) => !(left == right); } diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index 24d6f17eb0..c173e766e0 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -4,64 +4,148 @@ using Umbraco.Cms.Core.Models.Editors; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Provides a builder collection for items. +/// public class DataValueReferenceFactoryCollection : BuilderCollectionBase { - public DataValueReferenceFactoryCollection(Func> items) - : base(items) - { - } - // TODO: We could further reduce circular dependencies with PropertyEditorCollection by not having IDataValueReference implemented // by property editors and instead just use the already built in IDataValueReferenceFactory and/or refactor that into a more normal collection - public IEnumerable GetAllReferences( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var trackedRelations = new HashSet(); - foreach (IProperty p in properties) + /// + /// Initializes a new instance of the class. + /// + /// The items. + public DataValueReferenceFactoryCollection(Func> items) + : base(items) + { } + + /// + /// Gets all unique references from the specified properties. + /// + /// The properties. + /// The property editors. + /// + /// The unique references from the specified properties. + /// + public ISet GetAllReferences(IPropertyCollection properties, PropertyEditorCollection propertyEditors) + { + var references = new HashSet(); + + foreach (IProperty property in properties) { - if (!propertyEditors.TryGet(p.PropertyType.PropertyEditorAlias, out IDataEditor? editor)) + if (!propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { continue; } - // TODO: We will need to change this once we support tracking via variants/segments - // for now, we are tracking values from ALL variants - foreach (IPropertyValue propertyVal in p.Values) + // Only use edited value for now + references.UnionWith(GetReferences(dataEditor, property.Values.Select(x => x.EditedValue))); + } + + return references; + } + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, params object?[] values) + => GetReferences(dataEditor, (IEnumerable)values); + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, IEnumerable values) + { + // TODO: We will need to change this once we support tracking via variants/segments + // for now, we are tracking values from ALL variants + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + foreach (UmbracoEntityReference reference in values.SelectMany(dataValueReference.GetReferences)) { - var val = propertyVal.EditedValue; - - IDataValueEditor? valueEditor = editor?.GetValueEditor(); - if (valueEditor is IDataValueReference reference) - { - IEnumerable refs = reference.GetReferences(val); - foreach (UmbracoEntityReference r in refs) - { - trackedRelations.Add(r); - } - } - - // Loop over collection that may be add to existing property editors - // implementation of GetReferences in IDataValueReference. - // Allows developers to add support for references by a - // package /property editor that did not implement IDataValueReference themselves - foreach (IDataValueReferenceFactory item in this) - { - // Check if this value reference is for this datatype/editor - // Then call it's GetReferences method - to see if the value stored - // in the dataeditor/property has referecnes to media/content items - if (item.IsForEditor(editor)) - { - foreach (UmbracoEntityReference r in item.GetDataValueReference().GetReferences(val)) - { - trackedRelations.Add(r); - } - } - } + yield return reference; } } - return trackedRelations; + // Loop over collection that may be add to existing property editors + // implementation of GetReferences in IDataValueReference. + // Allows developers to add support for references by a + // package /property editor that did not implement IDataValueReference themselves + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + // Check if this value reference is for this datatype/editor + // Then call it's GetReferences method - to see if the value stored + // in the dataeditor/property has references to media/content items + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + IDataValueReference factoryDataValueReference = dataValueReferenceFactory.GetDataValueReference(); + foreach (UmbracoEntityReference reference in values.SelectMany(factoryDataValueReference.GetReferences)) + { + yield return reference; + } + } + } + } + + /// + /// Gets all relation type aliases that are automatically tracked. + /// + /// The property editors. + /// + /// All relation type aliases that are automatically tracked. + /// + public ISet GetAllAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) + { + // Always add default automatic relation types + var automaticRelationTypeAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); + + // Add relation types for all property editors + foreach (IDataEditor dataEditor in propertyEditors) + { + automaticRelationTypeAliases.UnionWith(GetAutomaticRelationTypesAliases(dataEditor)); + } + + return automaticRelationTypeAliases; + } + + /// + /// Gets the automatic relation types aliases. + /// + /// The data editor. + /// + /// The automatic relation types aliases. + /// + public IEnumerable GetAutomaticRelationTypesAliases(IDataEditor dataEditor) + { + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + // Return custom relation types from value editor implementation + foreach (var alias in dataValueReference.GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + // Return custom relation types from factory + foreach (var alias in dataValueReferenceFactory.GetDataValueReference().GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 4f6aa228d0..6569ad4d89 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1083,91 +1083,59 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected void PersistRelations(TEntity entity) { // Get all references from our core built in DataEditors/Property Editors - // Along with seeing if deverlopers want to collect additional references from the DataValueReferenceFactories collection - var trackedRelations = new List(); - trackedRelations.AddRange(_dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors)); - - var relationTypeAliases = GetAutomaticRelationTypesAliases(entity.Properties, PropertyEditors).ToList(); - - // At this point we potentially have a problem (see for example: https://github.com/umbraco/Umbraco.Forms.Issues/issues/1129). - // If we have a custom relation type (i.e. not document or media) and use of this within a block grid/list property editor, - // we'll get an error when saving the relations. - // It happens because the block grid doesn't expose the automatic relation type aliases for all of it's nested properties, and - // as such relationTypeAliases does not contain the custom relation type alias. - // Auto-relations of that type are then then not deleted, and we get duplicate error on insert. - // To resolve we can look at the relations we are going to be saving, and if they include any relation type aliases we haven't - // already identified, we'll add them in, so they will also be removed along with the other auto-relations. - relationTypeAliases.AddRange(trackedRelations.Select(x => x.RelationTypeAlias).Distinct().Except(relationTypeAliases)); + // Along with seeing if developers want to collect additional references from the DataValueReferenceFactories collection + ISet references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors); // First delete all auto-relations for this entity - RelationRepository.DeleteByParent(entity.Id, relationTypeAliases.ToArray()); + ISet automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors); + RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); - if (trackedRelations.Count == 0) + if (references.Count == 0) { + // No need to add new references/relations return; } - trackedRelations = trackedRelations.Distinct().ToList(); - var udiToGuids = trackedRelations.Select(x => x.Udi as GuidUdi) - .ToDictionary(x => (Udi)x!, x => x!.Guid); - - // lookup in the DB all INT ids for the GUIDs and chuck into a dictionary - var keyToIds = Database.Fetch(Sql() - .Select(x => x.NodeId, x => x.UniqueId) - .From() - .WhereIn(x => x.UniqueId, udiToGuids.Values)) - .ToDictionary(x => x.UniqueId, x => x.NodeId); - - var allRelationTypes = RelationTypeRepository.GetMany(Array.Empty())? - .ToDictionary(x => x.Alias, x => x); - - IEnumerable toSave = trackedRelations.Select(rel => - { - if (allRelationTypes is null || !allRelationTypes.TryGetValue(rel.RelationTypeAlias, out IRelationType? relationType)) - { - throw new InvalidOperationException($"The relation type {rel.RelationTypeAlias} does not exist"); - } - - if (!udiToGuids.TryGetValue(rel.Udi, out Guid guid)) - { - return null; // This shouldn't happen! - } - - if (!keyToIds.TryGetValue(guid, out var id)) - { - return null; // This shouldn't happen! - } - - return new ReadOnlyRelation(entity.Id, id, relationType.Id); - }).WhereNotNull(); - - // Save bulk relations - RelationRepository.SaveBulk(toSave); - } - - private IEnumerable GetAutomaticRelationTypesAliases( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var automaticRelationTypesAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); - - foreach (IProperty property in properties) + // Lookup node IDs for all GUID based UDIs + IEnumerable keys = references.Select(x => x.Udi).OfType().Select(x => x.Guid); + var keysLookup = Database.FetchByGroups(keys, Constants.Sql.MaxParameterCount, guids => { - if (propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? editor) is false ) - { - continue; - } + return Sql() + .Select(x => x.NodeId, x => x.UniqueId) + .From() + .WhereIn(x => x.UniqueId, guids); + }).ToDictionary(x => x.UniqueId, x => x.NodeId); - if (editor.GetValueEditor() is IDataValueReference reference) + // Lookup all relation type IDs + var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()).ToDictionary(x => x.Alias, x => x.Id); + + // Get all valid relations + var relations = new List(references.Count); + foreach (UmbracoEntityReference reference in references) + { + if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) { - foreach (var alias in reference.GetAutomaticRelationTypesAliases()) - { - automaticRelationTypesAliases.Add(alias); - } + // Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code + Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias); + } + else if (!relationTypeLookup.TryGetValue(reference.RelationTypeAlias, out int relationTypeId)) + { + // A non-existent relation type could be caused by an environment issue (e.g. it was manually removed) + Logger.LogWarning("The reference to {Udi} uses a relation type {RelationTypeAlias} that does not exist.", reference.Udi, reference.RelationTypeAlias); + } + else if (reference.Udi is not GuidUdi udi || !keysLookup.TryGetValue(udi.Guid, out var id)) + { + // Relations only support references to items that are stored in the NodeDto table (because of foreign key constraints) + Logger.LogInformation("The reference to {Udi} can not be saved as relation, because doesn't have a node ID.", reference.Udi); + } + else + { + relations.Add(new ReadOnlyRelation(entity.Id, id, relationTypeId)); } } - return automaticRelationTypesAliases; + // Save bulk relations + RelationRepository.SaveBulk(relations); } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index c524c2c39b..9739553b99 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -17,12 +17,14 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV { private BlockEditorValues? _blockEditorValues; private readonly IDataTypeService _dataTypeService; - private readonly ILogger _logger; private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; + private readonly ILogger _logger; protected BlockEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -32,6 +34,7 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV : base(textService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _dataTypeService = dataTypeService; _logger = logger; } @@ -42,73 +45,58 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV set => _blockEditorValues = value; } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) { - return Enumerable.Empty(); - } - - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) - { - foreach (KeyValuePair prop in row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) + { + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) + { + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) + { + continue; + } + + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; + } + } + } + + private IEnumerable GetAllPropertyValues(object? value) { var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + if (blockEditorData is null) { - return Enumerable.Empty(); + yield break; } - var result = new List(); - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) + // Return all property values from the content and settings data + IEnumerable data = blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData); + foreach (BlockItemData.BlockPropertyValue propertyValue in data.SelectMany(x => x.PropertyValues.Select(x => x.Value))) { - foreach (KeyValuePair prop in row.PropertyValues) - { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; - - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); - } + yield return propertyValue; } - - return result; } #region Convert database // editor diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs index 73b767bd4f..7e67c2d8a7 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs @@ -50,6 +50,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor public BlockGridEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -58,7 +59,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor IIOHelper ioHelper, IContentTypeService contentTypeService, IPropertyValidationService propertyValidationService) - : base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + : base(attribute, propertyEditors, dataValueReferenceFactories, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockGridEditorDataConverter(jsonSerializer), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs index 194383560e..d59af3817c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs @@ -46,6 +46,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor public BlockListEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService textService, @@ -54,7 +55,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : - base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + base(attribute, propertyEditors, dataValueReferenceFactories,dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockListEditorDataConverter(), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs index bf5781079a..87d80e3972 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs @@ -91,9 +91,10 @@ public class NestedContentPropertyEditor : DataEditor internal class NestedContentPropertyValueEditor : DataValueEditor, IDataValueReference, IDataValueTags { private readonly IDataTypeService _dataTypeService; + private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; private readonly ILogger _logger; private readonly NestedContentValues _nestedContentValues; - private readonly PropertyEditorCollection _propertyEditors; public NestedContentPropertyValueEditor( IDataTypeService dataTypeService, @@ -102,16 +103,19 @@ public class NestedContentPropertyEditor : DataEditor IShortStringHelper shortStringHelper, DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, ILogger logger, IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { - _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; + _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _logger = logger; _nestedContentValues = new NestedContentValues(contentTypeService); + Validators.Add(new NestedContentValidator(propertyValidationService, _nestedContentValues, contentTypeService)); } @@ -139,66 +143,45 @@ public class NestedContentPropertyEditor : DataEditor } } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in _nestedContentValues.GetPropertyValues(rawJson)) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in - row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) { - IReadOnlyList rows = - _nestedContentValues.GetPropertyValues(value); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in rows.ToList()) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in row.PropertyValues - .ToList()) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; } } - - return result; } + private IEnumerable GetAllPropertyValues(object? value) + => _nestedContentValues.GetPropertyValues(value).SelectMany(x => x.PropertyValues.Values); + #region DB to String public override string ConvertDbToString(IPropertyType propertyType, object? propertyValue) @@ -424,7 +407,8 @@ public class NestedContentPropertyEditor : DataEditor // set values to null row.PropertyValues[elementTypeProp.Alias] = new NestedContentValues.NestedContentPropertyValue { - PropertyType = elementTypeProp, Value = null, + PropertyType = elementTypeProp, + Value = null, }; row.RawPropertyValues[elementTypeProp.Alias] = null; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs index d88a9689ab..75d4f0d509 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs @@ -1,4 +1,3 @@ -using System.Linq; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -15,6 +14,7 @@ public class DataValueEditorReuseTests { private Mock _dataValueEditorFactoryMock; private PropertyEditorCollection _propertyEditorCollection; + private DataValueReferenceFactoryCollection _dataValueReferenceFactories; [SetUp] public void SetUp() @@ -31,6 +31,7 @@ public class DataValueEditorReuseTests Mock.Of())); _propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + _dataValueReferenceFactories = new DataValueReferenceFactoryCollection(Enumerable.Empty); _dataValueEditorFactoryMock .Setup(m => @@ -38,6 +39,7 @@ public class DataValueEditorReuseTests .Returns(() => new BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor( new DataEditorAttribute("a", "b", "c"), _propertyEditorCollection, + _dataValueReferenceFactories, Mock.Of(), Mock.Of(), Mock.Of(), @@ -93,7 +95,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); @@ -114,7 +116,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs index 6108f59e2e..38fc5125dc 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Collections.Generic; -using System.Linq; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; @@ -173,6 +171,33 @@ public class DataValueReferenceFactoryCollectionTests Assert.AreEqual(trackedUdi4, result.ElementAt(1).Udi.ToString()); } + [Test] + public void GetAutomaticRelationTypesAliases_ContainsDefault() + { + var collection = new DataValueReferenceFactoryCollection(Enumerable.Empty); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes; + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + + [Test] + public void GetAutomaticRelationTypesAliases_ContainsCustom() + { + var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield()); + + var labelPropertyEditor = new LabelPropertyEditor(DataValueEditorFactory, IOHelper, EditorConfigurationParser); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => labelPropertyEditor.Yield())); + var serializer = new ConfigurationEditorJsonSerializer(); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes.Append("umbTest"); + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + private class TestDataValueReferenceFactory : IDataValueReferenceFactory { public IDataValueReference GetDataValueReference() => new TestMediaDataValueReference(); @@ -196,6 +221,12 @@ public class DataValueReferenceFactoryCollectionTests yield return new UmbracoEntityReference(udi); } } + + public IEnumerable GetAutomaticRelationTypesAliases() => new[] + { + "umbTest", + "umbTest", // Duplicate on purpose to test distinct aliases + }; } } } From bee20b525fd6fa9311f7994a41db33420c85ffb9 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Sat, 20 Jan 2024 11:26:40 +0100 Subject: [PATCH 15/21] Post merge fix --- tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj b/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj index 9e70ea8382..8b66986898 100644 --- a/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj +++ b/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj @@ -13,8 +13,5 @@ - - - From cdbbd6a921084128d6789beb057104dac6493402 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 22 Jan 2024 14:53:20 +0100 Subject: [PATCH 16/21] Optimize relation tracking for adding new and keeping existing relations (#15596) * Include automatic relation type aliases from factory and fix SQL parameter overflow (#15141) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations (#15160) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations * Always get all automatic relation type aliases * Do not set relation type alias for unknown entity types * Get references from recursive (nested/block) properties * Optimize relation tracking for adding new and keeping existing relations * Optimize getting references by grouping by property editor alias and avoiding duplicate parsing of the same value --- .../DataValueReferenceFactoryCollection.cs | 16 +++++-- .../Implement/ContentRepositoryBase.cs | 44 +++++++++++++------ .../BlockEditorPropertyValueEditor.cs | 8 ++-- .../NestedContentPropertyEditor.cs | 8 ++-- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index c173e766e0..e15f581809 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -32,15 +32,23 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase(); - foreach (IProperty property in properties) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var propertyValuesByPropertyEditorAlias in properties.GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Values)) { - if (!propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!propertyEditors.TryGet(propertyValuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - // Only use edited value for now - references.UnionWith(GetReferences(dataEditor, property.Values.Select(x => x.EditedValue))); + // Use distinct values to avoid duplicate parsing of the same value + var values = new HashSet(properties.Count); + foreach (IPropertyValue propertyValue in propertyValuesByPropertyEditorAlias.SelectMany(x => x)) + { + values.Add(propertyValue.EditedValue); + values.Add(propertyValue.PublishedValue); + } + + references.UnionWith(GetReferences(dataEditor, values)); } return references; diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 6569ad4d89..56c356e7b4 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1082,20 +1082,24 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected void PersistRelations(TEntity entity) { - // Get all references from our core built in DataEditors/Property Editors - // Along with seeing if developers want to collect additional references from the DataValueReferenceFactories collection + // Get all references and automatic relation type aliases ISet references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors); - - // First delete all auto-relations for this entity ISet automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors); - RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); if (references.Count == 0) { + // Delete all relations using the automatic relation type aliases + RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); + // No need to add new references/relations return; } + // Lookup all relation type IDs + var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()) + .Where(x => automaticRelationTypeAliases.Contains(x.Alias)) + .ToDictionary(x => x.Alias, x => x.Id); + // Lookup node IDs for all GUID based UDIs IEnumerable keys = references.Select(x => x.Udi).OfType().Select(x => x.Guid); var keysLookup = Database.FetchByGroups(keys, Constants.Sql.MaxParameterCount, guids => @@ -1106,14 +1110,16 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement .WhereIn(x => x.UniqueId, guids); }).ToDictionary(x => x.UniqueId, x => x.NodeId); - // Lookup all relation type IDs - var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()).ToDictionary(x => x.Alias, x => x.Id); - // Get all valid relations - var relations = new List(references.Count); + var relations = new List<(int ChildId, int RelationTypeId)>(references.Count); foreach (UmbracoEntityReference reference in references) { - if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) + if (string.IsNullOrEmpty(reference.RelationTypeAlias)) + { + // Reference does not specify a relation type alias, so skip adding a relation + Logger.LogDebug("The reference to {Udi} does not specify a relation type alias, so it will not be saved as relation.", reference.Udi); + } + else if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) { // Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias); @@ -1130,12 +1136,24 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } else { - relations.Add(new ReadOnlyRelation(entity.Id, id, relationTypeId)); + relations.Add((id, relationTypeId)); } } - // Save bulk relations - RelationRepository.SaveBulk(relations); + // Get all existing relations (optimize for adding new and keeping existing relations) + var query = Query().Where(x => x.ParentId == entity.Id).WhereIn(x => x.RelationTypeId, relationTypeLookup.Values); + var existingRelations = RelationRepository.GetPagedRelationsByQuery(query, 0, int.MaxValue, out _, null) + .ToDictionary(x => (x.ChildId, x.RelationTypeId)); // Relations are unique by parent ID, child ID and relation type ID + + // Add relations that don't exist yet + var relationsToAdd = relations.Except(existingRelations.Keys).Select(x => new ReadOnlyRelation(entity.Id, x.ChildId, x.RelationTypeId)); + RelationRepository.SaveBulk(relationsToAdd); + + // Delete relations that don't exist anymore + foreach (IRelation relation in existingRelations.Where(x => !relations.Contains(x.Key)).Select(x => x.Value)) + { + RelationRepository.Delete(relation); + } } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index 9739553b99..c281b0758e 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -48,14 +48,16 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV /// public IEnumerable GetReferences(object? value) { - foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var valuesByPropertyEditorAlias in GetAllPropertyValues(value).GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Value)) { - if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!_propertyEditors.TryGet(valuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + // Use distinct values to avoid duplicate parsing of the same value + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, valuesByPropertyEditorAlias.Distinct())) { yield return reference; } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs index e34de1a3ad..6f30605c20 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs @@ -144,14 +144,16 @@ public class NestedContentPropertyEditor : DataEditor /// public IEnumerable GetReferences(object? value) { - foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var valuesByPropertyEditorAlias in GetAllPropertyValues(value).GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Value)) { - if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!_propertyEditors.TryGet(valuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + // Use distinct values to avoid duplicate parsing of the same value + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, valuesByPropertyEditorAlias.Distinct())) { yield return reference; } From b853a5e14a33e82c4c4fded7731b290c84f7289e Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 23 Jan 2024 14:11:42 +0100 Subject: [PATCH 17/21] Revert breaking changes --- .../DataValueReferenceFactoryCollection.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index f5eb0130fc..4a83a65405 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -73,7 +73,9 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase /// The references. /// - public IEnumerable GetReferences(IDataEditor dataEditor, IEnumerable values) + public ISet GetReferences(IDataEditor dataEditor, IEnumerable values) => + GetReferencesEnumerable(dataEditor, values).ToHashSet(); + private IEnumerable GetReferencesEnumerable(IDataEditor dataEditor, IEnumerable values) { // TODO: We will need to change this once we support tracking via variants/segments // for now, we are tracking values from ALL variants @@ -133,7 +135,9 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase /// The automatic relation types aliases. /// - public IEnumerable GetAutomaticRelationTypesAliases(IDataEditor dataEditor) + public ISet GetAutomaticRelationTypesAliases(IDataEditor dataEditor) => + GetAutomaticRelationTypesAliasesEnumerable(dataEditor).ToHashSet(); + private IEnumerable GetAutomaticRelationTypesAliasesEnumerable(IDataEditor dataEditor) { if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) { From 4421b920f0d58da423b127e3fffc450cae4351de Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 23 Jan 2024 14:26:47 +0100 Subject: [PATCH 18/21] Revert breaking changes --- .../PropertyEditors/DataValueReferenceFactoryCollection.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index 4a83a65405..d826e6e3e4 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -114,6 +114,9 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase /// All relation type aliases that are automatically tracked. /// + [Obsolete("Use GetAllAutomaticRelationTypesAliases. This will be removed in Umbraco 15.")] + public ISet GetAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) => + GetAllAutomaticRelationTypesAliases(propertyEditors); public ISet GetAllAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) { // Always add default automatic relation types From b3d6c60ce22d4065a3d471d3a6f6ef6127a79b0f Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 23 Jan 2024 14:46:28 +0100 Subject: [PATCH 19/21] Revert breaking changes --- .../DataValueReferenceFactoryCollection.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index d826e6e3e4..86c9c48fc0 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -130,6 +130,23 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase GetAutomaticRelationTypesAliases(IPropertyCollection properties, PropertyEditorCollection propertyEditors) + { + // Always add default automatic relation types + var automaticRelationTypeAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); + + // Only add relation types that are used in the properties + foreach (IProperty property in properties) + { + if (propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + { + automaticRelationTypeAliases.UnionWith(GetAutomaticRelationTypesAliasesEnumerable(dataEditor)); + } + } + + return automaticRelationTypeAliases; + } /// /// Gets the automatic relation types aliases. From b7e43a8def6e476c24ba5bfd93be7d7b5a4cf4d4 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Tue, 23 Jan 2024 16:08:17 +0100 Subject: [PATCH 20/21] Get references from macro parameters using IDataValueReferenceFactory (#15625) --- .../Templates/HtmlMacroParameterParser.cs | 52 ++++++++----------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/Umbraco.Infrastructure/Templates/HtmlMacroParameterParser.cs b/src/Umbraco.Infrastructure/Templates/HtmlMacroParameterParser.cs index 721e99bbd7..409c02ed02 100644 --- a/src/Umbraco.Infrastructure/Templates/HtmlMacroParameterParser.cs +++ b/src/Umbraco.Infrastructure/Templates/HtmlMacroParameterParser.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Newtonsoft.Json; using Umbraco.Cms.Core; @@ -6,6 +7,7 @@ using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Macros; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Templates; @@ -15,12 +17,23 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser private readonly ILogger _logger; private readonly IMacroService _macroService; private readonly ParameterEditorCollection _parameterEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; + [Obsolete("Use the non-obsolete overload instead, scheduled for removal in v14")] public HtmlMacroParameterParser(IMacroService macroService, ILogger logger, ParameterEditorCollection parameterEditors) + : this( + macroService, + logger, + parameterEditors, + StaticServiceProvider.Instance.GetRequiredService()) + { } + + public HtmlMacroParameterParser(IMacroService macroService, ILogger logger, ParameterEditorCollection parameterEditors, DataValueReferenceFactoryCollection dataValueReferenceFactories) { _macroService = macroService; _logger = logger; _parameterEditors = parameterEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; } /// @@ -41,6 +54,7 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser (macroAlias, macroAttributes) => foundMacros.Add(new Tuple>( macroAlias, new Dictionary(macroAttributes, StringComparer.OrdinalIgnoreCase)))); + foreach (UmbracoEntityReference umbracoEntityReference in GetUmbracoEntityReferencesFromMacros(foundMacros)) { yield return umbracoEntityReference; @@ -52,8 +66,7 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser /// /// /// - public IEnumerable FindUmbracoEntityReferencesFromGridControlMacros( - IEnumerable macroGridControls) + public IEnumerable FindUmbracoEntityReferencesFromGridControlMacros(IEnumerable macroGridControls) { var foundMacros = new List>>(); @@ -65,8 +78,7 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser // Collect any macro parameters that contain the media udi format if (gridMacro is not null && gridMacro.MacroParameters is not null && gridMacro.MacroParameters.Any()) { - foundMacros.Add( - new Tuple>(gridMacro.MacroAlias, gridMacro.MacroParameters)); + foundMacros.Add(new Tuple>(gridMacro.MacroAlias, gridMacro.MacroParameters)); } } @@ -101,14 +113,12 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser continue; } - foundMacroUmbracoEntityReferences.Add( - new UmbracoEntityReference(Udi.Create(Constants.UdiEntityType.Macro, macroConfig.Key))); + foundMacroUmbracoEntityReferences.Add(new UmbracoEntityReference(Udi.Create(Constants.UdiEntityType.Macro, macroConfig.Key))); // Only do this if the macros actually have parameters if (macroConfig.Properties.Keys.Any(f => f != "macroAlias")) { - foreach (UmbracoEntityReference umbracoEntityReference in GetUmbracoEntityReferencesFromMacroParameters( - macro.Item2, macroConfig, _parameterEditors)) + foreach (UmbracoEntityReference umbracoEntityReference in GetUmbracoEntityReferencesFromMacroParameters(macro.Item2, macroConfig, _parameterEditors)) { yield return umbracoEntityReference; } @@ -130,41 +140,23 @@ public sealed class HtmlMacroParameterParser : IHtmlMacroParameterParser /// look up the corresponding property editor for a macro parameter /// /// - private IEnumerable GetUmbracoEntityReferencesFromMacroParameters( - Dictionary macroParameters, IMacro macroConfig, ParameterEditorCollection parameterEditors) + private IEnumerable GetUmbracoEntityReferencesFromMacroParameters(Dictionary macroParameters, IMacro macroConfig, ParameterEditorCollection parameterEditors) { - var foundUmbracoEntityReferences = new List(); foreach (IMacroProperty parameter in macroConfig.Properties) { if (macroParameters.TryGetValue(parameter.Alias, out var parameterValue)) { var parameterEditorAlias = parameter.EditorAlias; - - // Lookup propertyEditor from the registered ParameterEditors with the implmementation to avoid looking up for each parameter - IDataEditor? parameterEditor = parameterEditors.FirstOrDefault(f => - string.Equals(f.Alias, parameterEditorAlias, StringComparison.OrdinalIgnoreCase)); + IDataEditor? parameterEditor = parameterEditors.FirstOrDefault(f => string.Equals(f.Alias, parameterEditorAlias, StringComparison.OrdinalIgnoreCase)); if (parameterEditor is not null) { - // Get the ParameterValueEditor for this PropertyEditor (where the GetReferences method is implemented) - cast as IDataValueReference to determine if 'it is' implemented for the editor - if (parameterEditor.GetValueEditor() is IDataValueReference parameterValueEditor) + foreach (UmbracoEntityReference entityReference in _dataValueReferenceFactories.GetReferences(parameterEditor, parameterValue)) { - foreach (UmbracoEntityReference entityReference in parameterValueEditor.GetReferences( - parameterValue)) - { - foundUmbracoEntityReferences.Add(entityReference); - } - } - else - { - _logger.LogInformation( - "{0} doesn't have a ValueEditor that implements IDataValueReference", - parameterEditor.Alias); + yield return entityReference; } } } } - - return foundUmbracoEntityReferences; } // Poco class to deserialise the Json for a Macro Control From a12361deb5413772e1fe48414a2084811ef3a79b Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 23 Jan 2024 18:56:47 +0100 Subject: [PATCH 21/21] Fixed test --- .../PropertyEditors/BlockValuePropertyValueEditorBase.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index c908a3cc81..775a3cd532 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -39,6 +39,7 @@ internal abstract class BlockValuePropertyValueEditorBase : DataValueEditor, IDa protected IEnumerable GetBlockValueReferences(BlockValue blockValue) { + var result = new HashSet(); BlockItemData.BlockPropertyValue[] propertyValues = blockValue.ContentData.Concat(blockValue.SettingsData) .SelectMany(x => x.PropertyValues.Values).ToArray(); foreach (IGrouping valuesByPropertyEditorAlias in propertyValues.GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Value)) @@ -54,7 +55,7 @@ internal abstract class BlockValuePropertyValueEditorBase : DataValueEditor, IDa { foreach (UmbracoEntityReference value in districtValues.SelectMany(reference.GetReferences)) { - yield return value; + result.Add(value); } } @@ -62,10 +63,11 @@ internal abstract class BlockValuePropertyValueEditorBase : DataValueEditor, IDa foreach (UmbracoEntityReference value in references) { - yield return value; + result.Add(value); } - } + + return result; } ///