From 917c24b7d5fec1b3f4181cc8944e76f8b3f30644 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Mon, 12 May 2025 17:01:13 +0100 Subject: [PATCH 1/4] Support secondary route for url mappers When starting the process, we want to be able to provide a secondary alternative route to the route lookup in template, so that we can skip to the next page should configuration allow it. This PR adds a 'sheetSelection' configuration property that can be set to 'manual' or 'automatic'. When set to automatic, and only a single sheet is found in the upload, the sheet selection step is skipped. --- lib/importer/package-lock.json | 10 ---------- lib/importer/src/config.js | 5 +++++ lib/importer/src/index.js | 15 +++++++++++++-- lib/importer/templates/upload.html | 2 +- prototypes/basic/app/config.json | 1 + prototypes/basic/app/views/upload.html | 2 +- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/importer/package-lock.json b/lib/importer/package-lock.json index ea07d061..c79a0919 100644 --- a/lib/importer/package-lock.json +++ b/lib/importer/package-lock.json @@ -2702,16 +2702,6 @@ "url": "https://opencollective.com/eslint" } }, - "node_modules/eslint/node_modules/@eslint/js": { - "version": "9.23.0", - "resolved": "https://registry.npmjs.org/@eslint/js/-/js-9.23.0.tgz", - "integrity": "sha512-35MJ8vCPU0ZMxo7zfev2pypqTwWTofFZO6m4KAtdoFhRpLJUpHTZZ+KB3C7Hb1d7bULYwO4lJXGCi5Se+8OMbw==", - "dev": true, - "license": "MIT", - "engines": { - "node": "^18.18.0 || ^20.9.0 || >=21.1.0" - } - }, "node_modules/eslint/node_modules/escape-string-regexp": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz", diff --git a/lib/importer/src/config.js b/lib/importer/src/config.js index 916298c6..d742ce52 100644 --- a/lib/importer/src/config.js +++ b/lib/importer/src/config.js @@ -31,6 +31,11 @@ exports.PluginConfig = class { this.uploadPath = config.uploadPath; this.uploadPathDefault = false; + // Should we auto-select a sheet when there is only one sheet? + // manual - No, always select a sheet even if there is only one. + // automatic - Yes, always use the single sheet when there is only one. + this.sheetSelection = config.sheetSelection || "automatic"; + // If the config fields are in the old format (just names) then move them to // the new structure with default values of text for the type, and not required. if (this.fields.find(Boolean)) { diff --git a/lib/importer/src/index.js b/lib/importer/src/index.js index 13a533ad..a3bcbaa8 100644 --- a/lib/importer/src/index.js +++ b/lib/importer/src/index.js @@ -93,7 +93,11 @@ exports.Initialise = (config, router, prototypeKit) => { prototypeKit.views.addFunction( k, - (next) => { + (next, other = null) => { + if (next && other) { + return `${v}?next=${encodeURIComponent(next)}&other=${encodeURIComponent(other)}`; + } + return `${v}?next=${encodeURIComponent(next)}`; }, {}, @@ -158,8 +162,15 @@ exports.Initialise = (config, router, prototypeKit) => { if (session.sheets.length == 1) { session.sheet = session.sheets[0]; - } + // When there is only a single sheet, and if the prototype is configured to + // automatically progress, then this will do so if the 'other' url is + // specified. + if (plugin_config.sheetSelection.toLowerCase() == "automatic" && "other" in request.query) { + const otherPage = decodeURIComponent(request.query.other) + if (otherPage) { request.query.next = request.query.other } + } + } // Ensure the session is persisted. Currently in session, eventually another way request.session.data[IMPORTER_SESSION_KEY] = session; redirectOnwards(request, response); diff --git a/lib/importer/templates/upload.html b/lib/importer/templates/upload.html index b442b36c..4947f9bd 100644 --- a/lib/importer/templates/upload.html +++ b/lib/importer/templates/upload.html @@ -12,7 +12,7 @@

Upload your data

-
+
{{ govukFileUpload({ id: "file-upload", diff --git a/prototypes/basic/app/config.json b/prototypes/basic/app/config.json index d3df2005..f2508e03 100644 --- a/prototypes/basic/app/config.json +++ b/prototypes/basic/app/config.json @@ -1,5 +1,6 @@ { "serviceName": "Upload monthly pension return", + "sheetSelection": "manual", "fields": [ { "name": "Title", diff --git a/prototypes/basic/app/views/upload.html b/prototypes/basic/app/views/upload.html index b5f2ce8e..1921d508 100644 --- a/prototypes/basic/app/views/upload.html +++ b/prototypes/basic/app/views/upload.html @@ -56,7 +56,7 @@

Check your dates

- +
{{ govukFileUpload({ id: "file-upload", From d171b19a8adc3f033db3ba90e75992ec88837ec7 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Tue, 13 May 2025 10:16:53 +0100 Subject: [PATCH 2/4] Test sheetSelection in plugin config --- lib/importer/src/config.js | 11 +++++- lib/importer/src/config.test.js | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/lib/importer/src/config.js b/lib/importer/src/config.js index d742ce52..e8278ebc 100644 --- a/lib/importer/src/config.js +++ b/lib/importer/src/config.js @@ -82,10 +82,18 @@ exports.PluginConfig = class { } }; + setSheetSelection = (mode) => { + if (!["manual", "automatic"].includes(mode)) { + throw (`'${mode}' is not a valid sheet selection mode`) + } + this.sheetSelection = mode + } + as_object = () => { return { uploadPath: this.uploadPathDefault ? TEMP_DIRECTORY_LABEL : this.uploadPath, - fields: this.fields + fields: this.fields, + sheetSelection: this.sheetSelection } } @@ -101,6 +109,7 @@ exports.PluginConfig = class { current.fields = this.fields; current.uploadPath = this.uploadPath; + current.sheetSelection = this.sheetSelection fse.writeJsonSync(configFilePath, current); }; diff --git a/lib/importer/src/config.test.js b/lib/importer/src/config.test.js index 16a93086..052b5cb1 100644 --- a/lib/importer/src/config.test.js +++ b/lib/importer/src/config.test.js @@ -35,6 +35,9 @@ describe("Configuration tests", () => { expect(original.uploadPath).not.toBeNull() expect(updated.uploadPath).not.toBeNull() expect(c.uploadPath).not.toBeNull() + + expect(updated.sheetSelection).not.toBeNull() + expect(updated.sheetSelection).toStrictEqual("automatic") } ); @@ -133,6 +136,68 @@ describe("Configuration tests", () => { mock_files.restore(); }); + test('manual sheet selection', () => { + mock_files({ + './fields/app/config.json': '{"fields": [{"name":"A"}, {"name": "B"}, {"name": "C"}]}', + }) + + withCurrent( + "./fields", + new cfg.PluginConfig(), + (c) => { + c.setSheetSelection("manual") + c.persistConfig() + }, + (original, updated, c) => { + expect(updated.sheetSelection).toStrictEqual("manual") + } + ); + + mock_files.restore(); + }); + + test('default sheet selection', () => { + mock_files({ + './fields/app/config.json': '{"fields": [{"name":"A"}, {"name": "B"}, {"name": "C"}]}', + }) + + withCurrent( + "./fields", + new cfg.PluginConfig(), + (c) => { + c.persistConfig() + }, + (original, updated, c) => { + expect(updated.sheetSelection).toStrictEqual("automatic") + } + ); + + mock_files.restore(); + }); + + + test('invalid sheet selection', () => { + mock_files({ + './fields/app/config.json': '{"fields": [{"name":"A"}, {"name": "B"}, {"name": "C"}]}', + }) + + withCurrent( + "./fields", + new cfg.PluginConfig(), + (c) => { + expect(() => { + c.setSheetSelection("invalid") + } + ).toThrow("'invalid' is not a valid sheet selection mode") + c.persistConfig() + }, + (original, updated, c) => { + expect(updated.sheetSelection).toStrictEqual("automatic") + } + ); + + mock_files.restore(); + }); }) From 07204e9c43300f9b319028733eb01ff1f7eb2cbb Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Tue, 13 May 2025 10:20:14 +0100 Subject: [PATCH 3/4] Appease the linter Unused function parameters (in some tests) result in a lint failure, which is a little extreme as now the function signature looks wrong for those tests. --- lib/importer/src/config.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/importer/src/config.test.js b/lib/importer/src/config.test.js index 052b5cb1..90aa44b7 100644 --- a/lib/importer/src/config.test.js +++ b/lib/importer/src/config.test.js @@ -148,7 +148,7 @@ describe("Configuration tests", () => { c.setSheetSelection("manual") c.persistConfig() }, - (original, updated, c) => { + (original, updated) => { expect(updated.sheetSelection).toStrictEqual("manual") } ); @@ -167,7 +167,7 @@ describe("Configuration tests", () => { (c) => { c.persistConfig() }, - (original, updated, c) => { + (original, updated) => { expect(updated.sheetSelection).toStrictEqual("automatic") } ); @@ -191,7 +191,7 @@ describe("Configuration tests", () => { ).toThrow("'invalid' is not a valid sheet selection mode") c.persistConfig() }, - (original, updated, c) => { + (original, updated) => { expect(updated.sheetSelection).toStrictEqual("automatic") } ); From d7eca84a8eaadd13fb8e0289c766a8c6327b8883 Mon Sep 17 00:00:00 2001 From: Ross Jones Date: Tue, 20 May 2025 14:46:22 +0100 Subject: [PATCH 4/4] Move tests and add multiple-projects to test config --- prototypes/basic/playwright.config.js | 33 ++++++++++++++++--- .../basic/tests/sheet-selection-auto/.gitkeep | 0 .../tests/sheet-selection-manual/.gitkeep | 0 .../basic/tests/{ => simple}/simple.spec.js | 0 .../tests/{ => simple}/small-file.spec.js | 12 +++---- .../basic/tests/{ => simple}/tribbles.spec.js | 12 +++---- 6 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 prototypes/basic/tests/sheet-selection-auto/.gitkeep create mode 100644 prototypes/basic/tests/sheet-selection-manual/.gitkeep rename prototypes/basic/tests/{ => simple}/simple.spec.js (100%) rename prototypes/basic/tests/{ => simple}/small-file.spec.js (89%) rename prototypes/basic/tests/{ => simple}/tribbles.spec.js (91%) diff --git a/prototypes/basic/playwright.config.js b/prototypes/basic/playwright.config.js index 1538f82d..58e1fd31 100644 --- a/prototypes/basic/playwright.config.js +++ b/prototypes/basic/playwright.config.js @@ -3,15 +3,38 @@ import { defineConfig, devices } from '@playwright/test'; export default defineConfig({ workers: 1, testDir: './tests', - reporter: process.env.CI ? [['github'], ['junit', {outputFile: "./test-results/results.xml"}]] : 'line', - use: { - baseURL: 'http://127.0.0.1:3000', - }, + reporter: process.env.CI ? [['github'], ['junit', { outputFile: "./test-results/results.xml" }]] : 'line', + // use: { + // baseURL: 'http://127.0.0.1:3000', + // }, + projects: [ + { + name: 'simple-tests', + use: { + baseURL: 'http://localhost:3000', + }, + testDir: './tests/simple', + }, + { + name: 'sheet-selection-auto', + use: { + baseURL: 'http://localhost:3000', + }, + testDir: './tests/sheet-selection-auto', + }, + { + name: 'sheet-selection-manual', + use: { + baseURL: 'http://localhost:3000', + }, + testDir: './tests/sheet-selection-manual', + }, + ], webServer: { command: 'npm run dev', url: 'http://127.0.0.1:3000', reuseExistingServer: !process.env.CI, stdout: 'pipe', stderr: 'pipe', - } + } }); diff --git a/prototypes/basic/tests/sheet-selection-auto/.gitkeep b/prototypes/basic/tests/sheet-selection-auto/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/prototypes/basic/tests/sheet-selection-manual/.gitkeep b/prototypes/basic/tests/sheet-selection-manual/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/prototypes/basic/tests/simple.spec.js b/prototypes/basic/tests/simple/simple.spec.js similarity index 100% rename from prototypes/basic/tests/simple.spec.js rename to prototypes/basic/tests/simple/simple.spec.js diff --git a/prototypes/basic/tests/small-file.spec.js b/prototypes/basic/tests/simple/small-file.spec.js similarity index 89% rename from prototypes/basic/tests/small-file.spec.js rename to prototypes/basic/tests/simple/small-file.spec.js index 358585b9..f993b4cc 100644 --- a/prototypes/basic/tests/small-file.spec.js +++ b/prototypes/basic/tests/simple/small-file.spec.js @@ -1,12 +1,12 @@ import { test, expect } from '@playwright/test'; -import * as fixtures from "./helpers/fixtures"; +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' +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'); diff --git a/prototypes/basic/tests/tribbles.spec.js b/prototypes/basic/tests/simple/tribbles.spec.js similarity index 91% rename from prototypes/basic/tests/tribbles.spec.js rename to prototypes/basic/tests/simple/tribbles.spec.js index 777d28f0..8f4d362e 100644 --- a/prototypes/basic/tests/tribbles.spec.js +++ b/prototypes/basic/tests/simple/tribbles.spec.js @@ -1,12 +1,12 @@ import { test, expect } from '@playwright/test'; -import * as fixtures from "./helpers/fixtures"; +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' +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');