NavigationNode.Url and NavigationNode.Text may be null

axuno · Feb 11, 2022 · fa67db2 · fa67db2
1 parent bae4e36
commit fa67db2
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 24 deletions.
diff --git a/League/Components/MainNavigationComponentModelExtensions.cs b/League/Components/MainNavigationComponentModelExtensions.cs
@@ -17,7 +17,7 @@ public static class MainNavigationComponentModelExtensions
         /// <returns></returns>
         public static bool HasVisibleChildNodes(this MainNavigationComponentModel.NavigationNode node)
         {
-            return node.ChildNodes.Any(childNode => childNode.IsVisible);
+            return node.ChildNodes.Any(childNode => childNode.ShouldShow());
         }
 
         /// <summary>
@@ -38,7 +38,7 @@ public static bool HasVisibleChildNodes(this MainNavigationComponentModel.Naviga
                     if (found != null) return found;
                 }
                 // Second check if this node contains the url
-                if (node.Url.Equals(httpContext.Request.Path.Value)) return node;
+                if (node.Url != null && node.Url.Equals(httpContext.Request.Path.Value)) return node;
             }
 
             return null;
@@ -96,6 +96,16 @@ public static bool IsActiveNode(this MainNavigationComponentModel model, Microso
             return node != null && currentNode != null && node.Key.Equals(currentNode.Key);
         }
 
+        /// <summary>
+        /// Checks, whether the node is visible and the node url is not null.
+        /// </summary>
+        /// <param name="node"></param>
+        /// <returns><see langref="true"/> if the node is visible and the node url is not null.</returns>
+        public static bool ShouldShow(this MainNavigationComponentModel.NavigationNode? node)
+        {
+            return node != null && (node.IsVisible && node.Url != null || node.ChildNodes.Any(n => n.ShouldShow()));
+        }
+
         /// <summary>
         /// Checks, whether the given <see cref="MainNavigationComponentModel.NavigationNode"/> contains an active child node.
         /// </summary>

diff --git a/League/Views/Shared/Components/MainNavigation/Default.cshtml b/League/Views/Shared/Components/MainNavigation/Default.cshtml
@@ -11,17 +11,17 @@
             <li class="ml-md-auto"></li>
         }
 
-        if (!node.IsVisible) { continue; }
+        if (!node.ShouldShow()) { continue; }
         if (!node.HasVisibleChildNodes())
         {
             <li class="@Model.GetClass(Context, node, "nav-item", "active", true)">
-                <a class="nav-link" href="@Url.Content(node.Url)">@Html.Raw(node.GetIcon())@node.Text</a>
+                <a class="nav-link" href="@Url.Content(node.Url!)">@Html.Raw(node.GetIcon())@node.Text</a>
             </li>
         }
         else
         {
             <li class="@Model.GetClass(Context, node, "nav-item dropdown", "active", true)">
-                <a class="nav-link dropdown-toggle" data-toggle="dropdown" href="@Url.Content(node.Url)" role="button">
+                <a class="nav-link dropdown-toggle" data-toggle="dropdown" href="@Url.Content(node.Url!)" role="button">
                     @Html.Raw(node.GetIcon())@node.Text
                 </a>
                 <partial name="@League.Views.ViewNames.Shared.NavigationNodeChildDropdownPartial" model="(Model, node)" />

diff --git a/League/Views/Shared/NavigationNodeChildDropdownPartial.cshtml b/League/Views/Shared/NavigationNodeChildDropdownPartial.cshtml
@@ -5,7 +5,7 @@
     <div class="dropdown-menu shadow@(ViewData["dropdown-menu-right"])" style="margin-top: -0.3rem">
         @foreach (var childNode in Model.Node.ChildNodes)
         {
-            if (!childNode.IsVisible)
+            if (!childNode.ShouldShow())
             {
                 continue;
             }
@@ -17,11 +17,11 @@
             }
             if (childNode.HasVisibleChildNodes())
             {
-                <a href="@Url.Content(childNode.Url)" class="@Model.MainNav.GetClass(Context, childNode, "dropdown-item", "active", true)">@childNode.Text</a>
+                <a href="@Url.Content(childNode.Url!)" class="@Model.MainNav.GetClass(Context, childNode, "dropdown-item", "active", true)">@childNode.Text</a>
             }
             else
             {
-                <a href="@Url.Content(childNode.Url)" class="@Model.MainNav.GetClass(Context, childNode, "dropdown-item", "active", true)">@childNode.Text</a>
+                <a href="@Url.Content(childNode.Url!)" class="@Model.MainNav.GetClass(Context, childNode, "dropdown-item", "active", true)">@childNode.Text</a>
                 <partial name="@League.Views.ViewNames.Shared.NavigationNodeChildDropdownPartial" model="(Model.MainNav, childNode)" /> @*recursion*@
             }
         }

diff --git a/LeagueDemo/Startup.cs b/LeagueDemo/Startup.cs
@@ -1,18 +1,13 @@
+using System;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
-using Microsoft.AspNetCore.HttpsPolicy;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 using NLog.Extensions.Logging;
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Threading.Tasks;
 using League.Components;
 
-
 namespace LeagueDemo
 {
     public class Startup
@@ -48,6 +43,7 @@ public Startup(IConfiguration configuration, IWebHostEnvironment webHostEnvironm
             {
                 builder.AddNLog();                
             });
+            Logger = LoggerFactory.CreateLogger<Startup>();
 
             League = new League.Startup(Configuration, WebHostEnvironment);
         }
@@ -83,12 +79,9 @@ public void Configure(IApplicationBuilder app, IHostEnvironment env, ILoggerFact
 
             app.UseAuthorization();
 
-            app.UseEndpoints(endpoints =>
-            {
-                endpoints.MapControllerRoute(
-                    name: "default",
-                    pattern: "{controller=Home}/{action=Index}/{id?}");
-            });
+            // DO NOT add the default endpoints! => Url.Action(...) could produce Urls violating attribute routes
+            // and e.g. showing wrong navigation links:
+            // app.UseEndpoints(endpoints => { endpoints.MapControllerRoute(name: "default", pattern: "{controller=Home}/{action=Index}/{id?}"); });
         }
     }
 }
diff --git a/LeagueDemo/ViewComponents/DemoMainNavigationNodeBuilder.cs b/LeagueDemo/ViewComponents/DemoMainNavigationNodeBuilder.cs
@@ -1,12 +1,8 @@
 using System;
-using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using League.Components;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
-using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.Localization;
 using Microsoft.Extensions.Logging;
 using TournamentManager.MultiTenancy;