Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V10/bugfix/14543 publish descendants #14763

Merged
merged 7 commits into from
Sep 5, 2023

Conversation

Migaroez
Copy link
Contributor

@Migaroez Migaroez commented Sep 4, 2023

Reason

Fix for #14543

Description

  • Renamed vars to clear up some things
  • Skipped the branchitem change detection on the root node as the action implies we want to save and publish the root
  • Seperated the root and children publish results to have seperate references to them later on
  • Used the seperated root reference to properly check if the root was succesful after all the children are processed.
  • Passed the state of root from the publishing to the published notification properly linking the pair together

Reproduction steps

Add the following classes to your project

using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Notifications;

public class Published : INotificationAsyncHandler<ContentPublishedNotification>
{
    public Task HandleAsync(ContentPublishedNotification notification, CancellationToken cancellationToken)
    {
        notification.State.TryGetValue("publishing", out var value);

        return Task.CompletedTask;
    }
}
public class Publishing : INotificationAsyncHandler<ContentPublishingNotification>
{
    public Task HandleAsync(ContentPublishingNotification notification, CancellationToken cancellationToken)
    {
        notification.State.Add("publishing", notification.PublishedEntities.Select(x => x.Id).ToList());

        return Task.CompletedTask;
    }
}

public class Saved : INotificationAsyncHandler<ContentSavedNotification>
{
    public Task HandleAsync(ContentSavedNotification notification, CancellationToken cancellationToken)
    {
        notification.State.TryGetValue("saving", out var value);

        return Task.CompletedTask;
    }
}
public class Saving : INotificationAsyncHandler<ContentSavingNotification>
{
    public Task HandleAsync(ContentSavingNotification notification, CancellationToken cancellationToken)
    {
        notification.State.Add("saving", notification.SavedEntities.Select(x => x.Id).ToList());

        return Task.CompletedTask;
    }
}

public class MyComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddNotificationAsyncHandler<ContentPublishingNotification, Publishing>();
        builder.AddNotificationAsyncHandler<ContentPublishedNotification, Published>();
        builder.AddNotificationAsyncHandler<ContentSavingNotification, Saving>();
        builder.AddNotificationAsyncHandler<ContentSavedNotification, Saved>();
    }
}

Each pair (save or publish) should be hit for every child that has unpublished changes when executing publish with descendants on a parent (or ancestor for that matter).
These pairs should also be called on the parent
And the state should be passed between each item in a pair.

@Migaroez Migaroez changed the base branch from contrib to v10/dev September 4, 2023 08:46
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few small things 👀

src/Umbraco.Core/Services/ContentService.cs Outdated Show resolved Hide resolved
src/Umbraco.Core/Services/ContentService.cs Outdated Show resolved Hide resolved
src/Umbraco.Core/Notifications/NotificationExtensions.cs Outdated Show resolved Hide resolved
Migaroez and others added 3 commits September 5, 2023 08:53
Co-authored-by: Nikolaj Geisle <[email protected]>
Co-authored-by: Nikolaj Geisle <[email protected]>
@Zeegaan
Copy link
Member

Zeegaan commented Sep 5, 2023

Looks good, tests good!

@Zeegaan Zeegaan merged commit f750bca into v10/dev Sep 5, 2023
14 of 15 checks passed
@Zeegaan Zeegaan deleted the v10/bugfix/14543-publish-descendants branch September 5, 2023 08:14
Zeegaan added a commit that referenced this pull request Sep 5, 2023
* WIP: Fix publish descendants and related notifications

* Removed related entitities from publish notification

* Fixed root not being saved when publishingWithDescendants

* Updated integrationtests to reflect the update view on when to save the root when its part of a branch

* PR formatting fix

Co-authored-by: Nikolaj Geisle <[email protected]>

* PR Cleanup

Co-authored-by: Nikolaj Geisle <[email protected]>

* Spicing up the codebase with some PR pattern matching

Co-authored-by: Nikolaj Geisle <[email protected]>

---------

Co-authored-by: Sven Geusens <[email protected]>
Co-authored-by: Nikolaj Geisle <[email protected]>
kjac added a commit that referenced this pull request Oct 5, 2023
Migaroez pushed a commit that referenced this pull request Oct 5, 2023
…ing with descendants (#14922)

* Revert "V10/bugfix/14543 publish descendants (#14763)"

This reverts commit f750bca.

* Add notification state to "final" published notification when publishing with descendants
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants