diff --git a/PlotLine.Tests/Program.cs b/PlotLine.Tests/Program.cs index daf4f45..aed8e52 100644 --- a/PlotLine.Tests/Program.cs +++ b/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 GetLocationsAsync", "public async Task GetCreateAsync"); + var treeMethod = ExtractBetween(service, "private IReadOnlyList 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"); diff --git a/PlotLine/Services/CoreServices.cs b/PlotLine/Services/CoreServices.cs index fa0bad7..f38e8ed 100644 --- a/PlotLine/Services/CoreServices.cs +++ b/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 logger) : ILocationService { public async Task 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 OrderLocationsAsTree(IReadOnlyList 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(projectLocations.Count); + var visited = new HashSet(); + var stack = new HashSet(); + + 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 postedAliases, string preferredName) { var aliases = AliasInput.Clean(postedAliases.Append(preferredName));