Redirects: Adding notification for redirect save and deletion - #22985
Conversation
AndyButland
left a comment
There was a problem hiding this comment.
Looks good @NillasKA - I think it's almost there. Found a few things to consider, and when you've done that and made updates I'll run some tests.
AndyButland
left a comment
There was a problem hiding this comment.
Looking good and working well. I've been testing it out with the following code that logs each redirect save and delete, and cancels them if the content begins with the letter "A".
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
namespace Umbraco.Cms.Web.UI.Custom.Redirects;
public class RedirectNotificationsComposer : IComposer
{
public void Compose(IUmbracoBuilder builder)
{
builder.AddNotificationHandler<RedirectUrlSavingNotification, RedirectUrlSavingNotificationHandler>();
builder.AddNotificationHandler<RedirectUrlSavedNotification, RedirectUrlSavedNotificationHandler>();
builder.AddNotificationHandler<RedirectUrlDeletingNotification, RedirectUrlDeletingNotificationHandler>();
builder.AddNotificationHandler<RedirectUrlDeletedNotification, RedirectUrlDeletedNotificationHandler>();
}
}
public class RedirectUrlDeletedNotificationHandler : INotificationHandler<RedirectUrlDeletedNotification>
{
private readonly ILogger<RedirectUrlDeletedNotificationHandler> _logger;
public RedirectUrlDeletedNotificationHandler(ILogger<RedirectUrlDeletedNotificationHandler> logger)
=> _logger = logger;
public void Handle(RedirectUrlDeletedNotification notification)
{
foreach (IRedirectUrl redirect in notification.DeletedEntities)
{
_logger.LogInformation(
"RedirectUrlDeleted: url='{Url}', contentKey={ContentKey}, culture='{Culture}'.",
redirect.Url,
redirect.ContentKey,
redirect.Culture);
}
}
}
public class RedirectUrlDeletingNotificationHandler : INotificationHandler<RedirectUrlDeletingNotification>
{
private readonly IContentService _contentService;
private readonly ILogger<RedirectUrlDeletingNotificationHandler> _logger;
public RedirectUrlDeletingNotificationHandler(
IContentService contentService,
ILogger<RedirectUrlDeletingNotificationHandler> logger)
{
_contentService = contentService;
_logger = logger;
}
public void Handle(RedirectUrlDeletingNotification notification)
{
IRedirectUrl[] redirects = notification.DeletedEntities.ToArray();
_logger.LogInformation(
"RedirectUrlDeleting: deleting {Count} redirect URL(s).",
redirects.Length);
foreach (IRedirectUrl redirect in redirects)
{
IContent? content = _contentService.GetById(redirect.ContentKey);
var contentName = content?.Name ?? "(unknown)";
_logger.LogInformation(
"RedirectUrlDeleting: url='{Url}', contentKey={ContentKey}, contentName='{ContentName}', culture='{Culture}'.",
redirect.Url,
redirect.ContentKey,
contentName,
redirect.Culture);
if (content?.Name?.StartsWith("A", StringComparison.OrdinalIgnoreCase) == true)
{
_logger.LogInformation(
"RedirectUrlDeleting: cancelling because content name '{ContentName}' starts with 'A'.",
contentName);
notification.CancelOperation(new EventMessage(
"Redirect URL Delete",
$"Redirects cannot be deleted for content whose name starts with 'A' (test handler). Content: '{contentName}'.",
EventMessageType.Error));
return;
}
}
}
}
public class RedirectUrlSavedNotificationHandler : INotificationHandler<RedirectUrlSavedNotification>
{
private readonly ILogger<RedirectUrlSavedNotificationHandler> _logger;
public RedirectUrlSavedNotificationHandler(ILogger<RedirectUrlSavedNotificationHandler> logger)
=> _logger = logger;
public void Handle(RedirectUrlSavedNotification notification)
{
foreach (IRedirectUrl redirect in notification.SavedEntities)
{
_logger.LogInformation(
"RedirectUrlSaved: url='{Url}', contentKey={ContentKey}, culture='{Culture}'.",
redirect.Url,
redirect.ContentKey,
redirect.Culture);
}
}
}
public class RedirectUrlSavingNotificationHandler : INotificationHandler<RedirectUrlSavingNotification>
{
private readonly IContentService _contentService;
private readonly ILogger<RedirectUrlSavingNotificationHandler> _logger;
public RedirectUrlSavingNotificationHandler(
IContentService contentService,
ILogger<RedirectUrlSavingNotificationHandler> logger)
{
_contentService = contentService;
_logger = logger;
}
public void Handle(RedirectUrlSavingNotification notification)
{
foreach (IRedirectUrl redirect in notification.SavedEntities)
{
IContent? content = _contentService.GetById(redirect.ContentKey);
var contentName = content?.Name ?? "(unknown)";
_logger.LogInformation(
"RedirectUrlSaving: url='{Url}', contentKey={ContentKey}, contentName='{ContentName}', culture='{Culture}'.",
redirect.Url,
redirect.ContentKey,
contentName,
redirect.Culture);
if (content?.Name?.StartsWith("A", StringComparison.OrdinalIgnoreCase) == true)
{
_logger.LogInformation(
"RedirectUrlSaving: cancelling because content name '{ContentName}' starts with 'A'.",
contentName);
notification.CancelOperation(new EventMessage(
"Redirect URL Creation",
$"Redirects are not allowed for content whose name starts with 'A' (test handler). Content: '{contentName}'.",
EventMessageType.Error));
return;
}
}
}
}All works well.
The only problem I see though is in the backoffice UI when saving a document. If it would create a redirect but the saving is cancelled, we get a notification in the UI:
And I don't think we really want to see this. Redirect creation is "silent" - there's no notification when they are created. And likely someone implementing a cancellable notification handler will be doing this because for example they don't want redirects for a given document type. Nothing the editor needs to know about and nothing they can do about it. So it would be better if this doesn't display.
It's different for deletion of redirects. There the editor is making an action specifically to delete one, so it's right they get a notification when it's cancelled. But we would want to avoid it for saving redirects.
|
Hi again @AndyButland thanks for the review. I have solved the primary concern you raised so cancelled saving notifications no longer are surfaced to the backoffice UI, but cancelling a deleting notification is still shown. |
AndyButland
left a comment
There was a problem hiding this comment.
All looks good me now @NillasKA - nice work. Approving and merging.
We should document this too. I'll DM you some thoughts.
Description
A was raised here calling for a need to control the creation of redirect URLs. This PR aims to build the foundation for this support by adding a
CreatingandCreatednotification for the creation of redirect URLs. This PR also createsDeletingandDeletednotification for further customization support.The purpose of these notifications are for consumers to have the ability to create notification handlers, that could be customized to "disable" the creation of redirects based on the consumers own criteria.
What has been implemented
RedirectUrlOperationStatusResultadds notification messages for clarityTesting
I have manually tested this by creating and removing redirects, none of my testing attempts have failed and all seems to work fine. Otherwise it's just the tests in the pipeline that should verify.