Improve handling of publish/unpublish operations

src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs

@@ -1,9 +1,12 @@
 using System.Security.Claims;
+using System.Text.Json;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Localization;
+using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.Extensions.Localization;
 using Microsoft.Extensions.Options;
 using OrchardCore.Admin;
@@ -219,6 +222,21 @@ public async Task<IActionResult> List(
             viewModel.Header = header;
         });
 
+        if (TempData.TryGetValue(nameof(ModelState), out var modelStateJson) && modelStateJson is string)
+        {
+            var errors = JsonSerializer.Deserialize<Dictionary<string, string[]>>((string)modelStateJson);
+            if (errors != null)
+            {
+                foreach (var error in errors)
+                {
+                    foreach (var errorMessage in error.Value)
+                    {
+                        ModelState.AddModelError(error.Key, errorMessage);
+                    }
+                }
+            }
+        }
+
         return View(shapeViewModel);
     }
 
@@ -256,36 +274,33 @@ public async Task<ActionResult> ListPOST(ContentOptionsViewModel options, long[]
             var checkedContentItems = await _session.Query<ContentItem, ContentItemIndex>()
                 .Where(x => x.DocumentId.IsIn(itemIds) && x.Latest)
                 .ListAsync(_contentManager);
-
+            bool authorized = false;
             switch (options.BulkAction)
             {
                 case ContentsBulkAction.None:
                     break;
                 case ContentsBulkAction.PublishNow:
                     foreach (var item in checkedContentItems)
                     {
-                        if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.PublishContent, item))
+                        if (!(authorized = await _authorizationService.AuthorizeAsync(User, CommonPermissions.PublishContent, item)) || !await _contentManager.PublishAsync(item))
                         {
                             await _notifier.WarningAsync(H["Couldn't publish selected content."]);
                             await _session.CancelAsync();
-                            return Forbid();
-                        }
 
-                        await _contentManager.PublishAsync(item);
+                            return authorized ? RedirectToListActionWithModelState() : Forbid();
+                        }
                     }
                     await _notifier.SuccessAsync(H["Content published successfully."]);
                     break;
                 case ContentsBulkAction.Unpublish:
                     foreach (var item in checkedContentItems)
                     {
-                        if (!await IsAuthorizedAsync(CommonPermissions.PublishContent, item))
+                        if (!(authorized = await IsAuthorizedAsync(CommonPermissions.PublishContent, item)) || !await _contentManager.UnpublishAsync(item))
                         {
                             await _notifier.WarningAsync(H["Couldn't unpublish selected content."]);
                             await _session.CancelAsync();
-                            return Forbid();
+                            return authorized ? RedirectToListActionWithModelState() : Forbid();
                         }
-
-                        await _contentManager.UnpublishAsync(item);
                     }
                     await _notifier.SuccessAsync(H["Content unpublished successfully."]);
                     break;
@@ -307,7 +322,6 @@ public async Task<ActionResult> ListPOST(ContentOptionsViewModel options, long[]
                     return BadRequest();
             }
         }
-
         return RedirectToAction(nameof(List));
     }
 
@@ -349,6 +363,8 @@ public Task<IActionResult> CreatePOST(
             await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayName)
                 ? H["Your content draft has been saved."]
                 : H["Your {0} draft has been saved.", typeDefinition.DisplayName]);
+
+            return true;
         });
     }
 
@@ -374,13 +390,17 @@ public async Task<IActionResult> CreateAndPublishPOST(
 
         return await CreateInternalAsync(id, returnUrl, stayOnSamePage, async contentItem =>
         {
-            await _contentManager.PublishAsync(contentItem);
+            if (await _contentManager.PublishAsync(contentItem))
+            {
+                var typeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);
 
-            var typeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);
+                await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition.DisplayName)
+                    ? H["Your content has been published."]
+                    : H["Your {0} has been published.", typeDefinition.DisplayName]);
 
-            await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition.DisplayName)
-                ? H["Your content has been published."]
-                : H["Your {0} has been published.", typeDefinition.DisplayName]);
+                return true;
+            }
+            return false;
         });
     }
 
@@ -443,6 +463,8 @@ public Task<IActionResult> EditPOST(
             await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayName)
                 ? H["Your content draft has been saved."]
                 : H["Your {0} draft has been saved.", typeDefinition.DisplayName]);
+
+            return true;
         });
     }
 
@@ -471,13 +493,18 @@ public async Task<IActionResult> EditAndPublishPOST(
         return await EditInternalAsync(contentItemId, returnUrl, stayOnSamePage, async contentItem =>
         {
             await _contentManager.UpdateAsync(contentItem);
-            await _contentManager.PublishAsync(contentItem);
+            var published = await _contentManager.PublishAsync(contentItem);
 
             var typeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);
 
-            await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayName)
+            if (published)
+            {
+                await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayName)
                 ? H["Your content has been published."]
                 : H["Your {0} has been published.", typeDefinition.DisplayName]);
+            }
+
+            return published;
         });
     }
 
@@ -587,22 +614,29 @@ public async Task<IActionResult> Publish(string contentItemId, string returnUrl)
             return Forbid();
         }
 
-        await _contentManager.PublishAsync(contentItem);
+        var published = await _contentManager.PublishAsync(contentItem);
 
         var typeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);
 
-        if (string.IsNullOrEmpty(typeDefinition?.DisplayName))
+        if (published)
         {
-            await _notifier.SuccessAsync(H["That content has been published."]);
+            if (string.IsNullOrEmpty(typeDefinition?.DisplayName))
+            {
+                await _notifier.SuccessAsync(H["That content has been published."]);
+            }
+            else
+            {
+                await _notifier.SuccessAsync(H["That {0} has been published.", typeDefinition.DisplayName]);
+            }
         }
-        else
+        else if (Url.IsLocalUrl(returnUrl))
         {
-            await _notifier.SuccessAsync(H["That {0} has been published.", typeDefinition.DisplayName]);
+            await _notifier.ErrorAsync(H["The operation was canceled."]);
         }
 
         return Url.IsLocalUrl(returnUrl)
             ? this.LocalRedirect(returnUrl, true)
-            : RedirectToAction(nameof(List));
+            : RedirectToListActionWithModelState();
     }
 
     [HttpPost]
@@ -620,29 +654,37 @@ public async Task<IActionResult> Unpublish(string contentItemId, string returnUr
             return Forbid();
         }
 
-        await _contentManager.UnpublishAsync(contentItem);
+        var unpublished = await _contentManager.UnpublishAsync(contentItem);
 
         var typeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);
 
-        if (string.IsNullOrEmpty(typeDefinition?.DisplayName))
+        if (unpublished)
         {
-            await _notifier.SuccessAsync(H["The content has been unpublished."]);
+            if (string.IsNullOrEmpty(typeDefinition?.DisplayName))
+            {
+                await _notifier.SuccessAsync(H["The content has been unpublished."]);
+            }
+            else
+            {
+                await _notifier.SuccessAsync(H["The {0} has been unpublished.", typeDefinition.DisplayName]);
+            }
         }
-        else
+        else if (Url.IsLocalUrl(returnUrl))
         {
-            await _notifier.SuccessAsync(H["The {0} has been unpublished.", typeDefinition.DisplayName]);
+            await _notifier.ErrorAsync(H["The operation was canceled."]);
         }
 
+
         return Url.IsLocalUrl(returnUrl)
             ? this.LocalRedirect(returnUrl, true)
-            : RedirectToAction(nameof(List));
+            : RedirectToListActionWithModelState();
     }
 
     private async Task<IActionResult> CreateInternalAsync(
         string id,
         string returnUrl,
         bool stayOnSamePage,
-        Func<ContentItem, Task> conditionallyPublish)
+        Func<ContentItem, Task<bool>> conditionallyPublish)
     {
         var contentItem = await CreateContentItemOwnedByCurrentUserAsync(id);
 
@@ -658,14 +700,12 @@ private async Task<IActionResult> CreateInternalAsync(
             await _contentManager.CreateAsync(contentItem, VersionOptions.Draft);
         }
 
-        if (!ModelState.IsValid)
+        if (!ModelState.IsValid || !await conditionallyPublish(contentItem))
         {
             await _session.CancelAsync();
             return View(model);
         }
 
-        await conditionallyPublish(contentItem);
-
         if (!string.IsNullOrEmpty(returnUrl) && !stayOnSamePage)
         {
             return this.LocalRedirect(returnUrl, true);
@@ -685,7 +725,7 @@ private async Task<IActionResult> EditInternalAsync(
         string contentItemId,
         string returnUrl,
         bool stayOnSamePage,
-        Func<ContentItem, Task> conditionallyPublish)
+        Func<ContentItem, Task<bool>> conditionallyPublish)
     {
         var contentItem = await _contentManager.GetAsync(contentItemId, VersionOptions.DraftRequired);
 
@@ -701,14 +741,12 @@ private async Task<IActionResult> EditInternalAsync(
 
         var model = await _contentItemDisplayManager.UpdateEditorAsync(contentItem, this, false);
 
-        if (!ModelState.IsValid)
+        if (!ModelState.IsValid || !(await conditionallyPublish(contentItem)))
         {
             await _session.CancelAsync();
             return View(nameof(Edit), model);
         }
 
-        await conditionallyPublish(contentItem);
-
         if (returnUrl == null)
         {
             return RedirectToAction(nameof(Edit), new RouteValueDictionary
@@ -796,4 +834,17 @@ private async Task<bool> IsAuthorizedAsync(Permission permission)
 
     private async Task<bool> IsAuthorizedAsync(Permission permission, object resource)
         => await _authorizationService.AuthorizeAsync(User, permission, resource);
+
+    private RedirectToActionResult RedirectToListActionWithModelState()
+    {
+        var errors = ModelState
+            .Where(ms => ms.Value.Errors.Any())
+            .ToDictionary(
+                kvp => kvp.Key,
+                kvp => kvp.Value.Errors.Select(e => e.ErrorMessage).ToArray()
+            );
+
+        TempData[nameof(ModelState)] = JsonSerializer.Serialize(errors);
+        return RedirectToAction(nameof(List));
+    }
 }

src/OrchardCore.Modules/OrchardCore.Contents/Views/Admin/List.cshtml

@@ -13,5 +13,6 @@
 <zone Name="Title"><h1>@RenderTitleSegments(pageTitle)</h1></zone>
 
 <form asp-action="List" asp-controller="Admin" method="post" id="items-form" autocomplete="off">
+    @Html.ValidationSummary()
     @await DisplayAsync(Model)
 </form>

src/OrchardCore/OrchardCore.ContentManagement.Abstractions/IContentManager.cs

@@ -120,9 +120,9 @@ public interface IContentManager
     /// <param name="contentItem"></param>
     Task SaveDraftAsync(ContentItem contentItem);
 
-    Task PublishAsync(ContentItem contentItem);
+    Task<bool> PublishAsync(ContentItem contentItem);
 
-    Task UnpublishAsync(ContentItem contentItem);
+    Task<bool> UnpublishAsync(ContentItem contentItem);
 
     Task<TAspect> PopulateAspectAsync<TAspect>(IContent content, TAspect aspect);