From a6976085294d08c849674c02bafced5e3c09446c Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 12 Jun 2018 17:05:37 +0200 Subject: [PATCH] Fix un/publishing with variants + urls --- .../Services/Implement/ContentService.cs | 147 ++++++----- src/Umbraco.Web.UI/Umbraco/config/lang/en.xml | 228 +++++++++--------- .../Umbraco/config/lang/en_us.xml | 228 +++++++++--------- src/Umbraco.Web/PublishedContentExtensions.cs | 17 +- src/Umbraco.Web/Routing/DefaultUrlProvider.cs | 27 +-- .../Routing/UrlProviderExtensions.cs | 82 +++++-- 6 files changed, 377 insertions(+), 352 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 0c0015019a..2e3fcf8522 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.Composing; using Umbraco.Core.Events; using Umbraco.Core.Exceptions; using Umbraco.Core.IO; @@ -857,6 +858,14 @@ namespace Umbraco.Core.Services.Implement // fixme - kill all those raiseEvents + // fixme + // the API is not consistent + // SaveAndPublish requires that SetPublishValues / ClearPublishValues is called beforehand + // Unpublish will unpublish one culture - no idea what happens if we SetPublishValues / ClearPublishValues beforehand + // and there's not way to just plainly unpublish the whole thing - refactor! + // + // plus, does it make any sense for the 'default' culture to also be 'mandatory'? + /// public OperationResult Save(IContent content, int userId = 0, bool raiseEvents = true) { @@ -944,6 +953,11 @@ namespace Umbraco.Core.Services.Implement /// public PublishResult SaveAndPublish(IContent content, int userId = 0, bool raiseEvents = true) { + // note: SaveAndPublish does *not* publishes value, it assumes that content.PublishValues() + // has been called for the variations that are to be published - those are are already + // published remain published - can also be used to simply put the content back into its + // previous published state after it had been completely unpublished + var evtMsgs = EventMessagesFactory.Get(); PublishResult result; @@ -1028,118 +1042,97 @@ namespace Umbraco.Core.Services.Implement /// public UnpublishResult Unpublish(IContent content, string culture = null, string segment = null, int userId = 0) { + // unpublish a variation - and that may end up unpublishing the document as a whole, + // if the variation was mandatory (ie for a mandatory language - no idea what would happen w/ segments) + + if (!segment.IsNullOrWhiteSpace()) + throw new NotImplementedException("Segments are not supported."); + var evtMsgs = EventMessagesFactory.Get(); using (var scope = ScopeProvider.CreateScope()) { scope.WriteLock(Constants.Locks.ContentTree); - 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 - var result = UnpublishInternal(scope, content, evtMsgs, userId); - - //not succesful, so return it - if (!result.Success) return result; - - //else check if there was a status returned from TryUnpublishVariationInternal and if so use that - return tryUnpublishVariation.Result ?? result; - } - } - - /// - /// 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()) + // not varying, or invariant culture + // simply unpublish the document + if (!content.ContentType.Variations.DoesSupportCulture() || culture.IsNullOrWhiteSpace()) { - //capture before we clear the values - var publishedCultureCount = content.PublishedCultures.Count(); + var unpublished = UnpublishScoped(scope, content, evtMsgs, userId); + if (unpublished.Success) scope.Complete(); + return unpublished; + } - //we need to unpublish everything if this is a mandatory language - var unpublishAll = _languageRepository.GetMany().Where(x => x.Mandatory).Select(x => x.IsoCode).Contains(culture, StringComparer.InvariantCultureIgnoreCase); + // varying, with culture = deal with cultures - content.ClearPublishedValues(culture, segment); - //now we just publish with the name cleared - SaveAndPublish(content, userId); + // if already unpublished, nothing to do + var publishedCultures = content.PublishedCultures.ToList(); + if (!publishedCultures.Contains(culture, StringComparer.OrdinalIgnoreCase)) + { + scope.Complete(); + return new UnpublishResult(UnpublishResultType.SuccessAlready, evtMsgs, content); + } + + // otherwise, in any case, clear the unpublishing variation + content.ClearPublishedValues(culture, segment); + + // would there be any culture left? + var mandatoryCultures = _languageRepository.GetMany().Where(x => x.Mandatory).Select(x => x.IsoCode); + var isMandatory = mandatoryCultures.Contains(culture, StringComparer.OrdinalIgnoreCase); + if (isMandatory || publishedCultures.Count == 1) + { + // nothing left = unpublish all + var unpublished = UnpublishScoped(scope, content, evtMsgs, userId); + if (unpublished.Success) scope.Complete(); + return unpublished; + } + + // else we need to republish, without that culture + var saved = SaveAndPublish(content, userId); + if (saved.Success) + { Audit(AuditType.UnPublish, $"UnPublish variation culture: {culture ?? string.Empty}, segment: {segment ?? string.Empty} performed by user", userId, content.Id); - - //We don't need to unpublish all and this is not the last one, so complete the scope and return - if (!unpublishAll && publishedCultureCount > 1) - { - scope.Complete(); - return Attempt.Succeed(new UnpublishResult(UnpublishResultType.SuccessVariant, evtMsgs, content)); - } - - if (unpublishAll) - { - //return a fail with a custom status here so the normal unpublish operation takes place but the result will be this flag - return Attempt.Fail(new UnpublishResult(UnpublishResultType.SuccessMandatoryCulture, evtMsgs, content)); - } + scope.Complete(); + return new UnpublishResult(UnpublishResultType.SuccessVariant, evtMsgs, content); } - 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"); - } + // failed - map result + var r = saved.Result == PublishResultType.FailedCancelledByEvent + ? UnpublishResultType.FailedCancelledByEvent + : UnpublishResultType.Failed; + return new UnpublishResult(r, evtMsgs, content); } - - //This is either a non variant document or this is the last one or this was a mandatory variant, 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 + /// Unpublishes an entire document. /// - /// - /// - /// - /// - /// - private UnpublishResult UnpublishInternal(IScope scope, IContent content, EventMessages evtMsgs, int userId) + private UnpublishResult UnpublishScoped(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 UnpublishResult(UnpublishResultType.SuccessAlready, evtMsgs, content); // already unpublished - } - // strategy + // if already unpublished, nothing to do + if (!content.Published) + return new UnpublishResult(UnpublishResultType.SuccessAlready, evtMsgs, content); + + // run strategies // 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 + // save content.WriterId = userId; _documentRepository.Save(content); + // events and audit 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 UnpublishResult(evtMsgs, content); } diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml index 39e578df45..450b33d816 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml @@ -147,6 +147,8 @@ Remove at This item has been changed after publication This item is not published + Culture '%0%' for this item is not published + Culture '%0%' for this item is not available Last published There are no items to show There are no items to show in the list. @@ -770,41 +772,41 @@ To manage your website, simply open the Umbraco back office and start adding con Umbraco: Reset Password - - - - - - + + + + + + + - - + +
+
- - - + + + -
- -
- -
+ +
+ +
-
+ - + - - - + + +
+

- - + + -
+
- - + +
+

Password reset requested

@@ -814,12 +816,12 @@ To manage your website, simply open the Umbraco back office and start adding con

- - + + +
+
Click this link to reset your password - -
@@ -831,22 +833,22 @@ To manage your website, simply open the Umbraco back office and start adding con %1% -

-
-
+ - -


- - - + +


+ + + - + ]]> @@ -889,53 +891,53 @@ To manage your website, simply open the Umbraco back office and start adding con - - - - - - + + + + + + + - - + +
+
- - + + -
- -
- -
+ +
+ +
-
+ - + - - - + + -
+

- - + - +
+
- - +
+

Hi %0%,

-

+

This is an automated mail to inform you that the task '%1%' has been performed on the page '%2%' by the user '%3%'

- - @@ -945,7 +947,7 @@ To manage your website, simply open the Umbraco back office and start adding con

Update summary:

+ +
EDIT
%6% -
+

Have a nice day!

@@ -957,10 +959,10 @@ To manage your website, simply open the Umbraco back office and start adding con

-


+


@@ -1519,9 +1521,9 @@ To manage your website, simply open the Umbraco back office and start adding con Hide this property value from content editors that don't have access to view sensitive information Show on member profile Allow this property value to be displayed on the member profile page - + tab has no sort order - + Where is this composition used? This composition is currently used in the composition of the following content types: @@ -1775,41 +1777,41 @@ To manage your website, simply open the Umbraco back office and start adding con Umbraco: Invitation - - - - - - + + + + + + + - - + +
+
- - - + + + -
- -
- -
+ +
+ +
-
+
+ - - - + + +
+

- - + + -
+
- - + +
+

Hi %0%,

@@ -1827,12 +1829,12 @@ To manage your website, simply open the Umbraco back office and start adding con 		- - + + +
+
Click this link to accept the invite - -
@@ -1847,22 +1849,22 @@ To manage your website, simply open the Umbraco back office and start adding con %3% -

-
-
+ - -


- - - + +


+ + + - + ]]> diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index b70cfe58e0..3b3d5b4b6e 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -181,6 +181,8 @@ Remove at This item has been changed after publication This item is not published + Culture '%0%' for this item is not published + Culture '%0%' for this item is not available Last published There are no items to show There are no items to show in the list. @@ -897,41 +899,41 @@ To manage your website, simply open the Umbraco back office and start adding con Umbraco: Reset Password - - - - - - + + + + + + + - - + +
+
- - - + + + -
- -
- -
+ +
+ +
-
+ - + - - - + + +
+

- - + + -
+
- - + +
+

Password reset requested

@@ -941,12 +943,12 @@ To manage your website, simply open the Umbraco back office and start adding con

- - + + +
+
Click this link to reset your password - -
@@ -958,22 +960,22 @@ To manage your website, simply open the Umbraco back office and start adding con %1% -

-
-
+ - -


- - - + +


+ + + - + ]]> @@ -1017,53 +1019,53 @@ To manage your website, simply open the Umbraco back office and start adding con - - - - - - + + + + + + + - - + +
+
- - + + -
- -
- -
+ +
+ +
-
+ - + - - - + + -
+

- - + - +
+
- - +
+

Hi %0%,

-

+

This is an automated mail to inform you that the task '%1%' has been performed on the page '%2%' by the user '%3%'

- - @@ -1073,7 +1075,7 @@ To manage your website, simply open the Umbraco back office and start adding con

Update summary:

+ +
EDIT
%6% -
+

Have a nice day!

@@ -1085,10 +1087,10 @@ To manage your website, simply open the Umbraco back office and start adding con

-


+


@@ -1646,9 +1648,9 @@ To manage your website, simply open the Umbraco back office and start adding con Hide this property value from content editors that don't have access to view sensitive information Show on member profile Allow this property value to be displayed on the member profile page - + tab has no sort order - + Where is this composition used? This composition is currently used in the composition of the following content types: @@ -1911,41 +1913,41 @@ To manage your website, simply open the Umbraco back office and start adding con Umbraco: Invitation - - - - - - + + + + + + + - - + +
+
- - - + + + -
- -
- -
+ +
+ +
-
+
+ - - - + + +
+

- - + + -
+
- - + +
+

Hi %0%,

@@ -1963,12 +1965,12 @@ To manage your website, simply open the Umbraco back office and start adding con 		- - + + +
+
Click this link to accept the invite - -
@@ -1983,22 +1985,22 @@ To manage your website, simply open the Umbraco back office and start adding con %3% -

-
-
+ - -


- - - + +


+ + + - + ]]> diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index 1adfb55ca9..1d3c747b6e 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -37,17 +37,6 @@ namespace Umbraco.Web return content.Url; } - /// - /// Gets the absolute url for the content. - /// - /// The content. - /// The absolute url for the content. - //[Obsolete("UrlWithDomain() is obsolete, use the UrlAbsolute() method instead.")] - public static string UrlWithDomain(this IPublishedContent content) - { - return content.UrlAbsolute(); - } - /// /// Gets the absolute url for the content. /// @@ -85,8 +74,12 @@ namespace Umbraco.Web if (!content.ContentType.Variations.Has(ContentVariation.CultureNeutral)) return content.UrlSegment; + // content.GetCulture(culture) will use the 'current' culture (via accessor) in case 'culture' + // is null (meaning, 'current') - and can return 'null' if that culture is not published - and + // will return 'null' if the content is variant and culture is invariant + // else try and get the culture info - // return the corresponding url segment, or null if none (ie the culture is not published) + // return the corresponding url segment, or null if none var cultureInfo = content.GetCulture(culture); return cultureInfo?.UrlSegment; } diff --git a/src/Umbraco.Web/Routing/DefaultUrlProvider.cs b/src/Umbraco.Web/Routing/DefaultUrlProvider.cs index e40dcd0560..7ea359fa67 100644 --- a/src/Umbraco.Web/Routing/DefaultUrlProvider.cs +++ b/src/Umbraco.Web/Routing/DefaultUrlProvider.cs @@ -79,23 +79,18 @@ namespace Umbraco.Web.Routing /// public virtual IEnumerable GetOtherUrls(UmbracoContext umbracoContext, int id, Uri current) { - //get the invariant route for this item, this will give us the Id of it's domain node if one is assigned - var invariantRoute = umbracoContext.ContentCache.GetRouteById(id); - - if (string.IsNullOrWhiteSpace(invariantRoute)) - { - _logger.Debug(() => $"Couldn't find any page with nodeId={id}. This is most likely caused by the page not being published."); - return null; - } - + var node = umbracoContext.ContentCache.GetById(id); var domainHelper = umbracoContext.GetDomainHelper(_siteDomainHelper); - // extract domainUri and path - // route is / or / - var pos = invariantRoute.IndexOf('/'); - var path = pos == 0 ? invariantRoute : invariantRoute.Substring(pos); - var domainUris = pos == 0 ? null : domainHelper.DomainsForNode(int.Parse(invariantRoute.Substring(0, pos)), current); + var n = node; + var domainUris = domainHelper.DomainsForNode(n.Id, current, false); + while (domainUris == null && n != null) // n is null at root + { + n = n.Parent; // move to parent node + domainUris = n == null ? null : domainHelper.DomainsForNode(n.Id, current, false); + } + // no domains = exit if (domainUris ==null) return Enumerable.Empty(); @@ -107,8 +102,8 @@ namespace Umbraco.Web.Routing if (route == null) continue; //need to strip off the leading ID for the route if it exists (occurs if the route is for a node with a domain assigned) - pos = route.IndexOf('/'); - path = pos == 0 ? route : route.Substring(pos); + var pos = route.IndexOf('/'); + var path = pos == 0 ? route : route.Substring(pos); var uri = new Uri(CombinePaths(d.Uri.GetLeftPart(UriPartial.Path), path)); uri = UriUtility.UriFromUmbraco(uri, _globalSettings, _requestSettings); diff --git a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs index f7a671b1e3..86fc51fe44 100644 --- a/src/Umbraco.Web/Routing/UrlProviderExtensions.cs +++ b/src/Umbraco.Web/Routing/UrlProviderExtensions.cs @@ -1,25 +1,22 @@ using System; using System.Collections.Generic; +using System.Threading; using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Core; using Umbraco.Core.Logging; using LightInject; -using Umbraco.Web.Composing; namespace Umbraco.Web.Routing { internal static class UrlProviderExtensions { /// - /// Gets the URLs for the content item + /// Gets the Urls of the content item. /// - /// - /// - /// /// - /// Use this when displaying URLs, if there are errors genertaing the urls the urls themselves will - /// contain the errors. + /// Use when displaying Urls. If errors occur when generating the Urls, they will show in the list. + /// Contains all the Urls that we can figure out (based upon domains, etc). /// public static IEnumerable GetContentUrls(this IContent content, UrlProvider urlProvider, ILocalizedTextService textService, IContentService contentService, ILogger logger) { @@ -31,44 +28,87 @@ namespace Umbraco.Web.Routing var urls = new HashSet(); + // going to build a list of urls (essentially for the back-office) + // which will contain + // - the 'main' url, which is what .Url would return, in the current culture + // - the 'other' urls we know (based upon domains, etc) + // + // this essentially happens when producing the urls for the back-office, and then we don't have + // a meaningful 'current culture' - so we need to explicitely pass some culture where required, + // and deal with whatever might happen + // + // if content is variant, go with the current culture - and that is NOT safe, there may be + // no 'main' url for that culture, deal with it later - otherwise, go with the invariant + // culture, and that is safe. + var varies = content.ContentType.Variations.DoesSupportCulture(); + var culture = varies ? Thread.CurrentThread.CurrentUICulture.Name : ""; + if (content.Published == false) { urls.Add(textService.Localize("content/itemNotPublished")); return urls; } - string url; - try + string url = null; + + if (varies) { - url = urlProvider.GetUrl(content.Id); + if (!content.IsCulturePublished(culture)) + { + urls.Add(textService.Localize("content/itemCultureNotPublished", culture)); + // but keep going, we want to add the 'other' urls + url = "#no"; + } + else if (!content.IsCultureAvailable(culture)) + { + urls.Add(textService.Localize("content/itemCultureNotAvailable", culture)); + // but keep going, we want to add the 'other' urls + url = "#no"; + } } - catch (Exception e) + + // get the 'main' url + if (url == null) { - logger.Error("GetUrl exception.", e); - url = "#ex"; + try + { + url = urlProvider.GetUrl(content.Id, culture); + } + catch (Exception e) + { + logger.Error("GetUrl exception.", e); + url = "#ex"; + } } - if (url == "#") + + if (url == "#") // deal with 'could not get the url' { - // document as a published version yet it's url is "#" => a parent must be + // document as a published version yet its url is "#" => a parent must be // unpublished, walk up the tree until we find it, and report. var parent = content; do { parent = parent.ParentId > 0 ? parent.Parent(contentService) : null; } - while (parent != null && parent.Published); + while (parent != null && parent.Published && (!varies || parent.IsCulturePublished(culture))); urls.Add(parent == null ? textService.Localize("content/parentNotPublishedAnomaly") // oops - internal error : textService.Localize("content/parentNotPublished", new[] { parent.Name })); } - else if (url == "#ex") + else if (url == "#ex") // deal with exceptions { urls.Add(textService.Localize("content/getUrlException")); } - else + else if (url == "#no") // deal with 'there is no main url' { - // test for collisions + // get the 'other' urls + foreach(var otherUrl in urlProvider.GetOtherUrls(content.Id)) + urls.Add(otherUrl); + } + else // detect collisions, etc + { + // test for collisions on the 'main' url var uri = new Uri(url.TrimEnd('/'), UriKind.RelativeOrAbsolute); if (uri.IsAbsoluteUri == false) uri = uri.MakeAbsolute(UmbracoContext.Current.CleanedUmbracoUrl); uri = UriUtility.UriToUmbraco(uri); @@ -105,10 +145,10 @@ namespace Umbraco.Web.Routing else { urls.Add(url); + + // get the 'other' urls foreach(var otherUrl in urlProvider.GetOtherUrls(content.Id)) - { urls.Add(otherUrl); - } } } return urls;