Improve handling of publish/unpublish operations #17441
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated AdminController to handle success/failure of publish/unpublish operations more robustly. Modified IContentManager interface to return Task<bool> for PublishAsync and UnpublishAsync methods. Implemented these changes in DefaultContentManager, including handling operation cancellations. Enhanced user feedback based on operation results.
hishamco
requested changes
Feb 1, 2025
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
Piedone
requested changes
Feb 1, 2025
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
…Controller.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
…Controller.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
…Controller.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
sebastienros
approved these changes
Feb 6, 2025
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
|
@MichaelPetrinolis could you please address @Piedone feedback, so that we can merge this PR |
Introduce `authorized` variable in `AdminController` to track authorization status during bulk actions, redirecting to list view or returning forbidden response as needed. Update `conditionallyPublish` to return `Task<bool>` for better control flow in `CreateInternalAsync` and `EditInternalAsync`. Enhance `DefaultContentManager` with `INotifier` and `IHtmlLocalizer` dependencies to provide user notifications when publishing or unpublishing is canceled, including content type and display text in messages.
Updated AdminController to handle ModelState errors across redirects using TempData. Added RedirectToListActionWithModelState method. Updated List.cshtml to display validation errors. Replaced INotifier and IHtmlLocalizer with IUpdateModelAccessor and IStringLocalizer in DefaultContentManager. Ensured ModelUpdater is checked before adding ModelState errors.
Piedone
requested changes
Feb 9, 2025
src/OrchardCore/OrchardCore.ContentManagement/DefaultContentManager.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.ContentManagement/DefaultContentManager.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
hishamco
requested changes
Feb 9, 2025
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Show resolved
Hide resolved
Co-authored-by: Hisham Bin Ateya <[email protected]> Co-authored-by: Zoltán Lehóczky <[email protected]>
Refactor error handling in AdminController to iterate through error keys and messages before adding them to ModelState. Rename variable 'authorized' to 'isAuthorized' for better readability and reformat return statement for clarity.
Piedone
approved these changes
Feb 11, 2025
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs
Outdated
Show resolved
Hide resolved
|
@Piedone feel free to merge once you are fine with the latest changes |
|
I am fine with the latest changes, hence the approve :). But @MichaelPetrinolis please address Sebastien's, and re-request review. |
Yes, but when I saw "for the future," I thought it was not necessary to be addressed right now |
The code refactor removes the `isAuthorized` variable and replaces it with an inline variable declaration using the `is var` pattern within the `if` statements. This change is applied to both the `PublishNow` and `Unpublish` cases within the `switch` statement handling `options.BulkAction`. The refactor simplifies the authorization and content management checks by combining them into a single `if` statement, improving readability. The return statements are updated to use the new `authorized` variable instead of the removed `isAuthorized` variable.
…tps://github.com/MichaelPetrinolis/OrchardCore into Improve-Handling-of-publish/unpublish-operations
Piedone
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes several changes to the
AdminController.csand related files to improve error handling, authorization checks, and the publishing process for content items. The most important changes include adding detailed error handling, updating methods to return boolean values indicating success, and refactoring the controller methods to use these new return values.Improvements to error handling:
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs: Added logic to deserialize and handleModelStateerrors fromTempDatain theListmethod.src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs: Added detailed error messages when publishing or unpublishing content fails, and updated the redirection to includeModelStateerrors. [1] [2] [3]Authorization and publishing checks:
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs: Added authorization checks and updated the methods to return appropriate responses based on the success of publishing or unpublishing actions. [1] [2] [3] [4] [5]Refactoring methods to return success status:
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs: UpdatedCreateInternalAsyncandEditInternalAsyncmethods to return a boolean indicating the success of the operation. [1] [2]src/OrchardCore/OrchardCore.ContentManagement.Abstractions/IContentManager.cs: ChangedPublishAsyncandUnpublishAsyncmethods to return boolean values.Adding support for error notifications:
src/OrchardCore/OrchardCore.ContentManagement/DefaultContentManager.cs: Added support for displaying error messages when publishing or unpublishing content is canceled. [1] [2] [3]Additional changes:
src/OrchardCore.Modules/OrchardCore.Contents/Views/Admin/List.cshtml: Added a validation summary to display errors on the list view.