From 1b9f0715813a90bca6f227bfe35cbb4d4ea3256a Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 7 Aug 2013 11:39:25 +1000 Subject: [PATCH 1/3] Fixes: U4-2589 Save and Publish is not creating version of document and adds a few unit tests --- src/Umbraco.Core/Models/ContentBase.cs | 15 +++++ src/Umbraco.Core/Models/ContentExtensions.cs | 43 ++++++++++++++ .../Repositories/ContentRepository.cs | 30 +++++----- .../Models/ContentExtensionsTests.cs | 59 +++++++++++++++++++ src/Umbraco.Tests/Models/ContentTests.cs | 15 +++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 6 files changed, 147 insertions(+), 16 deletions(-) create mode 100644 src/Umbraco.Tests/Models/ContentExtensionsTests.cs diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 6414ea2116..cc5cfcdee5 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -436,5 +436,20 @@ namespace Umbraco.Core.Models } public abstract void ChangeTrashedState(bool isTrashed, int parentId = -20); + + /// + /// We will override this method to ensure that when we reset the dirty properties that we + /// also reset the dirty changes made to the content's Properties (user defined) + /// + /// + internal override void ResetDirtyProperties(bool rememberPreviouslyChangedProperties) + { + base.ResetDirtyProperties(rememberPreviouslyChangedProperties); + + foreach (var prop in Properties) + { + prop.ResetDirtyProperties(rememberPreviouslyChangedProperties); + } + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/ContentExtensions.cs b/src/Umbraco.Core/Models/ContentExtensions.cs index 0a15cf941c..c30f96c214 100644 --- a/src/Umbraco.Core/Models/ContentExtensions.cs +++ b/src/Umbraco.Core/Models/ContentExtensions.cs @@ -12,6 +12,7 @@ using System.Xml.Linq; using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Media; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Strings; using Umbraco.Core.Persistence; @@ -23,6 +24,48 @@ namespace Umbraco.Core.Models public static class ContentExtensions { #region IContent + + /// + /// Determines if a new version should be created + /// + /// + /// + /// + /// A new version needs to be created when: + /// * Any property value is changed (to enable a rollback) + /// * The publish status is changed + /// * The language is changed + /// + internal static bool ShouldCreateNewVersion(this IContent entity) + { + var publishedState = ((Content)entity).PublishedState; + return ShouldCreateNewVersion(entity, publishedState); + } + + /// + /// Determines if a new version should be created + /// + /// + /// + /// + /// + /// A new version needs to be created when: + /// * Any property value is changed (to enable a rollback) + /// * The publish status is changed + /// * The language is changed + /// + internal static bool ShouldCreateNewVersion(this IContent entity, PublishedState publishedState) + { + var dirtyEntity = (ICanBeDirty)entity; + var contentChanged = + (dirtyEntity.IsPropertyDirty("Published") && publishedState != PublishedState.Unpublished) + || dirtyEntity.IsPropertyDirty("Language"); + + var propertyValueChanged = entity.Properties.Any(x => ((ICanBeDirty)x).IsDirty()); + + return contentChanged || propertyValueChanged; + } + /// /// Returns a list of the current contents ancestors, not including the content itself. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 921a7e75dd..ced4bfa07b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -275,8 +275,9 @@ namespace Umbraco.Core.Persistence.Repositories protected override void PersistUpdatedItem(IContent entity) { var publishedState = ((Content) entity).PublishedState; - //A new version should only be created if published state (or language) has changed - bool shouldCreateNewVersion = (((ICanBeDirty)entity).IsPropertyDirty("Published") && publishedState != PublishedState.Unpublished) || ((ICanBeDirty)entity).IsPropertyDirty("Language"); + + //check if we need to create a new version + bool shouldCreateNewVersion = entity.ShouldCreateNewVersion(publishedState); if (shouldCreateNewVersion) { //Updates Modified date and Version Guid @@ -338,22 +339,19 @@ namespace Umbraco.Core.Persistence.Repositories } } - //Look up (newest) entries by id in cmsDocument table to set newest = false - //NOTE: This should only be done for all other versions then the current one, so we don't cause the same entry to be updated multiple times. - var documentDtos = - Database.Query( - "WHERE nodeId = @Id AND newest = @IsNewest AND NOT(versionId = @VersionId)", - new {Id = entity.Id, IsNewest = true, VersionId = dto.ContentVersionDto.VersionId}); - foreach (var documentDto in documentDtos) - { - var docDto = documentDto; - docDto.Newest = false; - Database.Update(docDto); - } - var contentVersionDto = dto.ContentVersionDto; if (shouldCreateNewVersion) { + //Look up (newest) entries by id in cmsDocument table to set newest = false + //NOTE: This is only relevant when a new version is created, which is why its done inside this if-statement. + var documentDtos = Database.Fetch("WHERE nodeId = @Id AND newest = @IsNewest", new { Id = entity.Id, IsNewest = true }); + foreach (var documentDto in documentDtos) + { + var docDto = documentDto; + docDto.Newest = false; + Database.Update(docDto); + } + //Create a new version - cmsContentVersion //Assumes a new Version guid and Version date (modified date) has been set Database.Insert(contentVersionDto); @@ -484,7 +482,7 @@ namespace Umbraco.Core.Persistence.Repositories } #endregion - + /// /// Private method to create a content object from a DocumentDto, which is used by Get and GetByVersion. /// diff --git a/src/Umbraco.Tests/Models/ContentExtensionsTests.cs b/src/Umbraco.Tests/Models/ContentExtensionsTests.cs new file mode 100644 index 0000000000..3da5b12a7f --- /dev/null +++ b/src/Umbraco.Tests/Models/ContentExtensionsTests.cs @@ -0,0 +1,59 @@ +using System.Linq; +using NUnit.Framework; +using Umbraco.Core.Models; +using Umbraco.Tests.TestHelpers.Entities; + +namespace Umbraco.Tests.Models +{ + [TestFixture] + public class ContentExtensionsTests + { + [Test] + public void Should_Create_New_Version_When_Publish_Status_Changed() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + + content.ResetDirtyProperties(false); + + content.Published = true; + Assert.IsTrue(content.ShouldCreateNewVersion(PublishedState.Published)); + } + + [Test] + public void Should_Create_New_Version_When_Language_Changed() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + + content.ResetDirtyProperties(false); + + content.Language = "en-AU"; + Assert.IsTrue(content.ShouldCreateNewVersion(PublishedState.Unpublished)); + } + + [Test] + public void Should_Create_New_Version_When_Any_Property_Value_Changed() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + + content.ResetDirtyProperties(false); + + content.Properties.First().Value = "hello world"; + Assert.IsTrue(content.ShouldCreateNewVersion(PublishedState.Unpublished)); + } + + [Test] + public void Should_Not_Create_New_Version_When_Anything_Other_Than_Published_Language_Or_Property_Vals_Changed() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + + content.ResetDirtyProperties(false); + + Assert.IsFalse(content.ShouldCreateNewVersion(PublishedState.Unpublished)); + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 33af9c30e6..292fa89a80 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -13,6 +13,21 @@ namespace Umbraco.Tests.Models [TestFixture] public class ContentTests { + [Test] + public void All_Dirty_Properties_Get_Reset() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + + content.ResetDirtyProperties(false); + + Assert.IsFalse(content.IsDirty()); + foreach (var prop in content.Properties) + { + Assert.IsFalse(prop.IsDirty()); + } + } + [Test] public void Can_Verify_Mocked_Content() { diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 747cb52cee..d95b720b73 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -185,6 +185,7 @@ + From 14e4a8061c783955896e321f13196d05529a3b9c Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 7 Aug 2013 11:49:48 +1000 Subject: [PATCH 2/3] removes the vs profiling files from the sln. --- src/umbraco.sln | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/umbraco.sln b/src/umbraco.sln index eba2ad7b9b..43ec254562 100644 --- a/src/umbraco.sln +++ b/src/umbraco.sln @@ -67,11 +67,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "NuSpecs", "NuSpecs", "{227C ..\build\NuSpecs\UmbracoCms.nuspec = ..\build\NuSpecs\UmbracoCms.nuspec EndProjectSection EndProject -Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{99794542-89EC-43BA-88BE-E31A9D61423B}" - ProjectSection(SolutionItems) = preProject - Performance1.psess = Performance1.psess - EndProjectSection -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU From 5fce74194077a38dbe1a60b60ababf18798bf54b Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 7 Aug 2013 11:53:09 +1000 Subject: [PATCH 3/3] another sln update --- src/umbraco.sln | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/umbraco.sln b/src/umbraco.sln index 43ec254562..fee2486039 100644 --- a/src/umbraco.sln +++ b/src/umbraco.sln @@ -157,7 +157,4 @@ Global {73529637-28F5-419C-A6BB-D094E39DE614} = {DD32977B-EF54-475B-9A1B-B97A502C6E58} {B555AAE6-0F56-442F-AC9F-EF497DB38DE7} = {DD32977B-EF54-475B-9A1B-B97A502C6E58} EndGlobalSection - GlobalSection(Performance) = preSolution - HasPerformanceSessions = true - EndGlobalSection EndGlobal