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

(cherry picked from commit 2d9cfc8)

Author: kjac

src/Umbraco.Core/EmbeddedResources/Lang/da.xml

@@ -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>

src/Umbraco.Core/EmbeddedResources/Lang/en.xml

@@ -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>

src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml

@@ -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>

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

@@ -1134,8 +1134,12 @@ private bool EnsureUniqueName(string? name, IContent? content, string modelName)
             {
                 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 @@ private bool EnsureUniqueName(string? name, IContent? content, string modelName)
                 }
 
                 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 @@ private void AddPublishStatusNotifications(
         }
     }
 
+    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 @@ private PublishResult PublishInternal(ContentItemSave contentItem, string? defau
         }
     }
 
-    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 @@ private void AddDomainWarnings(IEnumerable<PublishResult> publishResults, string
     /// <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 @@ internal void AddDomainWarnings(IContent? persistedContent, string[]? culturesPu
         // 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 @@ internal void AddDomainWarnings(IContent? persistedContent, string[]? culturesPu
                 "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 @@ internal void AddDomainWarnings(IContent? persistedContent, string[]? culturesPu
                 persistedContent?.Name,
                 culture);
         }
+
+        return addedDomainWarnings;
     }
 
     /// <summary>

tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs

@@ -56,7 +56,7 @@ public void Root_Node_With_Domains_Causes_No_Warning()
         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 void Node_With_Single_Published_Culture_Causes_No_Warning()
         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 void Root_Node_Without_Domains_Causes_SingleWarning()
         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 void One_Warning_Per_Culture_Being_Published()
         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 void Ancestor_Domains_Counts()
         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 void Only_Warns_About_Cultures_Being_Published()
         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));