From 3930ae244c7c07f4d245c4f6c58d3ce767a9be90 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 24 Jul 2019 19:40:51 +1000 Subject: [PATCH 01/11] Created unit test to show the problem. --- .../Services/ContentServiceTests.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 222f40aeed..a719e04b2a 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -761,6 +761,61 @@ namespace Umbraco.Tests.Services } + [Test] + public void Unpublish_All_Cultures_Has_Unpublished_State() + { + // Arrange + + var langUk = new Language("en-GB") { IsDefault = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", Constants.System.Root, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(published.Success); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.AreEqual(PublishedState.Published, content.PublishedState); //still published + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + + unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //the last culture was unpublished so the document should also reflect this + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //just double checking + } + [Test] public void Pending_Invariant_Property_Changes_Affect_Default_Language_Edited_State() { From 1bfcc9053ebbe5f35321f89194b08e75d5837a31 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 24 Jul 2019 23:59:31 +1000 Subject: [PATCH 02/11] Fixes issues: we never properly used SuccessUnpublishMandatoryCulture, fixed issue of unpublishing last culture with a new status --- src/Umbraco.Core/Exceptions/PanicException.cs | 28 +++ .../Services/Implement/ContentService.cs | 172 +++++++++++++----- .../Services/PublishResultType.cs | 7 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Services/ContentServiceTests.cs | 38 +++- 5 files changed, 199 insertions(+), 47 deletions(-) create mode 100644 src/Umbraco.Core/Exceptions/PanicException.cs diff --git a/src/Umbraco.Core/Exceptions/PanicException.cs b/src/Umbraco.Core/Exceptions/PanicException.cs new file mode 100644 index 0000000000..4d41cafc65 --- /dev/null +++ b/src/Umbraco.Core/Exceptions/PanicException.cs @@ -0,0 +1,28 @@ +using System; +using System.Runtime.Serialization; + +namespace Umbraco.Core.Exceptions +{ + /// + /// Internal exception that in theory should never ben thrown, it is only thrown in circumstances that should never happen + /// + [Serializable] + internal class PanicException : Exception + { + public PanicException() + { + } + + public PanicException(string message) : base(message) + { + } + + public PanicException(string message, Exception innerException) : base(message, innerException) + { + } + + protected PanicException(SerializationInfo info, StreamingContext context) : base(info, context) + { + } + } +} diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index e49dcf4a12..b4e5f26de5 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -886,6 +886,8 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); + var allLangs = _languageRepository.GetMany().ToList(); + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); @@ -894,13 +896,13 @@ namespace Umbraco.Core.Services.Implement // if culture is '*', then publish them all (including variants) //this will create the correct culture impact even if culture is * or null - var impact = CultureImpact.Create(culture, _languageRepository.IsDefault(culture), content); + var impact = CultureImpact.Create(culture, IsDefaultCulture(allLangs, culture), content); // publish the culture(s) // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. content.PublishCulture(impact); - var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId, raiseEvents); scope.Complete(); return result; } @@ -921,6 +923,8 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); + var allLangs = _languageRepository.GetMany().ToList(); + var evtMsgs = EventMessagesFactory.Get(); var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) @@ -939,14 +943,14 @@ namespace Umbraco.Core.Services.Implement if (cultures.Any(x => x == null || x == "*")) throw new InvalidOperationException("Only valid cultures are allowed to be used in this method, wildcards or nulls are not allowed"); - var impacts = cultures.Select(x => CultureImpact.Explicit(x, _languageRepository.IsDefault(x))); + var impacts = cultures.Select(x => CultureImpact.Explicit(x, IsDefaultCulture(allLangs, x))); // publish the culture(s) // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. foreach (var impact in impacts) content.PublishCulture(impact); - var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId, raiseEvents); scope.Complete(); return result; } @@ -986,6 +990,8 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); + var allLangs = _languageRepository.GetMany().ToList(); + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); @@ -993,22 +999,39 @@ namespace Umbraco.Core.Services.Implement // all cultures = unpublish whole if (culture == "*" || (!content.ContentType.VariesByCulture() && culture == null)) { + // It's important to understand that when the document varies by culture but the "*" is used, + // we are just unpublishing the whole document but leaving all of the culture's as-is. This is expected + // because we don't want to actually unpublish every culture and then the document, we just want everything + // to be non-routable so that when it's re-published all variants were as they were. + content.PublishedState = PublishedState.Unpublishing; } else { - // If the culture we want to unpublish was already unpublished, nothing to do. - // To check for that we need to lookup the persisted content item - var persisted = content.HasIdentity ? GetById(content.Id) : null; - - if (persisted != null && !persisted.IsCulturePublished(culture)) - return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content); - - // unpublish the culture + // unpublish the culture, this will change the document state to Publishing! ... which is expected because this will + // essentially be re-publishing the document with the requested culture removed. + // The call to CommitDocumentChangesInternal will perform all the checks like if this is a mandatory culture or the last culture being unpublished + // and will then unpublish the document accordingly. content.UnpublishCulture(culture); + + + //TODO: Move this logic into CommitDocumentChangesInternal, we are already looking up the item there + + //// We are unpublishing a culture but there's a chance that the user might be trying + //// to unpublish a culture that is already unpublished so we need to lookup the current + //// persisted item and check since it would be quicker to do that than continue and re-publish everything. + //var persisted = content.HasIdentity ? GetById(content.Id) : null; + + //if (persisted != null && !persisted.IsCulturePublished(culture)) + //{ + // //it was already unpublished + // //TODO: We then need to check if all cultures are unpublished + // return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content); + //} + } - var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId); + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId); scope.Complete(); return result; } @@ -1047,15 +1070,35 @@ namespace Umbraco.Core.Services.Implement if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); - var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents); + var allLangs = _languageRepository.GetMany().ToList(); + + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId, raiseEvents); scope.Complete(); return result; } } + /// + /// Handles a lot of business logic cases for how the document should be persisted + /// + /// + /// + /// + /// + /// + /// + /// + /// + /// + /// + /// Business logic cases such: as unpublishing a mandatory culture, or unpublishing the last culture, checking for pending scheduled publishing, etc... is dealt with in this method. + /// There is quite a lot of cases to take into account along with logic that needs to deal with scheduled saving/publishing, branch saving/publishing, etc... + /// + /// private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content, - ContentSavingEventArgs saveEventArgs, - int userId = Constants.Security.SuperUserId, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) + ContentSavingEventArgs saveEventArgs, IReadOnlyCollection allLangs, + int userId = Constants.Security.SuperUserId, + bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) { if (scope == null) throw new ArgumentNullException(nameof(scope)); if (content == null) throw new ArgumentNullException(nameof(content)); @@ -1070,8 +1113,8 @@ namespace Umbraco.Core.Services.Implement if (content.PublishedState != PublishedState.Publishing && content.PublishedState != PublishedState.Unpublishing) content.PublishedState = PublishedState.Publishing; - // state here is either Publishing or Unpublishing - // (even though, Publishing to unpublish a culture may end up unpublishing everything) + // State here is either Publishing or Unpublishing + // Publishing to unpublish a culture may end up unpublishing everything so these flags can be flipped later var publishing = content.PublishedState == PublishedState.Publishing; var unpublishing = content.PublishedState == PublishedState.Unpublishing; @@ -1090,6 +1133,9 @@ namespace Umbraco.Core.Services.Implement if (publishing) { + //TODO: Check if it's a culture being unpublished and that's why we are publishing the document. In that case + // we should check if the culture is already unpublished since it might be the user is trying to unpublish an already unpublished culture? + //determine cultures publishing/unpublishing which will be based on previous calls to content.PublishCulture and ClearPublishInfo culturesUnpublishing = content.GetCulturesUnpublishing(); culturesPublishing = variesByCulture @@ -1097,11 +1143,21 @@ namespace Umbraco.Core.Services.Implement : null; // ensure that the document can be published, and publish handling events, business rules, etc - publishResult = StrategyCanPublish(scope, content, /*checkPath:*/ (!branchOne || branchRoot), culturesPublishing, culturesUnpublishing, evtMsgs, saveEventArgs); + publishResult = StrategyCanPublish(scope, content, /*checkPath:*/ (!branchOne || branchRoot), culturesPublishing, culturesUnpublishing, evtMsgs, saveEventArgs, allLangs); if (publishResult.Success) { // note: StrategyPublish flips the PublishedState to Publishing! publishResult = StrategyPublish(content, culturesPublishing, culturesUnpublishing, evtMsgs); + + //check if a culture has been unpublished and if there are no cultures left, and then unpublish document as a whole + if (publishResult.Result == PublishResultType.SuccessUnpublishCulture && content.PublishCultureInfos.Count == 0) + { + publishing = false; + unpublishing = content.Published; // if not published yet, nothing to do + + // we may end up in a state where we won't publish nor unpublish + // keep going, though, as we want to save anyways + } } else { @@ -1186,17 +1242,34 @@ namespace Umbraco.Core.Services.Implement if (culturesUnpublishing != null) { - //If we are here, it means we tried unpublishing a culture but it was mandatory so now everything is unpublished - var langs = string.Join(", ", _languageRepository.GetMany() + // This will mean that that we unpublished a mandatory culture or we unpublished the last culture. + + var langs = string.Join(", ", allLangs .Where(x => culturesUnpublishing.InvariantContains(x.IsoCode)) .Select(x => x.CultureName)); Audit(AuditType.UnpublishVariant, userId, content.Id, $"Unpublished languages: {langs}", langs); - //log that the whole content item has been unpublished due to mandatory culture unpublished - Audit(AuditType.Unpublish, userId, content.Id, "Unpublished (mandatory language unpublished)"); - } - else - Audit(AuditType.Unpublish, userId, content.Id); + if (publishResult == null) + throw new PanicException("publishResult == null - should not happen"); + + switch(publishResult.Result) + { + case PublishResultType.FailedPublishMandatoryCultureMissing: + //occurs when a mandatory culture was unpublished (which means we tried publishing the document without a mandatory culture) + + //log that the whole content item has been unpublished due to mandatory culture unpublished + Audit(AuditType.Unpublish, userId, content.Id, "Unpublished (mandatory language unpublished)"); + return new PublishResult(PublishResultType.SuccessUnpublishMandatoryCulture, evtMsgs, content); + case PublishResultType.SuccessUnpublishCulture: + //occurs when the last culture is unpublished + + Audit(AuditType.Unpublish, userId, content.Id, "Unpublished (last language unpublished)"); + return new PublishResult(PublishResultType.SuccessUnpublishLastCulture, evtMsgs, content); + } + + } + + Audit(AuditType.Unpublish, userId, content.Id); return new PublishResult(PublishResultType.SuccessUnpublish, evtMsgs, content); } @@ -1236,7 +1309,7 @@ namespace Umbraco.Core.Services.Implement case PublishResultType.SuccessPublishCulture: if (culturesPublishing != null) { - var langs = string.Join(", ", _languageRepository.GetMany() + var langs = string.Join(", ", allLangs .Where(x => culturesPublishing.InvariantContains(x.IsoCode)) .Select(x => x.CultureName)); Audit(AuditType.PublishVariant, userId, content.Id, $"Published languages: {langs}", langs); @@ -1245,7 +1318,7 @@ namespace Umbraco.Core.Services.Implement case PublishResultType.SuccessUnpublishCulture: if (culturesUnpublishing != null) { - var langs = string.Join(", ", _languageRepository.GetMany() + var langs = string.Join(", ", allLangs .Where(x => culturesUnpublishing.InvariantContains(x.IsoCode)) .Select(x => x.CultureName)); Audit(AuditType.UnpublishVariant, userId, content.Id, $"Unpublished languages: {langs}", langs); @@ -1259,14 +1332,14 @@ namespace Umbraco.Core.Services.Implement // should not happen if (branchOne && !branchRoot) - throw new Exception("panic"); + throw new PanicException("branchOne && !branchRoot - should not happen"); //if publishing didn't happen or if it has failed, we still need to log which cultures were saved if (!branchOne && (publishResult == null || !publishResult.Success)) { if (culturesChanging != null) { - var langs = string.Join(", ", _languageRepository.GetMany() + var langs = string.Join(", ", allLangs .Where(x => culturesChanging.InvariantContains(x.IsoCode)) .Select(x => x.CultureName)); Audit(AuditType.SaveVariant, userId, content.Id, $"Saved languages: {langs}", langs); @@ -1297,6 +1370,8 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); + var allLangs = _languageRepository.GetMany().ToList(); + foreach (var d in _documentRepository.GetContentForRelease(date)) { PublishResult result; @@ -1325,7 +1400,7 @@ namespace Umbraco.Core.Services.Implement //publish the culture values and validate the property values, if validation fails, log the invalid properties so the develeper has an idea of what has failed Property[] invalidProperties = null; - var impact = CultureImpact.Explicit(culture, _languageRepository.IsDefault(culture)); + var impact = CultureImpact.Explicit(culture, IsDefaultCulture(allLangs, culture)); var tryPublish = d.PublishCulture(impact) && _propertyValidationService.Value.IsPropertyDataValid(d, out invalidProperties, impact); if (invalidProperties != null && invalidProperties.Length > 0) Logger.Warn("Scheduled publishing will fail for document {DocumentId} and culture {Culture} because of invalid properties {InvalidProperties}", @@ -1340,7 +1415,7 @@ namespace Umbraco.Core.Services.Implement else if (!publishing) result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); else - result = CommitDocumentChangesInternal(scope, d, saveEventArgs, d.WriterId); + result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs, d.WriterId); if (result.Success == false) @@ -1390,7 +1465,7 @@ namespace Umbraco.Core.Services.Implement d.UnpublishCulture(c); } - result = CommitDocumentChangesInternal(scope, d, saveEventArgs, d.WriterId); + result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs, d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); yield return result; @@ -1416,7 +1491,7 @@ namespace Umbraco.Core.Services.Implement } // utility 'PublishCultures' func used by SaveAndPublishBranch - private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet culturesToPublish) + private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet culturesToPublish, IReadOnlyCollection allLangs) { //TODO: This does not support being able to return invalid property details to bubble up to the UI @@ -1426,7 +1501,7 @@ namespace Umbraco.Core.Services.Implement { return culturesToPublish.All(culture => { - var impact = CultureImpact.Create(culture, _languageRepository.IsDefault(culture), content); + var impact = CultureImpact.Create(culture, IsDefaultCulture(allLangs, culture), content); return content.PublishCulture(impact) && _propertyValidationService.Value.IsPropertyDataValid(content, out _, impact); }); } @@ -1535,7 +1610,7 @@ namespace Umbraco.Core.Services.Implement internal IEnumerable SaveAndPublishBranch(IContent document, bool force, Func> shouldPublish, - Func, bool> publishCultures, + Func, IReadOnlyCollection, bool> publishCultures, int userId = Constants.Security.SuperUserId) { if (shouldPublish == null) throw new ArgumentNullException(nameof(shouldPublish)); @@ -1549,6 +1624,8 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); + var allLangs = _languageRepository.GetMany().ToList(); + if (!document.HasIdentity) throw new InvalidOperationException("Cannot not branch-publish a new document."); @@ -1557,7 +1634,7 @@ namespace Umbraco.Core.Services.Implement throw new InvalidOperationException("Cannot mix PublishCulture and SaveAndPublishBranch."); // deal with the branch root - if it fails, abort - var result = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, publishedDocuments, evtMsgs, userId); + var result = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, publishedDocuments, evtMsgs, userId, allLangs); if (result != null) { results.Add(result); @@ -1588,7 +1665,7 @@ namespace Umbraco.Core.Services.Implement } // no need to check path here, parent has to be published here - result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, publishedDocuments, evtMsgs, userId); + result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, publishedDocuments, evtMsgs, userId, allLangs); if (result != null) { results.Add(result); @@ -1620,10 +1697,10 @@ namespace Umbraco.Core.Services.Implement // publishValues: a function publishing values (using the appropriate PublishCulture calls) private PublishResult SaveAndPublishBranchItem(IScope scope, IContent document, Func> shouldPublish, - Func, bool> publishCultures, + Func, IReadOnlyCollection, bool> publishCultures, bool isRoot, ICollection publishedDocuments, - EventMessages evtMsgs, int userId) + EventMessages evtMsgs, int userId, IReadOnlyCollection allLangs) { var culturesToPublish = shouldPublish(document); if (culturesToPublish == null) // null = do not include @@ -1636,13 +1713,13 @@ namespace Umbraco.Core.Services.Implement return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, document); // publish & check if values are valid - if (!publishCultures(document, culturesToPublish)) + if (!publishCultures(document, culturesToPublish, allLangs)) { //TODO: Based on this callback behavior there is no way to know which properties may have been invalid if this failed, see other results of FailedPublishContentInvalid return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); } - var result = CommitDocumentChangesInternal(scope, document, saveEventArgs, userId, branchOne: true, branchRoot: isRoot); + var result = CommitDocumentChangesInternal(scope, document, saveEventArgs, allLangs, userId, branchOne: true, branchRoot: isRoot); if (result.Success) publishedDocuments.Add(document); return result; @@ -2343,6 +2420,9 @@ namespace Umbraco.Core.Services.Implement _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Document), message, parameters)); } + private bool IsDefaultCulture(IReadOnlyCollection langs, string culture) => langs.Any(x => x.IsDefault && x.IsoCode.InvariantEquals(culture)); + private bool IsMandatoryCulture(IReadOnlyCollection langs, string culture) => langs.Any(x => x.IsMandatory && x.IsoCode.InvariantEquals(culture)); + #endregion #region Event Handlers @@ -2497,7 +2577,9 @@ namespace Umbraco.Core.Services.Implement /// /// /// - private PublishResult StrategyCanPublish(IScope scope, IContent content, bool checkPath, IReadOnlyList culturesPublishing, IReadOnlyCollection culturesUnpublishing, EventMessages evtMsgs, ContentSavingEventArgs savingEventArgs) + private PublishResult StrategyCanPublish(IScope scope, IContent content, bool checkPath, IReadOnlyList culturesPublishing, + IReadOnlyCollection culturesUnpublishing, EventMessages evtMsgs, ContentSavingEventArgs savingEventArgs, + IReadOnlyCollection allLangs) { // raise Publishing event if (scope.Events.DispatchCancelable(Publishing, this, savingEventArgs.ToContentPublishingEventArgs())) @@ -2510,7 +2592,7 @@ namespace Umbraco.Core.Services.Implement var impactsToPublish = culturesPublishing == null ? new[] {CultureImpact.Invariant} //if it's null it's invariant - : culturesPublishing.Select(x => CultureImpact.Explicit(x, _languageRepository.IsDefault(x))).ToArray(); + : culturesPublishing.Select(x => CultureImpact.Explicit(x, allLangs.Any(lang => lang.IsoCode.InvariantEquals(x) && lang.IsMandatory))).ToArray(); // publish the culture(s) if (!impactsToPublish.All(content.PublishCulture)) @@ -2535,7 +2617,7 @@ namespace Umbraco.Core.Services.Implement return new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); // missing mandatory culture = cannot be published - var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); + var mandatoryCultures = allLangs.Where(x => x.IsMandatory).Select(x => x.IsoCode); var mandatoryMissing = mandatoryCultures.Any(x => !content.PublishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); if (mandatoryMissing) return new PublishResult(PublishResultType.FailedPublishMandatoryCultureMissing, evtMsgs, content); diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index 1a2b52f9c9..66514a26fb 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -49,6 +49,11 @@ /// SuccessUnpublishMandatoryCulture = 6, + /// + /// The specified document culture was unpublished, and was the last published culture in the document, therefore the document itself was unpublished. + /// + SuccessUnpublishLastCulture = 8, + #endregion #region Success - Mixed @@ -115,7 +120,7 @@ /// /// The document could not be published because it has no publishing flags or values. /// - FailedPublishNothingToPublish = FailedPublish | 9, // TODO: in ContentService.StrategyCanPublish - weird + FailedPublishNothingToPublish = FailedPublish | 9, // TODO: in ContentService.StrategyCanPublish - weird - do we have a test for that case? /// /// The document could not be published because some mandatory cultures are missing. diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index ca9b7d4034..74af58b94f 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -204,6 +204,7 @@ + diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index a719e04b2a..07f4cdb661 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -806,7 +806,7 @@ namespace Umbraco.Tests.Services unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); Assert.IsTrue(unpublished.Success); - Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); + Assert.AreEqual(PublishResultType.SuccessUnpublishLastCulture, unpublished.Result); Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //the last culture was unpublished so the document should also reflect this @@ -816,6 +816,42 @@ namespace Umbraco.Tests.Services Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //just double checking } + [Test] + public void Unpublishing_Mandatory_Language_Unpublishes_Document() + { + // Arrange + + var langUk = new Language("en-GB") { IsDefault = true, IsMandatory = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", Constants.System.Root, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(published.Success); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + + var unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //unpublish mandatory lang + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishMandatoryCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); //remains published + Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); + } + [Test] public void Pending_Invariant_Property_Changes_Affect_Default_Language_Edited_State() { From 5a9ca8d09f30f396cdeed8fdebd961d93469b4ef Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Jul 2019 11:37:11 +1000 Subject: [PATCH 03/11] wip added more tests and notes --- .../Models/ContentRepositoryExtensions.cs | 56 ++++++++++++++----- .../Services/Implement/ContentService.cs | 12 +++- .../Services/ContentServiceTests.cs | 42 ++++++++++++++ 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index bf7228ca47..f9efc60142 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -222,7 +222,13 @@ namespace Umbraco.Core.Models return true; } - public static void UnpublishCulture(this IContent content, string culture = "*") + /// + /// Returns false if the culture is already unpublished + /// + /// + /// + /// + public static bool UnpublishCulture(this IContent content, string culture = "*") { culture = culture.NullOrWhiteSpaceAsNull(); @@ -230,16 +236,31 @@ namespace Umbraco.Core.Models if (!content.ContentType.SupportsPropertyVariation(culture, "*", true)) throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); - if (culture == "*") // all cultures + + var keepProcessing = true; + + if (culture == "*") + { + // all cultures content.ClearPublishInfos(); - else // one single culture - content.ClearPublishInfo(culture); + } + else + { + // one single culture + keepProcessing = content.ClearPublishInfo(culture); + } + - // property.PublishValues only publishes what is valid, variation-wise - foreach (var property in content.Properties) - property.UnpublishValues(culture); + if (keepProcessing) + { + // property.PublishValues only publishes what is valid, variation-wise + foreach (var property in content.Properties) + property.UnpublishValues(culture); - content.PublishedState = PublishedState.Publishing; + content.PublishedState = PublishedState.Publishing; + } + + return keepProcessing; } public static void ClearPublishInfos(this IContent content) @@ -247,15 +268,24 @@ namespace Umbraco.Core.Models content.PublishCultureInfos = null; } - public static void ClearPublishInfo(this IContent content, string culture) + /// + /// Returns false if the culture is already unpublished + /// + /// + /// + /// + public static bool ClearPublishInfo(this IContent content, string culture) { if (culture.IsNullOrWhiteSpace()) throw new ArgumentNullOrEmptyException(nameof(culture)); - content.PublishCultureInfos.Remove(culture); - - // set the culture to be dirty - it's been modified - content.TouchCulture(culture); + var removed = content.PublishCultureInfos.Remove(culture); + if (removed) + { + // set the culture to be dirty - it's been modified + content.TouchCulture(culture); + } + return removed; } /// diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index b4e5f26de5..91c26701b3 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1012,7 +1012,9 @@ namespace Umbraco.Core.Services.Implement // essentially be re-publishing the document with the requested culture removed. // The call to CommitDocumentChangesInternal will perform all the checks like if this is a mandatory culture or the last culture being unpublished // and will then unpublish the document accordingly. - content.UnpublishCulture(culture); + var removed = content.UnpublishCulture(culture); + + //TODO: if !removed then there is really nothing to process and we should exit here with SuccessUnpublishAlready. //TODO: Move this logic into CommitDocumentChangesInternal, we are already looking up the item there @@ -2613,8 +2615,14 @@ namespace Umbraco.Core.Services.Implement if (culturesPublishing == null) throw new InvalidOperationException("Internal error, variesByCulture but culturesPublishing is null."); - if (content.Published && culturesPublishing.Count == 0 && culturesUnpublishing.Count == 0) // no published cultures = cannot be published + if (content.Published && culturesPublishing.Count == 0 && culturesUnpublishing.Count == 0) + { + // no published cultures = cannot be published + // This will occur if for example, a culture that is already unpublished is sent to be unpublished again, or vice versa, in that case + // there will be nothing to publish/unpublish. return new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); + } + // missing mandatory culture = cannot be published var mandatoryCultures = allLangs.Where(x => x.IsMandatory).Select(x => x.IsoCode); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 07f4cdb661..a903bcedb0 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -852,6 +852,48 @@ namespace Umbraco.Tests.Services Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); } + [Test] + public void Unpublishing_Already_Unpublished_Culture() + { + // Arrange + + var langUk = new Language("en-GB") { IsDefault = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", Constants.System.Root, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(published.Success); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + + var unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + + content = ServiceContext.ContentService.GetById(content.Id); + + unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //unpublish again + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishAlready, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + + } + [Test] public void Pending_Invariant_Property_Changes_Affect_Default_Language_Edited_State() { From 40a55cb9dfe9978d7dfbab5067f932e2c400803f Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Jul 2019 15:28:50 +1000 Subject: [PATCH 04/11] Fixes issue/tests --- .../Services/Implement/ContentService.cs | 35 ++++++++----------- .../Services/PublishResultType.cs | 4 +-- .../Services/ContentServiceTests.cs | 8 +++++ 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 91c26701b3..a8b40b1bb1 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -937,6 +937,7 @@ namespace Umbraco.Core.Services.Implement //no cultures specified and doesn't vary, so publish it, else nothing to publish return !varies ? SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents) + //TODO: Though we may not have cultures to publish, shouldn't we continue to Save in this case?? : new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); } @@ -1005,39 +1006,33 @@ namespace Umbraco.Core.Services.Implement // to be non-routable so that when it's re-published all variants were as they were. content.PublishedState = PublishedState.Unpublishing; + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId); + scope.Complete(); + return result; } else { - // unpublish the culture, this will change the document state to Publishing! ... which is expected because this will + // Unpublish the culture, this will change the document state to Publishing! ... which is expected because this will // essentially be re-publishing the document with the requested culture removed. // The call to CommitDocumentChangesInternal will perform all the checks like if this is a mandatory culture or the last culture being unpublished // and will then unpublish the document accordingly. + // If the result of this is false it means there was no culture to unpublish (i.e. it was already unpublished or it did not exist) var removed = content.UnpublishCulture(culture); - //TODO: if !removed then there is really nothing to process and we should exit here with SuccessUnpublishAlready. + //save and publish any changes + var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId); + scope.Complete(); - //TODO: Move this logic into CommitDocumentChangesInternal, we are already looking up the item there - - //// We are unpublishing a culture but there's a chance that the user might be trying - //// to unpublish a culture that is already unpublished so we need to lookup the current - //// persisted item and check since it would be quicker to do that than continue and re-publish everything. - //var persisted = content.HasIdentity ? GetById(content.Id) : null; - - //if (persisted != null && !persisted.IsCulturePublished(culture)) - //{ - // //it was already unpublished - // //TODO: We then need to check if all cultures are unpublished - // return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content); - //} + // In one case the result will be PublishStatusType.FailedPublishNothingToPublish which means that no cultures + // were specified to be published which will be the case when removed is false. In that case + // we want to swap the result type to PublishResultType.SuccessUnpublishAlready (that was the expectation before). + if (result.Result == PublishResultType.FailedPublishNothingToPublish && !removed) + return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content); + return result; } - - var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, allLangs, userId); - scope.Complete(); - return result; } - } /// diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index 66514a26fb..66c1e38267 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -118,9 +118,9 @@ FailedPublishContentInvalid = FailedPublish | 8, /// - /// The document could not be published because it has no publishing flags or values. + /// The document could not be published because it has no publishing flags or values or if its a variant document, no cultures were specified to be published. /// - FailedPublishNothingToPublish = FailedPublish | 9, // TODO: in ContentService.StrategyCanPublish - weird - do we have a test for that case? + FailedPublishNothingToPublish = FailedPublish | 9, /// /// The document could not be published because some mandatory cultures are missing. diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index a903bcedb0..44b2eec66d 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -887,11 +887,19 @@ namespace Umbraco.Tests.Services content = ServiceContext.ContentService.GetById(content.Id); + //Change some data since Unpublish should always Save + content.SetCultureName("content-en-updated", langUk.IsoCode); + //var saveResult = ServiceContext.ContentService.Save(content); + //content = ServiceContext.ContentService.GetById(content.Id); + unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //unpublish again Assert.IsTrue(unpublished.Success); Assert.AreEqual(PublishResultType.SuccessUnpublishAlready, unpublished.Result); Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + content = ServiceContext.ContentService.GetById(content.Id); + //ensure that even though the culture was already unpublished that the data was still persisted + Assert.AreEqual("content-en-updated", content.GetCultureName(langUk.IsoCode)); } [Test] From 415916c527a93944196ed93136a950b607c9d42b Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Jul 2019 16:09:45 +1000 Subject: [PATCH 05/11] Fixes an issue with the content service + test --- .../Services/Implement/ContentService.cs | 7 +-- .../Services/ContentServiceTests.cs | 43 ++++++++++++++++++- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index a8b40b1bb1..b1aa92c4c8 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -932,13 +932,10 @@ namespace Umbraco.Core.Services.Implement var varies = content.ContentType.VariesByCulture(); - if (cultures.Length == 0) + if (cultures.Length == 0 && !varies) { //no cultures specified and doesn't vary, so publish it, else nothing to publish - return !varies - ? SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents) - //TODO: Though we may not have cultures to publish, shouldn't we continue to Save in this case?? - : new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); + return SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents); } if (cultures.Any(x => x == null || x == "*")) diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 44b2eec66d..b8ed0e1f41 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -889,8 +889,6 @@ namespace Umbraco.Tests.Services //Change some data since Unpublish should always Save content.SetCultureName("content-en-updated", langUk.IsoCode); - //var saveResult = ServiceContext.ContentService.Save(content); - //content = ServiceContext.ContentService.GetById(content.Id); unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //unpublish again Assert.IsTrue(unpublished.Success); @@ -902,6 +900,47 @@ namespace Umbraco.Tests.Services Assert.AreEqual("content-en-updated", content.GetCultureName(langUk.IsoCode)); } + [Test] + public void Publishing_No_Cultures_Still_Saves() + { + // Arrange + + var langUk = new Language("en-GB") { IsDefault = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", Constants.System.Root, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(published.Success); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + + //Change some data since SaveAndPublish should always Save + content.SetCultureName("content-en-updated", langUk.IsoCode); + + var saved = ServiceContext.ContentService.SaveAndPublish(content, new string [] { }); //save without cultures + Assert.AreEqual(PublishResultType.FailedPublishNothingToPublish, saved.Result); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + //ensure that even though nothing was published that the data was still persisted + Assert.AreEqual("content-en-updated", content.GetCultureName(langUk.IsoCode)); + } + + [Test] public void Pending_Invariant_Property_Changes_Affect_Default_Language_Edited_State() { From a2ba0f6a4c9a91c0a183b6f60d2be78ad989fd93 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Jul 2019 17:06:09 +1000 Subject: [PATCH 06/11] more tests - failing that need fixing --- .../Services/ContentServiceTests.cs | 151 +++++++++--------- 1 file changed, 75 insertions(+), 76 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index b8ed0e1f41..7954f0f024 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -717,21 +717,8 @@ namespace Umbraco.Tests.Services [Test] public void Can_Unpublish_Content_Variation() { - // Arrange + var content = CreateEnglishAndFrenchDocument(out var langUk, out var langFr, out var contentType); - var langUk = new Language("en-GB") { IsDefault = true }; - var langFr = new Language("fr-FR"); - - ServiceContext.LocalizationService.Save(langFr); - ServiceContext.LocalizationService.Save(langUk); - - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - - IContent content = new Content("content", Constants.System.Root, contentType); - content.SetCultureName("content-fr", langFr.IsoCode); - content.SetCultureName("content-en", langUk.IsoCode); content.PublishCulture(CultureImpact.Explicit(langFr.IsoCode, langFr.IsDefault)); content.PublishCulture(CultureImpact.Explicit(langUk.IsoCode, langUk.IsDefault)); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); @@ -761,25 +748,52 @@ namespace Umbraco.Tests.Services } + [Test] + public void Can_Publish_Culture_After_Last_Culture_Unpublished() + { + var content = CreateEnglishAndFrenchDocument(out var langUk, out var langFr, out var contentType); + + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + + var unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //first culture + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + + content = ServiceContext.ContentService.GetById(content.Id); + + unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); //last culture + Assert.IsTrue(unpublished.Success); + Assert.AreEqual(PublishResultType.SuccessUnpublishLastCulture, unpublished.Result); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); + + content = ServiceContext.ContentService.GetById(content.Id); + + published = ServiceContext.ContentService.SaveAndPublish(content, langUk.IsoCode); + Assert.AreEqual(PublishedState.Published, content.PublishedState); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + + content = ServiceContext.ContentService.GetById(content.Id); //reget + Assert.AreEqual(PublishedState.Published, content.PublishedState); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + + } + + + [Test] public void Unpublish_All_Cultures_Has_Unpublished_State() { - // Arrange + var content = CreateEnglishAndFrenchDocument(out var langUk, out var langFr, out var contentType); - var langUk = new Language("en-GB") { IsDefault = true }; - var langFr = new Language("fr-FR"); - - ServiceContext.LocalizationService.Save(langFr); - ServiceContext.LocalizationService.Save(langUk); - - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - - IContent content = new Content("content", Constants.System.Root, contentType); - content.SetCultureName("content-fr", langFr.IsoCode); - content.SetCultureName("content-en", langUk.IsoCode); - var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); @@ -792,7 +806,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); Assert.AreEqual(PublishedState.Published, content.PublishedState); - var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); + var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); //first culture Assert.IsTrue(unpublished.Success); Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); @@ -804,7 +818,7 @@ namespace Umbraco.Tests.Services Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); - unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); + unpublished = ServiceContext.ContentService.Unpublish(content, langUk.IsoCode); //last culture Assert.IsTrue(unpublished.Success); Assert.AreEqual(PublishResultType.SuccessUnpublishLastCulture, unpublished.Result); Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); @@ -814,13 +828,13 @@ namespace Umbraco.Tests.Services //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //just double checking + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); } [Test] public void Unpublishing_Mandatory_Language_Unpublishes_Document() { - // Arrange - var langUk = new Language("en-GB") { IsDefault = true, IsMandatory = true }; var langFr = new Language("fr-FR"); @@ -855,21 +869,7 @@ namespace Umbraco.Tests.Services [Test] public void Unpublishing_Already_Unpublished_Culture() { - // Arrange - - var langUk = new Language("en-GB") { IsDefault = true }; - var langFr = new Language("fr-FR"); - - ServiceContext.LocalizationService.Save(langFr); - ServiceContext.LocalizationService.Save(langUk); - - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - - IContent content = new Content("content", Constants.System.Root, contentType); - content.SetCultureName("content-fr", langFr.IsoCode); - content.SetCultureName("content-en", langUk.IsoCode); + var content = CreateEnglishAndFrenchDocument(out var langUk, out var langFr, out var contentType); var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); @@ -903,21 +903,7 @@ namespace Umbraco.Tests.Services [Test] public void Publishing_No_Cultures_Still_Saves() { - // Arrange - - var langUk = new Language("en-GB") { IsDefault = true }; - var langFr = new Language("fr-FR"); - - ServiceContext.LocalizationService.Save(langFr); - ServiceContext.LocalizationService.Save(langUk); - - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - - IContent content = new Content("content", Constants.System.Root, contentType); - content.SetCultureName("content-fr", langFr.IsoCode); - content.SetCultureName("content-en", langUk.IsoCode); + var content = CreateEnglishAndFrenchDocument(out var langUk, out var langFr, out var contentType); var published = ServiceContext.ContentService.SaveAndPublish(content, new[] { langFr.IsoCode, langUk.IsoCode }); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); @@ -991,17 +977,7 @@ namespace Umbraco.Tests.Services [Test] public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() { - // Arrange - - var langGB = new Language("en-GB") { IsDefault = true }; - var langFr = new Language("fr-FR"); - - ServiceContext.LocalizationService.Save(langFr); - ServiceContext.LocalizationService.Save(langGB); - - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); + CreateEnglishAndFrenchDocumentType(out var langUk, out var langFr, out var contentType); IContent content = new Content("content", Constants.System.Root, contentType); content.SetCultureName("content-fr", langFr.IsoCode); @@ -1012,8 +988,8 @@ namespace Umbraco.Tests.Services //re-get content = ServiceContext.ContentService.GetById(content.Id); - content.SetCultureName("content-en", langGB.IsoCode); - published = ServiceContext.ContentService.SaveAndPublish(content, langGB.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + published = ServiceContext.ContentService.SaveAndPublish(content, langUk.IsoCode); //audit log will only show that english was published lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); Assert.AreEqual($"Published languages: English (United Kingdom)", lastLog.Comment); @@ -3193,5 +3169,28 @@ namespace Umbraco.Tests.Services var repository = new DocumentRepository(accessor, AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } + + private void CreateEnglishAndFrenchDocumentType(out Language langUk, out Language langFr, out ContentType contentType) + { + langUk = new Language("en-GB") { IsDefault = true }; + langFr = new Language("fr-FR"); + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + } + + private IContent CreateEnglishAndFrenchDocument(out Language langUk, out Language langFr, out ContentType contentType) + { + CreateEnglishAndFrenchDocumentType(out langUk, out langFr, out contentType); + + IContent content = new Content("content", Constants.System.Root, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + + return content; + } } } From 45227357fd38b1498f4d53dbabb88de8520a6d6b Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Jul 2019 17:41:37 +1000 Subject: [PATCH 07/11] adds notes on why this is failing --- src/Umbraco.Tests/Services/ContentServiceTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 7954f0f024..01eb9a0e27 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -829,6 +829,13 @@ namespace Umbraco.Tests.Services content = ServiceContext.ContentService.GetById(content.Id); Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //just double checking Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + //TODO: This fails!? Why? Because in the ContentService.CommitDocumentChangesInternal method when we detect it's the last lang being unpublished, + // we swap the unpublishing flag to true which means we end up setting the property content.PublishedState = PublishedState.Unpublishing, because of this + // inside of the DocumentRepository we treat many things differently including not publishing the removed culture. + // So how do we fix that? Well when we unpublish the last culture we want to both save the unpublishing of the culture AND unpublish the document, the easy + // way to do this will be to perform a double save operation, though that's not the prettiest way to do it. Ideally we take care of this all in one process + // in the DocumentRepository but that might require another transitory status like PublishedState.UnpublishingLastCulture ... though that's not pretty either. + // It might be possible in some other magicaly way, i'm just leaving these notes here mostly for myself :) Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); } From b5b4ee79b406ade86d78a6ea0578683ee426c260 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 16:00:42 +1000 Subject: [PATCH 08/11] Adds notes, fixes logic with a double save --- .../TagsPropertyEditorAttribute.cs | 1 + .../Services/Implement/ContentService.cs | 34 +++++++++++++------ .../Services/ContentServiceTests.cs | 4 +++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/TagsPropertyEditorAttribute.cs b/src/Umbraco.Core/PropertyEditors/TagsPropertyEditorAttribute.cs index 2439c7c02e..6549f1a233 100644 --- a/src/Umbraco.Core/PropertyEditors/TagsPropertyEditorAttribute.cs +++ b/src/Umbraco.Core/PropertyEditors/TagsPropertyEditorAttribute.cs @@ -55,6 +55,7 @@ namespace Umbraco.Core.PropertyEditors /// /// Gets the type of the dynamic configuration provider. /// + //TODO: This is not used and should be implemented in a nicer way, see https://github.com/umbraco/Umbraco-CMS/issues/6017#issuecomment-516253562 public Type TagsConfigurationProviderType { get; } } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index b1aa92c4c8..7b003d0a26 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1125,6 +1125,18 @@ namespace Umbraco.Core.Services.Implement var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch; var previouslyPublished = content.HasIdentity && content.Published; + //inline method to persist the document with the documentRepository since this logic could be called a couple times below + void SaveDocument(IContent c) + { + // save, always + if (c.HasIdentity == false) + c.CreatorId = userId; + c.WriterId = userId; + + // saving does NOT change the published version, unless PublishedState is Publishing or Unpublishing + _documentRepository.Save(c); + } + if (publishing) { //TODO: Check if it's a culture being unpublished and that's why we are publishing the document. In that case @@ -1146,11 +1158,15 @@ namespace Umbraco.Core.Services.Implement //check if a culture has been unpublished and if there are no cultures left, and then unpublish document as a whole if (publishResult.Result == PublishResultType.SuccessUnpublishCulture && content.PublishCultureInfos.Count == 0) { - publishing = false; - unpublishing = content.Published; // if not published yet, nothing to do + // This is a special case! We are unpublishing the last culture and to persist that we need to re-publish without any cultures + // so the state needs to remain Publishing to do that. However, we then also need to unpublish the document and to do that + // the state needs to be Unpublishing and it cannot be both. This state is used within the documentRepository to know how to + // persist certain things. So before proceeding below, we need to save the Publishing state to publish no cultures, then we can + // mark the document for Unpublishing. + SaveDocument(content); - // we may end up in a state where we won't publish nor unpublish - // keep going, though, as we want to save anyways + //set the flag to unpublish and continue + unpublishing = content.Published; // if not published yet, nothing to do } } else @@ -1212,13 +1228,8 @@ namespace Umbraco.Core.Services.Implement } } - // save, always - if (content.HasIdentity == false) - content.CreatorId = userId; - content.WriterId = userId; - - // saving does NOT change the published version, unless PublishedState is Publishing or Unpublishing - _documentRepository.Save(content); + //Persist the document + SaveDocument(content); // raise the Saved event, always if (raiseEvents) @@ -2758,6 +2769,7 @@ namespace Umbraco.Core.Services.Implement { var attempt = new PublishResult(PublishResultType.SuccessUnpublish, evtMsgs, content); + //TODO: What is this check?? we just created this attempt and of course it is Success?! if (attempt.Success == false) return attempt; diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 01eb9a0e27..10bb63211b 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -836,6 +836,10 @@ namespace Umbraco.Tests.Services // way to do this will be to perform a double save operation, though that's not the prettiest way to do it. Ideally we take care of this all in one process // in the DocumentRepository but that might require another transitory status like PublishedState.UnpublishingLastCulture ... though that's not pretty either. // It might be possible in some other magicaly way, i'm just leaving these notes here mostly for myself :) + // UPDATE - ok, i 'think' instead of doing a check inside of CommitDocumentChangesInternal for the last culture being unpublished, we could actually do this check + // directly inside of the DocumentRepository + // UPDATE 2 - ok that won't work because there is some business logic that needs to occur in CommitDocumentChangesInternal when it needs to be unpublished, this will + // be tricky without doing a double save, hrm. Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); } From 27bc309c557f9b0adeca9f587feed9a199f80e36 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 18:49:48 +1000 Subject: [PATCH 09/11] Removes unused code --- .../Implement/ContentRepositoryBase.cs | 35 ++++++++++--------- .../Services/Implement/ContentService.cs | 3 -- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index aeb4c3774f..60beaca018 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -512,20 +512,21 @@ namespace Umbraco.Core.Persistence.Repositories.Implement foreach (var a in allPropertyDataDtos) a.PropertyTypeDto = indexedPropertyTypeDtos[a.PropertyTypeId]; - // prefetch configuration for tag properties - var tagEditors = new Dictionary(); - foreach (var propertyTypeDto in indexedPropertyTypeDtos.Values) - { - var editorAlias = propertyTypeDto.DataTypeDto.EditorAlias; - var editorAttribute = PropertyEditors[editorAlias].GetTagAttribute(); - if (editorAttribute == null) continue; - var tagConfigurationSource = propertyTypeDto.DataTypeDto.Configuration; - var tagConfiguration = string.IsNullOrWhiteSpace(tagConfigurationSource) - ? new TagConfiguration() - : JsonConvert.DeserializeObject(tagConfigurationSource); - if (tagConfiguration.Delimiter == default) tagConfiguration.Delimiter = editorAttribute.Delimiter; - tagEditors[editorAlias] = tagConfiguration; - } + //TODO: This logic was here but then we never used the data being passed to GetPropertyCollections, this will save a bit of CPU without it! + //// prefetch configuration for tag properties + //var tagEditors = new Dictionary(); + //foreach (var propertyTypeDto in indexedPropertyTypeDtos.Values) + //{ + // var editorAlias = propertyTypeDto.DataTypeDto.EditorAlias; + // var editorAttribute = PropertyEditors[editorAlias].GetTagAttribute(); + // if (editorAttribute == null) continue; + // var tagConfigurationSource = propertyTypeDto.DataTypeDto.Configuration; + // var tagConfiguration = string.IsNullOrWhiteSpace(tagConfigurationSource) + // ? new TagConfiguration() + // : JsonConvert.DeserializeObject(tagConfigurationSource); + // if (tagConfiguration.Delimiter == default) tagConfiguration.Delimiter = editorAttribute.Delimiter; + // tagEditors[editorAlias] = tagConfiguration; + //} // now we have // - the definitions @@ -533,10 +534,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // - tag editors // and we need to build the proper property collections - return GetPropertyCollections(temps, allPropertyDataDtos, tagEditors); + return GetPropertyCollections(temps, allPropertyDataDtos + /*, tagEditors*/); } - private IDictionary GetPropertyCollections(List> temps, IEnumerable allPropertyDataDtos, Dictionary tagConfigurations) + private IDictionary GetPropertyCollections(List> temps, IEnumerable allPropertyDataDtos + /*, Dictionary tagConfigurations*/) where T : class, IContentBase { var result = new Dictionary(); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 7b003d0a26..5492f97409 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1139,9 +1139,6 @@ namespace Umbraco.Core.Services.Implement if (publishing) { - //TODO: Check if it's a culture being unpublished and that's why we are publishing the document. In that case - // we should check if the culture is already unpublished since it might be the user is trying to unpublish an already unpublished culture? - //determine cultures publishing/unpublishing which will be based on previous calls to content.PublishCulture and ClearPublishInfo culturesUnpublishing = content.GetCulturesUnpublishing(); culturesPublishing = variesByCulture From 20d28ea2159f90b833ea14f8317f644763fbb3ea Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 19:08:37 +1000 Subject: [PATCH 10/11] removes commented out code --- .../Implement/ContentRepositoryBase.cs | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 60beaca018..7ab73f3f2d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -512,34 +512,16 @@ namespace Umbraco.Core.Persistence.Repositories.Implement foreach (var a in allPropertyDataDtos) a.PropertyTypeDto = indexedPropertyTypeDtos[a.PropertyTypeId]; - //TODO: This logic was here but then we never used the data being passed to GetPropertyCollections, this will save a bit of CPU without it! - //// prefetch configuration for tag properties - //var tagEditors = new Dictionary(); - //foreach (var propertyTypeDto in indexedPropertyTypeDtos.Values) - //{ - // var editorAlias = propertyTypeDto.DataTypeDto.EditorAlias; - // var editorAttribute = PropertyEditors[editorAlias].GetTagAttribute(); - // if (editorAttribute == null) continue; - // var tagConfigurationSource = propertyTypeDto.DataTypeDto.Configuration; - // var tagConfiguration = string.IsNullOrWhiteSpace(tagConfigurationSource) - // ? new TagConfiguration() - // : JsonConvert.DeserializeObject(tagConfigurationSource); - // if (tagConfiguration.Delimiter == default) tagConfiguration.Delimiter = editorAttribute.Delimiter; - // tagEditors[editorAlias] = tagConfiguration; - //} - // now we have // - the definitions // - all property data dtos - // - tag editors + // - tag editors (Actually ... no we don't since i removed that code, but we don't need them anyways it seems) // and we need to build the proper property collections - return GetPropertyCollections(temps, allPropertyDataDtos - /*, tagEditors*/); + return GetPropertyCollections(temps, allPropertyDataDtos); } - private IDictionary GetPropertyCollections(List> temps, IEnumerable allPropertyDataDtos - /*, Dictionary tagConfigurations*/) + private IDictionary GetPropertyCollections(List> temps, IEnumerable allPropertyDataDtos) where T : class, IContentBase { var result = new Dictionary(); From 108416638c837ad25a52dab9a142db909d789d46 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 22:28:06 +1000 Subject: [PATCH 11/11] removes commented out code --- src/Umbraco.Tests/Services/ContentServiceTests.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 10bb63211b..39bebbe90b 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -828,18 +828,7 @@ namespace Umbraco.Tests.Services //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.AreEqual(PublishedState.Unpublished, content.PublishedState); //just double checking - Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); - //TODO: This fails!? Why? Because in the ContentService.CommitDocumentChangesInternal method when we detect it's the last lang being unpublished, - // we swap the unpublishing flag to true which means we end up setting the property content.PublishedState = PublishedState.Unpublishing, because of this - // inside of the DocumentRepository we treat many things differently including not publishing the removed culture. - // So how do we fix that? Well when we unpublish the last culture we want to both save the unpublishing of the culture AND unpublish the document, the easy - // way to do this will be to perform a double save operation, though that's not the prettiest way to do it. Ideally we take care of this all in one process - // in the DocumentRepository but that might require another transitory status like PublishedState.UnpublishingLastCulture ... though that's not pretty either. - // It might be possible in some other magicaly way, i'm just leaving these notes here mostly for myself :) - // UPDATE - ok, i 'think' instead of doing a check inside of CommitDocumentChangesInternal for the last culture being unpublished, we could actually do this check - // directly inside of the DocumentRepository - // UPDATE 2 - ok that won't work because there is some business logic that needs to occur in CommitDocumentChangesInternal when it needs to be unpublished, this will - // be tricky without doing a double save, hrm. + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); Assert.IsFalse(content.IsCulturePublished(langUk.IsoCode)); }