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
Changes from 1 commit
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
110 changes: 95 additions & 15 deletions src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@

namespace Umbraco.Cms.Core.PropertyEditors;

// TODO (V17): Consider moving the notification handlers that are part of the is class to their own class, so we adhere
// better to the single responsibility principle.

[DataEditor(
Constants.PropertyEditors.Aliases.UploadField,
ValueEditorIsReusable = true)]
Expand All @@ -37,6 +40,7 @@
private readonly MediaFileManager _mediaFileManager;
private readonly UploadAutoFillProperties _uploadAutoFillProperties;
private readonly FileUploadValueParser _fileUploadValueParser;
private readonly IBlockEditorElementTypeCache _elementTypeCache;

private readonly BlockEditorValues<BlockListValue, BlockListLayoutItem> _blockListEditorValues;
private readonly BlockEditorValues<BlockGridValue, BlockGridLayoutItem> _blockGridEditorValues;
Expand All @@ -61,7 +65,8 @@
ioHelper,
StaticServiceProvider.Instance.GetRequiredService<IBlockEditorElementTypeCache>(),
StaticServiceProvider.Instance.GetRequiredService<IJsonSerializer>(),
StaticServiceProvider.Instance.GetRequiredService<ILogger<FileUploadPropertyEditor>>())
StaticServiceProvider.Instance.GetRequiredService<ILogger<FileUploadPropertyEditor>>(),
StaticServiceProvider.Instance.GetRequiredService<IBlockEditorElementTypeCache>())
{
}

Expand All @@ -77,7 +82,8 @@
IIOHelper ioHelper,
IBlockEditorElementTypeCache blockEditorElementTypeCache,
IJsonSerializer jsonSerializer,
ILogger<FileUploadPropertyEditor> logger)
ILogger<FileUploadPropertyEditor> logger,
IBlockEditorElementTypeCache elementTypeCache)
: base(dataValueEditorFactory)
{
_mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager));
Expand All @@ -86,6 +92,7 @@
_contentService = contentService;
_ioHelper = ioHelper;
_jsonSerializer = jsonSerializer;
_elementTypeCache = elementTypeCache;

SupportsReadOnly = true;

Expand Down Expand Up @@ -114,15 +121,15 @@
/// <inheritdoc/>
public void Handle(ContentCopiedNotification notification)
{
ArgumentNullException.ThrowIfNull(notification);

var isUpdated = false;

foreach (IProperty property in notification.Original.Properties)
{
if (IsUploadFieldPropertyType(property.PropertyType))
{
isUpdated |= UpdateUploadFieldProperty(notification, property);

Check notice on line 132 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Conditional

Handle no longer has a complex conditional. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.

continue;
}
Expand All @@ -140,6 +147,13 @@

continue;
}

if (IsRichTextPropertyType(property.PropertyType))
{
// TODO: Handle rich text properties with blocks.

continue;

Check warning on line 155 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

Handle has a cyclomatic complexity of 11, 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.
}
}

// if updated, re-save the copy with the updated value
Expand Down Expand Up @@ -197,31 +211,31 @@
return isUpdated;
}

private (bool, string?) UpdateBlockEditorData<TValue, TLayout>(ContentCopiedNotification notification, BlockEditorData<TValue, TLayout>? blockEditorData)
where TValue : BlockValue<TLayout>, new()
where TLayout : class, IBlockLayoutItem, new()
{
var isUpdated = false;

if (blockEditorData == null)
{
return (isUpdated, null);
}

IEnumerable<BlockPropertyValue> blockPropertyValues = blockEditorData.BlockValue.ContentData
.Concat(blockEditorData.BlockValue.SettingsData)
.SelectMany(x => x.Values);

foreach (BlockPropertyValue blockPropertyValue in blockPropertyValues)
{
if (blockPropertyValue.Value == null)
{
continue;
}

IPropertyType? propertyType = blockPropertyValue.PropertyType;

if (propertyType == null)

Check notice on line 238 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Conditional

GetFilePathsFromPropertyValues no longer has a complex conditional. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.
{
continue;
}
Expand Down Expand Up @@ -263,15 +277,15 @@

private bool UpdateUploadFieldBlockPropertyValue(BlockPropertyValue blockItemDataValue, ContentCopiedNotification notification, IPropertyType propertyType)
{
FileUploadValue? originalValue = _fileUploadValueParser.Parse(blockItemDataValue.Value);
FileUploadValue? fileUploadValue = _fileUploadValueParser.Parse(blockItemDataValue.Value);

// if original value is empty, we do not need to copy file
if (string.IsNullOrWhiteSpace(originalValue?.Src))
if (string.IsNullOrWhiteSpace(fileUploadValue?.Src))
{
return false;
}

var copyFileUrl = CopyFile(originalValue.Src, notification.Copy, propertyType);
var copyFileUrl = CopyFile(fileUploadValue.Src, notification.Copy, propertyType);

blockItemDataValue.Value = copyFileUrl;

Expand Down Expand Up @@ -362,39 +376,46 @@
}

/// <summary>
/// The paths to all file upload property files contained within a collection of content entities
/// The paths to all file upload property files contained within a collection of content entities.
/// </summary>
/// <param name="entities"></param>
private IReadOnlyList<string> ContainedFilePaths(IEnumerable<IContentBase> entities)
{
var paths = new List<string>();

foreach (IProperty? property in entities.SelectMany(x => x.Properties))
{
if (IsUploadFieldPropertyType(property.PropertyType))
{
paths.AddRange(GetPathsFromUploadFieldProperty(property));

continue;
}

if (IsBlockListPropertyType(property.PropertyType))
{
paths.AddRange(GetPathsFromBlockProperty(property, _blockListEditorValues));

continue;
}

if (IsBlockGridPropertyType(property.PropertyType))
{
paths.AddRange(GetPathsFromBlockProperty(property, _blockGridEditorValues));

continue;
}

if (IsRichTextPropertyType(property.PropertyType))
{
paths.AddRange(GetPathsFromRichTextProperty(property));

continue;
}
}

return paths.Distinct().ToList().AsReadOnly();
}

Check warning on line 418 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

ContainedFilePaths has a cyclomatic complexity of 10, 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.

private IEnumerable<string> GetPathsFromUploadFieldProperty(IProperty property)
{
Expand All @@ -420,73 +441,71 @@

foreach (IPropertyValue blockPropertyValue in property.Values)
{
paths.AddRange(GetPathsFromBlockEditorData(GetBlockEditorData(blockPropertyValue.PublishedValue, blockEditorValues)));
paths.AddRange(GetPathsFromBlockEditorData(GetBlockEditorData(blockPropertyValue.EditedValue, blockEditorValues)));
paths.AddRange(GetPathsFromBlockValue(GetBlockEditorData(blockPropertyValue.PublishedValue, blockEditorValues)?.BlockValue));
paths.AddRange(GetPathsFromBlockValue(GetBlockEditorData(blockPropertyValue.EditedValue, blockEditorValues)?.BlockValue));
}

return paths;
}

private IReadOnlyCollection<string> GetPathsFromBlockEditorData<TValue, TLayout>(BlockEditorData<TValue, TLayout>? blockEditorData)
where TValue : BlockValue<TLayout>, new()
where TLayout : class, IBlockLayoutItem, new()
private IReadOnlyCollection<string> GetPathsFromBlockValue(BlockValue? blockValue)
{
var paths = new List<string>();

if (blockEditorData == null)
if (blockValue is null)
{
return paths;
}

IEnumerable<BlockPropertyValue> blockPropertyValues = blockEditorData.BlockValue.ContentData
.Concat(blockEditorData.BlockValue.SettingsData)
IEnumerable<BlockPropertyValue> blockPropertyValues = blockValue.ContentData
.Concat(blockValue.SettingsData)
.SelectMany(x => x.Values);

foreach (BlockPropertyValue blockPropertyValue in blockPropertyValues)
{
if (blockPropertyValue.Value == null)
{
continue;
}

IPropertyType? propertyType = blockPropertyValue.PropertyType;

if (propertyType == null)
{
continue;
}

if (IsUploadFieldPropertyType(propertyType))
{
FileUploadValue? originalValue = _fileUploadValueParser.Parse(blockPropertyValue.Value);

if (string.IsNullOrWhiteSpace(originalValue?.Src))
{
continue;
}

paths.Add(_mediaFileManager.FileSystem.GetRelativePath(originalValue.Src));

continue;
}

if (IsBlockListPropertyType(propertyType))
{
paths.AddRange(GetPathsFromBlockPropertyValue(blockPropertyValue, _blockListEditorValues));

continue;
}

if (IsBlockGridPropertyType(propertyType))
{
paths.AddRange(GetPathsFromBlockPropertyValue(blockPropertyValue, _blockGridEditorValues));

continue;
}
}

return paths;
}

Check warning on line 508 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

GetPathsFromBlockValue has a cyclomatic complexity of 15, 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 508 in src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Bumpy Road Ahead

GetPathsFromBlockValue has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested 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.

private IReadOnlyCollection<string> GetPathsFromBlockPropertyValue<TValue, TLayout>(BlockPropertyValue blockItemDataValue, BlockEditorValues<TValue, TLayout> blockEditorValues)
where TValue : BlockValue<TLayout>, new()
Expand All @@ -494,7 +513,57 @@
{
BlockEditorData<TValue, TLayout>? blockItemEditorDataValue = GetBlockEditorData(blockItemDataValue.Value, blockEditorValues);

return GetPathsFromBlockEditorData(blockItemEditorDataValue);
return GetPathsFromBlockValue(blockItemEditorDataValue?.BlockValue);
}

private IReadOnlyCollection<string> GetPathsFromRichTextProperty(IProperty property)
{
var paths = new List<string>();

IPropertyValue? propertyValue = property.Values.FirstOrDefault();
if (propertyValue is null)
{
return paths;
}

paths.AddRange(GetPathsFromBlockValue(GetRichTextBlockValue(propertyValue.PublishedValue)));
paths.AddRange(GetPathsFromBlockValue(GetRichTextBlockValue(propertyValue.EditedValue)));

return paths;
}

private RichTextBlockValue? GetRichTextBlockValue(object? value)
{
if (value is null)
{
return null;
}

_jsonSerializer.TryDeserialize(value, out RichTextEditorValue? richTextEditorValue);
if (richTextEditorValue?.Blocks is null)
{
return null;
}

// Ensure the property type is populated on all blocks.
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);
}
}

return richTextEditorValue.Blocks;
}

#endregion
Expand Down Expand Up @@ -554,4 +623,15 @@
/// </returns>
private static bool IsBlockGridPropertyType(IPropertyType propertyType)
=> propertyType.PropertyEditorAlias == Constants.PropertyEditors.Aliases.BlockGrid;

/// <summary>
/// Gets a value indicating whether a property is an rich text field (supporting blocks).
/// </summary>
/// <param name="propertyType">The property type.</param>
/// <returns>
/// <c>true</c> if the specified property is an rich text field; otherwise, <c>false</c>.
/// </returns>
private static bool IsRichTextPropertyType(IPropertyType propertyType)
=> propertyType.PropertyEditorAlias == Constants.PropertyEditors.Aliases.RichText ||
propertyType.PropertyEditorAlias == "Umbraco.TinyMCE";
}
Loading