Sort locations as hierarchy

PlotLine.Tests/Program.cs

@@ -49,6 +49,7 @@ var tests = new (string Name, Action Test)[]
     ("Location canonical keys merge punctuation and safe plurals", LocationCanonicalKeysMergePunctuationAndSafePlurals),
     ("Location candidate consolidation handles generic review variants", LocationCandidateConsolidationHandlesGenericReviewVariants),
     ("Location candidate identity is parent-aware for generic sub-locations", LocationCandidateIdentityIsParentAwareForGenericSubLocations),
+    ("Locations page uses hierarchical sort order", LocationsPageUsesHierarchicalSortOrder),
     ("Location review UX supports large candidate sets", LocationReviewUxSupportsLargeCandidateSets),
     ("Story Intelligence auto resolves exact known locations before review", StoryIntelligenceAutoResolvesExactKnownLocationsBeforeReview),
     ("Story Intelligence location review uses single identity decision", StoryIntelligenceLocationReviewUsesSingleIdentityDecision),
@@ -2746,6 +2747,28 @@ static void LocationCandidateIdentityIsParentAwareForGenericSubLocations()
     Assert(persisted.Contains("ParentLocationOptions = parentOptions", StringComparison.Ordinal), "Persisted Location GET should hydrate parent options without rebuilding Scene JSON.");
 }
 
+static void LocationsPageUsesHierarchicalSortOrder()
+{
+    var root = Path.Combine(AppContext.BaseDirectory, "../../../../PlotLine");
+    var service = File.ReadAllText(Path.Combine(root, "Services/CoreServices.cs"));
+    var view = File.ReadAllText(Path.Combine(root, "Views/Locations/Index.cshtml"));
+
+    var listMethod = ExtractBetween(service, "public async Task<LocationListViewModel?> GetLocationsAsync", "public async Task<LocationEditViewModel?> GetCreateAsync");
+    var treeMethod = ExtractBetween(service, "private IReadOnlyList<LocationItem> OrderLocationsAsTree", "private async Task SyncLocationAliasesAsync");
+
+    Assert(listMethod.Contains("OrderLocationsAsTree(projectLocations)", StringComparison.Ordinal), "Locations index should use parent-aware tree ordering instead of repository row order.");
+    Assert(!listMethod.Contains("\n            Locations = await locations.ListByProjectAsync(projectId)", StringComparison.Ordinal), "Locations index should not flatten directly from globally ordered repository rows.");
+    Assert(treeMethod.Contains("ParentLocationID is null", StringComparison.Ordinal), "Tree ordering should begin with root locations.");
+    Assert(treeMethod.Contains("GroupBy(location => location.ParentLocationID", StringComparison.Ordinal), "Tree ordering should group direct children by parent.");
+    Assert(treeMethod.Contains("OrderBy(location => location.LocationName, StringComparer.OrdinalIgnoreCase)", StringComparison.Ordinal), "Tree ordering should sort siblings case-insensitively by location name.");
+    Assert(treeMethod.Contains("AddLocationAndChildren(child, depth + 1)", StringComparison.Ordinal), "Tree ordering should recurse through descendants.");
+    Assert(treeMethod.Contains("visited.Add(location.LocationID)", StringComparison.Ordinal), "Tree ordering should render each location once.");
+    Assert(treeMethod.Contains("stack.Add(location.LocationID)", StringComparison.Ordinal)
+        && treeMethod.Contains("logger.LogWarning(\"Location hierarchy cycle", StringComparison.Ordinal), "Tree ordering should guard and log circular parent data.");
+    Assert(view.Contains("@foreach (var location in Model.Locations)", StringComparison.Ordinal), "Locations view should preserve service-provided hierarchical order.");
+    Assert(view.Contains("style=\"--location-depth:@location.Depth\"", StringComparison.Ordinal), "Locations view should keep existing indentation.");
+}
+
 static void LocationReviewUxSupportsLargeCandidateSets()
 {
     var root = Path.Combine(AppContext.BaseDirectory, "../../../../PlotLine");

PlotLine/Services/CoreServices.cs

@@ -4,6 +4,7 @@ using System.Text;
 using System.Text.Json;
 using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.Data.SqlClient;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using PlotLine.Data;
 using PlotLine.Models;
@@ -7926,16 +7927,18 @@ public sealed class LocationService(
     IProjectRepository projects,
     ILocationRepository locations,
     IProjectActivityService activity,
-    ICurrentUserService currentUser) : ILocationService
+    ICurrentUserService currentUser,
+    ILogger<LocationService> logger) : ILocationService
 {
     public async Task<LocationListViewModel?> GetLocationsAsync(int projectId)
     {
         var project = await projects.GetAsync(projectId);
         var lookupData = await locations.GetLookupsAsync();
+        var projectLocations = await locations.ListByProjectAsync(projectId);
         return project is null ? null : new LocationListViewModel
         {
             Project = project,
-            Locations = await locations.ListByProjectAsync(projectId),
+            Locations = OrderLocationsAsTree(projectLocations),
             RelationshipTypeOptions = ChapterService.ToSelectList(lookupData.RelationshipTypes, x => x.LocationRelationshipTypeID, x => x.TypeName)
         };
     }
@@ -8140,6 +8143,65 @@ public sealed class LocationService(
         return model;
     }
 
+    private IReadOnlyList<LocationItem> OrderLocationsAsTree(IReadOnlyList<LocationItem> projectLocations)
+    {
+        var byId = projectLocations.ToDictionary(location => location.LocationID);
+        var childrenByParent = projectLocations
+            .Where(location => location.ParentLocationID.HasValue && byId.ContainsKey(location.ParentLocationID.Value))
+            .GroupBy(location => location.ParentLocationID!.Value)
+            .ToDictionary(
+                group => group.Key,
+                group => group.OrderBy(location => location.LocationName, StringComparer.OrdinalIgnoreCase).ThenBy(location => location.LocationID).ToList());
+
+        var ordered = new List<LocationItem>(projectLocations.Count);
+        var visited = new HashSet<int>();
+        var stack = new HashSet<int>();
+
+        foreach (var root in projectLocations
+            .Where(location => location.ParentLocationID is null || !byId.ContainsKey(location.ParentLocationID.Value))
+            .OrderBy(location => location.LocationName, StringComparer.OrdinalIgnoreCase)
+            .ThenBy(location => location.LocationID))
+        {
+            AddLocationAndChildren(root, 0);
+        }
+
+        foreach (var remaining in projectLocations
+            .Where(location => !visited.Contains(location.LocationID))
+            .OrderBy(location => location.LocationName, StringComparer.OrdinalIgnoreCase)
+            .ThenBy(location => location.LocationID))
+        {
+            logger.LogWarning("Location hierarchy cycle or disconnected parent chain encountered for LocationID {LocationID} in ProjectID {ProjectID}.", remaining.LocationID, remaining.ProjectID);
+            AddLocationAndChildren(remaining, Math.Max(0, remaining.Depth));
+        }
+
+        return ordered;
+
+        void AddLocationAndChildren(LocationItem location, int depth)
+        {
+            if (!stack.Add(location.LocationID))
+            {
+                logger.LogWarning("Location hierarchy cycle encountered at LocationID {LocationID} in ProjectID {ProjectID}.", location.LocationID, location.ProjectID);
+                return;
+            }
+
+            if (visited.Add(location.LocationID))
+            {
+                location.Depth = depth;
+                ordered.Add(location);
+
+                if (childrenByParent.TryGetValue(location.LocationID, out var children))
+                {
+                    foreach (var child in children)
+                    {
+                        AddLocationAndChildren(child, depth + 1);
+                    }
+                }
+            }
+
+            stack.Remove(location.LocationID);
+        }
+    }
+
     private async Task SyncLocationAliasesAsync(int locationId, IEnumerable<string> postedAliases, string preferredName)
     {
         var aliases = AliasInput.Clean(postedAliases.Append(preferredName));