From 0af73b0290432e63a461bf8555dc1c205cc37252 Mon Sep 17 00:00:00 2001 From: Mads Krohn Date: Thu, 12 Sep 2013 00:14:44 +0200 Subject: [PATCH 1/3] Fixed U4-2842 Umb 6.1.5 - moving a media item from recycle bin / trash --- src/Umbraco.Core/Services/MediaService.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 68b4253f77..fbf6d0f67b 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -479,16 +479,22 @@ namespace Umbraco.Core.Services if (Moving.IsRaisedEventCancelled(new MoveEventArgs(media, parentId), this)) return; - media.ParentId = parentId; + //If we aren't moving to recycle bin, we must be moving from recycle bin -> #U4-2842 + media.ChangeTrashedState(false, parentId); Save(media, userId); //Ensure that Path and Level is updated on children - var children = GetChildren(media.Id); + var children = GetChildren(media.Id).ToList(); //No need to enumerate twice? if (children.Any()) { var parentPath = media.Path; var parentLevel = media.Level; - var updatedDescendents = UpdatePathAndLevelOnChildren(children, parentPath, parentLevel); + var updatedDescendents = UpdatePathAndLevelOnChildren(children, parentPath, parentLevel).ToList(); + //Update trashed state for descendants -> #U4-2842 + foreach (var descendant in updatedDescendents) + { + descendant.ChangeTrashedState(false, descendant.ParentId); + } Save(updatedDescendents, userId); } @@ -910,7 +916,7 @@ namespace Umbraco.Core.Services child.Level = parentLevel + 1; list.Add(child); - var grandkids = GetChildren(child.Id); + var grandkids = GetChildren(child.Id).ToList(); //No need to enumerate twice? if (grandkids.Any()) { list.AddRange(UpdatePathAndLevelOnChildren(grandkids, child.Path, child.Level)); From 31f146fc6f1f62916041f2a58ff6826280069182 Mon Sep 17 00:00:00 2001 From: Mads Krohn Date: Fri, 13 Sep 2013 10:29:03 +0200 Subject: [PATCH 2/3] Updates to the media trashed state handling Tweaked the logic for determining when a media item is trashed or not. Added unit tests. Removed unnecessary comments. --- src/Umbraco.Core/Services/MediaService.cs | 33 +++++---- src/Umbraco.Tests/Services/BaseServiceTest.cs | 24 +++++++ .../Services/MediaServiceTests.cs | 72 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 4 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 src/Umbraco.Tests/Services/MediaServiceTests.cs diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index fbf6d0f67b..c45bbb1353 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -479,23 +479,22 @@ namespace Umbraco.Core.Services if (Moving.IsRaisedEventCancelled(new MoveEventArgs(media, parentId), this)) return; - //If we aren't moving to recycle bin, we must be moving from recycle bin -> #U4-2842 - media.ChangeTrashedState(false, parentId); + media.ParentId = parentId; + if (media.Trashed) + { + media.ChangeTrashedState(false, parentId); + } Save(media, userId); - //Ensure that Path and Level is updated on children - var children = GetChildren(media.Id).ToList(); //No need to enumerate twice? + //Ensure that relevant properties are updated on children + var children = GetChildren(media.Id); if (children.Any()) { var parentPath = media.Path; var parentLevel = media.Level; - var updatedDescendents = UpdatePathAndLevelOnChildren(children, parentPath, parentLevel).ToList(); - //Update trashed state for descendants -> #U4-2842 - foreach (var descendant in updatedDescendents) - { - descendant.ChangeTrashedState(false, descendant.ParentId); - } - Save(updatedDescendents, userId); + var parentTrashed = media.Trashed; + var updatedDescendants = UpdatePropertiesOnChildren(children, parentPath, parentLevel, parentTrashed); + Save(updatedDescendants, userId); } Moved.RaiseEvent(new MoveEventArgs(media, false, parentId), this); @@ -901,25 +900,31 @@ namespace Umbraco.Core.Services /// /// Updates the Path and Level on a collection of objects - /// based on the Parent's Path and Level. + /// based on the Parent's Path and Level. Also change the trashed state if relevant. /// /// Collection of objects to update /// Path of the Parent media /// Level of the Parent media + /// Indicates whether the Parent is trashed or not /// Collection of updated objects - private IEnumerable UpdatePathAndLevelOnChildren(IEnumerable children, string parentPath, int parentLevel) + private IEnumerable UpdatePropertiesOnChildren(IEnumerable children, string parentPath, int parentLevel, bool parentTrashed) { var list = new List(); foreach (var child in children) { child.Path = string.Concat(parentPath, ",", child.Id); child.Level = parentLevel + 1; + if (parentTrashed != child.Trashed) + { + child.ChangeTrashedState(parentTrashed, child.ParentId); + } + list.Add(child); var grandkids = GetChildren(child.Id).ToList(); //No need to enumerate twice? if (grandkids.Any()) { - list.AddRange(UpdatePathAndLevelOnChildren(grandkids, child.Path, child.Level)); + list.AddRange(UpdatePropertiesOnChildren(grandkids, child.Path, child.Level, child.Trashed)); } } return list; diff --git a/src/Umbraco.Tests/Services/BaseServiceTest.cs b/src/Umbraco.Tests/Services/BaseServiceTest.cs index cd4318b5a6..23487eb582 100644 --- a/src/Umbraco.Tests/Services/BaseServiceTest.cs +++ b/src/Umbraco.Tests/Services/BaseServiceTest.cs @@ -51,6 +51,30 @@ namespace Umbraco.Tests.Services Content trashed = MockedContent.CreateSimpleContent(contentType, "Text Page Deleted", -20); trashed.Trashed = true; ServiceContext.ContentService.Save(trashed, 0); + + //Create and Save folder-Media -> 1050 + var folderMediaType = ServiceContext.ContentTypeService.GetMediaType(1031); + var folder = MockedMedia.CreateMediaFolder(folderMediaType, -1); + ServiceContext.MediaService.Save(folder); + + //Create and Save folder-Media -> 1051 + var folder2 = MockedMedia.CreateMediaFolder(folderMediaType, -1); + ServiceContext.MediaService.Save(folder2); + + //Create and Save image-Media -> 1052 + var imageMediaType = ServiceContext.ContentTypeService.GetMediaType(1032); + var image = (Media)MockedMedia.CreateMediaImage(imageMediaType, 1050); + ServiceContext.MediaService.Save(image); + + //Create and Save folder-Media that is trashed -> 1053 + var folderTrashed = (Media)MockedMedia.CreateMediaFolder(folderMediaType, -21); + folderTrashed.Trashed = true; + ServiceContext.MediaService.Save(folderTrashed); + + //Create and Save image-Media child of folderTrashed -> 1054 + var imageTrashed = (Media)MockedMedia.CreateMediaImage(imageMediaType, folderTrashed.Id); + imageTrashed.Trashed = true; + ServiceContext.MediaService.Save(imageTrashed); } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/Services/MediaServiceTests.cs b/src/Umbraco.Tests/Services/MediaServiceTests.cs new file mode 100644 index 0000000000..658bff9de9 --- /dev/null +++ b/src/Umbraco.Tests/Services/MediaServiceTests.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using NUnit.Framework; + +namespace Umbraco.Tests.Services +{ + [TestFixture, RequiresSTA] + public class MediaServiceTests : BaseServiceTest + { + [SetUp] + public override void Initialize() + { + base.Initialize(); + } + + [TearDown] + public override void TearDown() + { + base.TearDown(); + } + + [Test] + public void Can_Move_Media() + { + // Arrange + var mediaService = ServiceContext.MediaService; + var media = mediaService.GetById(1052); + + // Act + mediaService.Move(media, 1051); + + // Assert + Assert.That(media.ParentId, Is.EqualTo(1051)); + Assert.That(media.Trashed, Is.False); + } + + [Test] + public void Can_Move_Media_To_RecycleBin() + { + // Arrange + var mediaService = ServiceContext.MediaService; + var media = mediaService.GetById(1050); + + // Act + mediaService.MoveToRecycleBin(media); + + // Assert + Assert.That(media.ParentId, Is.EqualTo(-21)); + Assert.That(media.Trashed, Is.True); + } + + [Test] + public void Can_Move_Media_From_RecycleBin() + { + // Arrange + var mediaService = ServiceContext.MediaService; + var media = mediaService.GetById(1053); + + // Act - moving out of recycle bin + mediaService.Move(media, 1050); + var mediaChild = mediaService.GetById(1054); + + // Assert + Assert.That(media.ParentId, Is.EqualTo(1050)); + Assert.That(media.Trashed, Is.False); + Assert.That(mediaChild.ParentId, Is.EqualTo(1053)); + Assert.That(mediaChild.Trashed, Is.False); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 032fcb8bfb..186b0736f5 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -200,6 +200,7 @@ + From 0fe33c120b73b13e47a9e6d9b4b24b66dda9529f Mon Sep 17 00:00:00 2001 From: Mads Krohn Date: Fri, 13 Sep 2013 10:42:08 +0200 Subject: [PATCH 3/3] Removing unwanted ToList() --- src/Umbraco.Core/Services/MediaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index c45bbb1353..09df9f6297 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -921,7 +921,7 @@ namespace Umbraco.Core.Services list.Add(child); - var grandkids = GetChildren(child.Id).ToList(); //No need to enumerate twice? + var grandkids = GetChildren(child.Id); if (grandkids.Any()) { list.AddRange(UpdatePropertiesOnChildren(grandkids, child.Path, child.Level, child.Trashed));