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

Webhooks administration from Backoffice #15050

Merged
merged 103 commits into from
Oct 31, 2023
Merged

Webhooks administration from Backoffice #15050

merged 103 commits into from
Oct 31, 2023

Conversation

Zeegaan
Copy link
Member

@Zeegaan Zeegaan commented Oct 26, 2023

Notes

  • Implemented a new webhook area in the Settings section 👀
    image
  • Implemented WebhookEventBase, which holds the logic for handling notifications and orchestrating the eventual firing & logging of the given webhook.
  • Implemented IWebhookFiringService which handles the logic for sending the webrequest to the given webhook url (and retry logic)
  • Implemented ´ÌWebhookLogService` to handle the logging.
  • Implemented IWebhookService to handle the CRUD of the webhooks.
  • Modelled Controller after v14 so it is easier to port forward, this means using ViewModels, not Display

What to expect of a webhook request

  • It is a POST request
  • The request body is a given request object, the request object should respond to the Get method of a type, so for content, it should be the same as ContentService.Get
  • We add a header out of the box Umb-Webhook-Event {eventName} so you can know which event triggered the request.

Implemented features

  • UI 🚀
  • Selecting multiple events
  • Selecting multiple Doc/Media types
  • Creating custom headers
  • Logging 📜
  • Creating your own webhook events (basically a webhook type)
  • Retry policy
  • Queueing, so firing of webhooks do not slow save and publish down
  • Limit the request payload
  • Manually retry a failed webhook
  • Member support

More background on choices

  • Basically, most choices boil down to aligning with hardcore, but this shouldn't be used as an excuse, if you disagree with the logic, feel free to let me know!
  • Why are webhook logs stored in the database? This is the same behavior in hardcore. It is also abstracted as a repository, so we could change this in the future if we need to.
  • Why are we using reflection to DI the WebhookEvents? Using reflection is more magical, but it also prevents us (and others if they want to create custom WebhookEvents) from having to register the webhooks both as a Collection, but also notification handlers. Reflection provides a neat way to just add them to the collection, and then make sure they also get registred as a notification handler.
  • Why hide media events when content is shown and vice versa? This is basically just a UX thing, while it would work to select both Media and content events, it's not really a use case we see and would cause more confusion than good IMO. This is why if you choose a content event, we try to be helpful, by removing the other events that doesn't have to do with content.

How to test

We should cover multiple use cases here.
We always need a DocumentType created, so in the cases, I will assume you have already made a content type named "Root" with "allow as root" set to true.

I have also put some code in the appendix that might be useful, such as a TestController.

Creating a webhook with 1 or more events

  • Go to the webhook section, and select "Create webhook"
  • Put in your URL that will be called, i used the URL for TestController: https://localhost:44331/umbraco/api/test/GetAllProducts
  • Select ContentPublish event
  • Save the webhook.
  • Set a breaking in TestController, in the GetAllProducts method
  • Save and publish some content
  • Assert the endpoint gets called, and a log is created for the request.

This should be tested with all different events.
We should also test adding custom headers, and asserting they get sent with the request (and are subsequently logged)

Creating a webhook, with one or more document types selected

  • Create a DocumentType named "DoNotSend" with "allow as root" set to true.
  • Go to the webhook section, and select "Create webhook"
  • Put in your URL that will be called, i used the URL for TestController: https://localhost:44331/umbraco/api/test/GetAllProducts
  • Select ContentPublish event
  • Select the "Root" content type.
  • Save the webhook.
  • Set a breaking in TestController, in the GetAllProducts method
  • Create a content from the "DoNotSend" document type, and save and publish
  • Assert the endpoint DOES NOT get called.
  • Create a content from the "Root" document type, and save and publish
  • Assert the endpoint DOES get called.

Creating your own webhook event

  • Implement TestWebhookEvent & TestComposer from appendix
  • Go to the webhook section, and select "Create webhook"
  • Put in your URL that will be called, i used the URL for TestController: https://localhost:44331/umbraco/api/test/GetAllProducts
  • Select MyEvent event that is your custom event
  • Save the webhook.
  • Set a breaking in TestController, in the GetAllProducts method
  • Create a member, then delete it.
  • Assert the endpoint gets called, and a log is created for the request.

Appendix

TestController

using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Web.Common.Controllers;

namespace Umbraco.Cms.Web.UI;

public class TestController : UmbracoApiController
{
    public IActionResult GetAllProducts() => Ok();
}

TestWebhookEvent

using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Webhooks;

namespace Umbraco.Cms.Web.UI;

public class TestWebhookEvent : WebhookEventBase<MemberDeletedNotification, IMember>
{
    public TestWebhookEvent(IWebhookFiringService webhookFiringService, IWebHookService webHookService, IWebhookLogService webhookLogService, IOptionsMonitor<WebhookSettings> webhookSettings, IWebhookLogFactory webhookLogFactory) : base(webhookFiringService, webHookService, webhookLogService, webhookSettings, webhookLogFactory, "MyEvent")
    {
    }

    protected override IEnumerable<IMember> GetEntitiesFromNotification(MemberDeletedNotification notification) => notification.DeletedEntities;
}

TestComposer

using Umbraco.Cms.Core.Composing;

namespace Umbraco.Cms.Web.UI;

public class TestComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder) => builder.WebhookEvents().Append<TestWebhookEvent>();
}

@bergmania bergmania changed the title V13: Webhooks Webhooks administration from Backoffice Oct 27, 2023
Copy link
Contributor

@kjac kjac left a comment

Choose a reason for hiding this comment

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

Looks great! 🥳

@Zeegaan Zeegaan merged commit 1b34d33 into v13/dev Oct 31, 2023
@Zeegaan Zeegaan deleted the v12/feature/webhooks branch October 31, 2023 09:06
/// <summary>
/// Webhooks icon
/// </summary>
public const string Webhooks = "icon-directions-alt";
Copy link
Contributor

@bjarnef bjarnef Oct 31, 2023

Choose a reason for hiding this comment

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

Would it be better to keep this in singular form @Zeegaan ?
So Webhook like we have User, Folder, Template... although Packages are plural. Maybe we should have a Package constant instead and eventually obsolete the Packages constant.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, sounds correct to me! 😁

Copy link
Contributor

@bjarnef bjarnef Oct 31, 2023

Choose a reason for hiding this comment

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

@Zeegaan does it require VS2022 17.8.0 preview to run v13/dev branch?

I tried to enable .NET 8 preview
https://anthonygiretti.com/2023/06/10/net-8-why-net-8-preview-doesnt-show-up-in-visual-studio-2022/

but it seems to return these warnings.

Targeting .NET 8.0 or higher in Visual Studio 2022 17.7 is not supported.

so not sure if the static assets are built correct as I get this error on startup:

The view '~/umbraco/UmbracoWebsite/Maintenance.cshtml' was not found.

Copy link
Member Author

@Zeegaan Zeegaan Oct 31, 2023

Choose a reason for hiding this comment

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

Not using VS personally (Rider 😍) But yes presumably you'd have to use a preview of VS, when using the dotnet 8 rc-2 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

https://dotnetthoughts.net/working-with-dotnet8-projects-in-vs2022/

Yes, I think it requires the latest VS2022 preview to run for now:
dotnet/core#8790

Copy link
Contributor

Choose a reason for hiding this comment

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

I just edited in VS 2022, but ran via command line next to it:

dotnet build
dotnet run

@bjarnef bjarnef mentioned this pull request Oct 31, 2023
1 task
@karlmacklin
Copy link
Contributor

I'm checking this out in v13 release candidate and it's probably my favorite feature of v13 so far!

Question, are there any plans to create any more default webhooks?

We use Umbraco as a headless CMS.
One thing this could replace for us is the need to listen to all content-related notifications to handle propagation of updated/changed/moved nodes.
When content is saved we tell our frontend to update said content.

One possible change is the slug of a page. In that case we tell our frontend to forget the previous url and rebuild the page for the new url.

When dealing with nested content, we suddenly have to subscribe to:

  • ContentPublishingNotification
  • ContentPublishedNotification
  • ContentUnpublishingNotification
  • ContentUnpublishedNotification
  • ContentMovingNotification
  • ContentMovedNotification
  • ContentMovingToRecycleBinNotification
  • ContentMovedToRecycleBinNotification

That is a lot of boilerplate to handle that specific scenario.

Now I assume that the idea here is that we could simply create our own webhooks and trigger those via these notifications, and already this is probably a big upgrade for us.

But it begs the question if there's any plan from the core team to expand on the default set, to cover such a use case?

@Zeegaan
Copy link
Member Author

Zeegaan commented Nov 13, 2023

@karlmacklin Great to hear! 💪 Yes the idea is that you could create webhooks for anything you need 💪
Warren is already in full swing of adding some more webhooks here: #15161
And nothing is set in stone, it's implemented in such a way that it would be non-breaking to add more (or there could even be a package adding some) 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants