Unify location review target selectors

This commit is contained in:
Nick Beckley 2026-09-01 20:50:09 +00:00
parent d1280583f6
commit 7990cb244e
5 changed files with 213 additions and 89 deletions

View File

@ -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("<optgroup label=\"@group.Key\">", 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("<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.");
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.");

View File

@ -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<IReadOnlyList<StoryIntelligenceLocationTargetOptionViewModel>> ToTargetOptionsAsync(
IReadOnlyList<LocationItem> projectLocations,
IReadOnlyList<LocationCandidate> pendingCandidates)
{
var canonicalKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var options = new List<StoryIntelligenceLocationTargetOptionViewModel>();
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<LocationIndex> BuildLocationIndexAsync(int projectId)
{
var index = new LocationIndex();

View File

@ -177,6 +177,7 @@ public sealed class StoryIntelligenceReviewCandidateService(
public async Task<StoryIntelligenceLocationReviewViewModel> GetLocationReviewAsync(OnboardingStoryIntelligenceBatch batch, int pageNumber, int pageSize)
{
var page = await LoadPageAsync<StoryIntelligenceLocationReviewCandidateViewModel>(batch.BookID, StoryIntelligenceReviewModules.Locations, pageNumber, pageSize);
var allPendingPage = await LoadPageAsync<StoryIntelligenceLocationReviewCandidateViewModel>(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<IReadOnlyList<StoryIntelligenceLocationTargetOptionViewModel>> BuildLocationTargetOptionsAsync(
IReadOnlyList<LocationItem> projectLocations,
IReadOnlyList<StoryIntelligenceLocationReviewCandidateViewModel> pendingCandidates)
{
var canonicalKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var options = new List<StoryIntelligenceLocationTargetOptionViewModel>();
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<int> MarkAssetDecisionsAsync(int bookId, IEnumerable<StoryIntelligenceAssetImportChoiceForm> choices)
{
StoryIntelligenceReviewCandidateGeneration? generation = null;

View File

@ -455,6 +455,7 @@ public sealed class StoryIntelligenceLocationReviewViewModel
public bool HasNextPage => PageNumber * PageSize < PendingCandidateCount;
public IReadOnlyList<StoryIntelligenceLocationParentOptionViewModel> ParentLocationOptions { get; init; } = [];
public IReadOnlyList<StoryIntelligenceLocationIdentityOptionViewModel> SameLocationOptions { get; init; } = [];
public IReadOnlyList<StoryIntelligenceLocationTargetOptionViewModel> LocationTargetOptions { get; init; } = [];
public IReadOnlyList<StoryIntelligenceLocationReviewCandidateViewModel> 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;

View File

@ -51,10 +51,15 @@
<div class="story-character-card-grid">
@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 });
<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>
@ -133,11 +138,11 @@
<div data-location-parent-panel>
<label class="form-label" for="location-parent-@i">Parent location</label>
<select id="location-parent-@i" class="form-select" name="Locations[@i].ParentSelection">
@if (!string.IsNullOrWhiteSpace(proposedParentLabel))
{
<option value="__proposed__" selected>Use proposed: @proposedParentLabel</option>
}
<select id="location-parent-@i" class="form-select" name="Locations[@i].ParentSelection">
@if (!string.IsNullOrWhiteSpace(proposedParentLabel))
{
<option value="@proposedParentValue" selected>Use proposed: @proposedParentLabel</option>
}
@if (string.IsNullOrWhiteSpace(proposedParentLabel))
{
<option value="__none__" selected>Not determined</option>
@ -146,45 +151,46 @@
{
<option value="__none__">Not determined</option>
}
@foreach (var parent in Model.LocationReview.ParentLocationOptions)
{
@if (candidate.ProposedParentLocationID == parent.LocationID)
{
<option value="@parent.LocationID" selected>@parent.DisplayName</option>
}
else
{
<option value="@parent.LocationID">@parent.DisplayName</option>
}
}
</select>
</div>
@foreach (var group in Model.LocationReview.LocationTargetOptions
.Where(target => !IsSelfTarget(target, candidate))
.GroupBy(target => target.GroupName))
{
<optgroup label="@group.Key">
@foreach (var target in group)
{
if (string.Equals(target.Value, proposedParentValue, StringComparison.OrdinalIgnoreCase))
{
continue;
}
<option value="@target.Value">@TargetLabel(target)</option>
}
</optgroup>
}
</select>
</div>
@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" for="location-same-target-@i">Same location as</label>
<select id="location-same-target-@i" class="form-select" data-location-unified-target>
<option value="">Choose location...</option>
@foreach (var group in Model.LocationReview.LocationTargetOptions
.Where(target => !IsSelfTarget(target, candidate))
.GroupBy(target => target.GroupName))
{
<optgroup label="@group.Key">
@foreach (var target in group)
{
<option value="@target.Value">@TargetLabel(target)</option>
}
</optgroup>
}
</select>
<input type="hidden" name="Locations[@i].ExistingLocationID" data-location-existing-target />
<input type="hidden" name="Locations[@i].AliasTargetKey" data-location-alias-target />
</div>
}
</div>
</details>
}
@ -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);
})();
</script>
@ -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));
}