diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index ff1925c72d..823d8bfb0a 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -194,15 +194,23 @@ namespace Umbraco.Core.Models Name = null; return; } + + if (!ContentTypeBase.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment)) + throw new NotSupportedException("Content type does not support varying name by culture."); - if (_names == null) return; + if (_names == null) return; + if (!_names.ContainsKey(culture)) + throw new InvalidOperationException($"Cannot unpublish culture {culture}, the document contains only cultures {string.Join(", ", _names.Keys)}"); _names.Remove(culture); if (_names.Count == 0) _names = null; } protected virtual void ClearNames() - { + { + if (!ContentTypeBase.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment)) + throw new NotSupportedException("Content type does not support varying name by culture."); + _names = null; OnPropertyChanged(Ps.Value.NamesSelector); } diff --git a/src/Umbraco.Core/Models/Entities/DocumentEntitySlim.cs b/src/Umbraco.Core/Models/Entities/DocumentEntitySlim.cs index 57cffa793c..50bec85af1 100644 --- a/src/Umbraco.Core/Models/Entities/DocumentEntitySlim.cs +++ b/src/Umbraco.Core/Models/Entities/DocumentEntitySlim.cs @@ -1,14 +1,24 @@ -namespace Umbraco.Core.Models.Entities +using System.Collections.Generic; + +namespace Umbraco.Core.Models.Entities { /// /// Implements . /// public class DocumentEntitySlim : ContentEntitySlim, IDocumentEntitySlim { + private static readonly IReadOnlyDictionary Empty = new Dictionary(); + private IReadOnlyDictionary _cultureNames; + public IReadOnlyDictionary CultureNames + { + get => _cultureNames ?? Empty; + set => _cultureNames = value; + } + /// public bool Published { get; set; } /// public bool Edited { get; set; } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Models/Entities/IDocumentEntitySlim.cs b/src/Umbraco.Core/Models/Entities/IDocumentEntitySlim.cs index 471c0ba1e1..19d5b7ee68 100644 --- a/src/Umbraco.Core/Models/Entities/IDocumentEntitySlim.cs +++ b/src/Umbraco.Core/Models/Entities/IDocumentEntitySlim.cs @@ -1,18 +1,30 @@ -namespace Umbraco.Core.Models.Entities +using System.Collections.Generic; + +namespace Umbraco.Core.Models.Entities { /// /// Represents a lightweight document entity, managed by the entity service. /// public interface IDocumentEntitySlim : IContentEntitySlim { - /// - /// Gets a value indicating whether the document is published. - /// - bool Published { get; } + //fixme we need to supply more information than this and change this property name. This will need to include Published/Editor per variation since we need this information for the tree + IReadOnlyDictionary CultureNames { get; } /// - /// Gets a value indicating whether the document has edited properties. + /// At least one variation is published /// - bool Edited { get; } + /// + /// If the document is invariant, this simply means there is a published version + /// + bool Published { get; set; } + + /// + /// At least one variation has pending changes + /// + /// + /// If the document is invariant, this simply means there is pending changes + /// + bool Edited { get; set; } + } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Models/PublishedState.cs b/src/Umbraco.Core/Models/PublishedState.cs index af9777710d..b7da2a25b1 100644 --- a/src/Umbraco.Core/Models/PublishedState.cs +++ b/src/Umbraco.Core/Models/PublishedState.cs @@ -1,7 +1,8 @@ using System; namespace Umbraco.Core.Models -{ +{ + /// /// The states of a content item. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs index 0dd7a25798..e7b88b3ebe 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs @@ -441,6 +441,9 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { [ResultColumn, Reference(ReferenceType.Many)] public List VariationInfo { get; set; } + + public bool Published { get; set; } + public bool Edited { get; set; } } /// @@ -477,8 +480,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement public DateTime CreateDate { get; set; } public int Children { get; set; } public int VersionId { get; set; } - public bool Published { get; set; } - public bool Edited { get; set; } public string Alias { get; set; } public string Icon { get; set; } public string Thumbnail { get; set; } @@ -826,7 +827,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement #region Factory - private static EntitySlim BuildEntity(bool isContent, bool isMedia, BaseDto dto) + private EntitySlim BuildEntity(bool isContent, bool isMedia, BaseDto dto) { if (isContent) return BuildDocumentEntity(dto); @@ -867,8 +868,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private static void BuildDocumentEntity(DocumentEntitySlim entity, BaseDto dto) { BuildContentEntity(entity, dto); - entity.Published = dto.Published; - entity.Edited = dto.Edited; } private static EntitySlim BuildContentEntity(BaseDto dto) @@ -879,8 +878,13 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return entity; } - private static EntitySlim BuildDocumentEntity(BaseDto dto) + private DocumentEntitySlim BuildDocumentEntity(BaseDto dto) { + if (dto is ContentEntityDto contentDto) + { + return BuildDocumentEntity(contentDto); + } + // EntitySlim does not track changes var entity = new DocumentEntitySlim(); BuildDocumentEntity(entity, dto); @@ -892,11 +896,16 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// /// /// - private EntitySlim BuildDocumentEntity(ContentEntityDto dto) + private DocumentEntitySlim BuildDocumentEntity(ContentEntityDto dto) { // EntitySlim does not track changes var entity = new DocumentEntitySlim(); BuildDocumentEntity(entity, dto); + + //fixme we need to set these statuses for each variant, see notes in IDocumentEntitySlim + entity.Edited = dto.Edited; + entity.Published = dto.Published; + var variantInfo = new Dictionary(StringComparer.InvariantCultureIgnoreCase); if (dto.VariationInfo != null) { @@ -906,7 +915,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (isoCode != null) variantInfo[isoCode] = info.Name; } - entity.AdditionalData["CultureNames"] = variantInfo; + entity.CultureNames = variantInfo; } return entity; } diff --git a/src/Umbraco.Core/Publishing/ScheduledPublisher.cs b/src/Umbraco.Core/Publishing/ScheduledPublisher.cs index b16cd961f0..c4ebfff821 100644 --- a/src/Umbraco.Core/Publishing/ScheduledPublisher.cs +++ b/src/Umbraco.Core/Publishing/ScheduledPublisher.cs @@ -14,11 +14,13 @@ namespace Umbraco.Core.Publishing { private readonly IContentService _contentService; private readonly ILogger _logger; + private readonly IUserService _userService; - public ScheduledPublisher(IContentService contentService, ILogger logger) + public ScheduledPublisher(IContentService contentService, ILogger logger, IUserService userService) { _contentService = contentService; _logger = logger; + _userService = userService; } /// @@ -40,7 +42,7 @@ namespace Umbraco.Core.Publishing { d.ReleaseDate = null; d.TryPublishValues(); // fixme variants? - var result = _contentService.SaveAndPublish(d, d.GetWriterProfile().Id); + var result = _contentService.SaveAndPublish(d, _userService.GetProfileById(d.WriterId).Id); _logger.Debug($"Result of publish attempt: {result.Result}"); if (result.Success == false) { @@ -66,7 +68,7 @@ namespace Umbraco.Core.Publishing try { d.ExpireDate = null; - var result = _contentService.Unpublish(d, d.GetWriterProfile().Id); + var result = _contentService.Unpublish(d, userId: _userService.GetProfileById(d.WriterId).Id); if (result.Success) { counter++; diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index abbab0ef39..21a9c9ad15 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -362,9 +362,9 @@ namespace Umbraco.Core.Services IEnumerable SaveAndPublishBranch(IContent content, bool force, Func editing, Func publishValues, int userId = 0); /// - /// Unpublishes a document. + /// Unpublishes a document or optionally unpublishes a culture and/or segment for the document. /// - PublishResult Unpublish(IContent content, int userId = 0); + PublishResult Unpublish(IContent content, string culture = null, string segment = null, int userId = 0); /// /// Gets a value indicating whether a document is path-publishable. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 919ca73273..0047c53982 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -859,7 +859,7 @@ namespace Umbraco.Core.Services.Implement /// public OperationResult Save(IContent content, int userId = 0, bool raiseEvents = true) { - var publishedState = ((Content) content).PublishedState; + var publishedState = content.PublishedState; if (publishedState != PublishedState.Published && publishedState != PublishedState.Unpublished) throw new InvalidOperationException("Cannot save a (un)publishing, use the dedicated (un)publish method."); @@ -1025,7 +1025,7 @@ namespace Umbraco.Core.Services.Implement } /// - public PublishResult Unpublish(IContent content, int userId = 0) + public PublishResult Unpublish(IContent content, string culture = null, string segment = null, int userId = 0) { var evtMsgs = EventMessagesFactory.Get(); @@ -1033,32 +1033,96 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); - var newest = GetById(content.Id); // ensure we have the newest version - if (content.VersionId != newest.VersionId) // but use the original object if it's already the newest version - content = newest; - if (content.Published == false) + var tryUnpublishVariation = TryUnpublishVariationInternal(scope, content, evtMsgs, culture, segment, userId); + + if (tryUnpublishVariation) return tryUnpublishVariation.Result; + + //continue the normal Unpublish operation to unpublish the entire content item + return UnpublishInternal(scope, content, evtMsgs, userId); + } + } + + /// + /// Used to unpublish the requested variant if possible + /// + /// + /// + /// + /// + /// + /// + /// + /// A successful attempt if a variant was unpublished and it wasn't the last, else a failed result if it's an invariant document or if it's the last. + /// A failed result indicates that a normal unpublish operation will need to be performed. + /// + private Attempt TryUnpublishVariationInternal(IScope scope, IContent content, EventMessages evtMsgs, string culture, string segment, int userId) + { + if (!culture.IsNullOrWhiteSpace() || !segment.IsNullOrWhiteSpace()) + { + //now we need to check if this is not the last culture/segment that is published + if (!culture.IsNullOrWhiteSpace()) { - scope.Complete(); - return new PublishResult(PublishResultType.SuccessAlready, evtMsgs, content); // already unpublished + //capture before we clear the values + var publishedCultureCount = content.PublishedCultures.Count(); + //fixme - this needs to be changed when 11227 is merged! + content.ClearPublishedValues(culture, segment); + //now we just publish with the name cleared + SaveAndPublish(content, userId); + Audit(AuditType.UnPublish, $"UnPublish variation culture: {culture ?? string.Empty}, segment: {segment ?? string.Empty} performed by user", userId, content.Id); + //This is not the last one, so complete the scope and return + if (publishedCultureCount > 1) + { + scope.Complete(); + return Attempt.Succeed(new PublishResult(PublishResultType.SuccessVariant, evtMsgs, content)); + } } - // strategy - // fixme should we still complete the uow? don't want to rollback here! - var attempt = StrategyCanUnpublish(scope, content, userId, evtMsgs); - if (attempt.Success == false) return attempt; // causes rollback - attempt = StrategyUnpublish(scope, content, true, userId, evtMsgs); - if (attempt.Success == false) return attempt; // causes rollback - - content.WriterId = userId; - _documentRepository.Save(content); - - scope.Events.Dispatch(UnPublished, this, new PublishEventArgs(content, false, false), "UnPublished"); - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshBranch).ToEventArgs()); - Audit(AuditType.UnPublish, "UnPublish performed by user", userId, content.Id); - - scope.Complete(); + if (!segment.IsNullOrWhiteSpace()) + { + //TODO: When segments are supported, implement this, a discussion needs to happen about what how a segment is 'published' or not + // since currently this is very different from a culture. + throw new NotImplementedException("Segments are currently not supported in Umbraco"); + } } + //This is either a non variant document or this is the last one, so return a failed result which indicates that a normal unpublish operation needs to also take place + return Attempt.Fail(); + } + + /// + /// Performs the logic to unpublish an entire document + /// + /// + /// + /// + /// + /// + private PublishResult UnpublishInternal(IScope scope, IContent content, EventMessages evtMsgs, int userId) + { + var newest = GetById(content.Id); // ensure we have the newest version + if (content.VersionId != newest.VersionId) // but use the original object if it's already the newest version + content = newest; + if (content.Published == false) + { + scope.Complete(); + return new PublishResult(PublishResultType.SuccessAlready, evtMsgs, content); // already unpublished + } + + // strategy + // fixme should we still complete the uow? don't want to rollback here! + var attempt = StrategyCanUnpublish(scope, content, userId, evtMsgs); + if (attempt.Success == false) return attempt; // causes rollback + attempt = StrategyUnpublish(scope, content, true, userId, evtMsgs); + if (attempt.Success == false) return attempt; // causes rollback + + content.WriterId = userId; + _documentRepository.Save(content); + + scope.Events.Dispatch(UnPublished, this, new PublishEventArgs(content, false, false), "UnPublished"); + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshBranch).ToEventArgs()); + Audit(AuditType.UnPublish, "UnPublish performed by user", userId, content.Id); + + scope.Complete(); return new PublishResult(PublishResultType.Success, evtMsgs, content); } @@ -1092,7 +1156,7 @@ namespace Umbraco.Core.Services.Implement try { d.ExpireDate = null; - var result = Unpublish(d, d.WriterId); + var result = Unpublish(d, userId: d.WriterId); if (result.Success == false) Logger.Error($"Failed to unpublish document id={d.Id}, reason={result.Result}."); } diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index c46a5e1725..e3c34b8439 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -18,6 +18,11 @@ /// SuccessAlready = 1, + /// + /// The specified variant was unpublished, the content item itself remains published. + /// + SuccessVariant = 2, + /// /// The operation failed. /// diff --git a/src/Umbraco.Examine/UmbracoExamineIndexer.cs b/src/Umbraco.Examine/UmbracoExamineIndexer.cs index f57ff07fa0..1c4624add2 100644 --- a/src/Umbraco.Examine/UmbracoExamineIndexer.cs +++ b/src/Umbraco.Examine/UmbracoExamineIndexer.cs @@ -387,7 +387,7 @@ namespace Umbraco.Examine //strip html of all users fields if we detect it has HTML in it. //if that is the case, we'll create a duplicate 'raw' copy of it so that we can return //the value of the field 'as-is'. - foreach (var value in e.IndexItem.ValueSet.Values) + foreach (var value in e.IndexItem.ValueSet.Values.ToList()) //ToList here to make a diff collection else we'll get collection modified errors { if (value.Value.Count > 0) { diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 8bb5f1bc9e..229a8c5ac2 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1214,7 +1214,7 @@ namespace Umbraco.Tests.Services } // Act - var unpublished = contentService.Unpublish(content, 0); + var unpublished = contentService.Unpublish(content, userId: 0); // Assert Assert.That(published.Success, Is.True); diff --git a/src/Umbraco.Tests/Services/EntityServiceTests.cs b/src/Umbraco.Tests/Services/EntityServiceTests.cs index 0546eb3c7b..cbcf0c3566 100644 --- a/src/Umbraco.Tests/Services/EntityServiceTests.cs +++ b/src/Umbraco.Tests/Services/EntityServiceTests.cs @@ -463,10 +463,11 @@ namespace Umbraco.Tests.Services var result = service.Get(c1.Id, UmbracoObjectTypes.Document); Assert.AreEqual("Test", result.Name); - Assert.IsTrue(result.AdditionalData.ContainsKey("CultureNames")); - var cultureNames = (IDictionary)result.AdditionalData["CultureNames"]; - Assert.AreEqual("Test - FR", cultureNames[_langFr.Id]); - Assert.AreEqual("Test - ES", cultureNames[_langEs.Id]); + Assert.IsNotNull(result as IDocumentEntitySlim); + var doc = (IDocumentEntitySlim)result; + var cultureNames = doc.CultureNames; + Assert.AreEqual("Test - FR", cultureNames[_langFr.IsoCode]); + Assert.AreEqual("Test - ES", cultureNames[_langEs.IsoCode]); } [Test] @@ -501,11 +502,9 @@ namespace Umbraco.Tests.Services if (i % 2 == 0) { Assert.AreEqual(1, entities[i].AdditionalData.Count); - Assert.AreEqual("CultureNames", entities[i].AdditionalData.Keys.First()); - var variantInfo = entities[i].AdditionalData.First().Value as IDictionary; - Assert.IsNotNull(variantInfo); - var keys = variantInfo.Keys.ToList(); - var vals = variantInfo.Values.ToList(); + var doc = (IDocumentEntitySlim)entities[i]; + var keys = doc.CultureNames.Keys.ToList(); + var vals = doc.CultureNames.Values.ToList(); Assert.AreEqual(_langFr.Id, keys[0]); Assert.AreEqual("Test " + i + " - FR", vals[0]); Assert.AreEqual(_langEs.Id, keys[1]); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.controller.js index 6601efef89..752636ab28 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.controller.js @@ -1,15 +1,16 @@ (function () { "use strict"; - + function PublishController($scope) { var vm = this; - var variants = $scope.model.variants; + vm.variants = $scope.model.variants; vm.changeSelection = changeSelection; vm.loading = true; - - vm.dirtyVariants = []; - vm.pristineVariants = []; + vm.dirtyVariantFilter = dirtyVariantFilter; + vm.pristineVariantFilter = pristineVariantFilter; + vm.hasDirtyVariants = false; + vm.hasPristineVariants = false; //watch this model, if it's reset, then re init $scope.$watch(function () { @@ -20,7 +21,7 @@ if (oldVal && oldVal.length) { //re-bind the selections for (var i = 0; i < oldVal.length; i++) { - var found = _.find(variants, function (v) { + var found = _.find(vm.variants, function (v) { return v.language.id === oldVal[i].language.id; }); if (found) { @@ -32,36 +33,45 @@ }); function changeSelection(variant) { - var firstSelected = _.find(variants, function (v) { + var firstSelected = _.find(vm.variants, function (v) { return v.publish; }); $scope.model.disableSubmitButton = !firstSelected; //disable submit button if there is none selected } + function dirtyVariantFilter(variant) { + return (variant.isEdited === true || (variant.isEdited === false && variant.state === "Unpublished")); + } + + function pristineVariantFilter(variant) { + return !(variant.isEdited === true || (variant.isEdited === false && variant.state === "Unpublished")); + } + function onInit() { - _.each(variants, + vm.hasDirtyVariants = false; + vm.hasPristineVariants = false; + + _.each(vm.variants, function (variant) { variant.compositeId = variant.language.id + "_" + (variant.segment ? variant.segment : ""); variant.htmlId = "publish_variant_" + variant.compositeId; - //separate "pristine" and "dirty" variants - if (variant.isEdited === true) { - vm.dirtyVariants.push(variant); - } else if (variant.isEdited === true || - variant.isEdited === false && variant.state === "Unpublished") { - vm.dirtyVariants.push(variant); - } else { - vm.pristineVariants.push(variant); + //check for dirty variants + if (variant.isEdited === true || (variant.isEdited === false && variant.state === "Unpublished")) { + vm.hasDirtyVariants = true; + } + else { + vm.hasPristineVariants = true; } }); - if (vm.dirtyVariants.length !== 0) { + if (vm.variants.length !== 0) { //now sort it so that the current one is at the top - vm.dirtyVariants = _.sortBy(vm.dirtyVariants, function (v) { + vm.variants = _.sortBy(vm.variants, function (v) { return v.current ? 0 : 1; }); //ensure that the current one is selected - vm.dirtyVariants[0].publish = true; + vm.variants[0].publish = true; } else { //disable Publish button if we have nothing to publish $scope.model.disableSubmitButton = true; @@ -72,5 +82,5 @@ } angular.module("umbraco").controller("Umbraco.Overlays.PublishController", PublishController); - + })(); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.html b/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.html index cfb205eb25..396ac9c8be 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/publish/publish.html @@ -1,17 +1,17 @@
-

-

+

+

-
+
-
+