From ee9dccd7fd96e9d5c576a3c8486b6813de3f2662 Mon Sep 17 00:00:00 2001 From: Phil Moorhouse Date: Wed, 25 Sep 2024 12:07:33 +0100 Subject: [PATCH 1/5] remove jwt logging --- src/middleware/ensure-authenticated.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/middleware/ensure-authenticated.ts b/src/middleware/ensure-authenticated.ts index c9c9555..2a38191 100644 --- a/src/middleware/ensure-authenticated.ts +++ b/src/middleware/ensure-authenticated.ts @@ -28,7 +28,6 @@ export const ensureAuthenticated: RequestHandler = (req: AuthedRequest, res, nex // store the token string in the request as we need it for Authorization header in API requests req.jwt = token; - logger.debug(`JWT: ${token}`); // store the user object in the request for use in the frontend req.user = decoded.user; From 6bc1138fecd9f7211ea0447554a4221d7eb689ae Mon Sep 17 00:00:00 2001 From: Phil Moorhouse Date: Thu, 26 Sep 2024 15:17:16 +0100 Subject: [PATCH 2/5] add initial tasklist page --- src/middleware/translations/en.json | 29 +++++- src/routes/publish.ts | 32 +++++- src/services/stats-wales-api.ts | 20 ++++ src/utils/latest.ts | 11 +++ src/utils/single-lang-dataset.ts | 14 +++ src/views/errors/not-found.ejs | 10 ++ src/views/publish/tasklist.ejs | 147 ++++++++++++++++++++++++++++ 7 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 src/utils/latest.ts create mode 100644 src/utils/single-lang-dataset.ts create mode 100644 src/views/errors/not-found.ejs create mode 100644 src/views/publish/tasklist.ejs diff --git a/src/middleware/translations/en.json b/src/middleware/translations/en.json index 7c86893..e5071bd 100644 --- a/src/middleware/translations/en.json +++ b/src/middleware/translations/en.json @@ -100,6 +100,32 @@ "IGNORE": "This column can be ignored", "UNKNOWN": "Select" } + }, + "tasklist": { + "heading": "Dataset Overview", + "data": { + "subheading": "Data", + "datatable": "Data table" + }, + "metadata": { + "subheading": "Metadata", + "title": "Title", + "summary": "Summary", + "data_collection": "Data collection", + "statistical_quality": "Statistical quality", + "data_sources": "Data sources", + "related_reports": "Related reports", + "update_frequency": "How often the dataset is updated", + "designation": "Designation", + "relevant_topics": "Relevant topics" + }, + "publishing": { + "subheading": "Publishing", + "when": "When the dataset should be published", + "export": "Export text fields for translation", + "import": "Import translations", + "submit": "Submit for approval" + } } }, "view": { @@ -153,7 +179,8 @@ "upload": { "no_csv": "No CSV data provided", "no_csv_data": "No CSV data available" - } + }, + "not_found": "Page not found" }, "api": { "available": "API is available" diff --git a/src/routes/publish.ts b/src/routes/publish.ts index 3a5957c..67aec4b 100644 --- a/src/routes/publish.ts +++ b/src/routes/publish.ts @@ -1,4 +1,4 @@ -import { Blob } from 'buffer'; +import { Blob } from 'node:buffer'; import { Response, Router } from 'express'; import multer from 'multer'; @@ -14,6 +14,7 @@ import { ConfirmedImportDTO } from '../dtos2/confirmed-import-dto'; import { DimensionCreationDTO } from '../dtos2/dimension-creation-dto'; import { SourceType } from '../enums/source-type'; import { ViewError } from '../dtos2/view-error'; +import { singleLangDataset } from '../utils/single-lang-dataset'; const t = i18next.t; const storage = multer.memoryStorage(); @@ -559,9 +560,12 @@ publish.post('/sources', upload.none(), async (req: AuthedRequest, res: Response currentFileImport.id, dimensionCreationRequest ); - res.status(200); - res.setHeader('Content-Type', 'application/json'); - res.json(updatedDataset); + + const datasetId = updatedDataset.dataset.id; + + res.redirect( + `/${lang}/${req.i18n.t('routes.publish.start', { lng: lang })}/${datasetId}/${req.i18n.t('routes.publish.tasklist', { lng: lang })}` + ); } catch (err) { logger.error(`Something went wrong with the Dimension Creation Request with the following error: ${err}`); const errs = generateViewErrors(undefined, 500, [ @@ -606,3 +610,23 @@ publish.delete('/session/currentImport', (req: AuthedRequest, res: Response) => res.status(200); res.json({ message: 'Current import has been deleted' }); }); + +publish.get('/:datasetId/tasklist', async (req: AuthedRequest, res: Response) => { + const lang = req.i18n.language; + const datasetId = req.params.datasetId as string; + const statsWalesApi = new StatsWalesApi(lang, req.jwt); + + try { + const dataset = await statsWalesApi.getDataset(datasetId); + setCurrentToSession(dataset, req); + + res.render('publish/tasklist', { + dataset: singleLangDataset(lang, dataset), + sourcesUrl: `/${lang}/${req.i18n.t('routes.publish.start', { lng: lang })}/${req.i18n.t('routes.publish.sources', { lng: lang })}` + }); + } catch (err) { + logger.error(`Something went wrong viewing the tasklist`); + res.status(404); + res.render('errors/not-found'); + } +}); diff --git a/src/services/stats-wales-api.ts b/src/services/stats-wales-api.ts index a12eb8c..c8022fc 100644 --- a/src/services/stats-wales-api.ts +++ b/src/services/stats-wales-api.ts @@ -57,6 +57,26 @@ export class StatsWalesApi { return filelist; } + public async getDataset(datasetId: string): Promise { + logger.info(`Fetching dataset from ${this.backendUrl}/${this.lang}/dataset/${datasetId}`); + const dataset = await fetch(`${this.backendUrl}/${this.lang}/dataset/${datasetId}`, { + headers: this.authHeader + }) + .then((response) => { + if (response.ok) { + return response.json(); + } + const err = new HttpError(response.status); + err.handleMessage(response.text()); + throw err; + }) + .then((api_res) => { + return api_res as DatasetDTO; + }); + + return dataset; + } + public async getDatasetView(datasetId: string, pageNumber: number, pageSize: number) { logger.info( `Fetching dataset view from ${this.backendUrl}/${this.lang}/dataset/${datasetId}/view?page_number=${pageNumber}&page_size=${pageSize}` diff --git a/src/utils/latest.ts b/src/utils/latest.ts new file mode 100644 index 0000000..eb6da36 --- /dev/null +++ b/src/utils/latest.ts @@ -0,0 +1,11 @@ +import { sortBy, last } from 'lodash'; + +import { DatasetDTO, ImportDTO, RevisionDTO } from '../dtos2/dataset-dto'; + +export const getLatestRevision = (dataset: DatasetDTO): RevisionDTO | undefined => { + return last(sortBy(dataset.revisions, 'revision_index')); +}; + +export const getLatestImport = (revision: RevisionDTO): ImportDTO | undefined => { + return last(sortBy(revision.imports, 'uploaded_at')); +}; diff --git a/src/utils/single-lang-dataset.ts b/src/utils/single-lang-dataset.ts new file mode 100644 index 0000000..3f87cef --- /dev/null +++ b/src/utils/single-lang-dataset.ts @@ -0,0 +1,14 @@ +import { DatasetDTO } from '../dtos2/dataset-dto'; + +export const singleLangDataset = (lang: string, dataset: DatasetDTO) => { + return { + ...dataset, + datasetInfo: dataset.datasetInfo?.find((info) => info.language === lang), + dimensions: dataset.dimensions?.map((dimension) => { + return { + ...dimension, + dimensionInfo: dimension.dimensionInfo?.find((info) => info.language === lang) + }; + }) + }; +}; diff --git a/src/views/errors/not-found.ejs b/src/views/errors/not-found.ejs new file mode 100644 index 0000000..d94aedd --- /dev/null +++ b/src/views/errors/not-found.ejs @@ -0,0 +1,10 @@ +<%- include("../partials/header"); %> + +
+ +
+

<%- t('errors.not_found') %>

+
+
+ +<%- include("../partials/footer"); %> diff --git a/src/views/publish/tasklist.ejs b/src/views/publish/tasklist.ejs new file mode 100644 index 0000000..3740757 --- /dev/null +++ b/src/views/publish/tasklist.ejs @@ -0,0 +1,147 @@ +<%- include("../partials/header"); %> + +
+
+ +
+
+

<%= t('publish.tasklist.heading') %>

+

<%= dataset?.datasetInfo?.title %>

+
+
+ +
+
+

<%= t('publish.tasklist.data.subheading') %>

+ + +

<%= t('publish.tasklist.metadata.subheading') %>

+ + +

<%= t('publish.tasklist.publishing.subheading') %>

+ +
+
+
+
+ + + +<%- include("../partials/footer"); %> From 1341dc1269ec4f2275b2114d67c3d199c3375ab5 Mon Sep 17 00:00:00 2001 From: Phil Moorhouse Date: Thu, 26 Sep 2024 15:28:18 +0100 Subject: [PATCH 3/5] fix linting and CQ issues --- src/routes/publish.ts | 4 +++- src/routes/view.ts | 3 --- test/helpers/mock-server.ts | 2 +- test/publish.test.ts | 6 ++---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/routes/publish.ts b/src/routes/publish.ts index 67aec4b..991b72c 100644 --- a/src/routes/publish.ts +++ b/src/routes/publish.ts @@ -2,6 +2,7 @@ import { Blob } from 'node:buffer'; import { Response, Router } from 'express'; import multer from 'multer'; +import { validate as validateUUID } from 'uuid'; import { logger } from '../utils/logger'; import { StatsWalesApi } from '../services/stats-wales-api'; @@ -617,6 +618,7 @@ publish.get('/:datasetId/tasklist', async (req: AuthedRequest, res: Response) => const statsWalesApi = new StatsWalesApi(lang, req.jwt); try { + if (!validateUUID(datasetId)) throw new Error('Invalid dataset ID'); const dataset = await statsWalesApi.getDataset(datasetId); setCurrentToSession(dataset, req); @@ -625,7 +627,7 @@ publish.get('/:datasetId/tasklist', async (req: AuthedRequest, res: Response) => sourcesUrl: `/${lang}/${req.i18n.t('routes.publish.start', { lng: lang })}/${req.i18n.t('routes.publish.sources', { lng: lang })}` }); } catch (err) { - logger.error(`Something went wrong viewing the tasklist`); + logger.error(`Something went wrong viewing the tasklist: ${err}`); res.status(404); res.render('errors/not-found'); } diff --git a/src/routes/view.ts b/src/routes/view.ts index d8a1c10..835f309 100644 --- a/src/routes/view.ts +++ b/src/routes/view.ts @@ -4,11 +4,9 @@ import { validate as validateUUID } from 'uuid'; import { StatsWalesApi } from '../services/stats-wales-api'; import { FileList } from '../dtos2/filelist'; import { ViewErrDTO } from '../dtos2/view-dto'; -import { i18next } from '../middleware/translation'; import { logger } from '../utils/logger'; import { AuthedRequest } from '../interfaces/authed-request'; -const t = i18next.t; export const view = Router(); const statsWalesApi = (req: AuthedRequest) => { @@ -24,7 +22,6 @@ view.get('/', async (req: AuthedRequest, res: Response) => { }); view.get('/:datasetId', async (req: AuthedRequest, res: Response) => { - const lang = req.i18n.language; const page_number: number = Number.parseInt(req.query.page_number as string, 10) || 1; const page_size: number = Number.parseInt(req.query.page_size as string, 10) || 100; diff --git a/test/helpers/mock-server.ts b/test/helpers/mock-server.ts index ef114d5..ba31b98 100644 --- a/test/helpers/mock-server.ts +++ b/test/helpers/mock-server.ts @@ -93,7 +93,7 @@ const brokenPreviewDataset: DatasetDTO = { ] } ] -} +}; export const completedDataset: DatasetDTO = { id: '5caeb8ed-ea64-4a58-8cf0-b728308833e5', diff --git a/test/publish.test.ts b/test/publish.test.ts index 1196c76..deb217d 100644 --- a/test/publish.test.ts +++ b/test/publish.test.ts @@ -107,7 +107,7 @@ describe('Publisher Journey Tests', () => { .set('User-Agent', 'supertest') .set('Cookie', cookies); expect(res.status).toBe(200); - expect(res.body).toEqual({ message: 'All session data has been cleared' }) + expect(res.body).toEqual({ message: 'All session data has been cleared' }); }); test('Delete current revision returns 200 with message and removes the current revision from session', async () => { @@ -555,9 +555,7 @@ describe('Publisher Journey Tests', () => { describe('Session issues for sources', () => { test('No dataset in the session when posting to sources returns 302 back to start', async () => { - const res = await request(app) - .post('/en-GB/publish/sources') - .set('User-Agent', 'supertest'); + const res = await request(app).post('/en-GB/publish/sources').set('User-Agent', 'supertest'); expect(res.status).toBe(302); expect(res.header.location).toBe(`/en-GB/publish/`); }); From fc9960721ddccdc12c5f132ba811b1ceeec6f39a Mon Sep 17 00:00:00 2001 From: Phil Moorhouse Date: Thu, 26 Sep 2024 16:00:46 +0100 Subject: [PATCH 4/5] add tests --- test/publish.test.ts | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/test/publish.test.ts b/test/publish.test.ts index deb217d..4a0f8d5 100644 --- a/test/publish.test.ts +++ b/test/publish.test.ts @@ -9,7 +9,7 @@ import { DatasetDTO, ImportDTO, RevisionDTO } from '../src/dtos2/dataset-dto'; import { ViewErrDTO } from '../src/dtos2/view-dto'; import { DimensionCreationDTO } from '../src/dtos2/dimension-creation-dto'; -import { completedDataset, server } from './helpers/mock-server'; +import { server } from './helpers/mock-server'; const t = i18next.t; @@ -472,7 +472,7 @@ describe('Publisher Journey Tests', () => { }); describe('Setting up sources and dimensions', () => { - test('Confirming a single set of datavalies, a single footnote and dimensions returns 200 and a JSON blob', async () => { + test('Confirming a single set of datavalues, a single footnote and dimensions returns 200 and a JSON blob', async () => { const cookies = await setSourcesIntoSession(); const res = await request(app) .post('/en-GB/publish/sources') @@ -483,11 +483,8 @@ describe('Publisher Journey Tests', () => { .field('8b2ef050-fe84-4150-b124-f993a5e56dc3', 'DIMENSION') .set('User-Agent', 'supertest') .set('Cookie', cookies); - expect(res.status).toBe(200); - expect(res.body).toEqual({ - success: true, - dataset: completedDataset - }); + expect(res.status).toBe(302); + expect(res.header.location).toBe(`/en-GB/publish/5caeb8ed-ea64-4a58-8cf0-b728308833e5/tasklist`); }); test('Confirming a multiple datavalies, a single footnote and dimensions returns 400 and a message to the user', async () => { @@ -601,4 +598,30 @@ describe('Publisher Journey Tests', () => { }); }); }); + + describe('Tasklist', () => { + test('It loads the dataset', async () => { + const cookies = await setSourcesIntoSession(); + const res = await request(app) + .get(`/en-GB/publish/5caeb8ed-ea64-4a58-8cf0-b728308833e5/tasklist`) + .set('User-Agent', 'supertest') + .set('Cookie', cookies); + expect(res.status).toBe(200); + expect(res.text).toContain(t('publish.tasklist.heading')); + expect(res.text).toContain('test dataset 1'); + expect(res.text).toContain(t('publish.tasklist.data.datatable')); + expect(res.text).toContain(t('publish.tasklist.metadata.update_frequency')); + expect(res.text).toContain(t('publish.tasklist.publishing.when')); + }); + + test('It throws a 404 if the dataset id is invalid', async () => { + const cookies = await setSourcesIntoSession(); + const res = await request(app) + .get(`/en-GB/publish/not-a-dataset-uuid/tasklist`) + .set('User-Agent', 'supertest') + .set('Cookie', cookies); + expect(res.status).toBe(404); + expect(res.text).toContain(t('errors.not_found')); + }); + }); }); From b6a97a151c7ee9a3f581d677d367c747cb55cb62 Mon Sep 17 00:00:00 2001 From: Phil Moorhouse Date: Thu, 26 Sep 2024 16:06:17 +0100 Subject: [PATCH 5/5] fix lint errors temporarily until we find cause --- src/routes/publish.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/routes/publish.ts b/src/routes/publish.ts index 991b72c..055f764 100644 --- a/src/routes/publish.ts +++ b/src/routes/publish.ts @@ -291,6 +291,7 @@ publish.get('/preview', async (req: AuthedRequest, res: Response) => { ); if (!previewData.success) { logger.error('Failed to get preview data from the backend'); + // eslint-disable-next-line require-atomic-updates req.session.errors = generateViewErrors(undefined, 500, [ generateError('preview', 'errors.preview.failed_to_get_preview', {}) ]); @@ -318,6 +319,7 @@ async function confirmFileUpload( currentFileImport.id ); if (confirmedImport.success) { + // eslint-disable-next-line require-atomic-updates req.session.currentImport = confirmedImport.fileImport; req.session.save(); res.redirect( @@ -328,6 +330,7 @@ async function confirmFileUpload( logger.error( `An HTTP error occurred trying to confirm import from the dataset with the following error: ${err}` ); + // eslint-disable-next-line require-atomic-updates req.session.errors = generateViewErrors(currentDataset.id, 500, [ generateError('confirm', 'errors.preview.confirm_error', {}) ]);