From d9d68cf6bf46198a87e4d365372bd65acbdde46c Mon Sep 17 00:00:00 2001 From: Claus Date: Wed, 20 Feb 2019 16:05:42 +0100 Subject: [PATCH] The content service is not returning the invalid properties when publishing fails (cherry picked from commit d94e383ed7778c72e11cbb303d1cce4798f780c7) --- .../Models/ContentRepositoryExtensions.cs | 10 +++- .../Services/ContentServiceExtensions.cs | 6 +-- src/Umbraco.Core/Services/IContentService.cs | 48 +++++++++---------- .../Services/Implement/ContentService.cs | 31 +++++++++--- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 27678c047c..01812f8469 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -193,6 +193,13 @@ namespace Umbraco.Core.Models public static bool PublishCulture(this IContent content, string culture = "*") { + return PublishCulture(content, out _, culture); + } + + public static bool PublishCulture(this IContent content, out Property[] invalidProperties, string culture = "*") + { + invalidProperties = null; + culture = culture.NullOrWhiteSpaceAsNull(); // the variation should be supported by the content type properties @@ -202,7 +209,8 @@ namespace Umbraco.Core.Models throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); // the values we want to publish should be valid - if (content.ValidateProperties(culture).Any()) + invalidProperties = content.ValidateProperties(culture); + if (invalidProperties.Length > 0) return false; var alsoInvariant = false; diff --git a/src/Umbraco.Core/Services/ContentServiceExtensions.cs b/src/Umbraco.Core/Services/ContentServiceExtensions.cs index 4d673fc902..1175df81dc 100644 --- a/src/Umbraco.Core/Services/ContentServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentServiceExtensions.cs @@ -31,16 +31,16 @@ namespace Umbraco.Core.Services /// /// /// - /// + /// /// /// - public static IContent CreateContent(this IContentService contentService, string name, Udi parentId, string mediaTypeAlias, int userId = Constants.Security.SuperUserId) + public static IContent CreateContent(this IContentService contentService, string name, Udi parentId, string contentTypeAlias, int userId = Constants.Security.SuperUserId) { var guidUdi = parentId as GuidUdi; if (guidUdi == null) throw new InvalidOperationException("The UDI provided isn't of type " + typeof(GuidUdi) + " which is required by content"); var parent = contentService.GetById(guidUdi.Guid); - return contentService.Create(name, parent, mediaTypeAlias, userId); + return contentService.Create(name, parent, contentTypeAlias, userId); } /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index b2fce8f3e6..784d04864e 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -391,30 +391,30 @@ namespace Umbraco.Core.Services /// IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = Constants.Security.SuperUserId); - /// - /// Saves and publishes a document branch. - /// - /// The root document. - /// A value indicating whether to force-publish documents that are not already published. - /// A function determining cultures to publish. - /// A function publishing cultures. - /// The identifier of the user performing the operation. - /// - /// The parameter determines which documents are published. When false, - /// only those documents that are already published, are republished. When true, all documents are - /// published. The root of the branch is always published, regardless of . - /// The parameter is a function which determines whether a document has - /// changes to publish (else there is no need to publish it). If one wants to publish only a selection of - /// cultures, one may want to check that only properties for these cultures have changed. Otherwise, other - /// cultures may trigger an unwanted republish. - /// The parameter is a function to execute to publish cultures, on - /// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating - /// whether the cultures could be published. - /// - IEnumerable SaveAndPublishBranch(IContent content, bool force, - Func> shouldPublish, - Func, bool> publishCultures, - int userId = Constants.Security.SuperUserId); + ///// + ///// Saves and publishes a document branch. + ///// + ///// The root document. + ///// A value indicating whether to force-publish documents that are not already published. + ///// A function determining cultures to publish. + ///// A function publishing cultures. + ///// The identifier of the user performing the operation. + ///// + ///// The parameter determines which documents are published. When false, + ///// only those documents that are already published, are republished. When true, all documents are + ///// published. The root of the branch is always published, regardless of . + ///// The parameter is a function which determines whether a document has + ///// changes to publish (else there is no need to publish it). If one wants to publish only a selection of + ///// cultures, one may want to check that only properties for these cultures have changed. Otherwise, other + ///// cultures may trigger an unwanted republish. + ///// The parameter is a function to execute to publish cultures, on + ///// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating + ///// whether the cultures could be published. + ///// + //IEnumerable SaveAndPublishBranch(IContent content, bool force, + // Func> shouldPublish, + // Func, bool> publishCultures, + // int userId = Constants.Security.SuperUserId); /// /// Unpublishes a document. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 02117d064c..922eb4e2db 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -869,19 +869,28 @@ namespace Umbraco.Core.Services.Implement // if culture is specific, first publish the invariant values, then publish the culture itself. // if culture is '*', then publish them all (including variants) + Property[] invalidProperties; + // explicitly SaveAndPublish a specific culture also publishes invariant values if (!culture.IsNullOrWhiteSpace() && culture != "*") { // publish the invariant values - var publishInvariant = content.PublishCulture(null); + var publishInvariant = content.PublishCulture(out invalidProperties, null); if (!publishInvariant) - return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); + return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; + } // publish the culture(s) - var publishCulture = content.PublishCulture(culture); + var publishCulture = content.PublishCulture(out invalidProperties, culture); if (!publishCulture) - return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); + return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; // finally, "save publishing" // what happens next depends on whether the content can be published or not @@ -1242,6 +1251,7 @@ namespace Umbraco.Core.Services.Implement .Distinct() .ToList(); + Property[] invalidProperties = null; var publishing = true; foreach (var culture in pendingCultures) { @@ -1250,14 +1260,17 @@ namespace Umbraco.Core.Services.Implement if (d.Trashed) continue; // won't publish - publishing &= d.PublishCulture(culture); //set the culture to be published + publishing &= d.PublishCulture(out invalidProperties, culture); //set the culture to be published if (!publishing) break; // no point continuing } if (d.Trashed) result = new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d); else if (!publishing) - result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); + result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; else result = CommitDocumentChanges(d, d.WriterId); @@ -1436,7 +1449,7 @@ namespace Umbraco.Core.Services.Implement } /// - public IEnumerable SaveAndPublishBranch(IContent document, bool force, + internal IEnumerable SaveAndPublishBranch(IContent document, bool force, Func> shouldPublish, Func, bool> publishCultures, int userId = Constants.Security.SuperUserId) @@ -1536,7 +1549,11 @@ namespace Umbraco.Core.Services.Implement // publish & check if values are valid if (!publishCultures(document, culturesToPublish)) + { + //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, userId, branchOne: true, branchRoot: isRoot); if (result.Success)