perf(ui): show list settings modal before its seven-round-trip load finishes
OpenListSettingsAsync/CreateListAsync used to await LoadAsync (name/agent config/list_config x2/ticket settings/ticket projects, all serial) before showing the window -- an unreachable ticket system alone could add a 10s HTTP timeout to every open. LoadAsync now runs via BeginLoad (fire-and-forget, mirrors UsageMonitorModalViewModel), the modal shows immediately and fields fill in behind it. GetListConfigAsync no longer runs twice per open -- AgentConfigEditorViewModel exposes the row it already fetched (LastLoadedListConfig) instead of the caller re-fetching it. Ticket-project loading is split into its own fire-and-forget step so a slow/unreachable ticket system can't block the rest of the fields, and its tri-state Save logic now also treats "never finished loading" like "not available" -- leaving the stored TicketProjectId link untouched instead of overwriting it with 0. Save/Delete are gated (CanExecute) on the core load being done, so a save can't land before the fields it writes are populated. Not measured with OperationTiming against a live worker in this session (no running app instance available) -- verified instead via 4 new unit tests that assert GetListConfigAsync fires exactly once and that a hung/failing ticket-projects call neither blocks LoadAsync's other fields nor corrupts the saved TicketProjectId. Before/after OperationTiming numbers and a visual pass are still open follow-ups.
This commit is contained in:
@@ -56,6 +56,20 @@ public sealed partial class ListSettingsModalViewModel : ViewModelBase
|
||||
[ObservableProperty] private TicketProjectOption? _selectedTicketProject;
|
||||
public ObservableCollection<TicketProjectOption> TicketProjects { get; } = new();
|
||||
|
||||
// Gates Save/Delete while the core fields (name/agent/verify command) are still loading, so a
|
||||
// save can't land before they're populated and clobber the list_config row with blanks. Ticket
|
||||
// projects load separately (see LoadTicketProjectsAsync) and are NOT covered by this — Save's
|
||||
// own tri-state handles that case instead.
|
||||
[ObservableProperty]
|
||||
[NotifyCanExecuteChangedFor(nameof(SaveCommand))]
|
||||
[NotifyCanExecuteChangedFor(nameof(DeleteCommand))]
|
||||
private bool _isLoading;
|
||||
|
||||
// True once GetTicketProjectsAsync has completed successfully for the current load — Save
|
||||
// only writes an explicit TicketProjectId when this is true, so a slow/unreachable ticket
|
||||
// system leaves the stored link untouched instead of wiping it to "none".
|
||||
private bool _ticketProjectsLoaded;
|
||||
|
||||
public ObservableCollection<string> CommitTypeOptions { get; } = new(CommitTypeRegistry.Types);
|
||||
|
||||
public const string DefaultIcon = "Folder";
|
||||
@@ -91,6 +105,19 @@ public sealed partial class ListSettingsModalViewModel : ViewModelBase
|
||||
Agent = new AgentConfigEditorViewModel(worker, AgentConfigScope.List);
|
||||
}
|
||||
|
||||
// Fires the load without blocking the caller, so the modal window can be shown right away
|
||||
// instead of appearing only after seven serial hub round-trips (one of them a 10s-timeout
|
||||
// ticket-system HTTP call) have finished — same pattern as UsageMonitorModalViewModel.BeginLoad.
|
||||
public void BeginLoad(
|
||||
string listId,
|
||||
string name,
|
||||
string? workingDir,
|
||||
string defaultCommitType,
|
||||
bool isManual = false,
|
||||
bool findingsTracked = false,
|
||||
string? icon = null)
|
||||
=> _ = LoadAsync(listId, name, workingDir, defaultCommitType, isManual, findingsTracked, icon);
|
||||
|
||||
public async Task LoadAsync(
|
||||
string listId,
|
||||
string name,
|
||||
@@ -101,43 +128,69 @@ public sealed partial class ListSettingsModalViewModel : ViewModelBase
|
||||
string? icon = null,
|
||||
CancellationToken ct = default)
|
||||
{
|
||||
ListId = listId;
|
||||
Name = name;
|
||||
IsManual = isManual;
|
||||
FindingsTracked = findingsTracked;
|
||||
// An unknown/legacy key would leave the picker with no selection and silently reset the
|
||||
// icon on the next save — fall back to the default instead.
|
||||
Icon = IconOptions.Contains(icon) ? icon! : DefaultIcon;
|
||||
WorkingDir = workingDir ?? "";
|
||||
DefaultCommitType = string.IsNullOrWhiteSpace(defaultCommitType) ? CommitTypeRegistry.DefaultType : defaultCommitType;
|
||||
|
||||
await Agent.LoadForListAsync(listId, ct);
|
||||
var cfg = await _worker.GetListConfigAsync(listId);
|
||||
VerifyCommand = cfg?.VerifyCommand ?? "";
|
||||
PowerUseAllowed = cfg?.PowerUseAllowed ?? false;
|
||||
|
||||
var ticketSettings = TicketSettingsTabViewModel.FeatureEnabled
|
||||
? await _worker.GetTicketSettingsAsync()
|
||||
: null;
|
||||
TicketsAvailable = !string.IsNullOrWhiteSpace(ticketSettings?.ApiBaseUrl);
|
||||
TicketProjects.Clear();
|
||||
if (TicketsAvailable)
|
||||
ListConfigDto? cfg;
|
||||
IsLoading = true;
|
||||
try
|
||||
{
|
||||
ListId = listId;
|
||||
Name = name;
|
||||
IsManual = isManual;
|
||||
FindingsTracked = findingsTracked;
|
||||
// An unknown/legacy key would leave the picker with no selection and silently reset the
|
||||
// icon on the next save — fall back to the default instead.
|
||||
Icon = IconOptions.Contains(icon) ? icon! : DefaultIcon;
|
||||
WorkingDir = workingDir ?? "";
|
||||
DefaultCommitType = string.IsNullOrWhiteSpace(defaultCommitType) ? CommitTypeRegistry.DefaultType : defaultCommitType;
|
||||
|
||||
await Agent.LoadForListAsync(listId, ct);
|
||||
cfg = Agent.LastLoadedListConfig;
|
||||
VerifyCommand = cfg?.VerifyCommand ?? "";
|
||||
PowerUseAllowed = cfg?.PowerUseAllowed ?? false;
|
||||
}
|
||||
finally { IsLoading = false; }
|
||||
|
||||
// Deliberately NOT awaited: an unreachable ticket system (10s HTTP timeout) must not hold
|
||||
// up the rest of the modal. The dropdown just stays empty until this finishes.
|
||||
_ = LoadTicketProjectsAsync(cfg?.TicketProjectId);
|
||||
}
|
||||
|
||||
private async Task LoadTicketProjectsAsync(int? existingTicketProjectId)
|
||||
{
|
||||
_ticketProjectsLoaded = false;
|
||||
TicketProjects.Clear();
|
||||
try
|
||||
{
|
||||
var ticketSettings = TicketSettingsTabViewModel.FeatureEnabled
|
||||
? await _worker.GetTicketSettingsAsync()
|
||||
: null;
|
||||
TicketsAvailable = !string.IsNullOrWhiteSpace(ticketSettings?.ApiBaseUrl);
|
||||
if (!TicketsAvailable)
|
||||
{
|
||||
SelectedTicketProject = null;
|
||||
return;
|
||||
}
|
||||
|
||||
TicketProjects.Add(new TicketProjectOption(0, Loc.T("vm.listSettings.ticketProjectNone")));
|
||||
foreach (var p in await _worker.GetTicketProjectsAsync())
|
||||
TicketProjects.Add(new TicketProjectOption(p.Id, $"{p.DepartmentName} / {p.Title}"));
|
||||
|
||||
SelectedTicketProject = cfg?.TicketProjectId is int id
|
||||
SelectedTicketProject = existingTicketProjectId is int id
|
||||
? TicketProjects.FirstOrDefault(t => t.Id == id) ?? TicketProjects[0]
|
||||
: TicketProjects[0];
|
||||
_ticketProjectsLoaded = true;
|
||||
}
|
||||
else
|
||||
catch (Exception)
|
||||
{
|
||||
SelectedTicketProject = null;
|
||||
// Ticket system unreachable/slow — leave the dropdown as-is. Save's tri-state (below)
|
||||
// skips writing TicketProjectId when _ticketProjectsLoaded is false, so the stored
|
||||
// link survives instead of being wiped to "none".
|
||||
}
|
||||
}
|
||||
|
||||
[RelayCommand]
|
||||
private bool CanSave() => !IsLoading;
|
||||
private bool CanDelete() => !IsLoading;
|
||||
|
||||
[RelayCommand(CanExecute = nameof(CanSave))]
|
||||
private async Task SaveAsync()
|
||||
{
|
||||
await _worker.UpdateListAsync(new UpdateListDto(
|
||||
@@ -149,10 +202,11 @@ public sealed partial class ListSettingsModalViewModel : ViewModelBase
|
||||
FindingsTracked,
|
||||
Icon));
|
||||
|
||||
// Tri-state: dropdown never shown (no base URL) -> null, leave the stored link alone.
|
||||
// Dropdown shown -> always an explicit value, 0 for "— none —" to actively clear it.
|
||||
// Never null here, or an explicit unlink would silently fail to reach the worker.
|
||||
var ticketProjectId = TicketsAvailable ? (SelectedTicketProject?.Id ?? 0) : (int?)null;
|
||||
// Tri-state: dropdown never shown (no base URL), or ticket projects failed/never loaded
|
||||
// (e.g. an unreachable ticket system) -> null, leave the stored link alone. Dropdown shown
|
||||
// and loaded -> always an explicit value, 0 for "— none —" to actively clear it. Never
|
||||
// null in that case, or an explicit unlink would silently fail to reach the worker.
|
||||
var ticketProjectId = TicketsAvailable && _ticketProjectsLoaded ? (SelectedTicketProject?.Id ?? 0) : (int?)null;
|
||||
|
||||
// Always explicit (never null) — this modal owns the checkbox, so unchecking it has to reach
|
||||
// the worker as a real false rather than a "leave it alone".
|
||||
@@ -165,7 +219,7 @@ public sealed partial class ListSettingsModalViewModel : ViewModelBase
|
||||
CloseAction?.Invoke();
|
||||
}
|
||||
|
||||
[RelayCommand]
|
||||
[RelayCommand(CanExecute = nameof(CanDelete))]
|
||||
private async Task DeleteAsync()
|
||||
{
|
||||
var displayName = string.IsNullOrWhiteSpace(Name) ? Loc.T("vm.listSettings.untitled") : Name;
|
||||
|
||||
Reference in New Issue
Block a user