Use proper notifications instead of event messages

This commit is contained in:
nikolajlauridsen
2021-10-08 11:24:10 +02:00
parent f0f8874727
commit 8e7fb897d1
3 changed files with 49 additions and 37 deletions

View File

@@ -441,6 +441,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Web.BackOffice.Controllers
string body = await response.Content.ReadAsStringAsync();
body = body.TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix);
ContentItemDisplay display = JsonConvert.DeserializeObject<ContentItemDisplay>(body);
Assert.IsNotNull(display);
Assert.AreEqual(1, display.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
}
}

View File

@@ -1,4 +1,6 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Microsoft.AspNetCore.Authorization;
using Microsoft.Extensions.Logging.Abstractions;
@@ -9,6 +11,7 @@ using Umbraco.Cms.Core.Dictionary;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Scoping;
@@ -49,13 +52,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> { "en-us", "da-dk" };
var publishResult = new PublishResult(new EventMessages(), rootNode);
var notifications = new SimpleNotificationModel();
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(rootNode, culturesPublished, publishResult);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
IEnumerable<EventMessage> eventMessages = publishResult.EventMessages.GetAll();
Assert.IsEmpty(eventMessages);
Assert.IsEmpty(notifications.Notifications);
}
[Test]
@@ -76,12 +78,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> {"da-dk" };
var publishResult = new PublishResult(new EventMessages(), rootNode);
var notifications = new SimpleNotificationModel();
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(rootNode, culturesPublished, publishResult);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
Assert.IsEmpty(publishResult.EventMessages.GetAll());
Assert.IsEmpty(notifications.Notifications);
}
[Test]
@@ -105,11 +107,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> { "en-us", "da-dk" };
var publishResult = new PublishResult(new EventMessages(), rootNode);
var notifications = new SimpleNotificationModel();
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(rootNode, culturesPublished, publishResult);
Assert.AreEqual(1, publishResult.EventMessages.Count);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
[Test]
@@ -134,11 +136,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> { "en-us", "da-dk", "nl-bk", "se-sv" };
var publishResult = new PublishResult(new EventMessages(), rootNode);
var notifications = new SimpleNotificationModel();
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(rootNode, culturesPublished, publishResult);
Assert.AreEqual(3, publishResult.EventMessages.Count);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
Assert.AreEqual(3, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
[Test]
@@ -183,12 +185,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> { "en-us", "da-dk", "nl-bk", "se-sv", "de-de" };
var publishResult = new PublishResult(new EventMessages(), level3Node);
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(level3Node, culturesPublished, publishResult);
var notifications = new SimpleNotificationModel();
contentController.AddDomainWarnings(level3Node, culturesPublished, notifications);
// We expect one error because all domains except "de-de" is registered somewhere in the ancestor path
Assert.AreEqual(1, publishResult.EventMessages.Count);
Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
[Test]
@@ -218,23 +221,30 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers
.Build();
var culturesPublished = new List<string> { "en-us", "se-sv" };
var publishResult = new PublishResult(new EventMessages(), rootNode);
var notifications = new SimpleNotificationModel();
ContentController contentController = CreateContentController(domainServiceMock.Object);
contentController.VerifyDomainsForCultures(rootNode, culturesPublished, publishResult);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
// We only get two errors, one for each culture being published, so no errors from previously published cultures.
Assert.AreEqual(2, publishResult.EventMessages.Count);
Assert.AreEqual(2, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
private ContentController CreateContentController(IDomainService domainService)
{
// We have to configure ILocalizedTextService to return a new string every time Localize is called
// Otherwise it won't add the notification because it skips dupes
var localizedTextServiceMock = new Mock<ILocalizedTextService>();
localizedTextServiceMock.Setup(x => x.Localize(It.IsAny<string>(),
It.IsAny<string>(), It.IsAny<CultureInfo>(), It.IsAny<IDictionary<string, string>>()))
.Returns(() => Guid.NewGuid().ToString());
var controller = new ContentController(
Mock.Of<ICultureDictionary>(),
NullLoggerFactory.Instance,
Mock.Of<IShortStringHelper>(),
Mock.Of<IEventMessagesFactory>(),
Mock.Of<ILocalizedTextService>(),
localizedTextServiceMock.Object,
new PropertyEditorCollection(new DataEditorCollection(() => null)),
Mock.Of<IContentService>(),
Mock.Of<IUserService>(),

View File

@@ -879,7 +879,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
case ContentSaveAction.Publish:
case ContentSaveAction.PublishNew:
{
var publishStatus = PublishInternal(contentItem, defaultCulture, cultureForInvariantErrors, out wasCancelled, out var successfulCultures);
PublishResult publishStatus = PublishInternal(contentItem, defaultCulture, cultureForInvariantErrors, out wasCancelled, out var successfulCultures);
// Add warnings if domains are not set up correctly
AddDomainWarnings(publishStatus.Content, successfulCultures, globalNotifications);
AddPublishStatusNotifications(new[] { publishStatus }, globalNotifications, notifications, successfulCultures);
}
break;
@@ -1413,11 +1415,6 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
wasCancelled = publishStatus.Result == PublishResultType.FailedPublishCancelledByEvent;
successfulCultures = culturesToPublish;
// Verify that there's appropriate cultures configured.
if (publishStatus.Success)
{
VerifyDomainsForCultures(contentItem.PersistedContent, culturesToPublish, publishStatus);
}
return publishStatus;
}
else
@@ -1435,14 +1432,20 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
/// Verifies that there's an appropriate domain setup for the published cultures
/// </summary>
/// <remarks>
/// Adds a warning and logs a message if, a node varies but culture, there's at least 1 culture already published,
/// Adds a warning and logs a message if a node varies by culture, there's at least 1 culture already published,
/// and there's no domain added for the published cultures
/// </remarks>
/// <param name="persistedContent"></param>
/// <param name="culturesPublished"></param>
/// <param name="publishResult"></param>
internal void VerifyDomainsForCultures(IContent persistedContent, IEnumerable<string> culturesPublished, PublishResult publishResult)
/// <param name="globalNotifications"></param>
internal void AddDomainWarnings(IContent persistedContent, IEnumerable<string> culturesPublished, SimpleNotificationModel globalNotifications)
{
// Don't try to verify if no cultures were published
if (culturesPublished is null)
{
return;
}
var publishedCultures = persistedContent.PublishedCultures.ToList();
// If only a single culture is published we shouldn't have any routing issues
if (publishedCultures.Count < 2)
@@ -1461,10 +1464,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
// No domains at all, add a warning, to add domains.
if (assignedDomains.Count == 0)
{
publishResult.EventMessages.Add(new EventMessage(
globalNotifications.AddWarningNotification(
_localizedTextService.Localize("auditTrails", "publish"),
_localizedTextService.Localize("speechBubbles", "publishWithNoDomains"),
EventMessageType.Warning));
_localizedTextService.Localize("speechBubbles", "publishWithNoDomains"));
_logger.LogWarning("The root node {RootNodeName} was published with multiple cultures, but no domains are configured, this will cause routing and caching issues, please register domains for: {Cultures}",
persistedContent.Name, string.Join(", ", publishedCultures));
@@ -1475,11 +1477,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers
foreach (var culture in culturesPublished
.Where(culture => assignedDomains.Any(x => x.LanguageIsoCode.Equals(culture, StringComparison.OrdinalIgnoreCase)) is false))
{
publishResult.EventMessages.Add(new EventMessage(
globalNotifications.AddWarningNotification(
_localizedTextService.Localize("auditTrails", "publish"),
_localizedTextService.Localize("speechBubbles", "publishWithMissingDomain", new []{culture}),
EventMessageType.Warning
));
_localizedTextService.Localize("speechBubbles", "publishWithMissingDomain", new []{culture}));
_logger.LogWarning("The root node {RootNodeName} was published in culture {Culture}, but there's no domain configured for it, this will cause routing and caching issues, please register a domain for it",
persistedContent.Name, culture);