Skip to content

Add support for file upload property editor within the block list and grid #18976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
5841e38
Fix for https://github.com/umbraco/Umbraco-CMS/issues/18872
PeterKvayt Apr 8, 2025
7ea9270
Parsing added for current value
PeterKvayt Apr 8, 2025
c6fc1e5
Build fix.
PeterKvayt Apr 8, 2025
0ed2358
Cyclomatic complexity fix
PeterKvayt Apr 8, 2025
dd7eb0a
Merge branch 'contrib' into temp/18872
PeterKvayt Apr 9, 2025
7292d9d
Merge branch 'v15/dev' into contrib
AndyButland Apr 28, 2025
f01efa2
Merge branch 'contrib' into temp/18872
AndyButland Apr 28, 2025
10bde34
Resolved breaking change.
AndyButland Apr 28, 2025
5414937
Pass content key.
AndyButland Apr 28, 2025
bba1f21
Simplified collections.
AndyButland Apr 28, 2025
d164bcc
Merge branch 'contrib' of https://github.com/umbraco/Umbraco-CMS into…
AndyButland Apr 28, 2025
bbd2387
Merge branch 'contrib' into temp/18872
AndyButland Apr 28, 2025
5586531
Added unit tests to verify behaviour.
AndyButland Apr 28, 2025
e6e007e
Allow file upload on block list.
AndyButland Apr 28, 2025
ea06fb2
Added unit test verifying added property.
AndyButland Apr 29, 2025
e0f1c1b
Added unit test verifying removed property.
AndyButland Apr 29, 2025
468beb1
Restored null return for null value fixing failing integration tests.
AndyButland Apr 29, 2025
fd5c8bc
Merge branch 'main' into temp/18872
AndyButland May 22, 2025
a9f5e81
Merge branch 'umbraco:main' into temp/18872
PeterKvayt May 24, 2025
f3e928d
Logic has been updated according edge cases
PeterKvayt May 24, 2025
a53ddec
Logic to copy files from block list items has been added.
PeterKvayt May 24, 2025
54d1b7e
Logic to delete files from block list items on content deletion has b…
PeterKvayt May 24, 2025
8ecd279
Test fix.
PeterKvayt May 24, 2025
d2f45e8
Refactoring.
PeterKvayt May 25, 2025
430b90f
WIP: Resolved breaking changes, minor refactoring.
AndyButland May 26, 2025
270cd88
Consistently return null over empty, resolving failure in integration…
AndyButland May 26, 2025
b2fa3d9
Removed unnecessary code nesting.
AndyButland May 26, 2025
653a3c2
Handle distinct paths.
AndyButland May 26, 2025
d40a7cd
Handles clean up of files added via file upload in rich text blocks o…
AndyButland May 28, 2025
f016933
Update src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyE…
AndyButland May 28, 2025
7cd6f81
Merge branch 'main' into temp/18872
AndyButland May 29, 2025
fc5a257
Fixed build of integration tests project.
AndyButland May 29, 2025
579a812
Merge branch 'temp/18872' of https://github.com/PeterKvayt/Umbraco-CM…
AndyButland May 29, 2025
0d77fc6
Handled delete of file uploads when deleting a block from an RTE usin…
AndyButland May 29, 2025
0070a3f
Refactored ensure of property type property populated on rich text va…
AndyButland May 29, 2025
2211eb6
Fixed integration tests build.
AndyButland May 29, 2025
0af6e98
Handle create of new file from file upload block in an RTE when the d…
AndyButland May 29, 2025
0b77239
Fixed failing integration tests.
AndyButland May 29, 2025
4c8c9c9
Refactored notification handlers relating to file uploads into separa…
AndyButland May 30, 2025
71357ed
Handle nested rich text editor block with file upload when copying co…
AndyButland May 30, 2025
1808b0e
Handle nested rich text editor block with file upload when deleting c…
AndyButland May 30, 2025
9492892
Minor refactor.
AndyButland May 30, 2025
831b34d
Merge branch 'umbraco:main' into temp/18872
PeterKvayt Jun 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
using Umbraco.Cms.Infrastructure.Migrations.Install;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.Mappers;
using Umbraco.Cms.Infrastructure.PropertyEditors.NotificationHandlers;
using Umbraco.Cms.Infrastructure.Routing;
using Umbraco.Cms.Infrastructure.Runtime;
using Umbraco.Cms.Infrastructure.Runtime.RuntimeModeValidators;
Expand Down Expand Up @@ -352,11 +353,11 @@ public static IUmbracoBuilder AddCoreNotifications(this IUmbracoBuilder builder)
.AddNotificationHandler<ContentSavingNotification, BlockGridPropertyNotificationHandler>()
.AddNotificationHandler<ContentCopyingNotification, BlockGridPropertyNotificationHandler>()
.AddNotificationHandler<ContentScaffoldedNotification, BlockGridPropertyNotificationHandler>()
.AddNotificationHandler<ContentCopiedNotification, FileUploadPropertyEditor>()
.AddNotificationHandler<ContentDeletedNotification, FileUploadPropertyEditor>()
.AddNotificationHandler<MediaDeletedNotification, FileUploadPropertyEditor>()
.AddNotificationHandler<MediaSavingNotification, FileUploadPropertyEditor>()
.AddNotificationHandler<MemberDeletedNotification, FileUploadPropertyEditor>()
.AddNotificationHandler<ContentCopiedNotification, FileUploadContentCopiedNotificationHandler>()
.AddNotificationHandler<ContentDeletedNotification, FileUploadContentDeletedNotificationHandler>()
.AddNotificationHandler<MediaDeletedNotification, FileUploadMediaDeletedNotificationHandler>()
.AddNotificationHandler<MediaSavingNotification, FileUploadMediaSavingNotificationHandler>()
.AddNotificationHandler<MemberDeletedNotification, FileUploadMemberDeletedNotificationHandler>()
.AddNotificationHandler<ContentCopiedNotification, ImageCropperPropertyEditor>()
.AddNotificationHandler<ContentDeletedNotification, ImageCropperPropertyEditor>()
.AddNotificationHandler<MediaDeletedNotification, ImageCropperPropertyEditor>()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache.PropertyEditors;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Blocks;

namespace Umbraco.Cms.Infrastructure.Extensions;

/// <summary>
/// Defines extensions on <see cref="RichTextEditorValue"/>.
/// </summary>
internal static class RichTextEditorValueExtensions
{
/// <summary>
/// Ensures that the property type property is populated on all blocks.
/// </summary>
/// <param name="richTextEditorValue">The <see cref="RichTextEditorValue"/> providing the blocks.</param>
/// <param name="elementTypeCache">Cache for element types.</param>
public static void EnsurePropertyTypePopulatedOnBlocks(this RichTextEditorValue richTextEditorValue, IBlockEditorElementTypeCache elementTypeCache)
{
Guid[] elementTypeKeys = (richTextEditorValue.Blocks?.ContentData ?? [])
.Select(x => x.ContentTypeKey)
.Union((richTextEditorValue.Blocks?.SettingsData ?? [])
.Select(x => x.ContentTypeKey))
.Distinct()
.ToArray();

IEnumerable<IContentType> elementTypes = elementTypeCache.GetMany(elementTypeKeys);

foreach (BlockItemData dataItem in (richTextEditorValue.Blocks?.ContentData ?? [])
.Union(richTextEditorValue.Blocks?.SettingsData ?? []))
{
foreach (BlockPropertyValue item in dataItem.Values)
{
item.PropertyType = elementTypes.FirstOrDefault(x => x.Key == dataItem.ContentTypeKey)?.PropertyTypes.FirstOrDefault(pt => pt.Alias == item.Alias);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using System.Diagnostics.CodeAnalysis;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Blocks;
Expand All @@ -16,10 +14,16 @@

namespace Umbraco.Cms.Core.PropertyEditors;

/// <summary>
/// Provides an abstract base class for property value editors based on block editors.
/// </summary>
public abstract class BlockEditorPropertyValueEditor<TValue, TLayout> : BlockValuePropertyValueEditorBase<TValue, TLayout>
where TValue : BlockValue<TLayout>, new()
where TLayout : class, IBlockLayoutItem, new()
{
/// <summary>
/// Initializes a new instance of the <see cref="BlockEditorPropertyValueEditor{TValue, TLayout}"/> class.
/// </summary>
protected BlockEditorPropertyValueEditor(
PropertyEditorCollection propertyEditors,
DataValueReferenceFactoryCollection dataValueReferenceFactories,
Expand Down Expand Up @@ -62,13 +66,7 @@ public override IEnumerable<ITag> GetTags(object? value, object? dataTypeConfigu
return BlockEditorValues.DeserializeAndClean(rawJson)?.BlockValue;
}

/// <summary>
/// Ensure that sub-editor values are translated through their ToEditor methods
/// </summary>
/// <param name="property"></param>
/// <param name="culture"></param>
/// <param name="segment"></param>
/// <returns></returns>
/// <inheritdoc />
public override object ToEditor(IProperty property, string? culture = null, string? segment = null)
{
var val = property.GetValue(culture, segment);
Expand All @@ -95,38 +93,48 @@ public override object ToEditor(IProperty property, string? culture = null, stri
return blockEditorData.BlockValue;
}

/// <summary>
/// Ensure that sub-editor values are translated through their FromEditor methods
/// </summary>
/// <param name="editorValue"></param>
/// <param name="currentValue"></param>
/// <returns></returns>
/// <inheritdoc />
public override object? FromEditor(ContentPropertyData editorValue, object? currentValue)
{
if (editorValue.Value == null || string.IsNullOrWhiteSpace(editorValue.Value.ToString()))
// Note: we can't early return here if editorValue is null or empty, because these is the following case:
// - current value not null (which means doc has at least one element in block list)
// - editor value (new value) is null (which means doc has no elements in block list)
// If we check editor value for null value and return before MapBlockValueFromEditor, then we will not trigger updates for properties.
// For most of the properties this is fine, but for properties which contain other state it might be critical (e.g. file upload field).
// So, we must run MapBlockValueFromEditor even if editorValue is null or string.IsNullOrWhiteSpace(editorValue.Value.ToString()) is true.

BlockEditorData<TValue, TLayout>? currentBlockEditorData = GetBlockEditorData(currentValue);
BlockEditorData<TValue, TLayout>? blockEditorData = GetBlockEditorData(editorValue.Value);

// We can skip MapBlockValueFromEditor if both editorValue and currentValue values are empty.
if (IsBlockEditorDataEmpty(currentBlockEditorData) && IsBlockEditorDataEmpty(blockEditorData))
{
return null;
}

BlockEditorData<TValue, TLayout>? blockEditorData;
try
MapBlockValueFromEditor(blockEditorData?.BlockValue, currentBlockEditorData?.BlockValue, editorValue.ContentKey);

if (IsBlockEditorDataEmpty(blockEditorData))
{
blockEditorData = BlockEditorValues.DeserializeAndClean(editorValue.Value);
return null;
}
catch

return JsonSerializer.Serialize(blockEditorData.BlockValue);
}

private BlockEditorData<TValue, TLayout>? GetBlockEditorData(object? value)
{
try
{
// if this occurs it means the data is invalid, shouldn't happen but has happened if we change the data format.
return string.Empty;
return BlockEditorValues.DeserializeAndClean(value);
}

if (blockEditorData == null || blockEditorData.BlockValue.ContentData.Count == 0)
catch
{
return string.Empty;
// If this occurs it means the data is invalid. It shouldn't happen could if we change the data format.
return null;
}

MapBlockValueFromEditor(blockEditorData.BlockValue);

// return json
return JsonSerializer.Serialize(blockEditorData.BlockValue);
}

private static bool IsBlockEditorDataEmpty([NotNullWhen(false)] BlockEditorData<TValue, TLayout>? editorData)
=> editorData is null || editorData.BlockValue.ContentData.Count == 0;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Umbraco.Cms.Core.Cache.PropertyEditors;
using Umbraco.Cms.Core.Cache.PropertyEditors;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Blocks;
using Umbraco.Cms.Core.Models.Validation;
Expand Down Expand Up @@ -88,7 +88,13 @@

foreach (var group in itemDataGroups)
{
var allElementTypes = _elementTypeCache.GetMany(group.Items.Select(x => x.ContentTypeKey).ToArray()).ToDictionary(x => x.Key);
Guid[] elementTypeKeys = group.Items.Select(x => x.ContentTypeKey).ToArray();
if (elementTypeKeys.Length == 0)
{
continue;
}

var allElementTypes = _elementTypeCache.GetMany(elementTypeKeys).ToDictionary(x => x.Key);

Check warning on line 97 in src/Umbraco.Infrastructure/PropertyEditors/BlockEditorValidatorBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method

GetBlockEditorDataValidation increases in cyclomatic complexity from 24 to 26, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 97 in src/Umbraco.Infrastructure/PropertyEditors/BlockEditorValidatorBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Bumpy Road Ahead

GetBlockEditorDataValidation increases from 4 to 5 logical blocks with deeply nested code, threshold is one single block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

for (var i = 0; i < group.Items.Length; i++)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Umbraco.Cms.Core.Cache;

Check notice on line 1 in src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Code Duplication

The module no longer contains too many functions with similar structure

Check notice on line 1 in src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 33.33% to 32.35%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Blocks;
Expand Down Expand Up @@ -129,10 +129,115 @@
return result;
}

protected void MapBlockValueFromEditor(TValue blockValue)
[Obsolete("This method is no longer used within Umbraco. Please use the overload taking all parameters. Scheduled for removal in Umbraco 17.")]
protected void MapBlockValueFromEditor(TValue blockValue) => MapBlockValueFromEditor(blockValue, null, Guid.Empty);

protected void MapBlockValueFromEditor(TValue? editedBlockValue, TValue? currentBlockValue, Guid contentKey)
{
MapBlockItemDataFromEditor(
editedBlockValue?.ContentData ?? [],
currentBlockValue?.ContentData ?? [],
contentKey);

MapBlockItemDataFromEditor(
editedBlockValue?.SettingsData ?? [],
currentBlockValue?.SettingsData ?? [],
contentKey);
}

private void MapBlockItemDataFromEditor(List<BlockItemData> editedItems, List<BlockItemData> currentItems, Guid contentKey)
{
// Create mapping between edited and current block items.
IEnumerable<BlockStateMapping<BlockItemData>> itemsMapping = GetBlockStatesMapping(editedItems, currentItems, (mapping, current) => mapping.Edited?.Key == current.Key);

foreach (BlockStateMapping<BlockItemData> itemMapping in itemsMapping)
{
// Create mapping between edited and current block item values.
IEnumerable<BlockStateMapping<BlockPropertyValue>> valuesMapping = GetBlockStatesMapping(itemMapping.Edited?.Values, itemMapping.Current?.Values, (mapping, current) => mapping.Edited?.Alias == current.Alias);

foreach (BlockStateMapping<BlockPropertyValue> valueMapping in valuesMapping)
{
BlockPropertyValue? editedValue = valueMapping.Edited;
BlockPropertyValue? currentValue = valueMapping.Current;

IPropertyType propertyType = editedValue?.PropertyType
?? currentValue?.PropertyType
?? throw new ArgumentException("One or more block properties did not have a resolved property type. Block editor values must be resolved before attempting to map them from editor.", nameof(editedItems));

// Lookup the property editor.
IDataEditor? propertyEditor = _propertyEditors[propertyType.PropertyEditorAlias];
if (propertyEditor is null)
{
continue;
}

// Fetch the property types prevalue.
var configuration = _dataTypeConfigurationCache.GetConfiguration(propertyType.DataTypeKey);

// Create a real content property data object.
var propertyData = new ContentPropertyData(editedValue?.Value, configuration)
{
ContentKey = contentKey,
PropertyTypeKey = propertyType.Key,
};

// Get the property editor to do it's conversion.
IDataValueEditor valueEditor = propertyEditor.GetValueEditor();
var newValue = valueEditor.FromEditor(propertyData, currentValue?.Value);

// Update the raw value since this is what will get serialized out.
if (editedValue != null)
{
editedValue.Value = newValue;
}
}
}
}

private sealed class BlockStateMapping<T>
{
MapBlockItemDataFromEditor(blockValue.ContentData);
MapBlockItemDataFromEditor(blockValue.SettingsData);
public T? Edited { get; set; }

public T? Current { get; set; }
}

private static IEnumerable<BlockStateMapping<T>> GetBlockStatesMapping<T>(IList<T>? editedItems, IList<T>? currentItems, Func<BlockStateMapping<T>, T, bool> condition)
{
// filling with edited items first
List<BlockStateMapping<T>> mapping = editedItems?
.Select(editedItem => new BlockStateMapping<T>
{
Current = default,
Edited = editedItem,
})
.ToList()
?? [];

if (currentItems is null)
{
return mapping;
}

// then adding current items
foreach (T currentItem in currentItems)
{
BlockStateMapping<T>? mappingItem = mapping.FirstOrDefault(x => condition(x, currentItem));

if (mappingItem == null) // if there is no edited item, then adding just current
{
mapping.Add(new BlockStateMapping<T>
{
Current = currentItem,
Edited = default,
});
}
else
{
mappingItem.Current = currentItem;
}
}

return mapping;
}

protected void MapBlockValueToEditor(IProperty property, TValue blockValue, string? culture, string? segment)
Expand Down Expand Up @@ -197,40 +302,6 @@
}
}

private void MapBlockItemDataFromEditor(List<BlockItemData> items)
{
foreach (BlockItemData item in items)
{
foreach (BlockPropertyValue blockPropertyValue in item.Values)
{
IPropertyType? propertyType = blockPropertyValue.PropertyType;
if (propertyType is null)
{
throw new ArgumentException("One or more block properties did not have a resolved property type. Block editor values must be resolved before attempting to map them from editor.", nameof(items));
}

// Lookup the property editor
IDataEditor? propertyEditor = _propertyEditors[propertyType.PropertyEditorAlias];
if (propertyEditor is null)
{
continue;
}

// Fetch the property types prevalue
var configuration = _dataTypeConfigurationCache.GetConfiguration(propertyType.DataTypeKey);

// Create a fake content property data object
var propertyData = new ContentPropertyData(blockPropertyValue.Value, configuration);

// Get the property editor to do it's conversion
var newValue = propertyEditor.GetValueEditor().FromEditor(propertyData, blockPropertyValue.Value);

// update the raw value since this is what will get serialized out
blockPropertyValue.Value = newValue;
}
}
}

/// <summary>
/// Updates the invariant data in the source with the invariant data in the value if allowed
/// </summary>
Expand Down
Loading
Loading