diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 4f88c2b803..9d65960f64 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -23,6 +23,33 @@ namespace Umbraco.Core #region IContent + /// + /// Gets the current status of the Content + /// + public static ContentStatus GetStatus(this IContent content, string culture = null) + { + if (content.Trashed) + return ContentStatus.Trashed; + + if (!content.ContentType.VariesByCulture()) + culture = string.Empty; + else if (culture.IsNullOrWhiteSpace()) + throw new ArgumentNullException($"{nameof(culture)} cannot be null or empty"); + + var expires = content.ContentSchedule.GetSchedule(culture, ContentScheduleChange.End); + if (expires != null && expires.Any(x => x.Date > DateTime.MinValue && DateTime.Now > x.Date)) + return ContentStatus.Expired; + + var release = content.ContentSchedule.GetSchedule(culture, ContentScheduleChange.Start); + if (release != null && release.Any(x => x.Date > DateTime.MinValue && x.Date > DateTime.Now)) + return ContentStatus.AwaitingRelease; + + if (content.Published) + return ContentStatus.Published; + + return ContentStatus.Unpublished; + } + /// /// Returns true if this entity was just published as part of a recent save operation (i.e. it wasn't previously published) /// diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 94f6d0beb7..33f9b4f088 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -139,33 +139,6 @@ namespace Umbraco.Core.Models set => SetPropertyValueAndDetectChanges(value, ref _template, Ps.Value.TemplateSelector); } - /// - /// Gets the current status of the Content - /// - [IgnoreDataMember] - public ContentStatus Status - { - get - { - if(Trashed) - return ContentStatus.Trashed; - - //fixme - deal with variants - var expires = ContentSchedule.GetSchedule(ContentScheduleChange.End); - if (expires != null && expires.Any(x => x.Date > DateTime.MinValue && DateTime.Now > x.Date)) - return ContentStatus.Expired; - - //fixme - deal with variants - var release = ContentSchedule.GetSchedule(ContentScheduleChange.Start); - if (release != null && release.Any(x => x.Date > DateTime.MinValue && x.Date > DateTime.Now)) - return ContentStatus.AwaitingRelease; - - if(Published) - return ContentStatus.Published; - - return ContentStatus.Unpublished; - } - } /// /// Gets or sets a value indicating whether this content item is published or not. diff --git a/src/Umbraco.Core/Models/ContentScheduleCollection.cs b/src/Umbraco.Core/Models/ContentScheduleCollection.cs index 52dcad9e45..14fb30984a 100644 --- a/src/Umbraco.Core/Models/ContentScheduleCollection.cs +++ b/src/Umbraco.Core/Models/ContentScheduleCollection.cs @@ -9,7 +9,8 @@ namespace Umbraco.Core.Models public class ContentScheduleCollection : INotifyCollectionChanged, IDeepCloneable { //underlying storage for the collection backed by a sorted list so that the schedule is always in order of date - private readonly Dictionary> _schedule = new Dictionary>(); + private readonly Dictionary> _schedule + = new Dictionary>(StringComparer.InvariantCultureIgnoreCase); public event NotifyCollectionChangedEventHandler CollectionChanged; @@ -161,14 +162,12 @@ namespace Umbraco.Core.Models return Enumerable.Empty(); } + //fixme - should this just return IEnumerable since the culture is part of the ContentSchedule object already? /// /// Returns all schedules for both invariant and variant cultures /// /// - public IReadOnlyDictionary> GetFullSchedule() - { - return _schedule.ToDictionary(x => x.Key, x => (IEnumerable)x.Value.Values); - } + public IReadOnlyDictionary> FullSchedule => _schedule.ToDictionary(x => x.Key, x => (IEnumerable)x.Value.Values); public object DeepClone() { diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 20590a8d36..6ce96e5170 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -71,11 +71,6 @@ namespace Umbraco.Core.Models /// IContentType ContentType { get; } - /// - /// Gets the current status of the content. - /// - ContentStatus Status { get; } - /// /// Gets a value indicating whether a culture is published. /// @@ -168,7 +163,7 @@ namespace Umbraco.Core.Models /// /// A value indicating whether the culture can be published. /// - /// Fails if properties don't pass variant validtion rules. + /// Fails if properties don't pass variant validation rules. /// Publishing must be finalized via the content service SavePublishing method. /// bool PublishCulture(string culture = "*"); diff --git a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs index fc81f3e6a4..d821195df5 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs @@ -176,7 +176,7 @@ namespace Umbraco.Core.Persistence.Factories public static IEnumerable BuildScheduleDto(IContent entity, ILanguageRepository languageRepository) { var schedule = new List(); - foreach (var schedByCulture in entity.ContentSchedule.GetFullSchedule()) + foreach (var schedByCulture in entity.ContentSchedule.FullSchedule) { foreach (var cultureSched in schedByCulture.Value) { diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 903aa28679..d80081ea7a 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Events; using Umbraco.Core.Exceptions; @@ -1064,30 +1065,6 @@ namespace Umbraco.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { - // is the content going to end up published, or unpublished? - if (publishing && content.ContentType.VariesByCulture()) - { - var publishedCultures = content.PublishedCultures.ToList(); - var cannotBePublished = publishedCultures.Count == 0; // no published cultures = cannot be published - if (!cannotBePublished) - { - var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); - 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 - } - else - { - culturesChanging = content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList(); - } - } var isNew = !content.HasIdentity; var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch; @@ -1107,12 +1084,30 @@ namespace Umbraco.Core.Services.Implement { // ensure that the document can be published, and publish // handling events, business rules, etc - // note: StrategyPublish flips the PublishedState to Publishing! publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ true, evtMsgs); if (publishResult.Success) - publishResult = StrategyPublish(scope, content, /*canPublish:*/ true, userId, evtMsgs); - if (!publishResult.Success) - ((Content) content).Published = content.Published; // reset published state = save unchanged + { + culturesChanging = content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList(); + // note: StrategyPublish flips the PublishedState to Publishing! + publishResult = StrategyPublish(scope, content, userId, evtMsgs); + } + else + { + //check for mandatory culture missing, if this is the case we'll switch the + //unpublishing flag + //SD: this is the logic that was happening above but is now moved into the StrategyCanPublish + if (publishResult.Result == PublishResultType.FailedMandatoryCultureMissing) + { + 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 + } + + //fixme - casting + ((Content)content).Published = content.Published; // reset published state = save unchanged + } + } if (unpublishing) @@ -1129,9 +1124,12 @@ namespace Umbraco.Core.Services.Implement // note: This unpublishes the entire document (not different variants) unpublishResult = StrategyCanUnpublish(scope, content, userId, evtMsgs); if (unpublishResult.Success) - unpublishResult = StrategyUnpublish(scope, content, true, userId, evtMsgs); - if (!unpublishResult.Success) - ((Content) content).Published = content.Published; // reset published state = save unchanged + unpublishResult = StrategyUnpublish(scope, content, userId, evtMsgs); + else + { + //fixme - casting + ((Content)content).Published = content.Published; // reset published state = save unchanged + } } else { @@ -1209,21 +1207,13 @@ namespace Umbraco.Core.Services.Implement return publishResult; } - // or, failed - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Complete(); // compete the save - return publishResult; } - // 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 + // or, failed scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); scope.Complete(); // compete the save - return new PublishResult(PublishResultType.FailedByCulture, evtMsgs, content); + return publishResult; } } @@ -1412,7 +1402,7 @@ namespace Umbraco.Core.Services.Implement return result; // publish - should be successful - var publishResult = StrategyPublish(scope, document, /*canPublish:*/ true, userId, evtMsgs); + var publishResult = StrategyPublish(scope, document, userId, evtMsgs); if (!publishResult.Success) throw new Exception("oops: failed to publish."); @@ -2239,9 +2229,17 @@ namespace Umbraco.Core.Services.Implement #region Publishing Strategies - // ensures that a document can be published - internal PublishResult StrategyCanPublish(IScope scope, IContent content, int userId, bool checkPath, EventMessages evtMsgs) - { + /// + /// Ensures that a document can be published + /// + /// + /// + /// + /// + /// + /// + private PublishResult StrategyCanPublish(IScope scope, IContent content, int userId, bool checkPath, EventMessages evtMsgs) + { // raise Publishing event if (scope.Events.DispatchCancelable(Publishing, this, new PublishEventArgs(content, evtMsgs))) { @@ -2249,28 +2247,63 @@ namespace Umbraco.Core.Services.Implement return new PublishResult(PublishResultType.FailedCancelledByEvent, evtMsgs, content); } + var variesByCulture = content.ContentType.VariesByCulture(); + var culturesChanging = variesByCulture ? new List() : null; + + //First check if mandatory languages fails, we are checking this first because this logic used to occur + //before the call to this method so I'm ensuring the order remains the same since we check the response + //specifically for FailedMandatoryCultureMissing + if (variesByCulture) + { + var publishedCultures = content.PublishedCultures.ToList(); + var cannotBePublished = publishedCultures.Count == 0; // no published cultures = cannot be published + if (!cannotBePublished) + { + var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); + cannotBePublished = mandatoryCultures.Any(x => !publishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); // missing mandatory culture = cannot be published + } + + if (cannotBePublished) + return new PublishResult(PublishResultType.FailedMandatoryCultureMissing, evtMsgs, content); + + //track which cultures are being published + culturesChanging = content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList(); + } + // ensure that the document has published values // either because it is 'publishing' or because it already has a published version + //fixme - casting if (((Content) content).PublishedState != PublishedState.Publishing && content.PublishedVersionId == 0) { Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document does not have published values"); return new PublishResult(PublishResultType.FailedNoPublishedValues, evtMsgs, content); } - // ensure that the document status is correct - switch (content.Status) + //loop over each culture changing - or string.Empty for invariant + foreach(var culture in culturesChanging == null ? new[] { string.Empty } : (IEnumerable)culturesChanging) { - case ContentStatus.Expired: - Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document has expired"); - return new PublishResult(PublishResultType.FailedHasExpired, evtMsgs, content); + // ensure that the document status is correct + // note: culture will be string.Empty for invariant + switch (content.GetStatus(culture)) + { + case ContentStatus.Expired: + if (!variesByCulture) + Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document has expired"); + else + Logger.Info("Document {ContentName} (id={ContentId}) culture {Culture} cannot be published: {Reason}", content.Name, content.Id, culture, "document culture has expired"); + return new PublishResult(!variesByCulture ? PublishResultType.FailedHasExpired : PublishResultType.FailedCultureHasExpired, evtMsgs, content); - case ContentStatus.AwaitingRelease: - Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document is awaiting release"); - return new PublishResult(PublishResultType.FailedAwaitingRelease, evtMsgs, content); + case ContentStatus.AwaitingRelease: + if (!variesByCulture) + Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document is awaiting release"); + else + Logger.Info("Document {ContentName} (id={ContentId}) culture {Culture} cannot be published: {Reason}", content.Name, content.Id, culture, "document is culture awaiting release"); + return new PublishResult(!variesByCulture ? PublishResultType.FailedAwaitingRelease : PublishResultType.FailedCultureAwaitingRelease, evtMsgs, content); - case ContentStatus.Trashed: - Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document is trashed"); - return new PublishResult(PublishResultType.FailedIsTrashed, evtMsgs, content); + case ContentStatus.Trashed: + Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document is trashed"); + return new PublishResult(PublishResultType.FailedIsTrashed, evtMsgs, content); + } } if (!checkPath) return new PublishResult(evtMsgs, content); @@ -2288,29 +2321,41 @@ namespace Umbraco.Core.Services.Implement return new PublishResult(evtMsgs, content); } - // publishes a document - internal PublishResult StrategyPublish(IScope scope, IContent content, bool canPublish, int userId, EventMessages evtMsgs) + /// + /// Publishes a document + /// + /// + /// + /// + /// + /// + /// + /// It is assumed that all publishing checks have passed before calling this method like + /// + private PublishResult StrategyPublish(IScope scope, IContent content, int userId, EventMessages evtMsgs) { - // note: when used at top-level, StrategyCanPublish with checkPath=true should have run already - // and alreadyCheckedCanPublish should be true, so not checking again. when used at nested level, - // there is no need to check the path again. so, checkPath=false in StrategyCanPublish below - - var result = canPublish - ? new PublishResult(evtMsgs, content) // already know we can - : StrategyCanPublish(scope, content, userId, /*checkPath:*/ false, evtMsgs); // else check + var result = new PublishResult(evtMsgs, content); if (result.Success == false) return result; // change state to publishing + // fixme - casting ((Content) content).PublishedState = PublishedState.Publishing; Logger.Info("Document {ContentName} (id={ContentId}) has been published.", content.Name, content.Id); return result; } - // ensures that a document can be unpublished - internal UnpublishResult StrategyCanUnpublish(IScope scope, IContent content, int userId, EventMessages evtMsgs) + /// + /// Ensures that a document can be unpublished + /// + /// + /// + /// + /// + /// + private UnpublishResult StrategyCanUnpublish(IScope scope, IContent content, int userId, EventMessages evtMsgs) { // raise Unpublishing event if (scope.Events.DispatchCancelable(Unpublishing, this, new PublishEventArgs(content, evtMsgs))) @@ -2322,12 +2367,20 @@ namespace Umbraco.Core.Services.Implement return new UnpublishResult(evtMsgs, content); } - // unpublishes a document - internal UnpublishResult StrategyUnpublish(IScope scope, IContent content, bool canUnpublish, int userId, EventMessages evtMsgs) + /// + /// Unpublishes a document + /// + /// + /// + /// + /// + /// + /// + /// It is assumed that all unpublishing checks have passed before calling this method like + /// + internal UnpublishResult StrategyUnpublish(IScope scope, IContent content, int userId, EventMessages evtMsgs) { - var attempt = canUnpublish - ? new UnpublishResult(evtMsgs, content) // already know we can - : StrategyCanUnpublish(scope, content, userId, evtMsgs); // else check + var attempt = new UnpublishResult(evtMsgs, content); if (attempt.Success == false) return attempt; @@ -2336,7 +2389,7 @@ namespace Umbraco.Core.Services.Implement // they should be removed so they don't interrupt an unpublish // otherwise it would remain released == published - var pastReleases = content.ContentSchedule.GetFullSchedule().SelectMany(x => x.Value) + var pastReleases = content.ContentSchedule.FullSchedule.SelectMany(x => x.Value) .Where(x => x.Change == ContentScheduleChange.End && x.Date <= DateTime.Now) .ToList(); foreach (var p in pastReleases) @@ -2345,6 +2398,7 @@ namespace Umbraco.Core.Services.Implement Logger.Info("Document {ContentName} (id={ContentId}) had its release date removed, because it was unpublished.", content.Name, content.Id); // change state to unpublishing + // fixme - casting ((Content) content).PublishedState = PublishedState.Unpublishing; Logger.Info("Document {ContentName} (id={ContentId}) has been unpublished.", content.Name, content.Id); diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index 15b2f503c7..d724e4cda9 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -42,39 +42,48 @@ /// FailedAwaitingRelease = Failed | 3, + /// + /// A culture on the content item was scheduled to be un-published and it has expired so we cannot force it to be + /// published again as part of a bulk publish operation. + /// + FailedCultureHasExpired = Failed | 4, + + /// + /// A culture on the content item is scheduled to be released in the future and therefore we cannot force it to + /// be published during a bulk publish operation. + /// + FailedCultureAwaitingRelease = Failed | 5, + /// /// The content item could not be published because it is in the trash. /// - FailedIsTrashed = Failed | 4, + FailedIsTrashed = Failed | 6, /// /// The publish action has been cancelled by an event handler. /// - FailedCancelledByEvent = Failed | 5, + FailedCancelledByEvent = Failed | 7, /// /// The content item could not be published because it contains invalid data (has not passed validation requirements). /// - FailedContentInvalid = Failed | 6, + FailedContentInvalid = Failed | 8, /// /// Cannot republish a document that hasn't been published. /// - 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? + FailedNoPublishedValues = Failed | 9, // in ContentService.StrategyCanPublish - fixme weird /// /// Publishing changes triggered an unpublishing, due to missing mandatory cultures, and unpublishing failed. /// - FailedToUnpublish = Failed | 9, // in ContentService.SavePublishing + FailedToUnpublish = Failed | 10, // in ContentService.SavePublishing /// /// Some mandatory cultures are missing. /// - FailedByCulture = Failed | 10, // in ContentService.SavePublishing + FailedMandatoryCultureMissing = Failed | 11, // in ContentService.SavePublishing + + } } diff --git a/src/Umbraco.Tests/Models/ContentScheduleTests.cs b/src/Umbraco.Tests/Models/ContentScheduleTests.cs index 1e28b2e9b8..345c345bd4 100644 --- a/src/Umbraco.Tests/Models/ContentScheduleTests.cs +++ b/src/Umbraco.Tests/Models/ContentScheduleTests.cs @@ -44,7 +44,7 @@ namespace Umbraco.Tests.Models schedule.Add(now, null); var invariantSched = schedule.GetSchedule(string.Empty); schedule.Remove(invariantSched.First()); - Assert.AreEqual(0, schedule.GetFullSchedule().Count()); + Assert.AreEqual(0, schedule.FullSchedule.Count()); } [Test] @@ -57,11 +57,11 @@ namespace Umbraco.Tests.Models var invariantSched = schedule.GetSchedule(string.Empty); schedule.Remove(invariantSched.First()); Assert.AreEqual(0, schedule.GetSchedule(string.Empty).Count()); - Assert.AreEqual(1, schedule.GetFullSchedule().Count()); + Assert.AreEqual(1, schedule.FullSchedule.Count()); var variantSched = schedule.GetSchedule("en-US"); schedule.Remove(variantSched.First()); Assert.AreEqual(0, schedule.GetSchedule("en-US").Count()); - Assert.AreEqual(0, schedule.GetFullSchedule().Count()); + Assert.AreEqual(0, schedule.FullSchedule.Count()); } [Test] @@ -75,7 +75,7 @@ namespace Umbraco.Tests.Models Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleChange.Start).Count()); Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.End).Count()); - Assert.AreEqual(1, schedule.GetFullSchedule().Count()); + Assert.AreEqual(1, schedule.FullSchedule.Count()); } [Test] @@ -92,7 +92,7 @@ namespace Umbraco.Tests.Models Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.Start).Count()); Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.End).Count()); Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.Start).Count()); - Assert.AreEqual(2, schedule.GetFullSchedule().Count()); + Assert.AreEqual(2, schedule.FullSchedule.Count()); schedule.Clear("en-US", ContentScheduleChange.End); @@ -100,7 +100,7 @@ namespace Umbraco.Tests.Models Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.Start).Count()); Assert.AreEqual(0, schedule.GetSchedule("en-US", ContentScheduleChange.End).Count()); Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.Start).Count()); - Assert.AreEqual(2, schedule.GetFullSchedule().Count()); + Assert.AreEqual(2, schedule.FullSchedule.Count()); } } diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 35b5ed0c0d..14abb342a2 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -260,7 +260,7 @@ namespace Umbraco.Tests.Services contentService.Save(content, Constants.Security.SuperUserId); content = contentService.GetById(content.Id); - var sched = content.ContentSchedule.GetFullSchedule(); + var sched = content.ContentSchedule.FullSchedule; Assert.AreEqual(1, sched.Count()); Assert.AreEqual(1, sched[string.Empty].Count()); content.ContentSchedule.Clear(ContentScheduleChange.End); @@ -269,7 +269,7 @@ namespace Umbraco.Tests.Services // Assert content = contentService.GetById(content.Id); - sched = content.ContentSchedule.GetFullSchedule(); + sched = content.ContentSchedule.FullSchedule; Assert.AreEqual(0, sched.Count()); Assert.IsTrue(contentService.SaveAndPublish(content).Success); } @@ -1521,6 +1521,26 @@ namespace Umbraco.Tests.Services Assert.That(parentPublished.Success, Is.True); Assert.That(published.Success, Is.False); Assert.That(content.Published, Is.False); + Assert.AreEqual(PublishResultType.FailedHasExpired, published.Result); + } + + [Test] + public void Cannot_Publish_Expired_Culture() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + var content = MockedContent.CreateBasicContent(contentType); + content.SetCultureName("Hello", "en-US"); + content.ContentSchedule.Add("en-US", null, DateTime.Now.AddMinutes(-5)); + ServiceContext.ContentService.Save(content); + + var published = ServiceContext.ContentService.SaveAndPublish(content, "en-US", Constants.Security.SuperUserId); + + Assert.IsFalse(published.Success); + Assert.AreEqual(PublishResultType.FailedCultureHasExpired, published.Result); + Assert.That(content.Published, Is.False); } [Test] @@ -1544,6 +1564,26 @@ namespace Umbraco.Tests.Services Assert.That(parentPublished.Success, Is.True); Assert.That(published.Success, Is.False); Assert.That(content.Published, Is.False); + Assert.AreEqual(PublishResultType.FailedAwaitingRelease, published.Result); + } + + [Test] + public void Cannot_Publish_Culture_Awaiting_Release() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + var content = MockedContent.CreateBasicContent(contentType); + content.SetCultureName("Hello", "en-US"); + content.ContentSchedule.Add("en-US", DateTime.Now.AddHours(2), null); + ServiceContext.ContentService.Save(content); + + var published = ServiceContext.ContentService.SaveAndPublish(content, "en-US", Constants.Security.SuperUserId); + + Assert.IsFalse(published.Success); + Assert.AreEqual(PublishResultType.FailedCultureAwaitingRelease, published.Result); + Assert.That(content.Published, Is.False); } [Test] diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 7f16d66480..ef1afaa2e2 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -845,7 +845,8 @@ namespace Umbraco.Web.Editors if (canPublish) { - //try to publish all the values on the model + //try to publish all the values on the model - this will generally only fail if someone is tampering with the request + //since there's no reason variant rules would be violated in normal cases. canPublish = PublishCulture(contentItem.PersistedContent, cultureVariants); } @@ -860,7 +861,7 @@ namespace Umbraco.Web.Editors { //can only save var saveResult = Services.ContentService.Save(contentItem.PersistedContent, Security.CurrentUser.Id); - publishStatus = new PublishResult(PublishResultType.FailedCannotPublish, null, contentItem.PersistedContent); + publishStatus = new PublishResult(PublishResultType.FailedMandatoryCultureMissing, null, contentItem.PersistedContent); wasCancelled = saveResult.Result == OperationResultType.FailedCancelledByEvent; successfulCultures = Array.Empty(); } @@ -904,6 +905,9 @@ namespace Umbraco.Web.Editors /// /// /// + /// + /// This would generally never fail unless someone is tampering with the request + /// private bool PublishCulture(IContent persistentContent, IEnumerable cultureVariants) { foreach(var variant in cultureVariants.Where(x => x.Publish)) @@ -1573,7 +1577,7 @@ namespace Umbraco.Web.Editors string.Join(",", status.InvalidProperties.Select(x => x.Alias)) }).Trim()); break; - case PublishResultType.FailedByCulture: + case PublishResultType.FailedMandatoryCultureMissing: display.AddWarningNotification( Services.TextService.Localize("publish"), "publish/contentPublishedFailedByCulture"); // fixme properly localize, these keys are missing from lang files!