diff --git a/PlotLine.Tests/Program.cs b/PlotLine.Tests/Program.cs index 241ed4c..d9e4d07 100644 --- a/PlotLine.Tests/Program.cs +++ b/PlotLine.Tests/Program.cs @@ -2687,6 +2687,7 @@ static void LocationCandidateIdentityIsParentAwareForGenericSubLocations() Assert(view.Contains("name=\"Locations[@i].ParentSelection\"", StringComparison.Ordinal), "Location review should let the author choose or clear parent context."); Assert(view.Contains("Use proposed:", StringComparison.Ordinal), "Location review should allow accepting proposed parent context."); Assert(view.Contains("Not determined", StringComparison.Ordinal), "Location review should allow unresolved parent context."); + Assert(view.Contains("Model.LocationReview.LocationTargetOptions", StringComparison.Ordinal), "Parent selection should use the unified Location target list."); Assert(viewModels.Contains("ProposedParentLocationID", StringComparison.Ordinal) && viewModels.Contains("ProposedParentCandidateKey", StringComparison.Ordinal) && viewModels.Contains("ParentIdentityKey", StringComparison.Ordinal), "Persisted Location candidate payload should include first-class parent identity."); @@ -2760,17 +2761,20 @@ static void StoryIntelligenceLocationReviewUsesSingleIdentityDecision() Assert(service.Contains("TryAddAliasAsync(locationId.Value, candidate.DisplayName", StringComparison.Ordinal), "Same-location import should attach the detected name directly to the target LocationID."); Assert(service.Contains("UpdateExistingParentAsync(locationId.Value, parentId)", StringComparison.Ordinal), "Parent review for a known Location should update hierarchy on the same LocationID."); Assert(view.Contains("Same location as", StringComparison.Ordinal), "Review should present one clear identity decision."); - Assert(view.Contains("Same location as existing", StringComparison.Ordinal), "Review should clearly distinguish canonical Location targets."); - Assert(view.Contains("Same location as pending candidate", StringComparison.Ordinal), "Review should preserve pending candidate-to-candidate matching."); + Assert(view.Contains("", StringComparison.Ordinal), "Review should clearly distinguish canonical and pending Location targets."); Assert(view.Contains("name=\"Locations[@i].ExistingLocationID\"", StringComparison.Ordinal), "Same-location dropdown should submit LocationID values."); Assert(view.Contains("candidate.IsExistingMatch", StringComparison.Ordinal) && view.Contains(" target.LocationName, NaturalStringComparer.OrdinalIgnoreCase)", StringComparison.Ordinal), - "Pending candidate targets should be sorted in natural alphabetical order."); - Assert(candidateService.Contains(".OrderBy(location => location.LocationName, NaturalStringComparer.OrdinalIgnoreCase)", StringComparison.Ordinal), - "Canonical Location identity selectors should be sorted in natural alphabetical order by preferred name."); + Assert(model.Contains("LocationTargetOptions", StringComparison.Ordinal), "Review model should expose one unified Location target source."); + Assert(view.Contains("data-location-unified-target", StringComparison.Ordinal), "Same-location selector should use the unified target source."); + Assert(view.Contains("IsSelfTarget(target, candidate)", StringComparison.Ordinal), "Selectors should exclude the current candidate."); + Assert(view.Contains("TargetLabel(target)", StringComparison.Ordinal), "Selectors should share display naming for canonical and pending targets."); + Assert(candidateService.Contains("BuildLocationTargetOptionsAsync(projectLocations, allPendingPage.Items)", StringComparison.Ordinal), + "Persisted review GET should hydrate unified canonical and pending targets."); + Assert(candidateService.Contains("canonicalKeys.Contains(key)", StringComparison.Ordinal), "Pending targets duplicating canonical identity should be suppressed."); + Assert(candidateService.Contains(".ThenBy(option => option.DisplayName, NaturalStringComparer.OrdinalIgnoreCase)", StringComparison.Ordinal), + "Unified Location targets should be sorted in natural alphabetical order within groups."); Assert(File.ReadAllText(Path.Combine(root, "Services/NaturalStringComparer.cs")).Contains("CompareNumberRuns", StringComparison.Ordinal), "Location selector sorting should compare embedded numbers naturally."); Assert(!view.Contains("Link existing location", StringComparison.Ordinal), "Review should not expose the old separate link-existing wording."); diff --git a/PlotLine/Services/StoryIntelligenceLocationImportService.cs b/PlotLine/Services/StoryIntelligenceLocationImportService.cs index e0d2484..a4585a6 100644 --- a/PlotLine/Services/StoryIntelligenceLocationImportService.cs +++ b/PlotLine/Services/StoryIntelligenceLocationImportService.cs @@ -131,6 +131,7 @@ public sealed class StoryIntelligenceLocationImportService( var projectLocations = (await locations.ListByProjectAsync(batch.ProjectID)).ToList(); var parentOptions = ToParentOptions(projectLocations); var identityOptions = await ToIdentityOptionsAsync(projectLocations); + var targetOptions = await ToTargetOptionsAsync(projectLocations, visibleCandidates); var existingIndex = await BuildLocationIndexAsync(batch.ProjectID); return new StoryIntelligenceLocationReviewViewModel { @@ -140,6 +141,7 @@ public sealed class StoryIntelligenceLocationImportService( IsComplete = data.HasCommittedScenes && visibleCandidates.Count == 0, ParentLocationOptions = parentOptions, SameLocationOptions = identityOptions, + LocationTargetOptions = targetOptions, Candidates = visibleCandidates.Select(candidate => new StoryIntelligenceLocationReviewCandidateViewModel { Key = candidate.Key, @@ -218,6 +220,19 @@ public sealed class StoryIntelligenceLocationImportService( return null; } + if (selection.StartsWith("existing:", StringComparison.OrdinalIgnoreCase) + && int.TryParse(selection["existing:".Length..], out var existingParentId) + && existingParentId > 0) + { + return existingParentId; + } + + if (selection.StartsWith("candidate:", StringComparison.OrdinalIgnoreCase) + && candidateByKey.TryGetValue(selection["candidate:".Length..], out var selectedParentCandidate)) + { + return await MaterialiseLocationCandidateAsync(selectedParentCandidate, choices.GetValueOrDefault(selectedParentCandidate.Key), isDependency: true); + } + if (int.TryParse(selection, out var selectedParentId) && selectedParentId > 0) { return selectedParentId; @@ -1016,6 +1031,55 @@ public sealed class StoryIntelligenceLocationImportService( .ToList(); } + private async Task> ToTargetOptionsAsync( + IReadOnlyList projectLocations, + IReadOnlyList pendingCandidates) + { + var canonicalKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var options = new List(); + foreach (var location in projectLocations) + { + var aliasNames = (await locations.ListAliasesAsync(location.LocationID)) + .Select(alias => alias.Alias) + .Where(alias => !string.IsNullOrWhiteSpace(alias) && !string.Equals(alias, location.LocationName, StringComparison.OrdinalIgnoreCase)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(alias => alias, NaturalStringComparer.OrdinalIgnoreCase) + .ToList(); + canonicalKeys.Add(CanonicalLocationKey(location.LocationName)); + options.Add(new StoryIntelligenceLocationTargetOptionViewModel + { + Value = $"existing:{location.LocationID}", + LocationID = location.LocationID, + DisplayName = location.LocationName, + GroupName = "Existing locations", + AliasSummary = string.Join(", ", aliasNames.Take(4)) + }); + } + + foreach (var candidate in pendingCandidates.Where(candidate => !candidate.ExistingLocationID.HasValue)) + { + var key = CanonicalLocationKey(candidate.DisplayName); + if (string.IsNullOrWhiteSpace(key) || canonicalKeys.Contains(key)) + { + continue; + } + + options.Add(new StoryIntelligenceLocationTargetOptionViewModel + { + Value = $"candidate:{candidate.Key}", + CandidateKey = candidate.Key, + DisplayName = candidate.DisplayName, + GroupName = "Pending suggestions" + }); + } + + return options + .OrderBy(option => option.GroupName, StringComparer.Ordinal) + .ThenBy(option => option.DisplayName, NaturalStringComparer.OrdinalIgnoreCase) + .ThenBy(option => option.Value, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + private async Task BuildLocationIndexAsync(int projectId) { var index = new LocationIndex(); diff --git a/PlotLine/Services/StoryIntelligenceReviewCandidateService.cs b/PlotLine/Services/StoryIntelligenceReviewCandidateService.cs index 95ccf3d..c3e817c 100644 --- a/PlotLine/Services/StoryIntelligenceReviewCandidateService.cs +++ b/PlotLine/Services/StoryIntelligenceReviewCandidateService.cs @@ -177,6 +177,7 @@ public sealed class StoryIntelligenceReviewCandidateService( public async Task GetLocationReviewAsync(OnboardingStoryIntelligenceBatch batch, int pageNumber, int pageSize) { var page = await LoadPageAsync(batch.BookID, StoryIntelligenceReviewModules.Locations, pageNumber, pageSize); + var allPendingPage = await LoadPageAsync(batch.BookID, StoryIntelligenceReviewModules.Locations, 1, MaxPageSize); var projectLocations = (await locations.ListByProjectAsync(batch.ProjectID)).ToList(); var parentOptions = projectLocations .OrderBy(location => location.LocationPath, StringComparer.OrdinalIgnoreCase) @@ -189,6 +190,7 @@ public sealed class StoryIntelligenceReviewCandidateService( }) .ToList(); var sameLocationOptions = await BuildSameLocationOptionsAsync(projectLocations); + var targetOptions = await BuildLocationTargetOptionsAsync(projectLocations, allPendingPage.Items); return new StoryIntelligenceLocationReviewViewModel { HasCommittedScenes = true, @@ -201,6 +203,7 @@ public sealed class StoryIntelligenceReviewCandidateService( PendingCandidateCount = page.PendingCount, ParentLocationOptions = parentOptions, SameLocationOptions = sameLocationOptions, + LocationTargetOptions = targetOptions, Candidates = page.Items }; } @@ -331,6 +334,58 @@ public sealed class StoryIntelligenceReviewCandidateService( .ToList(); } + private async Task> BuildLocationTargetOptionsAsync( + IReadOnlyList projectLocations, + IReadOnlyList pendingCandidates) + { + var canonicalKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + var options = new List(); + foreach (var location in projectLocations) + { + var aliasNames = (await locations.ListAliasesAsync(location.LocationID)) + .Select(alias => alias.Alias) + .Where(alias => !string.IsNullOrWhiteSpace(alias) && !string.Equals(alias, location.LocationName, StringComparison.OrdinalIgnoreCase)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(alias => alias, NaturalStringComparer.OrdinalIgnoreCase) + .ToList(); + canonicalKeys.Add(LocationTargetKey(location.LocationName)); + options.Add(new StoryIntelligenceLocationTargetOptionViewModel + { + Value = $"existing:{location.LocationID}", + LocationID = location.LocationID, + DisplayName = location.LocationName, + GroupName = "Existing locations", + AliasSummary = string.Join(", ", aliasNames.Take(4)) + }); + } + + foreach (var candidate in pendingCandidates.Where(candidate => !candidate.ExistingLocationID.HasValue)) + { + var key = LocationTargetKey(candidate.LocationName); + if (string.IsNullOrWhiteSpace(key) || canonicalKeys.Contains(key)) + { + continue; + } + + options.Add(new StoryIntelligenceLocationTargetOptionViewModel + { + Value = $"candidate:{candidate.Key}", + CandidateKey = candidate.Key, + DisplayName = candidate.LocationName, + GroupName = "Pending suggestions" + }); + } + + return options + .OrderBy(option => option.GroupName, StringComparer.Ordinal) + .ThenBy(option => option.DisplayName, NaturalStringComparer.OrdinalIgnoreCase) + .ThenBy(option => option.Value, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + private static string LocationTargetKey(string? name) + => StoryIntelligenceEntityTextNormaliser.SafeKey(StoryIntelligenceEntityTextNormaliser.StripLeadingArticle(name ?? string.Empty)); + public async Task MarkAssetDecisionsAsync(int bookId, IEnumerable choices) { StoryIntelligenceReviewCandidateGeneration? generation = null; diff --git a/PlotLine/ViewModels/OnboardingViewModels.cs b/PlotLine/ViewModels/OnboardingViewModels.cs index f2d81be..17124c8 100644 --- a/PlotLine/ViewModels/OnboardingViewModels.cs +++ b/PlotLine/ViewModels/OnboardingViewModels.cs @@ -455,6 +455,7 @@ public sealed class StoryIntelligenceLocationReviewViewModel public bool HasNextPage => PageNumber * PageSize < PendingCandidateCount; public IReadOnlyList ParentLocationOptions { get; init; } = []; public IReadOnlyList SameLocationOptions { get; init; } = []; + public IReadOnlyList LocationTargetOptions { get; init; } = []; public IReadOnlyList Candidates { get; init; } = []; } @@ -473,6 +474,18 @@ public sealed class StoryIntelligenceLocationIdentityOptionViewModel public string? AliasSummary { get; init; } } +public sealed class StoryIntelligenceLocationTargetOptionViewModel +{ + public string Value { get; init; } = string.Empty; + public string DisplayName { get; init; } = string.Empty; + public string GroupName { get; init; } = string.Empty; + public int? LocationID { get; init; } + public string? CandidateKey { get; init; } + public string? AliasSummary { get; init; } + public bool IsCanonical => LocationID.HasValue; + public bool IsPendingCandidate => !string.IsNullOrWhiteSpace(CandidateKey); +} + public sealed class StoryIntelligenceLocationReviewCandidateViewModel { public string Key { get; init; } = string.Empty; diff --git a/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml b/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml index 42a1ecd..4108437 100644 --- a/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml +++ b/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml @@ -51,10 +51,15 @@
@for (var i = 0; i < locationCandidates.Count; i++) { - var candidate = locationCandidates[i]; - var status = candidate.IsExistingMatch ? "existing" : "new"; - var proposedParentLabel = candidate.ProposedParentLocationName ?? candidate.ParentLocationHint; - var searchText = string.Join(" ", new[] { candidate.LocationName, candidate.ExistingLocationName, candidate.Category, candidate.ParentLocationHint, candidate.ProposedParentLocationName }); + var candidate = locationCandidates[i]; + var status = candidate.IsExistingMatch ? "existing" : "new"; + var proposedParentLabel = candidate.ProposedParentLocationName ?? candidate.ParentLocationHint; + var proposedParentValue = candidate.ProposedParentLocationID.HasValue + ? $"existing:{candidate.ProposedParentLocationID.Value}" + : !string.IsNullOrWhiteSpace(candidate.ProposedParentCandidateKey) + ? $"candidate:{candidate.ProposedParentCandidateKey}" + : string.Empty; + var searchText = string.Join(" ", new[] { candidate.LocationName, candidate.ExistingLocationName, candidate.Category, candidate.ParentLocationHint, candidate.ProposedParentLocationName });
@@ -133,11 +138,11 @@
- + @if (!string.IsNullOrWhiteSpace(proposedParentLabel)) + { + + } @if (string.IsNullOrWhiteSpace(proposedParentLabel)) { @@ -146,45 +151,46 @@ { } - @foreach (var parent in Model.LocationReview.ParentLocationOptions) - { - @if (candidate.ProposedParentLocationID == parent.LocationID) - { - - } - else - { - - } - } - -
+ @foreach (var group in Model.LocationReview.LocationTargetOptions + .Where(target => !IsSelfTarget(target, candidate)) + .GroupBy(target => target.GroupName)) + { + + @foreach (var target in group) + { + if (string.Equals(target.Value, proposedParentValue, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + } + + } + +
@if (!candidate.IsExistingMatch) { - } + + + + + + } } @@ -227,38 +233,30 @@ } }; const actionFor = (card) => card.querySelector("[data-location-action]:checked")?.value || "@StoryIntelligenceLocationImportActions.CreateNew"; + const applyUnifiedTarget = (card) => { + const selector = card.querySelector("[data-location-unified-target]"); + const existing = card.querySelector("[data-location-existing-target]"); + const pending = card.querySelector("[data-location-alias-target]"); + const value = selector?.value || ""; + if (existing && selector) existing.value = value.startsWith("existing:") ? value.substring("existing:".length) : ""; + if (pending && selector) pending.value = value.startsWith("candidate:") ? value.substring("candidate:".length) : ""; + }; const isCanonicalTarget = (card) => { + applyUnifiedTarget(card); const action = actionFor(card); const pendingTarget = card.querySelector("[data-location-alias-target]")?.value || ""; return action !== "@StoryIntelligenceLocationImportActions.Ignore" && !pendingTarget; }; const updateAliasTargets = () => { - const allCards = cards(); - for (const card of allCards) { - const body = card.querySelector("[data-location-key]"); - const ownKey = body?.getAttribute("data-location-key") || ""; - const select = card.querySelector("[data-location-alias-target]"); - if (!select) continue; - - const previous = select.value; - for (const option of Array.from(select.options)) { - if (!option.value) continue; - const targetCard = allCards.find(item => item.querySelector("[data-location-key]")?.getAttribute("data-location-key") === option.value); - option.disabled = option.value === ownKey || !targetCard || !isCanonicalTarget(targetCard); - } - if (select.selectedOptions[0]?.disabled) { - select.value = ""; - } else { - select.value = previous; - } - } + cards().forEach(applyUnifiedTarget); }; const updateCard = (card) => { const action = actionFor(card); const isExistingMatch = card.getAttribute("data-location-existing-match") === "true"; const namePanel = card.querySelector("[data-location-import-name-panel]"); const samePanel = card.querySelector("[data-location-same-panel]"); + applyUnifiedTarget(card); if (namePanel) namePanel.hidden = action === "@StoryIntelligenceLocationImportActions.Ignore" || action === "@StoryIntelligenceLocationImportActions.LinkExisting"; const parentPanel = card.querySelector("[data-location-parent-panel]"); if (parentPanel) parentPanel.hidden = action === "@StoryIntelligenceLocationImportActions.Ignore" || (action === "@StoryIntelligenceLocationImportActions.LinkExisting" && !isExistingMatch); @@ -280,25 +278,9 @@ form.querySelectorAll("[data-location-action]").forEach(input => { input.addEventListener("change", () => updateCard(input.closest("[data-location-card]"))); }); - form.querySelectorAll("[data-location-existing-target]").forEach(select => { - select.addEventListener("change", () => { - if (select.value) { - const card = select.closest("[data-location-card]"); - const pending = card?.querySelector("[data-location-alias-target]"); - if (pending) pending.value = ""; - } - }); - }); - form.querySelectorAll("[data-location-alias-target]").forEach(select => { - select.addEventListener("change", () => { - if (select.value) { - const card = select.closest("[data-location-card]"); - const existing = card?.querySelector("[data-location-existing-target]"); - if (existing) existing.value = ""; - } - cards().forEach(updateCard); - }); - }); + form.querySelectorAll("[data-location-unified-target]").forEach(select => { + select.addEventListener("change", () => cards().forEach(updateCard)); + }); cards().forEach(updateCard); })(); @@ -307,4 +289,10 @@ @functions { private static string Display(string? value) => string.IsNullOrWhiteSpace(value) ? "Not detected" : value; + private static string TargetLabel(StoryIntelligenceLocationTargetOptionViewModel target) + => string.IsNullOrWhiteSpace(target.AliasSummary) ? target.DisplayName : $"{target.DisplayName} ({target.AliasSummary})"; + + private static bool IsSelfTarget(StoryIntelligenceLocationTargetOptionViewModel target, StoryIntelligenceLocationReviewCandidateViewModel candidate) + => (target.LocationID.HasValue && candidate.ExistingLocationID == target.LocationID) + || (!string.IsNullOrWhiteSpace(target.CandidateKey) && string.Equals(target.CandidateKey, candidate.Key, StringComparison.OrdinalIgnoreCase)); }