yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Warn about un-routable content at publish time (#17705)

Browse Source 
(cherry picked from commit 2d9cfc880b)
This commit is contained in:
Kenn Jacobsen
2024-12-09 11:07:32 +01:00
committed by Nikolaj Geisle
parent ee8bdfc4ac
commit a627930b58
5 changed files with 110 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/Umbraco.Core/EmbeddedResources/Lang/da.xml
View File

@@ -1469,6 +1469,7 @@ Mange hilsner fra Umbraco robotten
<key alias="publishWithMissingDomain">Der er ikke noget domæne konfigureret for %0%, kontakt vensligst en
administrator, se loggen for mere information
</key>
<key alias="publishWithNoUrl">Dokumentet har ikke nogen URL, muligvis grundet en kollision med et andet dokuments navn. Flere detaljer kan ses under Info.</key>
<key alias="copySuccessMessage">Dit systems information er blevet kopieret til udklipsholderen</key>
<key alias="cannotCopyInformation">Kunne desværre ikke kopiere dit systems information til udklipsholderen</key>
<key alias="webhookSaved">Webhook gemt</key>

1
src/Umbraco.Core/EmbeddedResources/Lang/en.xml
View File

@@ -1700,6 +1700,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont
<key alias="publishWithMissingDomain">There is no domain configured for %0%, please contact an administrator, see
log for more information
</key>
<key alias="publishWithNoUrl">The document does not have a URL, possibly due to a naming collision with another document. More details can be found under Info.</key>
<key alias="copySuccessMessage">Your system information has successfully been copied to the clipboard</key>
<key alias="cannotCopyInformation">Could not copy your system information to the clipboard</key>
<key alias="webhookSaved">Webhook saved</key>

1
src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml
View File

@@ -1732,6 +1732,7 @@ To manage your website, simply open the Umbraco backoffice and start adding cont
<key alias="publishWithMissingDomain">There is no domain configured for %0%, please contact an administrator, see
log for more information
</key>
<key alias="publishWithNoUrl">The document does not have a URL, possibly due to a naming collision with another document. More details can be found under Info.</key>
<key alias="preventCleanupEnableError">An error occurred while enabling version cleanup for %0%</key>
<key alias="preventCleanupDisableError">An error occurred while disabling version cleanup for %0%</key>
<key alias="copySuccessMessage">Your system information has successfully been copied to the clipboard</key>

82
src/Umbraco.Web.BackOffice/Controllers/ContentController.cs
View File

@@ -1134,8 +1134,12 @@ public class ContentController : ContentControllerBase
{
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);
var addedDomainWarnings = AddDomainWarnings(publishStatus.Content, successfulCultures, globalNotifications, defaultCulture);
AddPublishStatusNotifications(new[] { publishStatus }, globalNotifications, notifications, successfulCultures);
if (addedDomainWarnings is false)
{
AddPublishRoutableErrorNotifications(new[] { publishStatus }, globalNotifications, successfulCultures);
}
}
break;
case ContentSaveAction.PublishWithDescendants:
@@ -1151,8 +1155,12 @@ public class ContentController : ContentControllerBase
}
var publishStatus = PublishBranchInternal(contentItem, false, cultureForInvariantErrors, out wasCancelled, out var successfulCultures).ToList();
AddDomainWarnings(publishStatus, successfulCultures, globalNotifications);
var addedDomainWarnings = AddDomainWarnings(publishStatus, successfulCultures, globalNotifications, defaultCulture);
AddPublishStatusNotifications(publishStatus, globalNotifications, notifications, successfulCultures);
if (addedDomainWarnings is false)
{
AddPublishRoutableErrorNotifications(publishStatus, globalNotifications, successfulCultures);
}
}
break;
case ContentSaveAction.PublishWithDescendantsForce:
@@ -1235,6 +1243,48 @@ public class ContentController : ContentControllerBase
}
}
private void AddPublishRoutableErrorNotifications(
IReadOnlyCollection<PublishResult> publishStatus,
SimpleNotificationModel globalNotifications,
string[]? successfulCultures)
{
IContent? content = publishStatus.FirstOrDefault()?.Content;
if (content is null)
{
return;
}
if (content.ContentType.VariesByCulture() is false)
{
// successfulCultures will be null here - change it to a wildcard and utilize this below
successfulCultures = ["*"];
}
if (successfulCultures?.Any() is not true)
{
return;
}
ContentItemDisplay? contentItemDisplay = _umbracoMapper.Map<ContentItemDisplay>(publishStatus.FirstOrDefault()?.Content);
if (contentItemDisplay?.Urls is null)
{
return;
}
foreach (var culture in successfulCultures)
{
if (contentItemDisplay.Urls.Where(u => u.Culture == culture || culture == "*").All(u => u.IsUrl is false))
{
globalNotifications.AddWarningNotification(
_localizedTextService.Localize("auditTrails", "publish"),
_localizedTextService.Localize("speechBubbles", "publishWithNoUrl"));
// only add one warning here, even though there might actually be more
break;
}
}
}
/// <summary>
/// Validates critical data for persistence and updates the ModelState and result accordingly
/// </summary>
@@ -1770,12 +1820,15 @@ public class ContentController : ContentControllerBase
}
}
private void AddDomainWarnings(IEnumerable<PublishResult> publishResults, string[]? culturesPublished, SimpleNotificationModel globalNotifications)
private bool AddDomainWarnings(IEnumerable<PublishResult> publishResults, string[]? culturesPublished, SimpleNotificationModel globalNotifications, string? defaultCulture)
{
var addedDomainWarnings = false;
foreach (PublishResult publishResult in publishResults)
{
AddDomainWarnings(publishResult.Content, culturesPublished, globalNotifications);
addedDomainWarnings &= AddDomainWarnings(publishResult.Content, culturesPublished, globalNotifications, defaultCulture);
}
return addedDomainWarnings;
}
/// <summary>
@@ -1788,24 +1841,25 @@ public class ContentController : ContentControllerBase
/// <param name="persistedContent"></param>
/// <param name="culturesPublished"></param>
/// <param name="globalNotifications"></param>
internal void AddDomainWarnings(IContent? persistedContent, string[]? culturesPublished, SimpleNotificationModel globalNotifications)
/// <param name="defaultCulture"></param>
internal bool AddDomainWarnings(IContent? persistedContent, string[]? culturesPublished, SimpleNotificationModel globalNotifications, string? defaultCulture)
{
if (_contentSettings.ShowDomainWarnings is false)
{
return;
return false;
}
// Don't try to verify if no cultures were published
if (culturesPublished is null)
{
return;
return false;
}
var publishedCultures = GetPublishedCulturesFromAncestors(persistedContent).ToList();
// If only a single culture is published we shouldn't have any routing issues
if (publishedCultures.Count < 2)
{
return;
return false;
}
// If more than a single culture is published we need to verify that there's a domain registered for each published culture
@@ -1827,6 +1881,12 @@ public class ContentController : ContentControllerBase
// No domains at all, add a warning, to add domains.
if (assignedDomains is null || assignedDomains.Count == 0)
{
// If only the default culture was published we shouldn't have any routing issues
if (culturesPublished.Length == 1 && culturesPublished[0].InvariantEquals(defaultCulture))
{
return false;
}
globalNotifications.AddWarningNotification(
_localizedTextService.Localize("auditTrails", "publish"),
_localizedTextService.Localize("speechBubbles", "publishWithNoDomains"));
@@ -1835,14 +1895,16 @@ public class ContentController : ContentControllerBase
"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));
return;
return true;
}
// If there is some domains, verify that there's a domain for each of the published cultures
var addedDomainWarnings = false;
foreach (var culture in culturesPublished
.Where(culture => assignedDomains.Any(x =>
x.LanguageIsoCode?.Equals(culture, StringComparison.OrdinalIgnoreCase) ?? false) is false))
{
addedDomainWarnings = true;
globalNotifications.AddWarningNotification(
_localizedTextService.Localize("auditTrails", "publish"),
_localizedTextService.Localize("speechBubbles", "publishWithMissingDomain", new[] { culture }));
@@ -1852,6 +1914,8 @@ public class ContentController : ContentControllerBase
persistedContent?.Name,
culture);
}
return addedDomainWarnings;
}
/// <summary>

40
tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs
View File

@@ -56,7 +56,7 @@ public class ContentControllerTests
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
Assert.IsEmpty(notifications.Notifications);
}
@@ -82,7 +82,7 @@ public class ContentControllerTests
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
Assert.IsEmpty(notifications.Notifications);
}
@@ -111,7 +111,7 @@ public class ContentControllerTests
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
@@ -139,10 +139,38 @@ public class ContentControllerTests
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
Assert.AreEqual(3, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
}
[Test]
public void Does_Not_Warn_When_Only_Publishing_The_Default_Culture_With_No_Domains_Assigned()
{
var domainServiceMock = new Mock<IDomainService>();
domainServiceMock.Setup(x => x.GetAssignedDomains(It.IsAny<int>(), It.IsAny<bool>()))
.Returns(Enumerable.Empty<IDomain>());
var rootNode = new ContentBuilder()
.WithContentType(CreateContentType())
.WithId(1060)
.AddContentCultureInfosCollection()
.AddCultureInfos()
.WithCultureIso("da-dk")
.Done()
.AddCultureInfos()
.WithCultureIso("en-us")
.Done()
.Done()
.Build();
var culturesPublished = new[] { "en-us" };
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
Assert.IsEmpty(notifications.Notifications);
}
[Test]
public void Ancestor_Domains_Counts()
{
@@ -189,7 +217,7 @@ public class ContentControllerTests
var contentController = CreateContentController(domainServiceMock.Object);
var notifications = new SimpleNotificationModel();
contentController.AddDomainWarnings(level3Node, culturesPublished, notifications);
contentController.AddDomainWarnings(level3Node, culturesPublished, notifications, "en-us");
// We expect one error because all domains except "de-de" is registered somewhere in the ancestor path
Assert.AreEqual(1, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));
@@ -225,7 +253,7 @@ public class ContentControllerTests
var notifications = new SimpleNotificationModel();
var contentController = CreateContentController(domainServiceMock.Object);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications);
contentController.AddDomainWarnings(rootNode, culturesPublished, notifications, "en-us");
// We only get two errors, one for each culture being published, so no errors from previously published cultures.
Assert.AreEqual(2, notifications.Notifications.Count(x => x.NotificationType == NotificationStyle.Warning));