From b515aea8dda90f9e5cb851409f70ecb75c89a810 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 15 Nov 2018 16:25:08 +1100 Subject: [PATCH] Fixes issue with *, fixes some of the logic in tests, adds another test, adds fixme notes --- src/Umbraco.Core/Models/Content.cs | 2 + src/Umbraco.Core/Models/ContentBase.cs | 1 + .../Services/Implement/ContentService.cs | 24 +++++-- .../ContentServicePublishBranchTests.cs | 68 +++++++++++++++---- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 3f6e387dec..b396e6a4f2 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -208,6 +208,7 @@ namespace Umbraco.Core.Models public IEnumerable PublishedCultures => _publishInfos?.Keys ?? Enumerable.Empty(); /// + // fixme/review: Do we deal with passing in * here since this can happen when publishing branches public bool IsCulturePublished(string culture) // just check _publishInfos // a non-available culture could not become published anyways @@ -240,6 +241,7 @@ namespace Umbraco.Core.Models } /// + // fixme/review: Do we deal with passing in * here since this can happen when publishing branches public bool IsCultureEdited(string culture) => IsCultureAvailable(culture) && // is available, and (!IsCulturePublished(culture) || // is not published, or diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index b0c786d4b0..d3e46a90ad 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -153,6 +153,7 @@ namespace Umbraco.Core.Models => _cultureInfos?.Keys ?? Enumerable.Empty(); /// + // fixme/review: Do we deal with passing in * here since this can happen when publishing branches public bool IsCultureAvailable(string culture) => _cultureInfos != null && _cultureInfos.ContainsKey(culture); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 613737bf06..d5308dae1f 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1303,10 +1303,26 @@ namespace Umbraco.Core.Services.Implement if (c.ContentType.VariesByCulture()) { - // variant content type - // add culture if edited, and already published or forced - if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force || isRoot)) - return new HashSet { culture.ToLowerInvariant() }; + //we need to check all available cultures when * + if (culture == "*") + { + var culturesToPublish = new HashSet(); + foreach (var availableCulture in c.AvailableCultures) + { + // variant content type + // add culture if edited, and already published or forced + if (c.IsCultureEdited(availableCulture) && (c.IsCulturePublished(availableCulture) || force || isRoot)) + culturesToPublish.Add(availableCulture.ToLowerInvariant()); + } + return culturesToPublish; + } + else + { + // variant content type + // add culture if edited, and already published or forced + if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force || isRoot)) + return new HashSet { culture.ToLowerInvariant() }; + } } else { diff --git a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs index fd55f10e2d..49edd24d9f 100644 --- a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs +++ b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs @@ -141,7 +141,7 @@ namespace Umbraco.Tests.Services } [Test] - public void Can_Publish_Variant_Branch_When_No_Changes_On_Root() + public void Can_Publish_Variant_Branch_When_No_Changes_On_Root_All_Cultures() { CreateTypes(out _, out var vContentType); @@ -171,7 +171,43 @@ namespace Umbraco.Tests.Services iv1.SetValue("vp", "UPDATED-iv1.de", "de"); ServiceContext.ContentService.Save(iv1); - var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false).ToArray(); + var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false).ToArray(); //no culture specified so "*" is used, so all cultures + Assert.AreEqual(PublishResultType.SuccessPublishAlready, r[0].Result); + Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[1].Result); + } + + [Test] + public void Can_Publish_Variant_Branch_When_No_Changes_On_Root_Specific_Culture() + { + CreateTypes(out _, out var vContentType); + + //create/publish root + IContent vRoot = new Content("vroot", -1, vContentType, "de"); + vRoot.SetCultureName("vroot.de", "de"); + vRoot.SetCultureName("vroot.ru", "ru"); + vRoot.SetCultureName("vroot.es", "es"); + vRoot.SetValue("ip", "vroot"); + vRoot.SetValue("vp", "vroot.de", "de"); + vRoot.SetValue("vp", "vroot.ru", "ru"); + vRoot.SetValue("vp", "vroot.es", "es"); + ServiceContext.ContentService.SaveAndPublish(vRoot); + + //create/publish child + IContent iv1 = new Content("iv1", vRoot, vContentType, "de"); + iv1.SetCultureName("iv1.de", "de"); + iv1.SetCultureName("iv1.ru", "ru"); + iv1.SetCultureName("iv1.es", "es"); + iv1.SetValue("ip", "iv1"); + iv1.SetValue("vp", "iv1.de", "de"); + iv1.SetValue("vp", "iv1.ru", "ru"); + iv1.SetValue("vp", "iv1.es", "es"); + ServiceContext.ContentService.SaveAndPublish(iv1); + + //update the child + iv1.SetValue("vp", "UPDATED-iv1.de", "de"); + ServiceContext.ContentService.Save(iv1); + + var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false, "de").ToArray(); Assert.AreEqual(PublishResultType.SuccessPublishAlready, r[0].Result); Assert.AreEqual(PublishResultType.SuccessPublishCulture, r[1].Result); } @@ -218,24 +254,24 @@ namespace Umbraco.Tests.Services // !force = publishes those that are actually published, and have changes // here: nothing - var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false).ToArray(); + var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false).ToArray(); //no culture specified = all cultures AssertPublishResults(r, x => x.Content.Name, "vroot.de", "iv1.de", "iv2.de"); AssertPublishResults(r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublishCulture, //the root will always get published PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublishAlready); // prepare - ServiceContext.ContentService.SaveAndPublish(vRoot, "de"); + //ServiceContext.ContentService.SaveAndPublish(vRoot, "de"); //fixme/review no need for this, all cultures in the root are published above vRoot.SetValue("ip", "changed"); vRoot.SetValue("vp", "changed.de", "de"); vRoot.SetValue("vp", "changed.ru", "ru"); vRoot.SetValue("vp", "changed.es", "es"); - ServiceContext.ContentService.Save(vRoot); + ServiceContext.ContentService.Save(vRoot); //now there's drafts in all cultures iv1.PublishCulture("de"); iv1.PublishCulture("ru"); - ServiceContext.ContentService.SavePublishing(iv1); + ServiceContext.ContentService.SavePublishing(iv1); iv1.SetValue("ip", "changed"); iv1.SetValue("vp", "changed.de", "de"); iv1.SetValue("vp", "changed.ru", "ru"); @@ -244,13 +280,13 @@ namespace Umbraco.Tests.Services // validate Assert.IsTrue(vRoot.Published); - Assert.IsTrue(vRoot.IsCulturePublished("de")); - Assert.IsFalse(vRoot.IsCulturePublished("ru")); - Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.IsTrue(vRoot.IsCulturePublished("de")); //all cultures are published because "*" was specified + Assert.IsTrue(vRoot.IsCulturePublished("ru")); //all cultures are published because "*" was specified + Assert.IsTrue(vRoot.IsCulturePublished("es")); //all cultures are published because "*" was specified Assert.IsTrue(iv1.Published); Assert.IsTrue(iv1.IsCulturePublished("de")); Assert.IsTrue(iv1.IsCulturePublished("ru")); - Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.IsFalse(iv1.IsCulturePublished("es")); r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false, "de").ToArray(); AssertPublishResults(r, x => x.Content.Name, @@ -267,8 +303,12 @@ namespace Umbraco.Tests.Services // de is published, ru and es have not been published Assert.IsTrue(vRoot.Published); Assert.IsTrue(vRoot.IsCulturePublished("de")); - Assert.IsFalse(vRoot.IsCulturePublished("ru")); - Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.IsFalse(vRoot.IsCultureEdited("de")); //no drafts, this was just published + Assert.IsTrue(vRoot.IsCulturePublished("ru")); + Assert.IsTrue(vRoot.IsCultureEdited("ru")); //has draft + Assert.IsTrue(vRoot.IsCulturePublished("es")); + Assert.IsTrue(vRoot.IsCultureEdited("es")); //has draft + Assert.AreEqual("changed", vRoot.GetValue("ip", published: true)); // publishing de implies publishing invariants Assert.AreEqual("changed.de", vRoot.GetValue("vp", "de", published: true)); @@ -276,7 +316,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(iv1.Published); Assert.IsTrue(iv1.IsCulturePublished("de")); Assert.IsTrue(iv1.IsCulturePublished("ru")); - Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.IsFalse(iv1.IsCulturePublished("es")); Assert.AreEqual("changed", iv1.GetValue("ip", published: true)); Assert.AreEqual("changed.de", iv1.GetValue("vp", "de", published: true)); Assert.AreEqual("iv1.ru", iv1.GetValue("vp", "ru", published: true));