From e2637425145d452ac2156850f0a511485175cd61 Mon Sep 17 00:00:00 2001 From: Dmitry Ratner <6830384+dmitrat@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:59:28 +0300 Subject: [PATCH] An import that reports fifteen failures and can name ten Phase 7 of the fix plan: import and export. Studio 1000 green, 989 before. **Finding 27 was half built, which is why it was reported as absent.** The CSV path has kept every rejection since WS-36, and the window has a button that writes them beside the source with the line, the reason and the text. The JSON path filled the display list and recorded nothing - so an import of a JSON file really did report "15 failed", name ten, and have nothing to save. Both paths record now, and the rule counts the recording sites against the display sites rather than trusting either. **Step 1 said "0 columns" for every file** because the count asked ColumnMappings, which is built on step 3. It reads the parse now - and the rows themselves are drawn, which is what a preview is for: a wrong delimiter is obvious in the rows and invisible in a count. **The result of an import was painted across the line saying what the file held**, because the preview banner and the error banner were both Grid.Row="5". Two borders in one cell are drawn over each other at full opacity, in every state - the same shape as the conflict message in phase 4. The cell holds a column now. **The collision policy moved to the step it is about.** It sat on Columns beside the mapping, so that step held two unrelated decisions while Destination held one control. **The export's first frame said "0 / 0 rows" at "0.0 %"**, because the panel opens when the export starts and the rows are fetched after that. Zero out of zero is not a small number; it is no number, and the numbers wait for the total now. **Two dialogs called Export Data are two dialogs.** Export table and Export query results, in the title bar and in the Tools menu - only one of them carries the three scopes with their counts, and somebody looking in Tools for them would not have found them. And the output path can be typed as well as browsed for. The census of hard-coded values went red again, at 69 against 68: the import preview's MaxHeight. The ledger records it with its reason - a box round a block of content, so the wizard does not grow with the file. Co-Authored-By: Claude Opus 5 (1M context) --- .../Themes/DesignTokenTests.cs | 7 +- ...TheExportDialogSaysWhichExportItIsTests.cs | 165 +++++++++++++++ .../TheImportWizardSaysWhatItSawTests.cs | 199 ++++++++++++++++++ .../Resources/Strings.en.json | 4 +- .../Resources/Strings.ru.json | 4 +- .../ViewModels/ExportViewModel.cs | 24 +++ .../ViewModels/ImportViewModel.cs | 19 +- .../Views/Dialogs/ExportDialog.axaml | 10 +- .../Views/Dialogs/ImportDialog.axaml | 33 ++- 9 files changed, 452 insertions(+), 13 deletions(-) create mode 100644 Tools/OutWit.Database.Studio.Tests/ViewModels/TheExportDialogSaysWhichExportItIsTests.cs create mode 100644 Tools/OutWit.Database.Studio.Tests/ViewModels/TheImportWizardSaysWhatItSawTests.cs diff --git a/Tools/OutWit.Database.Studio.Tests/Themes/DesignTokenTests.cs b/Tools/OutWit.Database.Studio.Tests/Themes/DesignTokenTests.cs index bea7805..f514e80 100644 --- a/Tools/OutWit.Database.Studio.Tests/Themes/DesignTokenTests.cs +++ b/Tools/OutWit.Database.Studio.Tests/Themes/DesignTokenTests.cs @@ -513,10 +513,13 @@ public void TheCensusOfHardCodedValuesIsWhatItWasMeasuredToBeTest() // to open at a fixed 480 for content needing about 900 and could not be resized, so // the footer was painted over. A window's minimum size is a window size, which is // the remainder this ledger exists to allow. - Assert.That(height, Is.EqualTo(68), "heights written by hand"); + // 69, and the newest is the import preview's MaxHeight: the wizard shows the rows it + // parsed, and without a bound it would grow with the file. A box round a block of + // content is the other thing this ledger allows, beside window sizes. + Assert.That(height, Is.EqualTo(69), "heights written by hand"); Assert.That(HeightSiteKinds(), Is.EqualTo(new[] { - "Border:23", "Button:1", "DataGrid:2", "Ellipse:1", "Grid:1", "GridSplitter:1", + "Border:24", "Button:1", "DataGrid:2", "Ellipse:1", "Grid:1", "GridSplitter:1", "ListBox:1", "PathIcon:2", "ProgressBar:4", "ScrollViewer:3", "TextBox:1", "Window:28", })); diff --git a/Tools/OutWit.Database.Studio.Tests/ViewModels/TheExportDialogSaysWhichExportItIsTests.cs b/Tools/OutWit.Database.Studio.Tests/ViewModels/TheExportDialogSaysWhichExportItIsTests.cs new file mode 100644 index 0000000..3cd9e72 --- /dev/null +++ b/Tools/OutWit.Database.Studio.Tests/ViewModels/TheExportDialogSaysWhichExportItIsTests.cs @@ -0,0 +1,165 @@ +using System.Text.Json; +using System.Text.RegularExpressions; +using OutWit.Database.Studio.Tests.Helpers; +using OutWit.Database.Studio.ViewModels; + +namespace OutWit.Database.Studio.Tests.ViewModels; + +/// +/// Two exports, two names - and a progress panel that waits until it has a number. +/// +/// +/// +/// Finding 5: Tools ▸ Export… and Export Results… are not the same dialog - the +/// first exports a TABLE chosen from a list, the second exports the query result and carries the three +/// scopes with their row counts - and both were called Export Data in the title bar, under a +/// menu entry called Export…. Somebody looking in Tools for the scopes will not find them. +/// +/// +/// Finding 32: the first tick of a large export read Exporting... 0 / 0 rows at +/// 0.0 %, because the panel opens when the export starts and the rows are fetched after that. +/// Zero out of zero is not a small number, it is no number - and it is the frame anybody +/// photographing an export in progress is most likely to catch. +/// +/// +/// Finding 4: the output path could only be set with Browse…, which opens the system +/// dialog on the user's own folders - awkward while a screen is being recorded, and slower than +/// typing a path somebody already knows. +/// +/// +[TestFixture] +public class TheExportDialogSaysWhichExportItIsTests +{ + #region Finding 5 + + [TestCase("en")] + [TestCase("ru")] + public void TheTwoExportsHaveTwoNamesTest(string language) + { + using var catalogue = JsonDocument.Parse(Source($"Resources/Strings.{language}.json")); + + var table = catalogue.RootElement.GetProperty("Dialog.Export.Title.Table").GetString(); + var results = catalogue.RootElement.GetProperty("Dialog.Export.Title.Results").GetString(); + var menu = catalogue.RootElement.GetProperty("Menu.Export").GetString(); + + Assert.Multiple(() => + { + Assert.That(table, Is.Not.EqualTo(results), + "the two windows do not share a name, because they do not do the same thing"); + + Assert.That(menu, Is.Not.Null.And.Not.Empty); + + Assert.That(menu!.Replace("_", string.Empty).TrimEnd('.', '…'), + Does.Contain(table!.Split(' ')[^1]), + "and the menu entry says which of the two it opens"); + }); + } + + [Test] + public async Task TheWindowNamesItselfAfterWhatItIsExportingTest() + { + await using var studio = await StudioFixture.CreateAsync(); + + using var export = new ExportViewModel(studio.App); + + var forTable = export.WindowTitle; + + // The same window, handed a query result: this is the call the result grid makes. + using var data = new System.Data.DataTable(); + data.Columns.Add("Id"); + + export.SetDataSource(data, "SELECT 1", null, 0); + + Assert.That(export.WindowTitle, Is.Not.EqualTo(forTable), + "the same window used for the other job says so in its title"); + } + + #endregion + + #region Finding 32 + + [Test] + public async Task NoNumberIsShownBeforeThereIsOneTest() + { + await using var studio = await StudioFixture.CreateAsync(); + + using var export = new ExportViewModel(studio.App); + + Assert.That(export.KnowsHowManyRows, Is.False, + "nothing has been counted yet, so there is nothing to say"); + + var markup = Source("Views/Dialogs/ExportDialog.axaml"); + + var progress = Regex.Match(markup, @"]*ExportProgressText[\s\S]*?/>"); + + Assert.That(progress.Success, Is.True); + + Assert.That(progress.Value, Does.Contain("KnowsHowManyRows"), + "and the panel's numbers wait for the total rather than opening at 0 / 0"); + } + + #endregion + + #region Finding 4 + + [Test] + public void TheOutputPathCanBeTypedTest() + { + var markup = Source("Views/Dialogs/ExportDialog.axaml"); + + var box = Regex.Match(markup, @"]*Text=""\{Binding OutputPath\}""[\s\S]*?/>"); + + Assert.That(box.Success, Is.True, "the dialog has a box for the path"); + + Assert.That(box.Value, Does.Not.Contain("IsReadOnly=\"True\""), + "a path can be typed as well as browsed for"); + } + + #endregion + + #region Finding 28 + + [Test] + public void TheCollisionPolicyIsOnTheStepAboutTheTargetTest() + { + var markup = Source("Views/Dialogs/ImportDialog.axaml"); + + var block = Regex.Match(markup, + @"]*Grid\.Row=""3""[\s\S]*?ImportConflictSkip"); + + Assert.That(block.Success, Is.True, "the collision policy is in the wizard"); + + Assert.That(block.Value, Does.Contain("IsDestination"), + "and on the step named after the thing it is about - it sat on «Columns», which left that " + + "step holding two unrelated decisions"); + } + + #endregion + + #region Tools + + private static string Source(string relative) + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + + while (directory != null) + { + var candidate = Path.Combine(directory.FullName, "Tools", "OutWit.Database.Studio"); + + if (Directory.Exists(Path.Combine(candidate, "Views"))) + { + var path = Path.Combine(candidate, relative.Replace('/', Path.DirectorySeparatorChar)); + + Assert.That(File.Exists(path), Is.True, $"{relative} must be where this fixture says"); + + return File.ReadAllText(path); + } + + directory = directory.Parent; + } + + throw new AssertionException("the Studio project was not found from " + AppContext.BaseDirectory); + } + + #endregion +} diff --git a/Tools/OutWit.Database.Studio.Tests/ViewModels/TheImportWizardSaysWhatItSawTests.cs b/Tools/OutWit.Database.Studio.Tests/ViewModels/TheImportWizardSaysWhatItSawTests.cs new file mode 100644 index 0000000..71ac9d1 --- /dev/null +++ b/Tools/OutWit.Database.Studio.Tests/ViewModels/TheImportWizardSaysWhatItSawTests.cs @@ -0,0 +1,199 @@ +using System.Text.RegularExpressions; +using OutWit.Database.Studio.Tests.Helpers; +using OutWit.Database.Studio.ViewModels; + +namespace OutWit.Database.Studio.Tests.ViewModels; + +/// +/// The import wizard reports the file it read, and keeps every row the database refused. +/// +/// +/// +/// Finding 25: step 1 said File contains approximately 13 rows 0 columns for a CSV +/// with four, and Refresh Preview changed nothing. The count was read from +/// ColumnMappings, which is built on step 3 - so on step 1 it was asking a list that does not +/// exist yet. Catching a wrong delimiter before anything is written is what a preview is for, and +/// "0 columns" is exactly what a wrong delimiter would look like if the number meant anything. +/// +/// +/// Finding 27 was half built, which is why it was reported as absent. The CSV path has kept +/// every rejection since WS-36 and the window has a button that writes them to a file beside the +/// source. The JSON path threw away everything past the tenth - it filled the display list and +/// recorded nothing - so an import of a JSON file really did report "15 failed", name ten, and have +/// nothing to save. Both paths record now. +/// +/// +/// Finding 26: the result was painted across the line it replaces because two borders share +/// one grid cell. A cell holds one thing. +/// +/// +[TestFixture] +public class TheImportWizardSaysWhatItSawTests +{ + #region Fields + + private StudioFixture m_studio = null!; + private ImportViewModel m_import = null!; + private string m_csv = null!; + + #endregion + + #region Setup + + [SetUp] + public async Task SetUp() + { + m_studio = await StudioFixture.CreateAsync(); + + m_csv = Path.Combine(m_studio.Root, "people.csv"); + + await File.WriteAllTextAsync(m_csv, + "Id,Name,Email,City" + Environment.NewLine + + "1,Ada,ada@example.com,London" + Environment.NewLine + + "2,Grace,grace@example.com,New York" + Environment.NewLine); + + m_import = new ImportViewModel(m_studio.App); + + await m_import.InitializeAsync(); + } + + [TearDown] + public async Task TearDown() + { + m_import.Dispose(); + + await m_studio.DisposeAsync(); + } + + #endregion + + #region Finding 25 + + [Test] + public async Task TheFirstStepCountsTheColumnsItReadTest() + { + m_import.InputPath = m_csv; + + await StudioFixture.PressAsync(m_import.PreviewCommand); + + Assert.Multiple(() => + { + Assert.That(m_import.PreviewData, Is.Not.Null, "the file was parsed"); + + Assert.That(m_import.PreviewData!.Columns, Has.Count.EqualTo(4)); + + Assert.That(m_import.PreviewColumnsSummary, Does.Contain("4"), + "the line says four - it used to ask the mapping list, which is built two steps later"); + }); + } + + [Test] + public async Task TheRowsItReadCanBeLookedAtTest() + { + m_import.InputPath = m_csv; + + await StudioFixture.PressAsync(m_import.PreviewCommand); + + Assert.That(m_import.PreviewData!.Rows, Has.Count.EqualTo(2), + "the parsed rows are there to be shown - a preview that shows nothing catches no delimiter"); + + Assert.That(Markup("Views/Dialogs/ImportDialog.axaml"), Does.Contain("ImportPreviewRowsGrid"), + "and the window draws them"); + } + + #endregion + + #region Finding 26 + + [Test] + public void TheResultDoesNotLandOnTopOfThePreviewTest() + { + // Comments stripped first: this fixture is about the markup, and the comment explaining + // the fix mentions the cell it is about. + var markup = Regex.Replace(Markup("Views/Dialogs/ImportDialog.axaml"), + @"", string.Empty); + + Assert.That(Regex.Matches(markup, @"Grid\.Row=""5"""), Has.Count.EqualTo(1), + "one thing in the cell: two borders in the same one are drawn over each other, which is " + + "how the result came to be painted across the line it replaces"); + } + + #endregion + + #region Finding 27 + + /// + /// Both import paths keep every rejection, so the button that writes them out has them all. + /// + /// + /// Asserted on the source, because reaching the JSON path here would mean importing into a table + /// whose rows the engine refuses one at a time - a scenario that measures the engine rather than + /// this wizard. What is checked is that neither path records ONLY the ten it shows. + /// + [Test] + public void NeitherImportPathThrowsAwayARejectionTest() + { + var source = Source("ViewModels/ImportViewModel.cs"); + + var displayed = Regex.Matches(source, @"ImportErrors\.Add\("); + var recorded = Regex.Matches(source, @"Rejected\.Add\("); + + Assert.Multiple(() => + { + Assert.That(displayed, Has.Count.EqualTo(2), "CSV and JSON, one display list each"); + + Assert.That(recorded, Has.Count.EqualTo(2), + "and each of them records the rejection as well - the JSON path used to fill the " + + "display list and keep nothing"); + }); + } + + [Test] + public void TheButtonThatWritesThemOutFollowsTheRejectionsTest() + { + var markup = Markup("Views/Dialogs/ImportDialog.axaml"); + + var button = Regex.Match(markup, @"]*ImportWriteReport[\s\S]*?/>"); + + Assert.That(button.Success, Is.True, "the window offers to write them out"); + + Assert.Multiple(() => + { + Assert.That(button.Value, Does.Contain("WriteReportCommand")); + + Assert.That(button.Value, Does.Contain("IsVisible=\"{Binding Rejected.Count}\""), + "and it is offered exactly when there is something to write"); + }); + } + + #endregion + + #region Tools + + private static string Markup(string relative) => Source(relative); + + private static string Source(string relative) + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + + while (directory != null) + { + var candidate = Path.Combine(directory.FullName, "Tools", "OutWit.Database.Studio"); + + if (Directory.Exists(Path.Combine(candidate, "Views"))) + { + var path = Path.Combine(candidate, relative.Replace('/', Path.DirectorySeparatorChar)); + + Assert.That(File.Exists(path), Is.True, $"{relative} must be where this fixture says"); + + return File.ReadAllText(path); + } + + directory = directory.Parent; + } + + throw new AssertionException("the Studio project was not found from " + AppContext.BaseDirectory); + } + + #endregion +} diff --git a/Tools/OutWit.Database.Studio/Resources/Strings.en.json b/Tools/OutWit.Database.Studio/Resources/Strings.en.json index 99aa6a8..83498e4 100644 --- a/Tools/OutWit.Database.Studio/Resources/Strings.en.json +++ b/Tools/OutWit.Database.Studio/Resources/Strings.en.json @@ -18,7 +18,7 @@ "Menu.SaveQueryAs": "Save Query _As…", "Menu.DumpDatabase": "_Dump Database…", "Menu.Import": "_Import…", - "Menu.Export": "_Export…", + "Menu.Export": "_Export table…", "Menu.Settings": "_Settings…", "Menu.Exit": "E_xit", "Menu.Copy": "_Copy", @@ -349,6 +349,8 @@ "Dialog.CreateIndex.CreateName": "Create index", "Dialog.CreateIndex.Creating": "Creating index...", "Dialog.Export.Title": "Export Data", + "Dialog.Export.Title.Table": "Export table", + "Dialog.Export.Title.Results": "Export query results", "Dialog.Export.Source": "Source", "Dialog.Export.QueryResult": "Query Result: {0} ({1} rows)", "Dialog.Export.SelectTable": "Select table to export...", diff --git a/Tools/OutWit.Database.Studio/Resources/Strings.ru.json b/Tools/OutWit.Database.Studio/Resources/Strings.ru.json index a49af8a..1ad341f 100644 --- a/Tools/OutWit.Database.Studio/Resources/Strings.ru.json +++ b/Tools/OutWit.Database.Studio/Resources/Strings.ru.json @@ -18,7 +18,7 @@ "Menu.SaveQueryAs": "Сохранить запрос _как…", "Menu.DumpDatabase": "_Дамп базы…", "Menu.Import": "_Импорт…", - "Menu.Export": "_Экспорт…", + "Menu.Export": "_Экспорт таблицы…", "Menu.Settings": "_Настройки…", "Menu.Exit": "В_ыход", "Menu.Copy": "_Копировать", @@ -355,6 +355,8 @@ "Dialog.CreateIndex.CreateName": "Создать индекс", "Dialog.CreateIndex.Creating": "Создание индекса…", "Dialog.Export.Title": "Экспорт данных", + "Dialog.Export.Title.Table": "Экспорт таблицы", + "Dialog.Export.Title.Results": "Экспорт результата запроса", "Dialog.Export.Source": "Источник", "Dialog.Export.QueryResult": "Результат запроса: {0} ({1} строк)", "Dialog.Export.SelectTable": "Выберите таблицу для экспорта…", diff --git a/Tools/OutWit.Database.Studio/ViewModels/ExportViewModel.cs b/Tools/OutWit.Database.Studio/ViewModels/ExportViewModel.cs index 31b3fe7..0325736 100644 --- a/Tools/OutWit.Database.Studio/ViewModels/ExportViewModel.cs +++ b/Tools/OutWit.Database.Studio/ViewModels/ExportViewModel.cs @@ -141,6 +141,8 @@ private void RefreshLanguage() OnPropertyChanged(nameof(EverythingLabel)); OnPropertyChanged(nameof(QuerySourceSummary)); OnPropertyChanged(nameof(ProgressText)); + OnPropertyChanged(nameof(KnowsHowManyRows)); + OnPropertyChanged(nameof(WindowTitle)); } #endregion @@ -717,6 +719,28 @@ private void OnPropertyChanged(object? sender, PropertyChangedEventArgs e) /// How far the export has got. Shown over the window while it runs. public string ProgressText => Localization.Format("Dialog.Export.Progress", RowsExported, TotalRows); + /// + /// Whether the number of rows is known yet. + /// + /// + /// The panel opens the moment the export starts, which is BEFORE the rows have been + /// fetched - so its first frame read "Exporting... 0 / 0 rows" at "0.0 %", and that is the + /// frame anybody photographing an export in progress is most likely to catch. Zero out of + /// zero is not a small number; it is no number at all, and the numbers wait for it. + /// + public bool KnowsHowManyRows => TotalRows > 0; + + /// + /// Which of the two exports this window is: a TABLE, chosen from the list, or the RESULT of + /// a query, with its three scopes and their counts. + /// + /// + /// Both were called Export Data, in the menu and in the title bar, and they are not + /// the same window: somebody looking in Tools for the scopes will not find them there. + /// + public string WindowTitle => + Localization[IsQueryResult ? "Dialog.Export.Title.Results" : "Dialog.Export.Title.Table"]; + /// /// Whether "Selection" can be chosen at all. An empty selection offered as a scope is a button /// that writes an empty file. diff --git a/Tools/OutWit.Database.Studio/ViewModels/ImportViewModel.cs b/Tools/OutWit.Database.Studio/ViewModels/ImportViewModel.cs index 93081f6..9190d9c 100644 --- a/Tools/OutWit.Database.Studio/ViewModels/ImportViewModel.cs +++ b/Tools/OutWit.Database.Studio/ViewModels/ImportViewModel.cs @@ -718,7 +718,12 @@ private async Task ImportJsonAsync(IDatabaseSession session, string targetColumn catch (Exception ex) { RowsFailed++; - + + // Every rejected row, not only the ten the window shows - the same as the CSV + // path beside it, which has done this since WS-36. Without it an import that + // reports "15 failed" can name ten of them and has thrown the rest away. + Rejected.Add(new ImportRejection(rowNumber, ex.Message, string.Empty)); + if (ImportErrors.Count < MAX_ERRORS_TO_SHOW) { ImportErrors.Add($"Row {rowNumber}: {ex.Message}"); @@ -1041,7 +1046,17 @@ private void OnPropertyChanged(object? sender, PropertyChangedEventArgs e) public string PreviewRowsSummary => Localization.Format("Dialog.Import.Preview.Approx", Localization.Plural("Count.Rows", TotalRows)); - public string PreviewColumnsSummary => Localization.Plural("Count.Columns", ColumnMappings?.Count ?? 0); + /// + /// How many columns the file was read as having. + /// + /// + /// From the parse, not from the mapping. This asked ColumnMappings, which is + /// built on step 3 - so on step 1 it answered "0 columns" for every file, which is exactly + /// what a wrong delimiter would look like if the number meant anything. Catching a wrong + /// delimiter before anything is written is what the preview is for. + /// + public string PreviewColumnsSummary => Localization.Plural("Count.Columns", + PreviewData?.Columns.Count ?? ColumnMappings?.Count ?? 0); /// How far the import has got, over the window while it runs. public string ProgressText => Localization.Format("Dialog.Import.Progress", RowsImported, TotalRows); diff --git a/Tools/OutWit.Database.Studio/Views/Dialogs/ExportDialog.axaml b/Tools/OutWit.Database.Studio/Views/Dialogs/ExportDialog.axaml index c6c538d..f031b08 100644 --- a/Tools/OutWit.Database.Studio/Views/Dialogs/ExportDialog.axaml +++ b/Tools/OutWit.Database.Studio/Views/Dialogs/ExportDialog.axaml @@ -9,7 +9,7 @@ d:DesignHeight="480" x:Class="OutWit.Database.Studio.Views.Dialogs.ExportDialog" x:DataType="vm:ExportViewModel" - Title="{DynamicResource S.Dialog.Export.Title}" + Title="{Binding WindowTitle}" Width="560" Height="620" MinWidth="480" @@ -141,7 +141,7 @@ + ToolTip.Tip="{DynamicResource S.Dialog.Export.Output}"/>