From fd100602c2b19640f919a97ad0aa8b09126cb64a Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Thu, 15 Aug 2024 07:11:17 +0200 Subject: [PATCH] V14/fix/element switch validation (#16421) * Added Element <-> Document type switch validation * Apply HasElementconfigured to block grid and block list Fix smalle bug + optimization * Moved some of the logic into warnings trough notifcationhandlers and eventmessages * Cleanup * Update openApi spec (merge changes) * Add IsElement check between parent and child on creation * Typos * Transformed HasElementConfigured into HasElementConfigured * Typo Co-authored-by: Kenn Jacobsen * IsElement Validation refactor Moved validation logic regarding doctype IsElement switch into its own service as it will be consumed by more things down the line * commit missing services... * Naming improvements * Bugfix * First batch of integration tests for ElementSwitchValidator * More integration tests! * Little reformatting * Changed the default values of block based configuration to match expected values. --------- Co-authored-by: Kenn Jacobsen --- .../DocumentTypeControllerBase.cs | 12 + src/Umbraco.Cms.Api.Management/OpenApi.json | 2 +- .../DependencyInjection/UmbracoBuilder.cs | 5 +- ...entTypeElementSwitchNotificationHandler.cs | 104 +++++ src/Umbraco.Core/Models/IDataValueEditor.cs | 3 +- .../PropertyEditors/BlockListConfiguration.cs | 2 +- .../PropertyEditors/DataEditor.cs | 4 + .../PropertyEditors/DataValueEditor.cs | 4 + .../PropertyEditors/IDataEditor.cs | 2 + .../PropertyEditors/RichTextConfiguration.cs | 2 +- .../ContentTypeEditingService.cs | 86 ++++- .../ContentTypeEditingServiceBase.cs | 6 + .../ElementSwitchValidator.cs | 66 ++++ .../IElementSwitchValidator.cs | 14 + src/Umbraco.Core/Services/DataTypeService.cs | 10 + src/Umbraco.Core/Services/IDataTypeService.cs | 7 + .../ContentTypeOperationStatus.cs | 3 + .../UmbracoBuilder.CoreServices.cs | 5 + .../BlockGridPropertyEditor.cs | 1 + .../BlockGridPropertyEditorBase.cs | 6 + .../BlockListPropertyEditor.cs | 2 + .../BlockListPropertyEditorBase.cs | 6 + .../BlockValuePropertyValueEditorBase.cs | 9 + .../PropertyEditors/RichTextPropertyEditor.cs | 8 + .../Services/ElementSwitchValidatorTests.cs | 358 ++++++++++++++++++ 25 files changed, 717 insertions(+), 10 deletions(-) create mode 100644 src/Umbraco.Core/Handlers/WarnDocumentTypeElementSwitchNotificationHandler.cs create mode 100644 src/Umbraco.Core/Services/ContentTypeEditing/ElementSwitchValidator.cs create mode 100644 src/Umbraco.Core/Services/ContentTypeEditing/IElementSwitchValidator.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ElementSwitchValidatorTests.cs diff --git a/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs index c65ed1631d..da1eeddfac 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs @@ -98,6 +98,18 @@ public abstract class DocumentTypeControllerBase : ManagementApiControllerBase .WithTitle("Name was too long") .WithDetail("Name cannot be more than 255 characters in length.") .Build()), + ContentTypeOperationStatus.InvalidElementFlagDocumentHasContent => new BadRequestObjectResult(problemDetailsBuilder + .WithTitle("Invalid IsElement flag") + .WithDetail("Cannot change to element type because content has already been created with this document type.") + .Build()), + ContentTypeOperationStatus.InvalidElementFlagElementIsUsedInPropertyEditorConfiguration => new BadRequestObjectResult(problemDetailsBuilder + .WithTitle("Invalid IsElement flag") + .WithDetail("Cannot change to document type because this element type is used in the configuration of a data type.") + .Build()), + ContentTypeOperationStatus.InvalidElementFlagComparedToParent => new BadRequestObjectResult(problemDetailsBuilder + .WithTitle("Invalid IsElement flag") + .WithDetail("Can not create a documentType with inheritance composition where the parent and the new type's IsElement flag are different.") + .Build()), _ => new ObjectResult("Unknown content type operation status") { StatusCode = StatusCodes.Status500InternalServerError }, }); diff --git a/src/Umbraco.Cms.Api.Management/OpenApi.json b/src/Umbraco.Cms.Api.Management/OpenApi.json index 5d2d532ebc..62bf741f9b 100644 --- a/src/Umbraco.Cms.Api.Management/OpenApi.json +++ b/src/Umbraco.Cms.Api.Management/OpenApi.json @@ -45275,4 +45275,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 11c9e91099..ffeafe17b7 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -398,11 +398,14 @@ namespace Umbraco.Cms.Core.DependencyInjection // Segments Services.AddUnique(); - + // definition Import/export Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); + + // add validation services + Services.AddUnique(); } } } diff --git a/src/Umbraco.Core/Handlers/WarnDocumentTypeElementSwitchNotificationHandler.cs b/src/Umbraco.Core/Handlers/WarnDocumentTypeElementSwitchNotificationHandler.cs new file mode 100644 index 0000000000..b4c13f8da3 --- /dev/null +++ b/src/Umbraco.Core/Handlers/WarnDocumentTypeElementSwitchNotificationHandler.cs @@ -0,0 +1,104 @@ +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Extensions; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; + +namespace Umbraco.Cms.Core.Handlers; + +public class WarnDocumentTypeElementSwitchNotificationHandler : + INotificationAsyncHandler, + INotificationAsyncHandler +{ + private const string NotificationStateKey = + "Umbraco.Cms.Core.Handlers.WarnDocumentTypeElementSwitchNotificationHandler"; + + private readonly IEventMessagesFactory _eventMessagesFactory; + private readonly IContentTypeService _contentTypeService; + private readonly IElementSwitchValidator _elementSwitchValidator; + + public WarnDocumentTypeElementSwitchNotificationHandler( + IEventMessagesFactory eventMessagesFactory, + IContentTypeService contentTypeService, + IElementSwitchValidator elementSwitchValidator) + { + _eventMessagesFactory = eventMessagesFactory; + _contentTypeService = contentTypeService; + _elementSwitchValidator = elementSwitchValidator; + } + + // To figure out whether a warning should be generated, we need both the state before and after saving + public async Task HandleAsync(ContentTypeSavingNotification notification, CancellationToken cancellationToken) + { + IEnumerable updatedKeys = notification.SavedEntities + .Where(e => e.HasIdentity) + .Select(e => e.Key); + + IEnumerable persistedItems = _contentTypeService.GetAll(updatedKeys); + + var stateInformation = persistedItems + .ToDictionary( + contentType => contentType.Key, + contentType => new DocumentTypeElementSwitchInformation { WasElement = contentType.IsElement }); + notification.State[NotificationStateKey] = stateInformation; + } + + public async Task HandleAsync(ContentTypeSavedNotification notification, CancellationToken cancellationToken) + { + if (notification.State[NotificationStateKey] is not Dictionary + stateInformation) + { + return; + } + + EventMessages eventMessages = _eventMessagesFactory.Get(); + + foreach (IContentType savedDocumentType in notification.SavedEntities) + { + if (stateInformation.ContainsKey(savedDocumentType.Key) is false) + { + continue; + } + + DocumentTypeElementSwitchInformation state = stateInformation[savedDocumentType.Key]; + if (state.WasElement == savedDocumentType.IsElement) + { + // no change + continue; + } + + await WarnIfAncestorsAreMisaligned(savedDocumentType, eventMessages); + await WarnIfDescendantsAreMisaligned(savedDocumentType, eventMessages); + } + } + + private async Task WarnIfAncestorsAreMisaligned(IContentType contentType, EventMessages eventMessages) + { + if (await _elementSwitchValidator.AncestorsAreAlignedAsync(contentType) == false) + { + // todo update this message when the format has been agreed upon on with the client + eventMessages.Add(new EventMessage( + "DocumentType saved", + "One or more ancestors have a mismatching element flag", + EventMessageType.Warning)); + } + } + + private async Task WarnIfDescendantsAreMisaligned(IContentType contentType, EventMessages eventMessages) + { + if (await _elementSwitchValidator.DescendantsAreAlignedAsync(contentType) == false) + { + // todo update this message when the format has been agreed upon on with the client + eventMessages.Add(new EventMessage( + "DocumentType saved", + "One or more descendants have a mismatching element flag", + EventMessageType.Warning)); + } + } + + private class DocumentTypeElementSwitchInformation + { + public bool WasElement { get; set; } + } +} diff --git a/src/Umbraco.Core/Models/IDataValueEditor.cs b/src/Umbraco.Core/Models/IDataValueEditor.cs index d733488be8..691ee70bf1 100644 --- a/src/Umbraco.Core/Models/IDataValueEditor.cs +++ b/src/Umbraco.Core/Models/IDataValueEditor.cs @@ -27,7 +27,6 @@ public interface IDataValueEditor /// bool SupportsReadOnly => false; - /// /// Gets the validators to use to validate the edited value. /// @@ -75,4 +74,6 @@ public interface IDataValueEditor XNode ConvertDbToXml(IPropertyType propertyType, object value); string ConvertDbToString(IPropertyType propertyType, object? value); + + IEnumerable ConfiguredElementTypeKeys() => Enumerable.Empty(); } diff --git a/src/Umbraco.Core/PropertyEditors/BlockListConfiguration.cs b/src/Umbraco.Core/PropertyEditors/BlockListConfiguration.cs index f398d75eac..7ebf2fd112 100644 --- a/src/Umbraco.Core/PropertyEditors/BlockListConfiguration.cs +++ b/src/Umbraco.Core/PropertyEditors/BlockListConfiguration.cs @@ -9,7 +9,7 @@ namespace Umbraco.Cms.Core.PropertyEditors; public class BlockListConfiguration { [ConfigurationField("blocks")] - public BlockConfiguration[] Blocks { get; set; } = null!; + public BlockConfiguration[] Blocks { get; set; } = Array.Empty(); [ConfigurationField("validationLimit")] public NumberRange ValidationLimit { get; set; } = new(); diff --git a/src/Umbraco.Core/PropertyEditors/DataEditor.cs b/src/Umbraco.Core/PropertyEditors/DataEditor.cs index f04e24c51b..3048162891 100644 --- a/src/Umbraco.Core/PropertyEditors/DataEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataEditor.cs @@ -75,6 +75,10 @@ public class DataEditor : IDataEditor [DataMember(Name = "supportsReadOnly", IsRequired = true)] public bool SupportsReadOnly { get; set; } + // Adding a virtual method that wraps the default implementation allows derived classes + // to override the default implementation without having to explicitly inherit the interface. + public virtual bool SupportsConfigurableElements => false; + /// [IgnoreDataMember] public bool IsDeprecated { get; } diff --git a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs index 5faa60507a..e9d131c75a 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs @@ -356,6 +356,10 @@ public class DataValueEditor : IDataValueEditor } } + // Adding a virtual method that wraps the default implementation allows derived classes + // to override the default implementation without having to explicitly inherit the interface. + public virtual IEnumerable ConfiguredElementTypeKeys() => Enumerable.Empty(); + /// /// Used to try to convert the string value to the correct CLR type based on the specified for /// this value editor. diff --git a/src/Umbraco.Core/PropertyEditors/IDataEditor.cs b/src/Umbraco.Core/PropertyEditors/IDataEditor.cs index 6db8049d23..5a95445fce 100644 --- a/src/Umbraco.Core/PropertyEditors/IDataEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/IDataEditor.cs @@ -16,6 +16,8 @@ public interface IDataEditor : IDiscoverable bool SupportsReadOnly => false; + bool SupportsConfigurableElements => false; + /// /// Gets a value indicating whether the editor is deprecated. /// diff --git a/src/Umbraco.Core/PropertyEditors/RichTextConfiguration.cs b/src/Umbraco.Core/PropertyEditors/RichTextConfiguration.cs index a0c76b1ffd..5ebcb13b5d 100644 --- a/src/Umbraco.Core/PropertyEditors/RichTextConfiguration.cs +++ b/src/Umbraco.Core/PropertyEditors/RichTextConfiguration.cs @@ -6,7 +6,7 @@ namespace Umbraco.Cms.Core.PropertyEditors; public class RichTextConfiguration : IIgnoreUserStartNodesConfig { [ConfigurationField("blocks")] - public RichTextBlockConfiguration[]? Blocks { get; set; } = null!; + public RichTextBlockConfiguration[]? Blocks { get; set; } = Array.Empty(); [ConfigurationField("mediaParentId")] public Guid? MediaParentId { get; set; } diff --git a/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingService.cs b/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingService.cs index 9c96fe64a4..e44b66b971 100644 --- a/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingService.cs +++ b/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingService.cs @@ -1,6 +1,9 @@ -using Umbraco.Cms.Core.Models; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.ContentTypeEditing; +using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Core.Strings; using Umbraco.Extensions; @@ -12,8 +15,24 @@ namespace Umbraco.Cms.Core.Services.ContentTypeEditing; internal sealed class ContentTypeEditingService : ContentTypeEditingServiceBase, IContentTypeEditingService { private readonly ITemplateService _templateService; + private readonly IElementSwitchValidator _elementSwitchValidator; private readonly IContentTypeService _contentTypeService; + public ContentTypeEditingService( + IContentTypeService contentTypeService, + ITemplateService templateService, + IDataTypeService dataTypeService, + IEntityService entityService, + IShortStringHelper shortStringHelper, + IElementSwitchValidator elementSwitchValidator) + : base(contentTypeService, contentTypeService, dataTypeService, entityService, shortStringHelper) + { + _contentTypeService = contentTypeService; + _templateService = templateService; + _elementSwitchValidator = elementSwitchValidator; + } + + [Obsolete("Use the constructor that is not marked obsolete, will be removed in v16")] public ContentTypeEditingService( IContentTypeService contentTypeService, ITemplateService templateService, @@ -24,6 +43,7 @@ internal sealed class ContentTypeEditingService : ContentTypeEditingServiceBase< { _contentTypeService = contentTypeService; _templateService = templateService; + _elementSwitchValidator = StaticServiceProvider.Instance.GetRequiredService(); } public async Task> CreateAsync(ContentTypeCreateModel model, Guid userKey) @@ -52,13 +72,20 @@ internal sealed class ContentTypeEditingService : ContentTypeEditingServiceBase< public async Task> UpdateAsync(IContentType contentType, ContentTypeUpdateModel model, Guid userKey) { - Attempt result = await ValidateAndMapForUpdateAsync(contentType, model); - if (result.Success is false) + // this needs to happen before the base call as that one is not a pure function + ContentTypeOperationStatus elementValidationStatus = await ValidateElementStatusForUpdateAsync(contentType, model); + if (elementValidationStatus is not ContentTypeOperationStatus.Success) { - return result; + return Attempt.Fail(elementValidationStatus); } - contentType = result.Result ?? throw new InvalidOperationException($"{nameof(ValidateAndMapForUpdateAsync)} succeeded but did not yield any result"); + Attempt baseValidationAttempt = await ValidateAndMapForUpdateAsync(contentType, model); + if (baseValidationAttempt.Success is false) + { + return baseValidationAttempt; + } + + contentType = baseValidationAttempt.Result ?? throw new InvalidOperationException($"{nameof(ValidateAndMapForUpdateAsync)} succeeded but did not yield any result"); UpdateHistoryCleanup(contentType, model); UpdateTemplates(contentType, model); @@ -77,6 +104,13 @@ internal sealed class ContentTypeEditingService : ContentTypeEditingServiceBase< bool isElement) => await FindAvailableCompositionsAsync(key, currentCompositeKeys, currentPropertyAliases, isElement); + protected override async Task AdditionalCreateValidationAsync( + ContentTypeEditingModelBase model) + { + // validate if the parent documentType (if set) has the same element status as the documentType being created + return await ValidateCreateParentElementStatusAsync(model); + } + // update content type history clean-up private void UpdateHistoryCleanup(IContentType contentType, ContentTypeModelBase model) { @@ -100,6 +134,48 @@ internal sealed class ContentTypeEditingService : ContentTypeEditingServiceBase< contentType.SetDefaultTemplate(allowedTemplates.FirstOrDefault(t => t.Key == model.DefaultTemplateKey)); } + private async Task ValidateElementStatusForUpdateAsync(IContentTypeBase contentType, ContentTypeModelBase model) + { + // no change, ignore rest of validation + if (contentType.IsElement == model.IsElement) + { + return ContentTypeOperationStatus.Success; + } + + // this method should only contain blocking validation, warnings are handled by WarnDocumentTypeElementSwitchNotificationHandler + + // => check whether the element was used in a block structure prior to updating + if (model.IsElement is false) + { + return await _elementSwitchValidator.ElementToDocumentNotUsedInBlockStructuresAsync(contentType) + ? ContentTypeOperationStatus.Success + : ContentTypeOperationStatus.InvalidElementFlagElementIsUsedInPropertyEditorConfiguration; + } + + return await _elementSwitchValidator.DocumentToElementHasNoContentAsync(contentType) + ? ContentTypeOperationStatus.Success + : ContentTypeOperationStatus.InvalidElementFlagDocumentHasContent; + } + + /// + /// Should be called after it has been established that the composition list is in a valid state and the (composition) parent exists + /// + private async Task ValidateCreateParentElementStatusAsync( + ContentTypeEditingModelBase model) + { + Guid? parentId = model.Compositions + .SingleOrDefault(composition => composition.CompositionType == CompositionType.Inheritance)?.Key; + if (parentId is null) + { + return ContentTypeOperationStatus.Success; + } + + IContentType? parent = await _contentTypeService.GetAsync(parentId.Value); + return parent!.IsElement == model.IsElement + ? ContentTypeOperationStatus.Success + : ContentTypeOperationStatus.InvalidElementFlagComparedToParent; + } + protected override IContentType CreateContentType(IShortStringHelper shortStringHelper, int parentId) => new ContentType(shortStringHelper, parentId); diff --git a/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs b/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs index 46c4508095..8e602a01f1 100644 --- a/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs +++ b/src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs @@ -91,6 +91,8 @@ internal abstract class ContentTypeEditingServiceBase(operationStatus, null); } + await AdditionalCreateValidationAsync(model); + // get the ID of the parent to create the content type under (we already validated that it exists) var parentId = GetParentId(model, containerKey) ?? throw new ArgumentException("Parent ID could not be found", nameof(model)); TContentType contentType = CreateContentType(_shortStringHelper, parentId); @@ -137,6 +139,10 @@ internal abstract class ContentTypeEditingServiceBase(ContentTypeOperationStatus.Success, contentType); } + protected virtual async Task AdditionalCreateValidationAsync( + ContentTypeEditingModelBase model) + => await Task.FromResult(ContentTypeOperationStatus.Success); + #region Sanitization private void SanitizeModelAliases(ContentTypeEditingModelBase model) diff --git a/src/Umbraco.Core/Services/ContentTypeEditing/ElementSwitchValidator.cs b/src/Umbraco.Core/Services/ContentTypeEditing/ElementSwitchValidator.cs new file mode 100644 index 0000000000..cc597b587f --- /dev/null +++ b/src/Umbraco.Core/Services/ContentTypeEditing/ElementSwitchValidator.cs @@ -0,0 +1,66 @@ +using Umbraco.Cms.Core.Extensions; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.PropertyEditors; + +namespace Umbraco.Cms.Core.Services.ContentTypeEditing; + +public class ElementSwitchValidator : IElementSwitchValidator +{ + private readonly IContentTypeService _contentTypeService; + private readonly PropertyEditorCollection _propertyEditorCollection; + private readonly IDataTypeService _dataTypeService; + + public ElementSwitchValidator( + IContentTypeService contentTypeService, + PropertyEditorCollection propertyEditorCollection, + IDataTypeService dataTypeService) + { + _contentTypeService = contentTypeService; + _propertyEditorCollection = propertyEditorCollection; + _dataTypeService = dataTypeService; + } + + public async Task AncestorsAreAlignedAsync(IContentType contentType) + { + // this call does not return the system roots + var ancestorIds = contentType.AncestorIds(); + if (ancestorIds.Length == 0) + { + // if there are no ancestors, validation passes + return true; + } + + // if there are any ancestors where IsElement is different from the contentType, the validation fails + return await Task.FromResult(_contentTypeService.GetAll(ancestorIds) + .Any(ancestor => ancestor.IsElement != contentType.IsElement) is false); + } + + public async Task DescendantsAreAlignedAsync(IContentType contentType) + { + IEnumerable descendants = _contentTypeService.GetDescendants(contentType.Id, false); + + // if there are any descendants where IsElement is different from the contentType, the validation fails + return await Task.FromResult(descendants.Any(descendant => descendant.IsElement != contentType.IsElement) is false); + } + + public async Task ElementToDocumentNotUsedInBlockStructuresAsync(IContentTypeBase contentType) + { + // get all propertyEditors that support block usage + IDataEditor[] editors = _propertyEditorCollection.Where(pe => pe.SupportsConfigurableElements).ToArray(); + var blockEditorAliases = editors.Select(pe => pe.Alias).ToArray(); + + // get all dataTypes that are based on those propertyEditors + IEnumerable dataTypes = await _dataTypeService.GetByEditorAliasAsync(blockEditorAliases); + + // if any dataType has a configuration where this element is selected as a possible block, the validation fails. + return dataTypes.Any(dataType => + editors.First(editor => editor.Alias == dataType.EditorAlias) + .GetValueEditor(dataType.ConfigurationObject) + .ConfiguredElementTypeKeys().Contains(contentType.Key)) is false; + } + + public async Task DocumentToElementHasNoContentAsync(IContentTypeBase contentType) => + + // if any content for the content type exists, the validation fails. + await Task.FromResult(_contentTypeService.HasContentNodes(contentType.Id) is false); +} diff --git a/src/Umbraco.Core/Services/ContentTypeEditing/IElementSwitchValidator.cs b/src/Umbraco.Core/Services/ContentTypeEditing/IElementSwitchValidator.cs new file mode 100644 index 0000000000..8816f99ec8 --- /dev/null +++ b/src/Umbraco.Core/Services/ContentTypeEditing/IElementSwitchValidator.cs @@ -0,0 +1,14 @@ +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Services.ContentTypeEditing; + +public interface IElementSwitchValidator +{ + Task AncestorsAreAlignedAsync(IContentType contentType); + + Task DescendantsAreAlignedAsync(IContentType contentType); + + Task ElementToDocumentNotUsedInBlockStructuresAsync(IContentTypeBase contentType); + + Task DocumentToElementHasNoContentAsync(IContentTypeBase contentType); +} diff --git a/src/Umbraco.Core/Services/DataTypeService.cs b/src/Umbraco.Core/Services/DataTypeService.cs index 5564802f3c..382238d583 100644 --- a/src/Umbraco.Core/Services/DataTypeService.cs +++ b/src/Umbraco.Core/Services/DataTypeService.cs @@ -330,6 +330,16 @@ namespace Umbraco.Cms.Core.Services.Implement return Task.FromResult(dataTypes); } + + /// + public async Task> GetByEditorAliasAsync(string[] propertyEditorAlias) + { + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => propertyEditorAlias.Contains(x.EditorAlias)); + IEnumerable dataTypes = _dataTypeRepository.Get(query).ToArray(); + ConvertMissingEditorsOfDataTypesToLabels(dataTypes); + return await Task.FromResult(dataTypes); + } /// public Task> GetByEditorUiAlias(string editorUiAlias) diff --git a/src/Umbraco.Core/Services/IDataTypeService.cs b/src/Umbraco.Core/Services/IDataTypeService.cs index 8aefd3b93f..8536c8a528 100644 --- a/src/Umbraco.Core/Services/IDataTypeService.cs +++ b/src/Umbraco.Core/Services/IDataTypeService.cs @@ -240,4 +240,11 @@ public interface IDataTypeService : IService /// The data type whose configuration to validate. /// One or more if the configuration data is invalid, an empty collection otherwise. IEnumerable ValidateConfigurationData(IDataType dataType); + + /// + /// Gets all for a set of property editors + /// + /// Aliases of the property editors + /// Collection of configured for the property editors + Task> GetByEditorAliasAsync(string[] propertyEditorAlias); } diff --git a/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs index 8b52d921ee..32f28eaf16 100644 --- a/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs @@ -21,4 +21,7 @@ public enum ContentTypeOperationStatus NotFound, NotAllowed, CancelledByNotification, + InvalidElementFlagDocumentHasContent, + InvalidElementFlagElementIsUsedInPropertyEditorConfiguration, + InvalidElementFlagComparedToParent, } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index e12c96557c..6201691d9f 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -407,6 +407,11 @@ public static partial class UmbracoBuilderExtensions builder .AddNotificationHandler(); + // Handlers for save warnings + builder + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler(); + return builder; } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditor.cs index 8b6ef4862c..6fe0e3882d 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditor.cs @@ -22,6 +22,7 @@ public class BlockGridPropertyEditor : BlockGridPropertyEditorBase : base(dataValueEditorFactory, blockValuePropertyIndexValueFactory) => _ioHelper = ioHelper; + public override bool SupportsConfigurableElements => true; #region Pre Value Editor diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs index bfd0e61e0b..254933eb8f 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs @@ -111,6 +111,12 @@ public abstract class BlockGridPropertyEditorBase : DataEditor return validationResults; } } + + public override IEnumerable ConfiguredElementTypeKeys() + { + var configuration = ConfigurationObject as BlockGridConfiguration; + return configuration?.Blocks.SelectMany(ConfiguredElementTypeKeys) ?? Enumerable.Empty(); + } } #endregion diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditor.cs index ca10cc8f76..bbb368dec1 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditor.cs @@ -36,6 +36,8 @@ public class BlockListPropertyEditor : BlockListPropertyEditorBase { } + public override bool SupportsConfigurableElements => true; + #region Pre Value Editor protected override IConfigurationEditor CreateConfigurationEditor() => diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs index 8d84254f16..2e37056cd2 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs @@ -93,6 +93,12 @@ public abstract class BlockListPropertyEditorBase : DataEditor return ValidateNumberOfBlocks(blockEditorData, validationLimit.Min, validationLimit.Max); } } + + public override IEnumerable ConfiguredElementTypeKeys() + { + var configuration = ConfigurationObject as BlockListConfiguration; + return configuration?.Blocks.SelectMany(ConfiguredElementTypeKeys) ?? Enumerable.Empty(); + } } #endregion diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index 39806b7a1b..f2178fd71a 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -114,6 +114,15 @@ public abstract class BlockValuePropertyValueEditorBase : DataV MapBlockItemDataToEditor(property, blockValue.SettingsData); } + protected IEnumerable ConfiguredElementTypeKeys(IBlockConfiguration configuration) + { + yield return configuration.ContentElementTypeKey; + if (configuration.SettingsElementTypeKey is not null) + { + yield return configuration.SettingsElementTypeKey.Value; + } + } + private void MapBlockItemDataToEditor(IProperty property, List items) { var valEditors = new Dictionary(); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs index 619e98743d..025bdd887c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs @@ -47,6 +47,8 @@ public class RichTextPropertyEditor : DataEditor public override IPropertyIndexValueFactory PropertyIndexValueFactory => _richTextPropertyIndexValueFactory; + public override bool SupportsConfigurableElements => true; + /// /// Create a custom value editor /// @@ -238,6 +240,12 @@ public class RichTextPropertyEditor : DataEditor return RichTextPropertyEditorHelper.SerializeRichTextEditorValue(cleanedUpRichTextEditorValue, _jsonSerializer); } + public override IEnumerable ConfiguredElementTypeKeys() + { + var configuration = ConfigurationObject as RichTextConfiguration; + return configuration?.Blocks?.SelectMany(ConfiguredElementTypeKeys) ?? Enumerable.Empty(); + } + private bool TryParseEditorValue(object? value, [NotNullWhen(true)] out RichTextEditorValue? richTextEditorValue) => RichTextPropertyEditorHelper.TryParseRichTextEditorValue(value, _jsonSerializer, _logger, out richTextEditorValue); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ElementSwitchValidatorTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ElementSwitchValidatorTests.cs new file mode 100644 index 0000000000..0b7ebd84c9 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ElementSwitchValidatorTests.cs @@ -0,0 +1,358 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Extensions; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; +using Umbraco.Cms.Tests.Common.Attributes; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +public class ElementSwitchValidatorTests : UmbracoIntegrationTest +{ + private IElementSwitchValidator ElementSwitchValidator => GetRequiredService(); + + private IContentTypeService ContentTypeService => GetRequiredService(); + + private IContentService ContentService => GetRequiredService(); + + private IDataTypeService DataTypeService => GetRequiredService(); + + [TestCase(new[] { true }, 0, true, true, TestName = "E=>E No Ancestor or children")] + [TestCase(new[] { false }, 0, false, true, TestName = "D=>D No Ancestor or children")] + [TestCase(new[] { true }, 0, false, true, TestName = "E=>D No Ancestor or children")] + [TestCase(new[] { false }, 0, true, true, TestName = "D=>E No Ancestor or children")] + [TestCase(new[] { true, true }, 1, true, true, TestName = "E Valid Parent")] + [TestCase(new[] { true, true }, 0, true, true, TestName = "E Valid Child")] + [TestCase(new[] { false, false }, 1, false, true, TestName = "D Valid Parent")] + [TestCase(new[] { false, false }, 0, false, true, TestName = "D Valid Child")] + [TestCase(new[] { false, false }, 1, true, false, TestName = "E InValid Parent")] + [TestCase(new[] { false, false }, 0, true, true, TestName = "E InValid Child")] + [TestCase(new[] { true, true }, 1, false, false, TestName = "D InValid Parent")] + [TestCase(new[] { true, true }, 0, false, true, TestName = "D InValid Child")] + [TestCase(new[] { true, false, false, true, false }, 2, true, false, + TestName = "D=>E InValid Child, Invalid Parent")] + [TestCase(new[] { false, true, false, true, false }, 2, true, false, + TestName = "D=>E InValid Child, Invalid Ancestor")] + [TestCase(new[] { true, false, false, true, true }, 2, true, false, + TestName = "D=>E Valid Children, Invalid Parent")] + [TestCase(new[] { false, true, false, true, true }, 2, true, false, + TestName = "D=>E Valid Children, Invalid Ancestor")] + [TestCase(new[] { false, false, false, false, false }, 2, true, false, TestName = "D=>E mismatch")] + [TestCase(new[] { false, false, true, false, false }, 2, false, true, TestName = "D=>E correction")] + [TestCase(new[] { true, true, true, true, true }, 2, false, false, TestName = "E=>D mismatch")] + [TestCase(new[] { true, true, false, true, true }, 2, true, true, TestName = "E=>D correction")] + [LongRunning] + public async Task AncestorsAreAligned( + bool[] isElementDoctypeChain, + int itemToTestIndex, + bool itemToTestNewIsElementValue, + bool validationShouldPass) + { + // Arrange + IContentType? parentItem = null; + IContentType? itemToTest = null; + for (var index = 0; index < isElementDoctypeChain.Length; index++) + { + var itemIsElement = isElementDoctypeChain[index]; + var builder = new ContentTypeBuilder() + .WithIsElement(itemIsElement); + if (parentItem is not null) + { + builder.WithParentContentType(parentItem); + } + + var contentType = builder.Build(); + await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + parentItem = contentType; + if (index == itemToTestIndex) + { + itemToTest = contentType; + } + } + + // Act + itemToTest!.IsElement = itemToTestNewIsElementValue; + var result = await ElementSwitchValidator.AncestorsAreAlignedAsync(itemToTest); + + // Assert + Assert.AreEqual(result, validationShouldPass); + } + + [TestCase(new[] { true }, 0, true, true, TestName = "E=>E No Ancestor or children")] + [TestCase(new[] { false }, 0, false, true, TestName = "D=>D No Ancestor or children")] + [TestCase(new[] { true }, 0, false, true, TestName = "E=>D No Ancestor or children")] + [TestCase(new[] { false }, 0, true, true, TestName = "D=>E No Ancestor or children")] + [TestCase(new[] { true, true }, 1, true, true, TestName = "E Valid Parent")] + [TestCase(new[] { true, true }, 0, true, true, TestName = "E Valid Child")] + [TestCase(new[] { false, false }, 1, false, true, TestName = "D Valid Parent")] + [TestCase(new[] { false, false }, 0, false, true, TestName = "D Valid Child")] + [TestCase(new[] { false, false }, 1, true, true, TestName = "E InValid Parent")] + [TestCase(new[] { false, false }, 0, true, false, TestName = "E InValid Child")] + [TestCase(new[] { true, true }, 1, false, true, TestName = "D InValid Parent")] + [TestCase(new[] { true, true }, 0, false, false, TestName = "D InValid Child")] + [TestCase(new[] { true, false, false, true, false }, 2, true, false, + TestName = "D=>E InValid Child, Invalid Parent")] + [TestCase(new[] { false, true, false, true, false }, 2, true, false, + TestName = "D=>E InValid Child, Invalid Ancestor")] + [TestCase(new[] { true, false, false, true, true }, 2, true, true, + TestName = "D=>E Valid Children, Invalid Parent")] + [TestCase(new[] { false, true, false, true, true }, 2, true, true, + TestName = "D=>E Valid Children, Invalid Ancestor")] + [TestCase(new[] { false, false, false, false, false }, 2, true, false, TestName = "D=>E mismatch")] + [TestCase(new[] { false, false, true, false, false }, 2, false, true, TestName = "D=>E correction")] + [TestCase(new[] { true, true, true, true, true }, 2, false, false, TestName = "E=>D mismatch")] + [TestCase(new[] { true, true, false, true, true }, 2, true, true, TestName = "E=>D correction")] + [LongRunning] + public async Task DescendantsAreAligned( + bool[] isElementDoctypeChain, + int itemToTestIndex, + bool itemToTestNewIsElementValue, + bool validationShouldPass) + { + // Arrange + IContentType? parentItem = null; + IContentType? itemToTest = null; + for (var index = 0; index < isElementDoctypeChain.Length; index++) + { + var itemIsElement = isElementDoctypeChain[index]; + var builder = new ContentTypeBuilder() + .WithIsElement(itemIsElement); + if (parentItem is not null) + { + builder.WithParentContentType(parentItem); + } + + var contentType = builder.Build(); + await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + parentItem = contentType; + if (index == itemToTestIndex) + { + itemToTest = contentType; + } + } + + // Act + itemToTest!.IsElement = itemToTestNewIsElementValue; + var result = await ElementSwitchValidator.DescendantsAreAlignedAsync(itemToTest); + + // Assert + Assert.AreEqual(result, validationShouldPass); + } + + [TestCase(0, true, TestName = "No Content")] + [TestCase(1, false, TestName = "One Content Item")] + [TestCase(5, false, TestName = "Many Content Items")] + public async Task DocumentToElementHasNoContent(int amountOfDocumentsCreated, bool validationShouldPass) + { + // Arrange + var contentType = await SetupContentType(false); + + for (int i = 0; i < amountOfDocumentsCreated; i++) + { + var contentBuilder = new ContentBuilder().WithContentType(contentType); + var content = contentBuilder.Build(); + ContentService.Save(content); + } + + // Act + contentType.IsElement = true; + var result = await ElementSwitchValidator.DocumentToElementHasNoContentAsync(contentType); + + // Assert + Assert.AreEqual(result, validationShouldPass); + } + + // Since the full permutation table would result in 64 tests and more block editors might be added later, + // we will at least test each single failure and a few combinations + // used in none + [TestCase(false, false, false, false, false, false, true)] + // used in one + [TestCase(true, false, false, false, false, false, false)] + [TestCase(false, true, false, false, false, false, false)] + [TestCase(false, false, true, false, false, false, false)] + [TestCase(false, false, false, true, false, false, false)] + [TestCase(false, false, false, false, true, false, false)] + [TestCase(false, false, false, false, false, true, false)] + // used in selection and setting + [TestCase(true, true, false, false, false, false, false)] + // used in 2 selections + [TestCase(true, false, true, false, false, false, false)] + // used in 2 settings + [TestCase(false, true, false, false, false, true, false)] + // used in all + [TestCase(true, true, true, true, true, true, false)] + public async Task ElementToDocumentNotUsedInBlockStructures( + bool isUsedInBlockList, + bool isUsedInBlockListBlockSetting, + bool isUsedInBlockGrid, + bool isUsedInBlockGridBlockSetting, + bool isUsedInRte, + bool isUsedInRteBlockSetting, + bool validationShouldPass) + { + // Arrange + var elementType = await SetupContentType(true); + + var otherElementType = await SetupContentType(true); + + if (isUsedInBlockList) + { + await SetupDataType(Constants.PropertyEditors.Aliases.BlockList, elementType.Key, null); + } + + if (isUsedInBlockListBlockSetting) + { + await SetupDataType(Constants.PropertyEditors.Aliases.BlockList, otherElementType.Key, elementType.Key); + } + + if (isUsedInBlockGrid) + { + await SetupDataType(Constants.PropertyEditors.Aliases.BlockGrid, elementType.Key, null); + } + + if (isUsedInBlockGridBlockSetting) + { + await SetupDataType(Constants.PropertyEditors.Aliases.BlockGrid, otherElementType.Key, elementType.Key); + } + + if (isUsedInRte) + { + await SetupDataType(Constants.PropertyEditors.Aliases.RichText, elementType.Key, null); + } + + if (isUsedInRteBlockSetting) + { + await SetupDataType(Constants.PropertyEditors.Aliases.RichText, otherElementType.Key, elementType.Key); + } + + // Act + var result = await ElementSwitchValidator.ElementToDocumentNotUsedInBlockStructuresAsync(elementType); + + // Assert + Assert.AreEqual(result, validationShouldPass); + } + + private async Task SetupContentType(bool isElement) + { + var typeBuilder = new ContentTypeBuilder() + .WithIsElement(isElement); + var contentType = typeBuilder.Build(); + await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + return contentType; + } + + // Constants.PropertyEditors.Aliases.BlockGrid + private async Task SetupDataType( + string editorAlias, + Guid elementKey, + Guid? elementSettingKey) + { + Dictionary configuration; + switch (editorAlias) + { + case Constants.PropertyEditors.Aliases.BlockGrid: + configuration = GetBlockGridBaseConfiguration(); + break; + case Constants.PropertyEditors.Aliases.RichText: + configuration = GetRteBaseConfiguration(); + break; + default: + configuration = new Dictionary(); + break; + } + + SetBlockConfiguration( + configuration, + elementKey, + elementSettingKey, + editorAlias == Constants.PropertyEditors.Aliases.BlockGrid ? true : null); + + + var dataTypeBuilder = new DataTypeBuilder() + .WithId(0) + .WithDatabaseType(ValueStorageType.Nvarchar) + .AddEditor() + .WithAlias(editorAlias); + + switch (editorAlias) + { + case Constants.PropertyEditors.Aliases.BlockGrid: + dataTypeBuilder.WithConfigurationEditor( + new BlockGridConfigurationEditor(IOHelper) { DefaultConfiguration = configuration }); + break; + case Constants.PropertyEditors.Aliases.BlockList: + dataTypeBuilder.WithConfigurationEditor( + new BlockListConfigurationEditor(IOHelper) { DefaultConfiguration = configuration }); + break; + case Constants.PropertyEditors.Aliases.RichText: + dataTypeBuilder.WithConfigurationEditor( + new RichTextConfigurationEditor(IOHelper) { DefaultConfiguration = configuration }); + break; + } + + var dataType = dataTypeBuilder.Done() + .Build(); + + await DataTypeService.CreateAsync(dataType, Constants.Security.SuperUserKey); + } + + private void SetBlockConfiguration( + Dictionary dictionary, + Guid? elementKey, + Guid? elementSettingKey, + bool? allowAtRoot) + { + if (elementKey is null) + { + return; + } + + dictionary["blocks"] = new[] { BuildBlockConfiguration(elementKey.Value, elementSettingKey, allowAtRoot) }; + } + + private Dictionary GetBlockGridBaseConfiguration() + => new Dictionary { ["gridColumns"] = 12 }; + + private Dictionary GetRteBaseConfiguration() + { + var dictionary = new Dictionary + { + ["maxImageSize"] = 500, + ["mode"] = "Classic", + ["toolbar"] = new[] + { + "styles", "bold", "italic", "alignleft", "aligncenter", "alignright", "bullist", "numlist", + "outdent", "indent", "sourcecode", "link", "umbmediapicker", "umbembeddialog" + }, + }; + return dictionary; + } + + private Dictionary BuildBlockConfiguration( + Guid? elementKey, + Guid? elementSettingKey, + bool? allowAtRoot) + { + var dictionary = new Dictionary(); + if (allowAtRoot is not null) + { + dictionary.Add("allowAtRoot", allowAtRoot.Value); + } + + dictionary.Add("contentElementTypeKey", elementKey.ToString()); + if (elementSettingKey is not null) + { + dictionary.Add("settingsElementTypeKey", elementSettingKey.ToString()); + } + + return dictionary; + } +}