V13: Refactor webhookservice to attempt pattern

src/Umbraco.Core/Services/IWebhookService.cs

@@ -1,4 +1,5 @@
 using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Services.OperationStatus;
 
 namespace Umbraco.Cms.Core.Services;
 
@@ -8,19 +9,19 @@ public interface IWebhookService
     ///     Creates a webhook.
     /// </summary>
     /// <param name="webhook"><see cref="Webhook" /> to create.</param>
-    Task<Webhook?> CreateAsync(Webhook webhook);
+    Task<Attempt<Webhook, WebhookOperationStatus>> CreateAsync(Webhook webhook);
 
     /// <summary>
     ///     Updates a webhook.
     /// </summary>
     /// <param name="webhook"><see cref="Webhook" /> to update.</param>
-    Task UpdateAsync(Webhook webhook);
+    Task<Attempt<Webhook, WebhookOperationStatus>> UpdateAsync(Webhook webhook);
 
     /// <summary>
     ///     Deletes a webhook.
     /// </summary>
     /// <param name="key">The unique key of the webhook.</param>
-    Task DeleteAsync(Guid key);
+    Task<Attempt<Webhook?, WebhookOperationStatus>> DeleteAsync(Guid key);
 
     /// <summary>
     ///     Gets a webhook by its key.

src/Umbraco.Core/Services/OperationStatus/WebhookOperationStatus.cs

@@ -0,0 +1,8 @@
+namespace Umbraco.Cms.Core.Services.OperationStatus;
+
+public enum WebhookOperationStatus
+{
+    Success,
+    CancelledByNotification,
+    NotFound,
+}

src/Umbraco.Core/Services/WebhookService.cs

@@ -3,6 +3,7 @@
 using Umbraco.Cms.Core.Notifications;
 using Umbraco.Cms.Core.Persistence.Repositories;
 using Umbraco.Cms.Core.Scoping;
+using Umbraco.Cms.Core.Services.OperationStatus;
 
 namespace Umbraco.Cms.Core.Services;
 
@@ -20,46 +21,46 @@ public WebhookService(ICoreScopeProvider provider, IWebhookRepository webhookRep
     }
 
     /// <inheritdoc />
-    public async Task<Webhook?> CreateAsync(Webhook webhook)
+    public async Task<Attempt<Webhook, WebhookOperationStatus>> CreateAsync(Webhook webhook)
     {
         using ICoreScope scope = _provider.CreateCoreScope();
 
         EventMessages eventMessages = _eventMessagesFactory.Get();
         var savingNotification = new WebhookSavingNotification(webhook, eventMessages);
-        if (scope.Notifications.PublishCancelable(savingNotification))
+        if (await scope.Notifications.PublishCancelableAsync(savingNotification))
         {
             scope.Complete();
-            return null;
+            return Attempt.FailWithStatus(WebhookOperationStatus.CancelledByNotification, webhook);
         }
 
         Webhook created = await _webhookRepository.CreateAsync(webhook);
 
-        scope.Notifications.Publish(
-            new WebhookSavedNotification(webhook, eventMessages).WithStateFrom(savingNotification));
+        scope.Notifications.Publish(new WebhookSavedNotification(webhook, eventMessages).WithStateFrom(savingNotification));
 
         scope.Complete();
 
-        return created;
+        return Attempt.SucceedWithStatus(WebhookOperationStatus.Success, created);
     }
 
     /// <inheritdoc />
-    public async Task UpdateAsync(Webhook webhook)
+    public async Task<Attempt<Webhook, WebhookOperationStatus>> UpdateAsync(Webhook webhook)
     {
         using ICoreScope scope = _provider.CreateCoreScope();
 
         Webhook? currentWebhook = await _webhookRepository.GetAsync(webhook.Key);
 
         if (currentWebhook is null)
         {
-            throw new ArgumentException("Webhook does not exist");
+            scope.Complete();
+            return Attempt.FailWithStatus(WebhookOperationStatus.NotFound, webhook);
         }
 
         EventMessages eventMessages = _eventMessagesFactory.Get();
         var savingNotification = new WebhookSavingNotification(webhook, eventMessages);
-        if (scope.Notifications.PublishCancelable(savingNotification))
+        if (await scope.Notifications.PublishCancelableAsync(savingNotification))
         {
             scope.Complete();
-            return;
+            return Attempt.FailWithStatus(WebhookOperationStatus.CancelledByNotification, webhook);
         }
 
         currentWebhook.Enabled = webhook.Enabled;
@@ -70,33 +71,37 @@ public async Task UpdateAsync(Webhook webhook)
 
         await _webhookRepository.UpdateAsync(currentWebhook);
 
-        scope.Notifications.Publish(
-            new WebhookSavedNotification(webhook, eventMessages).WithStateFrom(savingNotification));
+        scope.Notifications.Publish(new WebhookSavedNotification(webhook, eventMessages).WithStateFrom(savingNotification));
 
         scope.Complete();
+
+        return Attempt.SucceedWithStatus(WebhookOperationStatus.Success, webhook);
     }
 
     /// <inheritdoc />
-    public async Task DeleteAsync(Guid key)
+    public async Task<Attempt<Webhook?, WebhookOperationStatus>> DeleteAsync(Guid key)
     {
         using ICoreScope scope = _provider.CreateCoreScope();
         Webhook? webhook = await _webhookRepository.GetAsync(key);
-        if (webhook is not null)
+        if (webhook is null)
         {
-            EventMessages eventMessages = _eventMessagesFactory.Get();
-            var deletingNotification = new WebhookDeletingNotification(webhook, eventMessages);
-            if (scope.Notifications.PublishCancelable(deletingNotification))
-            {
-                scope.Complete();
-                return;
-            }
-
-            await _webhookRepository.DeleteAsync(webhook);
-            scope.Notifications.Publish(
-                new WebhookDeletedNotification(webhook, eventMessages).WithStateFrom(deletingNotification));
+            return Attempt.FailWithStatus(WebhookOperationStatus.NotFound, webhook);
         }
 
+        EventMessages eventMessages = _eventMessagesFactory.Get();
+        var deletingNotification = new WebhookDeletingNotification(webhook, eventMessages);
+        if (await scope.Notifications.PublishCancelableAsync(deletingNotification))
+        {
+            scope.Complete();
+            return Attempt.FailWithStatus<Webhook?, WebhookOperationStatus>(WebhookOperationStatus.CancelledByNotification, webhook);
+        }
+
+        await _webhookRepository.DeleteAsync(webhook);
+        scope.Notifications.Publish(new WebhookDeletedNotification(webhook, eventMessages).WithStateFrom(deletingNotification));
+
         scope.Complete();
+
+        return Attempt.SucceedWithStatus<Webhook?, WebhookOperationStatus>(WebhookOperationStatus.Success, webhook);
     }
 
     /// <inheritdoc />

src/Umbraco.Web.BackOffice/Controllers/WebhookController.cs

@@ -1,9 +1,12 @@
-using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.ContentEditing;
 using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Core.Webhooks;
 using Umbraco.Cms.Web.BackOffice.Services;
 using Umbraco.Cms.Web.Common.Attributes;
@@ -46,19 +49,16 @@ public async Task<IActionResult> Update(WebhookViewModel webhookViewModel)
     {
         Webhook webhook = _umbracoMapper.Map<Webhook>(webhookViewModel)!;
 
-        await _webhookService.UpdateAsync(webhook);
-
-        return Ok(_webhookPresentationFactory.Create(webhook));
+        Attempt<Webhook, WebhookOperationStatus> result = await _webhookService.UpdateAsync(webhook);
+        return result.Success ? Ok(_webhookPresentationFactory.Create(webhook)) : WebhookOperationStatusResult(result.Status);
     }
 
     [HttpPost]
     public async Task<IActionResult> Create(WebhookViewModel webhookViewModel)
     {
         Webhook webhook = _umbracoMapper.Map<Webhook>(webhookViewModel)!;
-
-        await _webhookService.CreateAsync(webhook);
-
-        return Ok(_webhookPresentationFactory.Create(webhook));
+        Attempt<Webhook, WebhookOperationStatus> result = await _webhookService.CreateAsync(webhook);
+        return result.Success ? Ok(_webhookPresentationFactory.Create(webhook)) : WebhookOperationStatusResult(result.Status);
     }
 
     [HttpGet]
@@ -72,9 +72,8 @@ public async Task<IActionResult> GetByKey(Guid key)
     [HttpDelete]
     public async Task<IActionResult> Delete(Guid key)
     {
-        await _webhookService.DeleteAsync(key);
-
-        return Ok();
+        Attempt<Webhook?, WebhookOperationStatus> result = await _webhookService.DeleteAsync(key);
+        return result.Success ? Ok() : WebhookOperationStatusResult(result.Status);
     }
 
     [HttpGet]
@@ -94,4 +93,15 @@ public async Task<IActionResult> GetLogs(int skip = 0, int take = int.MaxValue)
             Items = mappedLogs,
         });
     }
+
+    private IActionResult WebhookOperationStatusResult(WebhookOperationStatus status) =>
+        status switch
+        {
+            WebhookOperationStatus.CancelledByNotification => ValidationProblem(new SimpleNotificationModel(new BackOfficeNotification[]
+                {
+                    new("Cancelled by notification", "The operation was cancelled by a notification", NotificationStyle.Error),
+                })),
+            WebhookOperationStatus.NotFound => NotFound("Could not find the webhook"),
+            _ => StatusCode(StatusCodes.Status500InternalServerError),
+        };
 }

tests/Umbraco.Tests.Integration/Umbraco.Core/Services/WebhookServiceTests.cs

@@ -22,7 +22,7 @@ public class WebhookServiceTests : UmbracoIntegrationTest
     public async Task Can_Create_And_Get(string url, string webhookEvent, Guid key)
     {
         var createdWebhook = await WebhookService.CreateAsync(new Webhook(url, true, new[] { key }, new[] { webhookEvent }));
-        var webhook = await WebhookService.GetAsync(createdWebhook.Key);
+        var webhook = await WebhookService.GetAsync(createdWebhook.Result.Key);
 
         Assert.Multiple(() =>
         {
@@ -45,9 +45,9 @@ public async Task Can_Get_All()
         Assert.Multiple(() =>
         {
             Assert.IsNotEmpty(webhooks.Items);
-            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookOne.Key));
-            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookTwo.Key));
-            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookThree.Key));
+            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookOne.Result.Key));
+            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookTwo.Result.Key));
+            Assert.IsNotNull(webhooks.Items.FirstOrDefault(x => x.Key == createdWebhookThree.Result.Key));
         });
     }
 
@@ -60,19 +60,19 @@ public async Task Can_Get_All()
     public async Task Can_Delete(string url, string webhookEvent, Guid key)
     {
         var createdWebhook = await WebhookService.CreateAsync(new Webhook(url, true, new[] { key }, new[] { webhookEvent }));
-        var webhook = await WebhookService.GetAsync(createdWebhook.Key);
+        var webhook = await WebhookService.GetAsync(createdWebhook.Result.Key);
 
         Assert.IsNotNull(webhook);
         await WebhookService.DeleteAsync(webhook.Key);
-        var deletedWebhook = await WebhookService.GetAsync(createdWebhook.Key);
+        var deletedWebhook = await WebhookService.GetAsync(createdWebhook.Result.Key);
         Assert.IsNull(deletedWebhook);
     }
 
     [Test]
     public async Task Can_Create_With_No_EntityKeys()
     {
         var createdWebhook = await WebhookService.CreateAsync(new Webhook("https://example.com", events: new[] { Constants.WebhookEvents.Aliases.ContentPublish }));
-        var webhook = await WebhookService.GetAsync(createdWebhook.Key);
+        var webhook = await WebhookService.GetAsync(createdWebhook.Result.Key);
 
         Assert.IsNotNull(webhook);
         Assert.IsEmpty(webhook.ContentTypeKeys);
@@ -82,10 +82,10 @@ public async Task Can_Create_With_No_EntityKeys()
     public async Task Can_Update()
     {
         var createdWebhook = await WebhookService.CreateAsync(new Webhook("https://example.com", events: new[] { Constants.WebhookEvents.Aliases.ContentPublish }));
-        createdWebhook.Events = new[] { Constants.WebhookEvents.Aliases.ContentDelete };
-        await WebhookService.UpdateAsync(createdWebhook);
+        createdWebhook.Result.Events = new[] { Constants.WebhookEvents.Aliases.ContentDelete };
+        await WebhookService.UpdateAsync(createdWebhook.Result);
 
-        var updatedWebhook = await WebhookService.GetAsync(createdWebhook.Key);
+        var updatedWebhook = await WebhookService.GetAsync(createdWebhook.Result.Key);
         Assert.IsNotNull(updatedWebhook);
         Assert.AreEqual(1, updatedWebhook.Events.Length);
         Assert.IsTrue(updatedWebhook.Events.Contains(Constants.WebhookEvents.Aliases.ContentDelete));