Keep existing location matches from create flow
This commit is contained in:
parent
e37c761b46
commit
d1280583f6
@ -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("<input type=\"hidden\" name=\"Locations[@i].Action\" value=\"@StoryIntelligenceLocationImportActions.LinkExisting\"", StringComparison.Ordinal),
|
||||
"Known existing matches that reach review should be fixed to LinkExisting, not CreateNew.");
|
||||
Assert(view.Contains("Model.LocationReview.SameLocationOptions", StringComparison.Ordinal), "Selector should list one canonical row per LocationID.");
|
||||
Assert(view.Contains(".OrderBy(target => target.LocationName, NaturalStringComparer.OrdinalIgnoreCase)", StringComparison.Ordinal),
|
||||
"Pending candidate targets should be sorted in natural alphabetical order.");
|
||||
|
||||
@ -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; }
|
||||
|
||||
@ -492,8 +492,12 @@ public sealed class StoryIntelligenceLocationReviewCandidateViewModel
|
||||
public IReadOnlyList<string> 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
|
||||
|
||||
@ -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 });
|
||||
<details class="story-character-card" open data-location-card data-location-status="@status" data-location-search-text="@searchText">
|
||||
<details class="story-character-card" open data-location-card data-location-status="@status" data-location-existing-match="@candidate.IsExistingMatch.ToString().ToLowerInvariant()" data-location-search-text="@searchText">
|
||||
<summary>
|
||||
<span>
|
||||
<strong>@candidate.LocationName</strong>
|
||||
@ -97,31 +97,39 @@
|
||||
@if (candidate.IsExistingMatch)
|
||||
{
|
||||
<div class="story-review-note">
|
||||
<strong>Possible existing location</strong>
|
||||
<p>@candidate.LocationName may already be @candidate.ExistingLocationName. Choose whether to link them or create a separate location.</p>
|
||||
<strong>Existing location matched</strong>
|
||||
<p>@candidate.LocationName is linked to @candidate.ExistingLocationName. Review only the parent location if needed.</p>
|
||||
</div>
|
||||
}
|
||||
|
||||
<fieldset class="story-character-actions">
|
||||
<legend>Decision</legend>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.CreateNew" checked data-location-action />
|
||||
Create new location
|
||||
</label>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.LinkExisting" data-location-action />
|
||||
Same location as
|
||||
</label>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.Ignore" data-location-action />
|
||||
Ignore
|
||||
</label>
|
||||
</fieldset>
|
||||
@if (candidate.IsExistingMatch)
|
||||
{
|
||||
<input type="hidden" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.LinkExisting" data-location-action />
|
||||
<input type="hidden" name="Locations[@i].ExistingLocationID" value="@candidate.ExistingLocationID" />
|
||||
}
|
||||
else
|
||||
{
|
||||
<fieldset class="story-character-actions">
|
||||
<legend>Decision</legend>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.CreateNew" checked="@(candidate.DefaultAction == StoryIntelligenceLocationImportActions.CreateNew)" data-location-action />
|
||||
Create new location
|
||||
</label>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.LinkExisting" checked="@(candidate.DefaultAction == StoryIntelligenceLocationImportActions.LinkExisting)" data-location-action />
|
||||
Same location as
|
||||
</label>
|
||||
<label>
|
||||
<input type="radio" name="Locations[@i].Action" value="@StoryIntelligenceLocationImportActions.Ignore" data-location-action />
|
||||
Ignore
|
||||
</label>
|
||||
</fieldset>
|
||||
|
||||
<div data-location-import-name-panel>
|
||||
<label class="form-label" for="location-import-name-@i">Import name</label>
|
||||
<input id="location-import-name-@i" class="form-control" name="Locations[@i].ImportName" value="@candidate.ImportName" />
|
||||
</div>
|
||||
<div data-location-import-name-panel>
|
||||
<label class="form-label" for="location-import-name-@i">Import name</label>
|
||||
<input id="location-import-name-@i" class="form-control" name="Locations[@i].ImportName" value="@candidate.ImportName" />
|
||||
</div>
|
||||
}
|
||||
|
||||
<div data-location-parent-panel>
|
||||
<label class="form-label" for="location-parent-@i">Parent location</label>
|
||||
@ -152,28 +160,31 @@
|
||||
</select>
|
||||
</div>
|
||||
|
||||
<div data-location-same-panel hidden>
|
||||
<label class="form-label" for="location-same-existing-@i">Same location as existing</label>
|
||||
<select id="location-same-existing-@i" class="form-select" name="Locations[@i].ExistingLocationID" data-location-existing-target>
|
||||
<option value="">Choose existing location...</option>
|
||||
@foreach (var target in Model.LocationReview.SameLocationOptions)
|
||||
{
|
||||
var isSelected = candidate.ExistingLocationID == target.LocationID;
|
||||
<option value="@target.LocationID" selected="@isSelected">@target.LocationName@(string.IsNullOrWhiteSpace(target.AliasSummary) ? string.Empty : $" ({target.AliasSummary})")</option>
|
||||
}
|
||||
</select>
|
||||
@if (!candidate.IsExistingMatch)
|
||||
{
|
||||
<div data-location-same-panel hidden>
|
||||
<label class="form-label" for="location-same-existing-@i">Same location as existing</label>
|
||||
<select id="location-same-existing-@i" class="form-select" name="Locations[@i].ExistingLocationID" data-location-existing-target>
|
||||
<option value="">Choose existing location...</option>
|
||||
@foreach (var target in Model.LocationReview.SameLocationOptions)
|
||||
{
|
||||
var isSelected = candidate.ExistingLocationID == target.LocationID;
|
||||
<option value="@target.LocationID" selected="@isSelected">@target.LocationName@(string.IsNullOrWhiteSpace(target.AliasSummary) ? string.Empty : $" ({target.AliasSummary})")</option>
|
||||
}
|
||||
</select>
|
||||
|
||||
<label class="form-label mt-3" for="location-alias-target-@i">Same location as pending candidate</label>
|
||||
<select id="location-alias-target-@i" class="form-select" name="Locations[@i].AliasTargetKey" data-location-alias-target>
|
||||
<option value="">Choose pending location...</option>
|
||||
@foreach (var target in locationCandidates
|
||||
.Where(target => !string.Equals(target.Key, candidate.Key, StringComparison.OrdinalIgnoreCase))
|
||||
.OrderBy(target => target.LocationName, NaturalStringComparer.OrdinalIgnoreCase))
|
||||
{
|
||||
<option value="@target.Key">@target.LocationName@(target.IsExistingMatch ? $" -> {target.ExistingLocationName}" : string.Empty)</option>
|
||||
}
|
||||
</select>
|
||||
</div>
|
||||
<label class="form-label mt-3" for="location-alias-target-@i">Same location as pending candidate</label>
|
||||
<select id="location-alias-target-@i" class="form-select" name="Locations[@i].AliasTargetKey" data-location-alias-target>
|
||||
<option value="">Choose pending location...</option>
|
||||
@foreach (var target in locationCandidates
|
||||
.Where(target => !string.Equals(target.Key, candidate.Key, StringComparison.OrdinalIgnoreCase))
|
||||
.OrderBy(target => target.LocationName, NaturalStringComparer.OrdinalIgnoreCase))
|
||||
{
|
||||
<option value="@target.Key">@target.LocationName@(target.IsExistingMatch ? $" -> {target.ExistingLocationName}" : string.Empty)</option>
|
||||
}
|
||||
</select>
|
||||
</div>
|
||||
}
|
||||
</div>
|
||||
</details>
|
||||
}
|
||||
@ -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();
|
||||
};
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user