From 1b9f0715813a90bca6f227bfe35cbb4d4ea3256a Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 7 Aug 2013 11:39:25 +1000 Subject: [PATCH] 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 @@ +