From f9f0e71453a46b714faa83dd8ab8b1d8242f2f99 Mon Sep 17 00:00:00 2001 From: doyaGu Date: Sat, 3 Oct 2026 08:58:53 -0400 Subject: [PATCH] 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. --- .../ViewModels/BoardViewModelTests.cs | 46 +++++++++ .../ViewModels/TagsAndSearchViewModelTests.cs | 22 ++++- YKanBan/Assets/Locales/Resources.resx | 6 ++ YKanBan/Assets/Locales/Resources.zh-Hans.resx | 6 ++ YKanBan/ViewModels/Board/BoardViewModel.cs | 97 +++++++++++++++---- .../Dialogs/CardEditorDialogViewModel.cs | 13 ++- YKanBan/ViewModels/Dialogs/IDialogService.cs | 14 +++ YKanBan/ViewModels/Tags/TagsViewModel.cs | 41 ++++++-- YKanBan/ViewModels/WorkspaceViewModel.cs | 4 +- 9 files changed, 216 insertions(+), 33 deletions(-) diff --git a/YKanBan.Tests/ViewModels/BoardViewModelTests.cs b/YKanBan.Tests/ViewModels/BoardViewModelTests.cs index 123bc92..9f3c80f 100644 --- a/YKanBan.Tests/ViewModels/BoardViewModelTests.cs +++ b/YKanBan.Tests/ViewModels/BoardViewModelTests.cs @@ -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() { diff --git a/YKanBan.Tests/ViewModels/TagsAndSearchViewModelTests.cs b/YKanBan.Tests/ViewModels/TagsAndSearchViewModelTests.cs index 7b56cb2..ca638bb 100644 --- a/YKanBan.Tests/ViewModels/TagsAndSearchViewModelTests.cs +++ b/YKanBan.Tests/ViewModels/TagsAndSearchViewModelTests.cs @@ -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); diff --git a/YKanBan/Assets/Locales/Resources.resx b/YKanBan/Assets/Locales/Resources.resx index edbca1b..5a09083 100644 --- a/YKanBan/Assets/Locales/Resources.resx +++ b/YKanBan/Assets/Locales/Resources.resx @@ -343,6 +343,12 @@ The change could not be written to the workspace. The board shows the stored state. + + Reading failed + + + The data could not be read from the workspace. What is shown may be out of date. + New tag diff --git a/YKanBan/Assets/Locales/Resources.zh-Hans.resx b/YKanBan/Assets/Locales/Resources.zh-Hans.resx index 78af73d..31320d4 100644 --- a/YKanBan/Assets/Locales/Resources.zh-Hans.resx +++ b/YKanBan/Assets/Locales/Resources.zh-Hans.resx @@ -343,6 +343,12 @@ 更改未能写入工作区，看板显示的是已保存的状态。 + + 读取失败 + + + 无法从工作区读取数据，当前显示的内容可能不是最新的。 + 新建标签 diff --git a/YKanBan/ViewModels/Board/BoardViewModel.cs b/YKanBan/ViewModels/Board/BoardViewModel.cs index ad5fb97..2331715 100644 --- a/YKanBan/ViewModels/Board/BoardViewModel.cs +++ b/YKanBan/ViewModels/Board/BoardViewModel.cs @@ -107,10 +107,25 @@ public sealed partial class BoardViewModel : ViewModelBase /// /// Reloads the columns and the cards matching . /// - public void Refresh() + /// Reading the workspace failed; the board is left unchanged. + public void Refresh() => Load(ActiveFilter); + + /// + /// Reloads like , but reports a read failure in a + /// message dialog instead of throwing. + /// + /// when the board was reloaded. + public Task RefreshOrReportAsync() => TryLoadAsync(ActiveFilter); + + /// + /// Reloads the board with the given filter; the columns are only replaced + /// once both queries have succeeded. + /// + /// The search to apply; shows every card. + private void Load(CompiledSearch? filter) { IReadOnlyList columns = _repository.GetColumns(); - ILookup cardsByColumn = _repository.GetCards(ActiveFilter).ToLookup(card => card.ColumnId); + ILookup 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 } } + /// + /// Reloads the board with the given filter and reports a read failure in a message dialog. + /// + /// The search to apply; shows every card. + /// when the board was reloaded. + private async Task TryLoadAsync(CompiledSearch? filter) + { + try + { + Load(filter); + return true; + } + catch (Exception exception) + { + await _dialogs.ShowReadFailedAsync(exception); + return false; + } + } + /// /// 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 + /// only once its query has succeeded, so a failed + /// search leaves the previous result and later refreshes untouched. /// /// A task completing after the refresh or the error dialog. [RelayCommand] @@ -143,19 +179,24 @@ public sealed partial class BoardViewModel : ViewModelBase return; } - ActiveFilter = filter; - Refresh(); + if (await TryLoadAsync(filter)) + { + ActiveFilter = filter; + } } /// /// Empties the search box and shows every card again. /// + /// A task completing after the refresh. [RelayCommand] - private void ClearSearch() + private async Task ClearSearchAsync() { SearchText = ""; - ActiveFilter = null; - Refresh(); + if (await TryLoadAsync(null)) + { + ActiveFilter = null; + } } /// @@ -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(); } /// @@ -245,17 +286,27 @@ public sealed partial class BoardViewModel : ViewModelBase /// A task completing when the editor closes. 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(); } /// @@ -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(); } /// - /// 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. /// - private void RefreshAfterWrite() + /// A task completing after the refresh. + private async Task RefreshAfterWriteAsync() { - Refresh(); - DataWritten?.Invoke(this, EventArgs.Empty); + if (await RefreshOrReportAsync()) + { + DataWritten?.Invoke(this, EventArgs.Empty); + } } /// @@ -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(); } } diff --git a/YKanBan/ViewModels/Dialogs/CardEditorDialogViewModel.cs b/YKanBan/ViewModels/Dialogs/CardEditorDialogViewModel.cs index 59d1b07..5c7a11e 100644 --- a/YKanBan/ViewModels/Dialogs/CardEditorDialogViewModel.cs +++ b/YKanBan/ViewModels/Dialogs/CardEditorDialogViewModel.cs @@ -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)); diff --git a/YKanBan/ViewModels/Dialogs/IDialogService.cs b/YKanBan/ViewModels/Dialogs/IDialogService.cs index c8be85e..0be1ec6 100644 --- a/YKanBan/ViewModels/Dialogs/IDialogService.cs +++ b/YKanBan/ViewModels/Dialogs/IDialogService.cs @@ -1,3 +1,5 @@ +using System.Diagnostics; + namespace YKanBan.ViewModels.Dialogs; /// @@ -47,4 +49,16 @@ public static class DialogServiceExtensions /// A task completing when the dialog closes. public static Task ShowMessageAsync(this IDialogService dialogs, string title, string message, string? details = null) => dialogs.ShowAsync(new MessageDialogViewModel(title, message, details)); + + /// + /// Reports that reading from the workspace failed, so the view may be stale. + /// + /// The dialog service. + /// The read failure; its untranslated message is shown as details. + /// A task completing when the dialog closes. + 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); + } } diff --git a/YKanBan/ViewModels/Tags/TagsViewModel.cs b/YKanBan/ViewModels/Tags/TagsViewModel.cs index dbd5abe..f962325 100644 --- a/YKanBan/ViewModels/Tags/TagsViewModel.cs +++ b/YKanBan/ViewModels/Tags/TagsViewModel.cs @@ -57,10 +57,12 @@ public sealed partial class TagsViewModel : ViewModelBase /// /// Reloads the tags with their usage counts. /// + /// Reading the workspace failed; the list is left unchanged. 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)); } + /// + /// Reloads like , but reports a read failure in a + /// message dialog instead of throwing. + /// + /// when the list was reloaded. + public async Task RefreshOrReportAsync() + { + try + { + Refresh(); + return true; + } + catch (Exception exception) + { + await _dialogs.ShowReadFailedAsync(exception); + return false; + } + } + /// /// Opens the new-tag dialog. /// @@ -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(); } /// - /// 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. /// - private void AfterWrite() + /// A task completing after the reload. + private async Task AfterWriteAsync() { - Refresh(); - TagsChanged?.Invoke(this, EventArgs.Empty); + if (await RefreshOrReportAsync()) + { + TagsChanged?.Invoke(this, EventArgs.Empty); + } } } diff --git a/YKanBan/ViewModels/WorkspaceViewModel.cs b/YKanBan/ViewModels/WorkspaceViewModel.cs index b4019f7..8343658 100644 --- a/YKanBan/ViewModels/WorkspaceViewModel.cs +++ b/YKanBan/ViewModels/WorkspaceViewModel.cs @@ -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(); } ///