From 0af18b23765bd33899e2b9d7bb6b7dd97c97004f Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 5 Jul 2018 17:07:40 +0200 Subject: [PATCH] Fix publishing --- .../Services/Implement/ContentService.cs | 39 +++++++++++-------- .../Services/PublishResultType.cs | 21 ++++++++-- src/Umbraco.Web/Editors/ContentController.cs | 32 ++++++++------- 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 90619abf05..731f29ed4e 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1055,27 +1055,31 @@ namespace Umbraco.Core.Services.Implement if (content.PublishedState != PublishedState.Publishing && content.PublishedState != PublishedState.Unpublishing) ((Content) content).PublishedState = PublishedState.Publishing; + // state here is either Publishing or Unpublishing + var publishing = content.PublishedState == PublishedState.Publishing; + var unpublishing = content.PublishedState == PublishedState.Unpublishing; + using (var scope = ScopeProvider.CreateScope()) { // is the content going to end up published, or unpublished? - bool publishing, unpublishing; - if (content.ContentType.VariesByCulture()) + if (publishing && content.ContentType.VariesByCulture()) { var publishedCultures = content.PublishedCultures.ToList(); - var cannotBePublished= publishedCultures.Count == 0; // no published cultures = cannot be published + var cannotBePublished = publishedCultures.Count == 0; // no published cultures = cannot be published if (!cannotBePublished) { var mandatoryCultures = _languageRepository.GetMany().Where(x => x.Mandatory).Select(x => x.IsoCode); - cannotBePublished = mandatoryCultures.Any(x => !publishedCultures.Contains(x)); // missing mandatory culture = cannot be published + cannotBePublished = mandatoryCultures.Any(x => !publishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); // missing mandatory culture = cannot be published + } + + if (cannotBePublished) + { + 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 anways } - unpublishing = content.Published && cannotBePublished; // if we cannot be published, and we are published, we unpublish - publishing = !cannotBePublished; // if we can be published, we publish - } - else - { - // invariant: we can publish, no culture problem, no need to unpublish - publishing = content.PublishedState == PublishedState.Publishing; - unpublishing = content.PublishedState == PublishedState.Unpublishing; } var isNew = !content.HasIdentity; @@ -1160,7 +1164,7 @@ namespace Umbraco.Core.Services.Implement // or, failed scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); scope.Complete(); // compete the save - return new PublishResult(PublishResultType.Failed, evtMsgs, content); // bah + return new PublishResult(PublishResultType.FailedToUnpublish, evtMsgs, content); // bah } if (publishing) // we have tried to publish @@ -1194,12 +1198,15 @@ namespace Umbraco.Core.Services.Implement return publishResult; } - // we haven't tried anything - assume that is bad (may need to reconsider the case of unpublishing - // a culture, and the culture or content item was already unpublished...) - bah + // both publishing and unpublishing are false + // this means that we wanted to publish, in a variant scenario, a document that + // was not published yet, and we could not, due to cultures issues + // + // raise event (we saved), report scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); scope.Complete(); // compete the save - return new PublishResult(PublishResultType.Failed, evtMsgs, content); + return new PublishResult(PublishResultType.FailedByCulture, evtMsgs, content); } } diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index b4bfe078b7..15b2f503c7 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -18,7 +18,7 @@ /// The item was already published. /// SuccessAlready = 1, - + /// /// The operation failed. /// @@ -58,8 +58,23 @@ FailedContentInvalid = Failed | 6, /// - /// The document could not be published because it does not have published values. + /// Cannot republish a document that hasn't been published. /// - FailedNoPublishedValues = Failed | 7 + FailedNoPublishedValues = Failed | 7, // in ContentService.StrategyCanPublish - fixme weird + + /// + /// Some mandatory cultures are missing, or are not valid. + /// + FailedCannotPublish = Failed | 8, // in ContentController.PublishInternal - fixme // FailedByCulture? + + /// + /// Publishing changes triggered an unpublishing, due to missing mandatory cultures, and unpublishing failed. + /// + FailedToUnpublish = Failed | 9, // in ContentService.SavePublishing + + /// + /// Some mandatory cultures are missing. + /// + FailedByCulture = Failed | 10, // in ContentService.SavePublishing } } diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 6c0313bd1f..0033a97bce 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -772,7 +772,7 @@ namespace Umbraco.Web.Editors { //can only save var saveResult = Services.ContentService.Save(contentItem.PersistedContent, Security.CurrentUser.Id); - publishStatus = new PublishResult(PublishResultType.Failed, null, contentItem.PersistedContent); + publishStatus = new PublishResult(PublishResultType.FailedCannotPublish, null, contentItem.PersistedContent); wasCancelled = saveResult.Result == OperationResultType.FailedCancelledByEvent; } } @@ -1150,7 +1150,7 @@ namespace Umbraco.Web.Editors display.AddWarningNotification( Services.TextService.Localize("publish"), Services.TextService.Localize("publish/contentPublishedFailedByParent", - new[] { string.Format("{0} ({1})", status.Content.Name, status.Content.Id) }).Trim()); + new[] { $"{status.Content.Name} ({status.Content.Id})" }).Trim()); break; case PublishResultType.FailedCancelledByEvent: AddCancelMessage(display, "publish", "speechBubbles/contentPublishedFailedByEvent"); @@ -1159,32 +1159,36 @@ namespace Umbraco.Web.Editors display.AddWarningNotification( Services.TextService.Localize("publish"), Services.TextService.Localize("publish/contentPublishedFailedAwaitingRelease", - new[] { string.Format("{0} ({1})", status.Content.Name, status.Content.Id) }).Trim()); + new[] { $"{status.Content.Name} ({status.Content.Id})" }).Trim()); break; case PublishResultType.FailedHasExpired: display.AddWarningNotification( Services.TextService.Localize("publish"), Services.TextService.Localize("publish/contentPublishedFailedExpired", - new[] - { - string.Format("{0} ({1})", status.Content.Name, status.Content.Id), - }).Trim()); + new[] { $"{status.Content.Name} ({status.Content.Id})", }).Trim()); break; case PublishResultType.FailedIsTrashed: - //TODO: We should add proper error messaging for this! + display.AddWarningNotification( + Services.TextService.Localize("publish"), + "publish/contentPublishedFailedIsTrashed"); // fixme properly localize! break; case PublishResultType.FailedContentInvalid: display.AddWarningNotification( Services.TextService.Localize("publish"), Services.TextService.Localize("publish/contentPublishedFailedInvalid", - new[] - { - string.Format("{0} ({1})", status.Content.Name, status.Content.Id), - string.Join(",", status.InvalidProperties.Select(x => x.Alias)) - }).Trim()); + new[] + { + $"{status.Content.Name} ({status.Content.Id})", + string.Join(",", status.InvalidProperties.Select(x => x.Alias)) + }).Trim()); + break; + case PublishResultType.FailedByCulture: + display.AddWarningNotification( + Services.TextService.Localize("publish"), + "publish/contentPublishedFailedByCulture"); // fixme properly localize! break; default: - throw new IndexOutOfRangeException(); + throw new IndexOutOfRangeException($"PublishedResultType \"{status.Result}\" was not expected."); } }