Add webhooks extension methods

[erikjanwestendorp]

Prerequisites

• I have added steps to test this contribution in the description below

Description

Add two IUmbracoBuilder extension methods to the UmbracoBuilder.CollectionBuilders.cs class.

AddAllCmsWebhookEvents
This method allows for easy registration of all CMS WebhookEvents from the Startup.cs/Program.cs class. See the code example below:

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

builder.CreateUmbracoBuilder()
    .AddBackOffice()
    .AddWebsite()
    .AddDeliveryApi()
    .AddComposers()
    .AddAllCmsWebhookEvents()
    .Build();

WebApplication app = builder.Build();

ConfigureCmsWebhookEvents
This method can be used to configure WebhookEvents in the Startup.cs/Program.cs class, for example:

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

builder.CreateUmbracoBuilder()
    .AddBackOffice()
    .AddWebsite()
    .AddDeliveryApi()
    .AddComposers()
    .ConfigureCmsWebhookEvents(cmsBuilder => cmsBuilder.AddLanguage())
    .Build();

WebApplication app = builder.Build();

When #15424 is merged the default events can be deleted like so:

builder.CreateUmbracoBuilder()
    .AddBackOffice()
    .AddWebsite()
    .AddDeliveryApi()
    .AddComposers()
    .ConfigureCmsWebhookEvents(cmsBuilder => cmsBuilder.RemoveDefault().AddLanguage())
    .Build();

[github-actions]

Hi there @erikjanwestendorp, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[nul800sebastiaan]

Maybe I haven't merged everything into contrib yet? .AddAllCmsWebhookEvents() no longer exists, we've added a fluent API in the last few days and I think it covers this scenario as well. I'll do that merge back to contrib now just so it's available, but I think this PR is probably not needed now?

[erikjanwestendorp]

@nul800sebastiaan Thanks for your reply! I know that a fluent API has been added, and the methods added in this PR also use it. However, the methods added in this PR make it possible to do this directly from the Program.cs class (so without a Composer) because they are extensions for the IUmbracoBuilder.

[nul800sebastiaan]

I see, I didn't read much further since you mentioned an "old" method.. (AddAllCmsWebhookEvents was removed in the last few days) but you've now made a new one. I am not sure about the naming.

Now I see what you mean and I've learned something important: all those PRs from you to enhance Program.cs actually go against my advice to everyone, which is: you should never have to edit Program.cs; use a Composer. Anyway, too late for that now 😅

[erikjanwestendorp]

@nul800sebastiaan I can well imagine that you give that advice, although I think it can be convenient for small applications to quickly modify the Program.cs. However, of course, it is not necessary to do this; you can still use the methods from a Composer, like so:

public class MyComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddAllCmsWebhookEvents();
    }
}

or:

public class MyComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.ConfigureCmsWebhookEvents(cmsBuilder => cmsBuilder.AddContentType());
    }
}

[ronaldbarendse]

In my opinion we already have too many extension methods on IUmbracoBuilder, so adding even more would add to the existing clutter and result in multiple ways of accomplishing the same thing (which isn't always great for understanding what the code is doing, requires more code to be maintained, documented, etc.)...

The main reason for adding these is to keep the fluent/chained calls, so you don't have to use composers or resort to the following:

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

var umbracoBuilder = builder.CreateUmbracoBuilder()
    .AddBackOffice()
    .AddWebsite()
    .AddDeliveryApi()
    .AddComposers();

umbracoBuilder.WebhookEvents().AddCms(cmsBuilder => cmsBuilder.AddLanguage());

umbracoBuilder.Build();

WebApplication app = builder.Build();

---

But in that case, I'd suggest creating a single 'generic' Compose() extension method:

public static class UmbracoBuilderExtensions
{
    public static IUmbracoBuilder Compose(this IUmbracoBuilder builder, Action<IUmbracoBuilder> compose)
    {
        compose(builder);

        return builder;
    }
}

Which would allow you to keep the fluent approach:

WebApplicationBuilder builder = WebApplication.CreateBuilder(args);

builder.CreateUmbracoBuilder()
    .AddBackOffice()
    .AddWebsite()
    .AddDeliveryApi()
    .AddComposers()
    .Compose(builder => builder.WebhookEvents().AddCms(cmsBuilder => cmsBuilder.AddLanguage()))
    .Compose(builder =>
    {
        // You can even use multiple lines, e.g. to add more webhook events
        builder.WebhookEvents().AddCms(cmsBuilder => cmsBuilder.AddUser());

        // This would also make the following extension method obsolete:
        builder.AddWebhookEvent<ImportedPackageWebhookEvent>();
    })
    .Build();

WebApplication app = builder.Build();

Although I'm not 100% sure whether this makes sense though (and if it should be named Compose() or something else), as this would make it possible to infinitely nest them (e.g. Compose(builder => builder.Compose(builder => builder.AddComposers())))... But it at least would remove the need for having AddWebhookEvent<>(), AddContentApp<>(), AddContentFinder<>() and similar extension methods that are essentially forwarding their calls in a similar way as the above extension method...

[erikjanwestendorp]

@nul800sebastiaan @ronaldbarendse From what I read, the individual extension methods are certainly not desirable 🤣 . So, this PR can be closed for now I guess? However, it might be a good idea to decide whether to remove all extension methods or to remove them and create a generic Compose() method as @ronaldbarendse suggests. I believe that in both cases, it constitutes a breaking change and should be merged into the v14 branch (with the current methods marked as obsolete in v13?). In either scenario, I would like to create a new PR, but I am curious to know which of the two solutions is preferred.

[bielu]

Now I see what you mean and I've learned something important: all those PRs from you to enhance Program.cs actually go against my advice to everyone, which is: you should never have to edit Program.cs; use a Composer. Anyway, too late for that now 😅

@nul800sebastiaan / @erikjanwestendorp
I am little confused with this advice, as program.cs is definition of application so if you have multicontext application you need modify program.cs/startup.cs, so can you elaborate on this? As i can see 100 reasons why to not use composers, but not a good reason to not modify either of the mentioned files.

[nul800sebastiaan]

@bielu Never mind, I retract that, personal preference.

[ronaldbarendse]

My main concern regarding this PR is that it's adding an additional way of configuring webhooks, just so you can chain IUmbracoBuilder methods... This requires more code to be maintained and extra documentation to be written, while it doesn't support the full API that umbracoBuilder.WebhookEvents() exposes (as it's only specific to CMS events).

Adding a single Compose() extension method would solve the chaining issue and allow us to clean up (obsolete) the existing duplicated APIs that are similar (e.g. builder.AddDashboard<T>() would become builder.Compose(builder => builder.Dashboards().Add<T>()), builder.AddWebhookEvent<T>() would become builder.Compose(builder => builder.WebhookEvents().Add<T>()), etc.).

I do acknowledge this is slightly more verbose, but having a single way of doing things that also supports the full API (like inserting, removing, etc.) should be enough to justify that...

[erikjanwestendorp]

@ronaldbarendse Thanks again for you reply! I think this PR can be closed? I will create a new PR for the single Compose() method. I assume the methods can only be made obsolete in the v14 branch?

[emmagarland]

Thanks all! @erikjanwestendorp @ronaldbarendse we happy to close this one now we have the another PR? Do you have a link to that at all?