From b7f424756cbc225456e45f40847019d8a17baa61 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:50:53 +0100 Subject: [PATCH] V15: Dont create invalid media (#17534) * Don't allow create when there is validation errors * Fix tests * Add tests * Fix last test * Fix more tests --------- Co-authored-by: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> --- .../Media/CreateMediaController.cs | 2 +- .../Services/MediaEditingService.cs | 8 ++- .../UmbracoIntegrationTestWithMediaEditing.cs | 2 + .../Services/MediaEditingServiceTests.cs | 60 +++++++++++++++++++ .../Services/MediaNavigationServiceTests.cs | 2 + .../Services/MediaListViewServiceTests.cs | 2 + 6 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaEditingServiceTests.cs diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/CreateMediaController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/CreateMediaController.cs index 0220493ee5..60a68a7ba3 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Media/CreateMediaController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Media/CreateMediaController.cs @@ -46,6 +46,6 @@ public class CreateMediaController : CreateMediaControllerBase return result.Success ? CreatedAtId(controller => nameof(controller.ByKey), result.Result.Content!.Key) - : ContentEditingOperationStatusResult(result.Status); + : MediaEditingOperationStatusResult(result.Status, requestModel, result.Result.ValidationResult); }); } diff --git a/src/Umbraco.Core/Services/MediaEditingService.cs b/src/Umbraco.Core/Services/MediaEditingService.cs index f771dd3460..6484d1112b 100644 --- a/src/Umbraco.Core/Services/MediaEditingService.cs +++ b/src/Umbraco.Core/Services/MediaEditingService.cs @@ -55,12 +55,18 @@ internal sealed class MediaEditingService ContentEditingOperationStatus validationStatus = result.Status; ContentValidationResult validationResult = result.Result.ValidationResult; + // If we have property validation errors, don't allow saving, as media only supports "published" status. + if (result.Status == ContentEditingOperationStatus.PropertyValidationError) + { + return Attempt.FailWithStatus(validationStatus, new MediaCreateResult { ValidationResult = validationResult }); + } + IMedia media = result.Result.Content!; var currentUserId = await GetUserIdAsync(userKey); ContentEditingOperationStatus operationStatus = Save(media, currentUserId); return operationStatus == ContentEditingOperationStatus.Success - ? Attempt.SucceedWithStatus(validationStatus, new MediaCreateResult { Content = media, ValidationResult = validationResult }) + ? Attempt.SucceedWithStatus(validationStatus, new MediaCreateResult { Content = media }) : Attempt.FailWithStatus(operationStatus, new MediaCreateResult { Content = media }); } diff --git a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithMediaEditing.cs b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithMediaEditing.cs index eee8ddc4ed..9f0ffdb307 100644 --- a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithMediaEditing.cs +++ b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithMediaEditing.cs @@ -70,6 +70,8 @@ public abstract class UmbracoIntegrationTestWithMediaEditing : UmbracoIntegratio var mediaTypes = MediaTypeService.GetAll(); var mediaTypesList = mediaTypes.ToList(); var imageMediaType = mediaTypesList.FirstOrDefault(x => x.Alias == "Image"); + imageMediaType.PropertyTypes.First().Mandatory = false; + MediaTypeService.Save(imageMediaType); // Add CustomMediaType to FolderMediaType AllowedContentTypes var mediaTypeUpdateHelper = new MediaTypeUpdateHelper(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaEditingServiceTests.cs new file mode 100644 index 0000000000..aaa4b12848 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaEditingServiceTests.cs @@ -0,0 +1,60 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; +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 MediaEditingServiceTests : UmbracoIntegrationTest +{ + protected IMediaTypeService MediaTypeService => GetRequiredService(); + + protected IMediaEditingService MediaEditingService => GetRequiredService(); + + protected IMediaType ImageMediaType { get; set; } + + [SetUp] + public async Task Setup() + { + ImageMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Image); + } + + [Test] + public async Task Cannot_Create_Media_With_Mandatory_Property() + { + var imageModel = CreateMediaCreateModel("Image", new Guid(), ImageMediaType.Key); + var imageCreateAttempt = await MediaEditingService.CreateAsync(imageModel, Constants.Security.SuperUserKey); + + // Assert + Assert.IsFalse(imageCreateAttempt.Success); + Assert.AreEqual(ContentEditingOperationStatus.PropertyValidationError, imageCreateAttempt.Status); + } + + [Test] + public async Task Can_Create_Media_Without_Mandatory_Property() + { + ImageMediaType.PropertyTypes.First(x => x.Alias == "umbracoFile").Mandatory = false; + MediaTypeService.Save(ImageMediaType); + var imageModel = CreateMediaCreateModel("Image", new Guid(), ImageMediaType.Key); + var imageCreateAttempt = await MediaEditingService.CreateAsync(imageModel, Constants.Security.SuperUserKey); + + // Assert + Assert.IsTrue(imageCreateAttempt.Success); + Assert.AreEqual(ContentEditingOperationStatus.Success, imageCreateAttempt.Status); + } + + private MediaCreateModel CreateMediaCreateModel(string name, Guid key, Guid mediaTypeKey) + => new() + { + ContentTypeKey = mediaTypeKey, + ParentKey = Constants.System.RootKey, + InvariantName = name, + Key = key, + }; +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.cs index 6a4761fa81..dc72facf28 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.cs @@ -21,6 +21,8 @@ public partial class MediaNavigationServiceTests : MediaNavigationServiceTestsBa // Media Types FolderMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Folder); ImageMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Image); + ImageMediaType.PropertyTypes.First(x => x.Alias == "umbracoFile").Mandatory = false; + MediaTypeService.Save(ImageMediaType); // Media var albumModel = CreateMediaCreateModel("Album", new Guid("1CD97C02-8534-4B72-AE9E-AE52EC94CF31"), FolderMediaType.Key); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaListViewServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaListViewServiceTests.cs index d5a66439a7..9868a507d0 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaListViewServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaListViewServiceTests.cs @@ -202,6 +202,8 @@ public class MediaListViewServiceTests : ContentListViewServiceTestsBase private async Task CreateRootMediaWithFiveChildrenAsListViewItems(Guid? listViewDataTypeKey = null) { var childImageMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Image); + childImageMediaType.PropertyTypes.First(x => x.Alias == "umbracoFile").Mandatory = false; + MediaTypeService.Save(childImageMediaType); var mediaTypeWithListView = new MediaTypeBuilder() .WithAlias("album")