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 cc65a8dc5f..b22c7311fe 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 ed031d9ab0..a38d258e6b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -276,8 +276,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 @@ -339,22 +340,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); @@ -485,7 +483,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 d0f4357293..050b6c1b8b 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -57,6 +57,21 @@ namespace Umbraco.Tests.Models Assert.AreEqual(5, content.Properties.Count()); } + [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 9245eca8f5..6b7006367f 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -198,6 +198,7 @@ + diff --git a/src/umbraco.sln b/src/umbraco.sln index 3fcce7326d..40e367de59 100644 --- a/src/umbraco.sln +++ b/src/umbraco.sln @@ -89,12 +89,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", "{245872FE-BA7D-494A-AFFF-B364BB8FF170}" - ProjectSection(SolutionItems) = preProject - Performance1.psess = Performance1.psess - Performance2.psess = Performance2.psess - EndProjectSection -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -187,7 +181,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