From d64f388a2806ef820d834c14bec3575e02f8af66 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Mon, 30 Sep 2024 10:50:27 +0100 Subject: [PATCH] Fix off by one error in small files Currently there's an off-by-one error when displaying a small file with fewer rows than the number used in previews (10). This fixes the off-by-one and introduces a test to ensure we don't re-introduce it. --- fixtures/small-file.csv | 2 + lib/importer/sheets.js | 2 +- prototypes/basic/tests/small-file.spec.js | 96 +++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 fixtures/small-file.csv create mode 100644 prototypes/basic/tests/small-file.spec.js diff --git a/fixtures/small-file.csv b/fixtures/small-file.csv new file mode 100644 index 0000000..a3672ab --- /dev/null +++ b/fixtures/small-file.csv @@ -0,0 +1,2 @@ +A,B +1,2 diff --git a/lib/importer/sheets.js b/lib/importer/sheets.js index a46ff42..9b07eea 100644 --- a/lib/importer/sheets.js +++ b/lib/importer/sheets.js @@ -156,7 +156,7 @@ exports.GetRows = (session, start=0, count=10) => { const rowRange = { sheet: session.sheet, start: { row: start, column: 0}, - end: { row: Math.min(start + count - 1, sheetDimensions.rows-1), column: sheetDimensions.columns-1} + end: { row: Math.min(start + count, sheetDimensions.rows), column: sheetDimensions.columns-1} } const wantedRows = rowRange.end.row - start; diff --git a/prototypes/basic/tests/small-file.spec.js b/prototypes/basic/tests/small-file.spec.js new file mode 100644 index 0000000..15bf6b6 --- /dev/null +++ b/prototypes/basic/tests/small-file.spec.js @@ -0,0 +1,96 @@ +import { test, expect } from '@playwright/test'; + +import * as fixtures from "./helpers/fixtures"; + +import { UploadPage } from './helpers/upload_page'; +import { SheetSelectorPage } from './helpers/sheet_selector_page'; +import { HeaderSelectorPage } from './helpers/header_selector_page'; +import { FooterSelectorPage } from './helpers/footer_selector_page' +import { MappingPage } from './helpers/mapping_page' + +const path = require('node:path'); + +// Take a screenshot to help with debugging +const screenshot = async(page, name) => { + await page.screenshot({ path: name, fullPage: true }); +} + +test('tiny files', async ({ page }) => { + await page.goto('/'); + await page.getByText('Start now').click(); + + // --------------------------------------------------------------------------- + // Upload a file + await expect(page).toHaveURL(/.upload/) + + const uploadPage = new UploadPage(page) + const browseButton = await uploadPage.browseButton() + + await browseButton.click(); + await browseButton.setInputFiles(fixtures.getFixture('small-file.csv')); + await uploadPage.submit(); + + // --------------------------------------------------------------------------- + // Select sheet + await expect(page).toHaveURL(/.select_sheet/) + + const sheets = new SheetSelectorPage(page); + + // There is only a single sheet, it should default be selected and the table + // visible + const radioButton = await sheets.getRadio('Sheet1') + await expect(radioButton).toBeChecked(); + const vis = await sheets.getTableProperty(0, 'visibility') + await expect(vis).toBe('visible') + + // // Check the data is present in the sheet previews + const previewRow = await sheets.getTableRow(0, 0) + const zerozero = await previewRow.locator("td").nth(0).textContent() + const zeroone = await previewRow.locator("td").nth(1).textContent() + await expect(zerozero).toBe("A") + await expect(zeroone).toBe("B") + + await sheets.submit() + + // --------------------------------------------------------------------------- + // Select some cells for a header row + // We will use the entire first row as our selection + await expect(page).toHaveURL(/.select_header_row/) + + const headers = new HeaderSelectorPage(page) + await headers.select([0,0], [0,1]) + await headers.submit() + + // // --------------------------------------------------------------------------- + // // Do not choose a footer row, just click Skip + await expect(page).toHaveURL(/.select_footer_row/) + + const footers = new FooterSelectorPage(page) + await footers.skip() + + // // --------------------------------------------------------------------------- + // // Perform the mapping after checking the previews here are what we expect + await expect(page).toHaveURL(/.mapping/) + + const mapping = new MappingPage(page) + + const expectedColumns = ["A", "B"] + const expectedExamples = [ + "1", + "2", + ] + + const colNames = await mapping.getColumnNames() + expect(colNames).toStrictEqual(expectedColumns) + + const examples = await mapping.getExamples() + expect(examples).toStrictEqual(expectedExamples) + + await mapping.setMapping('A', 'Code') + await mapping.setMapping('B', 'Name') + await mapping.submit() + + // // --------------------------------------------------------------------------- + // // Should be on the success page + await expect(page).toHaveURL(/.success/) +});