fix(board): commit a search only after its query succeeds and report read failures
A failed read on a UI path (sort change, clear, refresh after a write, opening a card, the tag picker, the tags tab) used to escape as an unhandled exception. Reads now run before the view is cleared and report Error_Read_* instead; the active filter changes only once its query has run. DataWritten and TagsChanged fire only after a successful refresh, so a broken database shows one dialog.
This commit is contained in:
1 parent
23f7806e2d
commit
f9f0e71453
9 files changed
+216
-33
No files matched your search
@@ -285,6 +285,52 @@ public class BoardViewModelTests
|
||||
Assert.AreEqual(0, fixture.Repository.GetCards().Count);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public async Task FailedSearchQueryIsReportedAndKeepsThePreviousResultAndFilter()
|
||||
{
|
||||
using var fixture = new Fixture();
|
||||
ColumnModel column = fixture.Repository.AddColumn("A", "");
|
||||
fixture.Repository.AddCard(column.Id, "alpha", "", []);
|
||||
fixture.Repository.AddCard(column.Id, "beta", "", []);
|
||||
BoardViewModel board = fixture.CreateBoard();
|
||||
board.SearchText = "alpha";
|
||||
await board.SearchCommand.ExecuteAsync(null);
|
||||
CompiledSearch? alphaFilter = board.ActiveFilter;
|
||||
|
||||
// Every read fails from now on.
|
||||
fixture.Repository.Dispose();
|
||||
board.SearchText = "beta";
|
||||
await board.SearchCommand.ExecuteAsync(null);
|
||||
|
||||
var error = (MessageDialogViewModel)fixture.Dialogs.Shown.Single();
|
||||
Assert.AreEqual(Resources.Error_Read_Title, error.Title);
|
||||
Assert.AreSame(alphaFilter, board.ActiveFilter);
|
||||
CollectionAssert.AreEqual(new[] { "alpha" }, board.Columns.Single().Cards.Select(card => card.Title).ToArray());
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public async Task ReadFailuresOnBoardCommandsAreReportedInsteadOfThrown()
|
||||
{
|
||||
using var fixture = new Fixture();
|
||||
ColumnModel column = fixture.Repository.AddColumn("A", "");
|
||||
CardModel card = fixture.Repository.AddCard(column.Id, "a", "", []);
|
||||
BoardViewModel board = fixture.CreateBoard();
|
||||
var editor = CardEditorDialogViewModel.ForExistingCard(fixture.Repository, fixture.Dialogs, card, confirmDiscard: false);
|
||||
fixture.Repository.Dispose();
|
||||
|
||||
board.SelectedSort = board.SortOptions.Single(option => (CardSortOption)option.Value == CardSortOption.Title);
|
||||
await board.ClearSearchCommand.ExecuteAsync(null);
|
||||
await board.Columns.Single().Cards.Single().EditCommand.ExecuteAsync(null);
|
||||
await board.Columns.Single().Cards.Single().MoveToCommand.ExecuteAsync(column.Id);
|
||||
await editor.AddTagCommand.ExecuteAsync(null);
|
||||
|
||||
// Sort, clear, edit, the failed move with its refresh, and the tag picker.
|
||||
string read = Resources.Error_Read_Title;
|
||||
CollectionAssert.AreEqual(
|
||||
new[] { read, read, read, Resources.Error_Operation_Title, read, read },
|
||||
fixture.Dialogs.Shown.Select(dialog => ((MessageDialogViewModel)dialog).Title).ToArray());
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void TagPickerReportsInvalidColorAndDuplicateNameWithoutClosing()
|
||||
{
|
||||
|
||||
@@ -138,7 +138,7 @@ public class TagsAndSearchViewModelTests
|
||||
board.SearchText = "alpha";
|
||||
await board.SearchCommand.ExecuteAsync(null);
|
||||
Assert.IsTrue(board.HasSearchText);
|
||||
board.ClearSearchCommand.Execute(null);
|
||||
await board.ClearSearchCommand.ExecuteAsync(null);
|
||||
|
||||
Assert.AreEqual("", board.SearchText);
|
||||
Assert.IsFalse(board.HasSearchText);
|
||||
@@ -312,6 +312,24 @@ public class TagsAndSearchViewModelTests
|
||||
Assert.AreEqual(0, fixture.Repository.GetTags().Count);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public async Task TagReadFailuresAreReportedInsteadOfThrown()
|
||||
{
|
||||
using var fixture = new Fixture();
|
||||
fixture.Repository.AddTag("bug", new RgbColor(1, 2, 3), "");
|
||||
fixture.Config.Confirmations.DeleteTag = false;
|
||||
TagsViewModel tags = fixture.CreateTags();
|
||||
|
||||
// Every read and write fails from now on.
|
||||
fixture.Repository.Dispose();
|
||||
await tags.Tags[0].DeleteCommand.ExecuteAsync(null);
|
||||
|
||||
CollectionAssert.AreEqual(
|
||||
new[] { Resources.Error_Operation_Title, Resources.Error_Read_Title },
|
||||
fixture.Dialogs.Shown.Select(dialog => ((MessageDialogViewModel)dialog).Title).ToArray());
|
||||
Assert.AreEqual("bug", tags.Tags.Single().Tag.Name);
|
||||
}
|
||||
|
||||
#endregion
|
||||
|
||||
#region Workspace wiring and export
|
||||
@@ -334,7 +352,7 @@ public class TagsAndSearchViewModelTests
|
||||
Assert.AreEqual(0, workspace.Board.Columns.Sum(column => column.CardCount));
|
||||
|
||||
// Re-tag through the repository, then a board write updates the usage count.
|
||||
workspace.Board.ClearSearchCommand.Execute(null);
|
||||
await workspace.Board.ClearSearchCommand.ExecuteAsync(null);
|
||||
TagModel ui = fixture.Repository.AddTag("ui", new RgbColor(4, 5, 6), "");
|
||||
fixture.Repository.UpdateCard(card.Id, "a", "", [ui.Id]);
|
||||
Assert.AreEqual(0, workspace.Tags.Tags.Count);
|
||||
|
||||
@@ -343,6 +343,12 @@
|
||||
<data name="Error_Operation_Message" xml:space="preserve">
|
||||
<value>The change could not be written to the workspace. The board shows the stored state.</value>
|
||||
</data>
|
||||
<data name="Error_Read_Title" xml:space="preserve">
|
||||
<value>Reading failed</value>
|
||||
</data>
|
||||
<data name="Error_Read_Message" xml:space="preserve">
|
||||
<value>The data could not be read from the workspace. What is shown may be out of date.</value>
|
||||
</data>
|
||||
<data name="TagEditor_TitleNew" xml:space="preserve">
|
||||
<value>New tag</value>
|
||||
</data>
|
||||
|
||||
@@ -343,6 +343,12 @@
|
||||
<data name="Error_Operation_Message" xml:space="preserve">
|
||||
<value>更改未能写入工作区,看板显示的是已保存的状态。</value>
|
||||
</data>
|
||||
<data name="Error_Read_Title" xml:space="preserve">
|
||||
<value>读取失败</value>
|
||||
</data>
|
||||
<data name="Error_Read_Message" xml:space="preserve">
|
||||
<value>无法从工作区读取数据,当前显示的内容可能不是最新的。</value>
|
||||
</data>
|
||||
<data name="TagEditor_TitleNew" xml:space="preserve">
|
||||
<value>新建标签</value>
|
||||
</data>
|
||||
|
||||
@@ -107,10 +107,25 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
/// <summary>
|
||||
/// Reloads the columns and the cards matching <see cref="ActiveFilter"/>.
|
||||
/// </summary>
|
||||
public void Refresh()
|
||||
/// <exception cref="Exception">Reading the workspace failed; the board is left unchanged.</exception>
|
||||
public void Refresh() => Load(ActiveFilter);
|
||||
|
||||
/// <summary>
|
||||
/// Reloads like <see cref="Refresh"/>, but reports a read failure in a
|
||||
/// message dialog instead of throwing.
|
||||
/// </summary>
|
||||
/// <returns><see langword="true"/> when the board was reloaded.</returns>
|
||||
public Task<bool> RefreshOrReportAsync() => TryLoadAsync(ActiveFilter);
|
||||
|
||||
/// <summary>
|
||||
/// Reloads the board with the given filter; the columns are only replaced
|
||||
/// once both queries have succeeded.
|
||||
/// </summary>
|
||||
/// <param name="filter">The search to apply; <see langword="null"/> shows every card.</param>
|
||||
private void Load(CompiledSearch? filter)
|
||||
{
|
||||
IReadOnlyList<ColumnModel> columns = _repository.GetColumns();
|
||||
ILookup<long, CardModel> cardsByColumn = _repository.GetCards(ActiveFilter).ToLookup(card => card.ColumnId);
|
||||
ILookup<long, CardModel> cardsByColumn = _repository.GetCards(filter).ToLookup(card => card.ColumnId);
|
||||
var sort = (CardSortOption)SelectedSort.Value;
|
||||
|
||||
ColumnModels = columns;
|
||||
@@ -121,9 +136,30 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reloads the board with the given filter and reports a read failure in a message dialog.
|
||||
/// </summary>
|
||||
/// <param name="filter">The search to apply; <see langword="null"/> shows every card.</param>
|
||||
/// <returns><see langword="true"/> when the board was reloaded.</returns>
|
||||
private async Task<bool> TryLoadAsync(CompiledSearch? filter)
|
||||
{
|
||||
try
|
||||
{
|
||||
Load(filter);
|
||||
return true;
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
await _dialogs.ShowReadFailedAsync(exception);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Parses and runs the search box text; blank text shows every card. An
|
||||
/// invalid expression only shows the error dialog.
|
||||
/// invalid expression only shows the error dialog. The expression becomes
|
||||
/// <see cref="ActiveFilter"/> only once its query has succeeded, so a failed
|
||||
/// search leaves the previous result and later refreshes untouched.
|
||||
/// </summary>
|
||||
/// <returns>A task completing after the refresh or the error dialog.</returns>
|
||||
[RelayCommand]
|
||||
@@ -143,19 +179,24 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
return;
|
||||
}
|
||||
|
||||
ActiveFilter = filter;
|
||||
Refresh();
|
||||
if (await TryLoadAsync(filter))
|
||||
{
|
||||
ActiveFilter = filter;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Empties the search box and shows every card again.
|
||||
/// </summary>
|
||||
/// <returns>A task completing after the refresh.</returns>
|
||||
[RelayCommand]
|
||||
private void ClearSearch()
|
||||
private async Task ClearSearchAsync()
|
||||
{
|
||||
SearchText = "";
|
||||
ActiveFilter = null;
|
||||
Refresh();
|
||||
if (await TryLoadAsync(null))
|
||||
{
|
||||
ActiveFilter = null;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -168,7 +209,7 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
var dialog = new ColumnEditorDialogViewModel(null, (title, description) => _repository.AddColumn(title, description));
|
||||
if (await _dialogs.ShowAsync(dialog))
|
||||
{
|
||||
RefreshAfterWrite();
|
||||
await RefreshAfterWriteAsync();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -196,7 +237,7 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
column, (title, description) => _repository.UpdateColumn(column.Id, title, description));
|
||||
if (await _dialogs.ShowAsync(dialog))
|
||||
{
|
||||
RefreshAfterWrite();
|
||||
await RefreshAfterWriteAsync();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -235,7 +276,7 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
await _dialogs.ShowAsync(editor);
|
||||
|
||||
// Refresh even after a cancel: the nested picker may have created tags.
|
||||
RefreshAfterWrite();
|
||||
await RefreshAfterWriteAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -245,17 +286,27 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
/// <returns>A task completing when the editor closes.</returns>
|
||||
internal async Task EditCardAsync(long cardId)
|
||||
{
|
||||
CardModel? card = _repository.GetCard(cardId);
|
||||
CardModel? card;
|
||||
try
|
||||
{
|
||||
card = _repository.GetCard(cardId);
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
await _dialogs.ShowReadFailedAsync(exception);
|
||||
return;
|
||||
}
|
||||
|
||||
if (card is null)
|
||||
{
|
||||
Refresh();
|
||||
await RefreshOrReportAsync();
|
||||
return;
|
||||
}
|
||||
|
||||
var editor = CardEditorDialogViewModel.ForExistingCard(
|
||||
_repository, _dialogs, card, _config.Confirmations.DiscardEdit);
|
||||
await _dialogs.ShowAsync(editor);
|
||||
RefreshAfterWrite();
|
||||
await RefreshAfterWriteAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -314,16 +365,20 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
await _dialogs.ShowMessageAsync(
|
||||
Resources.Error_Operation_Title, Resources.Error_Operation_Message, exception.Message);
|
||||
}
|
||||
RefreshAfterWrite();
|
||||
await RefreshAfterWriteAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Refreshes after a write and announces it.
|
||||
/// Refreshes after a write and announces it; a failed refresh is reported
|
||||
/// and not announced, so a broken workspace yields one read-failure dialog.
|
||||
/// </summary>
|
||||
private void RefreshAfterWrite()
|
||||
/// <returns>A task completing after the refresh.</returns>
|
||||
private async Task RefreshAfterWriteAsync()
|
||||
{
|
||||
Refresh();
|
||||
DataWritten?.Invoke(this, EventArgs.Empty);
|
||||
if (await RefreshOrReportAsync())
|
||||
{
|
||||
DataWritten?.Invoke(this, EventArgs.Empty);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -333,6 +388,8 @@ public sealed partial class BoardViewModel : ViewModelBase
|
||||
partial void OnSelectedSortChanged(OptionItem value)
|
||||
{
|
||||
_config.Sort.Card = (CardSortOption)value.Value;
|
||||
Refresh();
|
||||
|
||||
// A property setter cannot await; the refresh reports its own failures.
|
||||
_ = RefreshOrReportAsync();
|
||||
}
|
||||
}
|
||||
@@ -156,7 +156,18 @@ public sealed partial class CardEditorDialogViewModel : DialogViewModelBase
|
||||
[RelayCommand]
|
||||
private async Task AddTagAsync()
|
||||
{
|
||||
var picker = new TagPickerDialogViewModel(_repository, Tags.Select(tag => tag.Tag.Id).ToHashSet());
|
||||
TagPickerDialogViewModel picker;
|
||||
try
|
||||
{
|
||||
picker = new TagPickerDialogViewModel(_repository, Tags.Select(tag => tag.Tag.Id).ToHashSet());
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
// The picker lists the tags as it opens; a read failure must not escape the command.
|
||||
await _dialogs.ShowReadFailedAsync(exception);
|
||||
return;
|
||||
}
|
||||
|
||||
if (await _dialogs.ShowAsync(picker) && picker.PickedTag is { } picked)
|
||||
{
|
||||
Tags.Add(new TagBadgeViewModel(picked));
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
using System.Diagnostics;
|
||||
|
||||
namespace YKanBan.ViewModels.Dialogs;
|
||||
|
||||
/// <summary>
|
||||
@@ -47,4 +49,16 @@ public static class DialogServiceExtensions
|
||||
/// <returns>A task completing when the dialog closes.</returns>
|
||||
public static Task ShowMessageAsync(this IDialogService dialogs, string title, string message, string? details = null) =>
|
||||
dialogs.ShowAsync(new MessageDialogViewModel(title, message, details));
|
||||
|
||||
/// <summary>
|
||||
/// Reports that reading from the workspace failed, so the view may be stale.
|
||||
/// </summary>
|
||||
/// <param name="dialogs">The dialog service.</param>
|
||||
/// <param name="exception">The read failure; its untranslated message is shown as details.</param>
|
||||
/// <returns>A task completing when the dialog closes.</returns>
|
||||
public static Task ShowReadFailedAsync(this IDialogService dialogs, Exception exception)
|
||||
{
|
||||
Debug.WriteLine(exception);
|
||||
return dialogs.ShowMessageAsync(Resources.Error_Read_Title, Resources.Error_Read_Message, exception.Message);
|
||||
}
|
||||
}
|
||||
@@ -57,10 +57,12 @@ public sealed partial class TagsViewModel : ViewModelBase
|
||||
/// <summary>
|
||||
/// Reloads the tags with their usage counts.
|
||||
/// </summary>
|
||||
/// <exception cref="Exception">Reading the workspace failed; the list is left unchanged.</exception>
|
||||
public void Refresh()
|
||||
{
|
||||
IReadOnlyList<(TagModel Tag, long UsageCount)> tags = _repository.GetTagsWithUsage();
|
||||
Tags.Clear();
|
||||
foreach ((TagModel tag, long usage) in _repository.GetTagsWithUsage())
|
||||
foreach ((TagModel tag, long usage) in tags)
|
||||
{
|
||||
Tags.Add(new TagRowViewModel(this, tag, usage));
|
||||
}
|
||||
@@ -68,6 +70,25 @@ public sealed partial class TagsViewModel : ViewModelBase
|
||||
OnPropertyChanged(nameof(IsEmpty));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reloads like <see cref="Refresh"/>, but reports a read failure in a
|
||||
/// message dialog instead of throwing.
|
||||
/// </summary>
|
||||
/// <returns><see langword="true"/> when the list was reloaded.</returns>
|
||||
public async Task<bool> RefreshOrReportAsync()
|
||||
{
|
||||
try
|
||||
{
|
||||
Refresh();
|
||||
return true;
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
await _dialogs.ShowReadFailedAsync(exception);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Opens the new-tag dialog.
|
||||
/// </summary>
|
||||
@@ -81,7 +102,7 @@ public sealed partial class TagsViewModel : ViewModelBase
|
||||
(name, color, description) => _repository.AddTag(name, color, description));
|
||||
if (await _dialogs.ShowAsync(dialog))
|
||||
{
|
||||
AfterWrite();
|
||||
await AfterWriteAsync();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,7 +119,7 @@ public sealed partial class TagsViewModel : ViewModelBase
|
||||
(name, color, description) => _repository.UpdateTag(tag.Id, name, color, description));
|
||||
if (await _dialogs.ShowAsync(dialog))
|
||||
{
|
||||
AfterWrite();
|
||||
await AfterWriteAsync();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -130,15 +151,19 @@ public sealed partial class TagsViewModel : ViewModelBase
|
||||
await _dialogs.ShowMessageAsync(
|
||||
Resources.Error_Operation_Title, Resources.Error_Operation_Message, exception.Message);
|
||||
}
|
||||
AfterWrite();
|
||||
await AfterWriteAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reloads the list and tells the board that tags changed.
|
||||
/// Reloads the list and tells the board that tags changed; a failed reload
|
||||
/// is reported and not announced.
|
||||
/// </summary>
|
||||
private void AfterWrite()
|
||||
/// <returns>A task completing after the reload.</returns>
|
||||
private async Task AfterWriteAsync()
|
||||
{
|
||||
Refresh();
|
||||
TagsChanged?.Invoke(this, EventArgs.Empty);
|
||||
if (await RefreshOrReportAsync())
|
||||
{
|
||||
TagsChanged?.Invoke(this, EventArgs.Empty);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -45,8 +45,8 @@ public sealed partial class WorkspaceViewModel : ViewModelBase
|
||||
_dialogs = dialogs;
|
||||
Board = new BoardViewModel(session.Repository, config, dialogs);
|
||||
Tags = new TagsViewModel(session.Repository, config, dialogs);
|
||||
Board.DataWritten += (_, _) => Tags.Refresh();
|
||||
Tags.TagsChanged += (_, _) => Board.Refresh();
|
||||
Board.DataWritten += async (_, _) => await Tags.RefreshOrReportAsync();
|
||||
Tags.TagsChanged += async (_, _) => await Board.RefreshOrReportAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
Reference in new issue
Block a user