From d1280583f6c23c32edfdb24ef88493f0afc9a8eb Mon Sep 17 00:00:00 2001 From: Nick Beckley Date: Tue, 1 Sep 2026 20:13:55 +0000 Subject: [PATCH] Keep existing location matches from create flow --- PlotLine.Tests/Program.cs | 11 ++ .../StoryIntelligenceLocationImportService.cs | 74 +++++++++++-- PlotLine/ViewModels/OnboardingViewModels.cs | 4 + .../StoryIntelligenceLocations.cshtml | 100 ++++++++++-------- 4 files changed, 138 insertions(+), 51 deletions(-) diff --git a/PlotLine.Tests/Program.cs b/PlotLine.Tests/Program.cs index 7e362c9..241ed4c 100644 --- a/PlotLine.Tests/Program.cs +++ b/PlotLine.Tests/Program.cs @@ -2732,10 +2732,17 @@ static void StoryIntelligenceAutoResolvesExactKnownLocationsBeforeReview() "Exact known locations should be recognised separately from reviewable new or ambiguous locations."); Assert(service.Contains("existingIndex.Find(candidate.DisplayName)?.LocationID == candidate.ExistingLocationID.Value", StringComparison.Ordinal), "Preferred-name and alias exact matches should auto-resolve only when the canonical lookup returns the same LocationID."); + Assert(service.Contains("!HasParentReviewIssue(candidate, existingIndex)", StringComparison.Ordinal), + "Exact known locations should only remain visible when parent hierarchy still needs review."); + Assert(service.Contains("candidate.ProposedParentLocationID.HasValue", StringComparison.Ordinal) + && service.Contains("existing.ParentLocationID != candidate.ProposedParentLocationID", StringComparison.Ordinal), + "Existing Locations with same parent should hide, while missing/conflicting parents should remain parent-review candidates."); Assert(service.Contains("LinkSceneLocationsAsync(candidate, locationId)", StringComparison.Ordinal), "Auto-resolution should preserve scene/location associations."); Assert(service.Contains("return null;", StringComparison.Ordinal) && service.Contains("if (linkExisting && !locationId.HasValue)", StringComparison.Ordinal), "A same-location decision without a canonical or pending target must not create a duplicate Location."); + Assert(service.Contains("Identity beats hierarchy: known Locations must not be duplicated because parent context changed.", StringComparison.Ordinal), + "Known existing LocationIDs should never be imported through the CreateNew path."); } static void StoryIntelligenceLocationReviewUsesSingleIdentityDecision() @@ -2751,10 +2758,14 @@ static void StoryIntelligenceLocationReviewUsesSingleIdentityDecision() Assert(candidateService.Contains("DecisionCanonicalID = choice.ExistingLocationID", StringComparison.Ordinal), "Persisted review decisions should record canonical LocationID targets."); Assert(service.Contains("selectedExisting?.LocationID", StringComparison.Ordinal), "Import should resolve same-location targets by canonical LocationID."); 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("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."); diff --git a/PlotLine/Services/StoryIntelligenceLocationImportService.cs b/PlotLine/Services/StoryIntelligenceLocationImportService.cs index 24e69d4..e0d2484 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 existingIndex = await BuildLocationIndexAsync(batch.ProjectID); return new StoryIntelligenceLocationReviewViewModel { HasCommittedScenes = data.HasCommittedScenes, @@ -157,6 +158,9 @@ public sealed class StoryIntelligenceLocationImportService( ExampleContext = candidate.Description, ExistingLocationID = candidate.ExistingLocationID, ExistingLocationName = candidate.ExistingLocationName, + ExistingParentLocationID = candidate.ExistingParentLocationID, + ExistingParentLocationName = candidate.ExistingParentLocationName, + HasParentReviewIssue = HasParentReviewIssue(candidate, existingIndex), EvidenceSummaries = candidate.Appearances .GroupBy(appearance => appearance.SceneID) .OrderBy(group => group.Min(appearance => appearance.SceneNumber)) @@ -253,6 +257,16 @@ public sealed class StoryIntelligenceLocationImportService( try { var action = choice?.Action ?? StoryIntelligenceLocationImportActions.CreateNew; + if (candidate.ExistingLocationID.HasValue + && string.Equals(action, StoryIntelligenceLocationImportActions.CreateNew, StringComparison.OrdinalIgnoreCase)) + { + // Identity beats hierarchy: known Locations must not be duplicated because parent context changed. + action = StoryIntelligenceLocationImportActions.LinkExisting; + if (choice is not null) + { + choice.Action = action; + } + } if (string.Equals(action, StoryIntelligenceLocationImportActions.Ignore, StringComparison.OrdinalIgnoreCase) || string.Equals(action, StoryIntelligenceLocationImportActions.Alias, StringComparison.OrdinalIgnoreCase)) { @@ -312,6 +326,8 @@ public sealed class StoryIntelligenceLocationImportService( aliasesAdded++; } } + + await UpdateExistingParentAsync(locationId.Value, parentId); } AddResolvedName(resolvedNames, candidate.DisplayName, locationId.Value); @@ -557,7 +573,8 @@ public sealed class StoryIntelligenceLocationImportService( return; } - var identity = ResolveLocationIdentity(cleanName, null, existingIndex); + var initialIdentity = ResolveLocationIdentity(cleanName, null, existingIndex); + var identity = initialIdentity; if (identity is null || !IsLocationNameCandidate(identity.DisplayName)) { return; @@ -565,7 +582,9 @@ public sealed class StoryIntelligenceLocationImportService( var parentContext = ResolveParentContext(cleanName, identity.DisplayName, genericRoomType, parentLocationHint, sceneParentContext, existingIndex); EnsureParentCandidate(groups, existingIndex, importedScene, parsed, parentContext); - identity = ResolveLocationIdentity(parentContext.LeafName, parentContext.ProposedParentLocationID, existingIndex); + identity = identity.ExistingMatch is not null + ? identity + : ResolveLocationIdentity(parentContext.LeafName, parentContext.ProposedParentLocationID, existingIndex); if (identity is null || !IsLocationNameCandidate(identity.DisplayName)) { return; @@ -574,9 +593,7 @@ public sealed class StoryIntelligenceLocationImportService( var key = ParentAwareCandidateKey(parentContext.ParentIdentityKey, identity.Key); if (!groups.TryGetValue(key, out var candidate)) { - var match = parentContext.ParentIdentityKey is not null && !parentContext.ProposedParentLocationID.HasValue - ? null - : identity.ExistingMatch; + var match = identity.ExistingMatch; candidate = new LocationCandidate(key, identity.DisplayName, match?.LocationID, match?.LocationName) { LeafName = identity.DisplayName, @@ -586,7 +603,9 @@ public sealed class StoryIntelligenceLocationImportService( ParentLocationHint = parentContext.DisplayHint, ProposedParentLocationID = parentContext.ProposedParentLocationID, ProposedParentLocationName = parentContext.ProposedParentLocationName, - ProposedParentCandidateKey = parentContext.ProposedParentCandidateKey + ProposedParentCandidateKey = parentContext.ProposedParentCandidateKey, + ExistingParentLocationID = match?.ParentLocationID, + ExistingParentLocationName = match?.ParentLocationName }; groups[key] = candidate; } @@ -1015,7 +1034,46 @@ public sealed class StoryIntelligenceLocationImportService( private static bool IsAutoResolvableKnownIdentity(LocationCandidate candidate, LocationIndex existingIndex) => candidate.ExistingLocationID.HasValue - && existingIndex.Find(candidate.DisplayName)?.LocationID == candidate.ExistingLocationID.Value; + && existingIndex.Find(candidate.DisplayName)?.LocationID == candidate.ExistingLocationID.Value + && !HasParentReviewIssue(candidate, existingIndex); + + private static bool HasParentReviewIssue(LocationCandidate candidate, LocationIndex existingIndex) + { + if (!candidate.ExistingLocationID.HasValue) + { + return false; + } + + var existing = existingIndex.Find(candidate.ExistingLocationID); + if (existing is null) + { + return false; + } + + if (candidate.ProposedParentLocationID.HasValue) + { + return existing.ParentLocationID != candidate.ProposedParentLocationID; + } + + return !string.IsNullOrWhiteSpace(candidate.ProposedParentCandidateKey) && existing.ParentLocationID is null; + } + + private async Task UpdateExistingParentAsync(int locationId, int? parentId) + { + if (!parentId.HasValue || parentId.Value == locationId) + { + return; + } + + var location = await locations.GetAsync(locationId); + if (location is null || location.ParentLocationID == parentId) + { + return; + } + + location.ParentLocationID = parentId; + await locations.SaveAsync(location); + } private static bool IsPendingSameLocationChoice(StoryIntelligenceLocationImportChoiceForm choice) => string.Equals(choice.Action, StoryIntelligenceLocationImportActions.Alias, StringComparison.OrdinalIgnoreCase) @@ -1734,6 +1792,8 @@ public sealed class StoryIntelligenceLocationImportService( public string LeafIdentityKey { get; set; } = key; public int? ExistingLocationID { get; } = existingLocationId; public string? ExistingLocationName { get; } = existingLocationName; + public int? ExistingParentLocationID { get; set; } + public string? ExistingParentLocationName { get; set; } public string Category { get; set; } = "Ambiguous location"; public string? ParentIdentityKey { get; set; } public string? ParentLocationHint { get; set; } diff --git a/PlotLine/ViewModels/OnboardingViewModels.cs b/PlotLine/ViewModels/OnboardingViewModels.cs index 0291d67..f2d81be 100644 --- a/PlotLine/ViewModels/OnboardingViewModels.cs +++ b/PlotLine/ViewModels/OnboardingViewModels.cs @@ -492,8 +492,12 @@ public sealed class StoryIntelligenceLocationReviewCandidateViewModel public IReadOnlyList EvidenceSummaries { get; init; } = []; public int? ExistingLocationID { get; init; } public string? ExistingLocationName { get; init; } + public int? ExistingParentLocationID { get; init; } + public string? ExistingParentLocationName { get; init; } + public bool HasParentReviewIssue { get; init; } public bool IsExistingMatch => ExistingLocationID.HasValue; public string ActionLabel => IsExistingMatch ? "Link existing" : "Create"; + public string DefaultAction => IsExistingMatch ? StoryIntelligenceLocationImportActions.LinkExisting : StoryIntelligenceLocationImportActions.CreateNew; } public sealed class StoryIntelligenceLocationImportForm diff --git a/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml b/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml index 874de70..42a1ecd 100644 --- a/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml +++ b/PlotLine/Views/Onboarding/StoryIntelligenceLocations.cshtml @@ -55,7 +55,7 @@ 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 }); -
+
@candidate.LocationName @@ -97,31 +97,39 @@ @if (candidate.IsExistingMatch) {
- Possible existing location -

@candidate.LocationName may already be @candidate.ExistingLocationName. Choose whether to link them or create a separate location.

+ Existing location matched +

@candidate.LocationName is linked to @candidate.ExistingLocationName. Review only the parent location if needed.

} -
- Decision - - - -
+ @if (candidate.IsExistingMatch) + { + + + } + else + { +
+ Decision + + + +
-
- - -
+
+ + +
+ }
@@ -152,28 +160,31 @@
- + }
} @@ -245,11 +256,12 @@ }; 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]"); 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"; + if (parentPanel) parentPanel.hidden = action === "@StoryIntelligenceLocationImportActions.Ignore" || (action === "@StoryIntelligenceLocationImportActions.LinkExisting" && !isExistingMatch); if (samePanel) samePanel.hidden = action !== "@StoryIntelligenceLocationImportActions.LinkExisting"; updateAliasTargets(); };