diff --git a/.husky/pre-commit b/.husky/pre-commit index 6f5cf54d4..3e58d0a63 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,5 +1,2 @@ -#!/usr/bin/env sh -. "$(dirname -- "$0")/_/husky.sh" - npm run lint:fix npm run lint diff --git a/config/default.yaml b/config/default.yaml index 2b89001bc..278cd98f6 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -174,4 +174,4 @@ features: nonAuthPages: # This feature enables pages to show non-authoritative datasets # If off, it will still check for local-plan datasets. - enabled: true + enabled: true \ No newline at end of file diff --git a/config/development.yaml b/config/development.yaml index 6701ad3e8..34d59afcd 100644 --- a/config/development.yaml +++ b/config/development.yaml @@ -5,6 +5,7 @@ aws: { region: eu-west-2, bucket: 'development-pub-async-request-files', } +manageServiceUrl: 'https://manage.development.planning.data.gov.uk' datasetteUrl: https://datasette.development.planning.data.gov.uk downloadUrl: 'https://download.development.planning.data.gov.uk' redis: { diff --git a/config/local.yaml b/config/local.yaml index 273fa47bd..a7e0cbae9 100644 --- a/config/local.yaml +++ b/config/local.yaml @@ -7,6 +7,7 @@ aws: { endpoint: 'http://localhost:4566', s3ForcePathStyle: true, } +manageServiceUrl: 'http://localhost:5000' organisationTypes: - local-authority - national-park-authority diff --git a/config/production.yaml b/config/production.yaml index 5d00f2d01..0035a4a79 100644 --- a/config/production.yaml +++ b/config/production.yaml @@ -10,6 +10,7 @@ aws: { region: eu-west-2, bucket: 'production-pub-async-request-files', } +manageServiceUrl: 'https://manage.planning.data.gov.uk' redis: { secure: true, host: 'production-pub-async-redis-eihmmv.serverless.euw2.cache.amazonaws.com', diff --git a/config/staging.yaml b/config/staging.yaml index 59baa932f..29469261d 100644 --- a/config/staging.yaml +++ b/config/staging.yaml @@ -10,6 +10,7 @@ aws: { region: eu-west-2, bucket: 'staging-pub-async-request-files', } +manageServiceUrl: 'https://manage.staging.planning.data.gov.uk/' redis: { secure: true, host: 'staging-pub-async-redis-kh1elu.serverless.euw2.cache.amazonaws.com', diff --git a/src/assets/js/statusPage.js b/src/assets/js/statusPage.js index 2664058f1..9cc290b86 100644 --- a/src/assets/js/statusPage.js +++ b/src/assets/js/statusPage.js @@ -16,6 +16,7 @@ export default class StatusPage { this.interval = null this.heading = document.querySelector('#js-async-processing-heading') this.processingMessage = document.querySelector('#js-async-processing-message') + this.secondaryProcessingMessage = document.querySelector('#js-async-processing-secondary-message') this.continueButton = document.querySelector('#js-async-continue-button') } @@ -29,7 +30,11 @@ export default class StatusPage { console.info('StatusPage: polled request and got a status of: ' + data.status) // ToDo: handle other status' here if (finishedProcessingStatuses.includes(data.status)) { - this.updatePageToComplete() + if (data.showColumnMapping) { + this.updatePageToColumnMapping(data) + } else { + this.updatePageToComplete() + } clearInterval(interval) } }).catch((reason) => { @@ -49,7 +54,9 @@ export default class StatusPage { updatePageToChecking () { // update the page this.heading.textContent = headingTexts.checking + this.processingMessage.style.display = 'block' this.processingMessage.textContent = messageTexts.checking + this.hideSecondaryMessage() this.continueButton.style.display = 'none' } @@ -57,20 +64,47 @@ export default class StatusPage { // update the page this.heading.textContent = headingTexts.checked this.processingMessage.style.display = 'none' + this.hideSecondaryMessage() this.continueButton.textContent = buttonTexts.checked this.continueButton.ariaLabel = buttonAriaLabels.checked this.continueButton.style.display = 'block' } + updatePageToColumnMapping (data) { + // update the page to show column mapping action + this.heading.textContent = headingTexts.columnMapping + this.processingMessage.style.display = 'block' + this.processingMessage.textContent = messageTexts.columnMapping.primary + this.showSecondaryMessage(messageTexts.columnMapping.secondary) + this.continueButton.textContent = buttonTexts.columnMapping + this.continueButton.ariaLabel = buttonAriaLabels.columnMapping + this.continueButton.style.display = 'block' + } + updatePageForPollingTimeout () { // update the page this.heading.textContent = headingTexts.checking this.processingMessage.style.display = 'block' this.processingMessage.textContent = messageTexts.checking + this.hideSecondaryMessage() this.continueButton.textContent = buttonTexts.checking this.continueButton.ariaLabel = buttonAriaLabels.checking this.continueButton.style.display = 'block' } + + hideSecondaryMessage () { + if (!this.secondaryProcessingMessage) return + + this.secondaryProcessingMessage.textContent = '' + this.secondaryProcessingMessage.style.display = 'none' + } + + showSecondaryMessage (message) { + if (!this.secondaryProcessingMessage) return + + this.secondaryProcessingMessage.textContent = message + this.secondaryProcessingMessage.style.display = 'block' + } } window.addEventListener('load', () => { diff --git a/src/assets/scss/index.scss b/src/assets/scss/index.scss index 3b706c57e..95cbd7998 100644 --- a/src/assets/scss/index.scss +++ b/src/assets/scss/index.scss @@ -249,6 +249,39 @@ code * { border-left: govuk-spacing(1) solid $govuk-error-colour; } +.column-mapping__row--error { + padding-left: 0; + + .govuk-table__cell:first-child { + border-left: govuk-spacing(1) solid $govuk-error-colour; + } +} + +.column-mapping-table { + th, + td { + padding-top: govuk-spacing(2); + padding-bottom: govuk-spacing(2); + } +} + +.column-mapping-panel { + background-color: govuk-colour("light-grey"); + padding: 15px 20px; +} + +.column-mapping__unmap-select { + background-color: govuk-colour("green"); + color: govuk-colour("white"); + font-weight: bold; + font-size: 0.875rem; + padding: 2px 8px; + border: none; + border-radius: 0; + cursor: pointer; + appearance: auto; +} + .app-content__markdown img { max-width: 100%; } diff --git a/src/content/statusPage.js b/src/content/statusPage.js index 64e3c2f30..0e15c2d90 100644 --- a/src/content/statusPage.js +++ b/src/content/statusPage.js @@ -1,19 +1,26 @@ export const headingTexts = { checking: 'Checking your data', - checked: 'Data checked' + checked: 'Data checked', + columnMapping: 'You need to map your fields' } export const messageTexts = { checking: 'Do not close this page. Larger files may take longer.', - checked: 'You can continue' + checked: 'You can continue', + columnMapping: { + primary: 'We could not map all of the fields in your data to what is in the data standard.', + secondary: "For all expected fields, you need to select a field found in your data or 'not provided'." + } } export const buttonTexts = { checking: 'Check latest status', - checked: 'Continue' + checked: 'Continue', + columnMapping: 'Map your fields' } export const buttonAriaLabels = { checking: 'Check latest status of data check', - checked: 'Continue to next step' + checked: 'Continue to next step', + columnMapping: 'Map your fields' } diff --git a/src/controllers/CheckAnswersController.js b/src/controllers/CheckAnswersController.js index bb81aa665..38239a808 100644 --- a/src/controllers/CheckAnswersController.js +++ b/src/controllers/CheckAnswersController.js @@ -1,6 +1,6 @@ import PageController from './pageController.js' import config from '../../config/index.js' -import { attachFileToIssue, createCustomerRequest } from '../services/jiraService.js' +import { addInternalNoteToIssue, attachFileToIssue, createCustomerRequest } from '../services/jiraService.js' import logger from '../utils/logger.js' import { types } from '../utils/logging.js' import { stringify } from 'csv-stringify/sync' @@ -42,6 +42,12 @@ class CheckAnswersController extends PageController { async post (req, res, next) { try { const issue = await this.createJiraServiceRequest(req, res, next) + if (issue?.localJiraFallback) { + return res.json({ + message: issue.message, + manageServiceLink: issue.manageServiceLink + }) + } if (issue) { req.sessionModel.set('reference', issue.issueKey) req.sessionModel.set('errors', []) @@ -81,6 +87,18 @@ class CheckAnswersController extends PageController { const checkTool = requestId ? `${config.url}check/results/${requestId}/1` : 'Check tool link unavailable' + const manageServiceLink = buildManageServiceLink(requestId, data) + + // Theres no point in attempting to create a Jira issue if we're going to fail, so we check + // first and return early with a message about using the Manage Service for local development + // if Jira isn't configured + if (config.environment === 'local' && !isJiraConfigured()) { + return { + localJiraFallback: true, + message: 'Jira is not configured for local development. Use this Manage Service link to add the data.', + manageServiceLink + } + } const isNonProd = ['local', 'development', 'staging'].includes(config.environment) const summary = `${isNonProd ? '[TEST] ' : ''}Dataset URL request: ${data.organisationName} for ${data.dataset}` @@ -113,14 +131,22 @@ class CheckAnswersController extends PageController { return null } - this.attachFileToIssue(requestId, data, description, response).catch((error) => { - logger.error('CheckAnswersController.attachFileToIssue(): Failed to attach CSV to Jira issue', { + const internalNote = `This request is ready to be added in the Manage Service.\n\n[Open this request in Manage Service|${manageServiceLink}]\n\nIf the link does not open, copy and paste this URL in your browser:\n${manageServiceLink}` + await addInternalNoteToIssue(response.data.issueKey, internalNote).catch((error) => { + logger.error('CheckAnswersController.addInternalNoteToIssue(): Failed to add internal note to Jira issue', { errorMessage: error.message, errorStack: error, type: types.External }) }) + await this.attachFileToIssue(requestId, data, description, response).catch((error) => { + logger.error('CheckAnswersController.attachFileToIssue(): Failed to attach CSV to Jira issue', { + errorMessage: error.message, + errorStack: error, + type: types.External + }) + }) return response.data } @@ -189,4 +215,13 @@ class CheckAnswersController extends PageController { } } +function buildManageServiceLink (requestId, data) { + const urlSearchParams = new URLSearchParams({ requestId, documentationUrl: data.documentationUrl }) + return `${config.manageServiceUrl}/datamanager${urlSearchParams.toString() ? `?${urlSearchParams.toString()}` : ''}` +} + +function isJiraConfigured () { + return Boolean(process.env.JIRA_URL && process.env.JIRA_API_KEY && process.env.JIRA_SERVICE_DESK_ID) +} + export default CheckAnswersController diff --git a/src/controllers/columnMappingController.js b/src/controllers/columnMappingController.js new file mode 100644 index 000000000..2c84d5984 --- /dev/null +++ b/src/controllers/columnMappingController.js @@ -0,0 +1,510 @@ +import PageController from './pageController.js' +import { postCheckRequest } from '../services/asyncRequestApi.js' +import { isStatutoryDataset } from '../utils/redisLoader.js' +import { getRequestDataMiddleware, updateSessionFromRequestData } from './resultsController.js' +import { processSpecificationMiddlewares } from '../middleware/common.middleware.js' +import platformApi from '../services/platformApi.js' +import { types } from '../utils/logging.js' +import logger from '../utils/logger.js' + +const LOCK_DETECTED_GEOMETRY_MAPPINGS = true +const GEOMETRY_FIELDS = ['geometry', 'point'] + +class ColumnMappingController extends PageController { + middlewareSetup () { + super.middlewareSetup() + this.use(getRequestDataMiddleware) + this.use(async (req, res, next) => { + const { requestData } = req.locals + const params = requestData.getParams() ?? {} + if (await isStatutoryDataset(params.organisationName, params.dataset)) { + return res.status(404).render('errors/404.html') + } + next() + }) + this.use(updateSessionFromRequestData) + // Populate req.params and dataset, then run specification processing middlewares + this.use(async (req, res, next) => { + const { requestData } = req.locals + const params = requestData?.getParams() ?? {} + try { + const { formattedData } = await platformApi.fetchDatasets({ dataset: params.dataset }) + // Bounds check TODO move to external bounds handling as in fetchOne + if (!formattedData || formattedData.length === 0) { + const error = new Error(`Dataset not found: ${req.params.dataset}`) + logger.warn('fetchDatasetPlatformInfo: no dataset returned', { type: types.App, dataset: req.params.dataset }) + return next(error) + } + const datasetInfo = formattedData[0] + req.dataset = { + collection: datasetInfo.collection, + name: datasetInfo.name, + dataset: datasetInfo.dataset, + typology: datasetInfo.typology + } + } catch (error) { + logger.warn('fetchDatasetPlatformInfo failed', { type: types.App, errorMessage: error.message, errorStack: error.stack }) + return next(error) + } + return next() + }) + // attach the standard specification processing middleware chain + processSpecificationMiddlewares.forEach(mw => this.use(mw)) + } + + async locals (req, res, next) { + try { + const { requestData } = req.locals + if (!requestData) return + + if (requestData.isFailed()) { + res.redirect(`/check/results/${req.params.id}/1`) + return + } + const uniqueDatasetFields = req.uniqueDatasetFields || [] + Object.assign(req.form.options, await buildColumnMappingOptions({ + requestData, + requestId: req.params.id, + uniqueDatasetFields + })) + super.locals(req, res, next) + } catch (error) { + next(error) + } + } + + async post (req, res, next) { + try { + const requestData = await getCompletedRequestData(req, res) + if (!requestData) return + + const options = await buildColumnMappingOptions({ + requestData, + requestId: req.params.id, + body: req.body, + uniqueDatasetFields: req.uniqueDatasetFields || [] + }) + const validationErrors = validateColumnMapping(req.body, options.mappingRows) + if (Object.keys(validationErrors).length > 0) { + options.columnMappingErrors = validationErrors + Object.assign(req.form.options, options) + Object.assign(res.locals, { + options: req.form.options, + errors: {} + }) + res.status(400) + res.render('check/column-mapping.html') + return + } + + const params = requestData.getParams() ?? {} + const uniqueDatasetFields = req.uniqueDatasetFields || [] + const { detectedGeometryMapping } = await prepareColumnMappingContext(requestData, uniqueDatasetFields) + const columnMapping = buildSubmittedColumnMapping({ + existingMapping: { + ...(params.column_mapping ?? {}), + ...detectedGeometryMapping + }, + body: req.body + }) + + const newRequestId = await postCheckRequest({ + ...params, + column_mapping: Object.keys(columnMapping).length > 0 ? columnMapping : null + }) + + req.sessionModel.set('request_id', newRequestId) + res.redirect(`/check/status/${newRequestId}`) + } catch (error) { + next(error) + } + } +} + +/** + * Build the options object passed to the column-mapping template. + * Returns UI-friendly data including expected mapping rows, selectable uploaded + * columns and any validation errors so the view can render the mapping form. + */ +async function buildColumnMappingOptions ({ requestData, requestId, body = {}, validationErrors = {}, uniqueDatasetFields = [] }) { + const { + columnMappingRows, + specFields, + requiredFields, + responseRows + } = await prepareColumnMappingContext(requestData, uniqueDatasetFields) + const mappingRows = buildExpectedFieldRows({ + columnMappingRows, + specFields, + requiredFields + }) + + applySubmittedFieldSelections(mappingRows, body) + + return { + id: requestId, + requestParams: requestData.getParams(), + mappingRows, + uploadedColumns: buildSelectableColumns(columnMappingRows, responseRows), + columnMappingErrors: validationErrors, + lastPage: `/check/status/${requestId}` + } +} + +/** + * Prepare the low-level data required to build the column mapping UI. + * + * - Fetches response details and the column-field detection log from `requestData`. + * - Builds `columnMappingRows` (combined auto-detected + user overrides). + * - Resolves `specFields` from the dataset and `requiredFields` from config. + */ +async function prepareColumnMappingContext (requestData, uniqueDatasetFields = []) { + let columnFieldLog = requestData.getColumnFieldLog() + if (uniqueDatasetFields.length > 0) { + columnFieldLog = columnFieldLog.filter(entry => uniqueDatasetFields.includes(entry?.field)) + } + const responseDetails = await requestData.fetchResponseDetails(0, 50) + const responseRows = responseDetails.getRows() + const detectedGeometryMapping = detectGeometryColumnMapping(columnFieldLog, responseRows) + columnFieldLog = applyDetectedGeometryColumnMapping(columnFieldLog, responseRows) + + const params = requestData.getParams() ?? {} + const userColumnMapping = params.column_mapping ?? {} + columnFieldLog = applyPersistedDetectedGeometryColumnMapping(columnFieldLog, responseRows, userColumnMapping) + const columnMappingRows = buildColumnMappingRows({ + columnFieldLog, + userColumnMapping + }) + + const specFields = buildSpecFields(columnFieldLog.map(entry => entry?.field).filter(Boolean)) + const requiredFields = columnFieldLog.filter(entry => entry?.mandatory).map(entry => entry.field) + return { + columnMappingRows, + specFields, + requiredFields, + responseRows, + detectedGeometryMapping + } +} + +async function getCompletedRequestData (req, res) { + const { requestData } = req.locals + if (!requestData) { + return null + } + if (!requestData.isComplete()) { + res.redirect(`/check/status/${req.params.id}`) + return null + } + return requestData +} + +export function validateColumnMapping (body = {}, mappingRows = []) { + const fieldMap = getBracketFields(body, 'fieldMap') + + // base errors: missing or explicit 'na' for required fields + const errors = Object.fromEntries( + Object.entries(fieldMap) + .filter(([field, value]) => value === '') + .map(([field]) => [field, { + text: `Select the ${field} field` + }]) + ) + + // check for duplicate selections (same column selected for multiple fields) + const selections = Object.entries(fieldMap) + .map(([field, value]) => [field, (value ?? '').trim()]) + .filter(([, col]) => col && col !== 'na') + + const counts = selections.reduce((acc, [, col]) => { + acc[col] = (acc[col] || 0) + 1 + return acc + }, {}) + + for (const [field, col] of selections) { + if (counts[col] > 1 && !errors[field]) { + errors[field] = { text: `${col} has been selected more than once` } + } + } + + return errors +} + +export function applySubmittedFieldSelections (mappingRows = [], body = {}) { + const fieldMap = getBracketFields(body, 'fieldMap') + mappingRows.forEach(row => { + if (Object.hasOwn(fieldMap, row.field)) { + const selectedColumn = (fieldMap[row.field] ?? '').trim() + row.userIgnored = selectedColumn === 'na' + row.column = row.userIgnored ? '' : selectedColumn + } + }) +} + +export function buildSubmittedColumnMapping ({ existingMapping = {}, body = {}, spareUploadedColumns = [] }) { + const columnMapping = { ...existingMapping } + const columns = Array.isArray(body.columns) + ? body.columns + : body.columns + ? [body.columns] + : [] + const selectedUploadedColumns = new Set() + + columns.forEach((column, index) => { + const field = body[`field-${index}`]?.trim() + const status = body[`status-${index}`] + + if (status === 'ignore') { + columnMapping[column] = 'IGNORE' + } else if (status === 'unmap') { + delete columnMapping[column] + } else if (field) { + columnMapping[column] = field + } + }) + + for (const [column, fieldValue] of Object.entries(getBracketFields(body, 'map'))) { + const field = fieldValue?.trim() + if (field === 'IGNORE') { + columnMapping[column] = 'IGNORE' + } else if (field) { + columnMapping[column] = field + } + } + + for (const [column, value] of Object.entries(getBracketFields(body, 'unmap'))) { + if (value === 'yes') { + delete columnMapping[column] + } + } + + for (const [field, columnValue] of Object.entries(getBracketFields(body, 'fieldMap'))) { + const column = columnValue?.trim() + if (!column) continue + + for (const [mappedColumn, mappedField] of Object.entries(columnMapping)) { + if (mappedField === field) delete columnMapping[mappedColumn] + } + + columnMapping[column] = column === 'na' ? 'IGNORE' : field + selectedUploadedColumns.add(column) + } + + spareUploadedColumns.forEach(column => { + if (!selectedUploadedColumns.has(column) && !columnMapping[column]) { + columnMapping[column] = 'IGNORE' + } + }) + + return Object.fromEntries( + Object.entries(columnMapping).filter(([, field]) => field) + ) +} + +export function getBracketFields (body = {}, fieldName) { + const values = { ...(body[fieldName] ?? {}) } + const prefix = `${fieldName}[` + + for (const [key, value] of Object.entries(body)) { + if (key.startsWith(prefix) && key.endsWith(']')) { + values[key.slice(prefix.length, -1)] = value + } + } + + return values +} + +export function buildSpecFields (datasetFields = []) { + const fields = new Set(datasetFields) + return [...fields].sort() +} + +export function buildColumnMappingRows ({ columnFieldLog = [], userColumnMapping = {} }) { + const entries = columnFieldLog + const rows = [] + + entries.forEach(entry => { + const column = entry?.column + if (!column) { + if (entry?.field) { + rows.push({ + column: '', + field: entry.field, + isMapped: false, + isMissing: entry.missing, + userDefined: false, + userIgnored: false + }) + } + return + } + + const userMappedField = userColumnMapping[column] + const isDetectedGeometryMapping = entry?.detectedGeometryMapping === true + const isLockedDetectedGeometryMapping = LOCK_DETECTED_GEOMETRY_MAPPINGS && isDetectedGeometryMapping + const field = entry.field + const isMapped = Boolean(field) && Boolean(column) + rows.push({ + column, + field, + isMapped, + isAutoMapped: isMapped && (isLockedDetectedGeometryMapping || (!userMappedField && !isDetectedGeometryMapping)), + isMissing: entry.missing, + userDefined: Boolean((userMappedField || isDetectedGeometryMapping) && !isLockedDetectedGeometryMapping) + }) + }) + + if (Object.keys(userColumnMapping).length > 0) { + rows.forEach(row => { + if (!row.column) { + row.userIgnored = true + } + }) + } + + return rows +} + +export function applyDetectedGeometryColumnMapping (columnFieldLog = [], responseRows = []) { + const detectedMapping = detectGeometryColumnMapping(columnFieldLog, responseRows) + if (Object.keys(detectedMapping).length === 0) return columnFieldLog + + const [[detectedColumn, detectedField]] = Object.entries(detectedMapping) + return columnFieldLog.map(entry => { + if (entry?.field !== detectedField) return entry + + return { + ...entry, + column: detectedColumn, + detectedGeometryMapping: true, + missing: false + } + }) +} + +export function detectGeometryColumnMapping (columnFieldLog = [], responseRows = []) { + const expectedGeometryFields = new Set( + columnFieldLog + .map(entry => entry?.field) + .filter(field => GEOMETRY_FIELDS.includes(field)) + ) + + if (expectedGeometryFields.size === 0) return {} + + const alreadyMapped = columnFieldLog.some(entry => + GEOMETRY_FIELDS.includes(entry?.field) && Boolean(entry?.column) + ) + if (alreadyMapped) return {} + + const detectedMapping = detectGeometryColumnFromFirstRow(responseRows, expectedGeometryFields) + if (!detectedMapping) return {} + + return { + [detectedMapping.column]: detectedMapping.field + } +} + +function applyPersistedDetectedGeometryColumnMapping (columnFieldLog = [], responseRows = [], userColumnMapping = {}) { + if (!LOCK_DETECTED_GEOMETRY_MAPPINGS) return columnFieldLog + + const firstRow = responseRows[0]?.converted_row ?? {} + const detectedUserMappings = Object.entries(userColumnMapping) + .filter(([column, field]) => GEOMETRY_FIELDS.includes(field) && getGeometryFieldForValue(firstRow[column]) === field) + + if (detectedUserMappings.length === 0) return columnFieldLog + + return columnFieldLog.map(entry => { + const isPersistedDetectedMapping = detectedUserMappings.some(([column, field]) => + entry?.column === column && entry?.field === field + ) + + return isPersistedDetectedMapping + ? { ...entry, detectedGeometryMapping: true } + : entry + }) +} + +export function detectGeometryColumnFromFirstRow (responseRows = [], expectedFields = new Set(['geometry', 'point'])) { + const firstRow = responseRows[0]?.converted_row ?? {} + + for (const [column, value] of Object.entries(firstRow)) { + const field = getGeometryFieldForValue(value) + if (field && expectedFields.has(field)) { + return { column, field } + } + } + + return null +} + +function getGeometryFieldForValue (value) { + if (typeof value !== 'string') return null + + const normalisedValue = value.trimStart().toUpperCase() + if (normalisedValue.startsWith('POINT')) return 'point' + if (normalisedValue.startsWith('POLYGON') || normalisedValue.startsWith('MULTIPOLYGON')) return 'geometry' + return null +} + +// selectable columns are converted rows that have not been auto-mapped by the system +export function buildSelectableColumns (columnMappingRows = [], responseRows = []) { + const autoMappedColumns = new Set(columnMappingRows.filter(row => row.isAutoMapped).map(row => row.column).filter(Boolean)) + const unmappedColumns = new Set() + responseRows.forEach(row => { + Object.keys(row?.converted_row ?? {}).forEach(column => { + if (!autoMappedColumns.has(column)) unmappedColumns.add(column) + }) + }) + return [...unmappedColumns].sort() +} + +export function buildExpectedFieldRows ({ columnMappingRows = [], specFields = [], requiredFields = [] }) { + const requiredFieldSet = new Set(requiredFields) + + let rows = specFields.map(field => { + const mappedRow = columnMappingRows.find(row => row.field === field && row.isMapped && !row.userIgnored) + const ignoredRow = columnMappingRows.find(row => row.field === field && row.userIgnored) + + const isAutoMapped = Boolean(mappedRow?.isMapped) && !mappedRow?.userDefined + return { + field, + column: mappedRow?.column ?? '', + isMapped: Boolean(mappedRow?.column), + isAutoMapped, + userDefined: Boolean(mappedRow?.userDefined), // if the user has explicitly mapped this field + userIgnored: Boolean(ignoredRow), + isEditable: !isAutoMapped, // shows as dropdown in the UI and also shows as an Unmapped badge + isRequired: requiredFieldSet.has(field) + } + }) + + // Business rule: geometry and point are mutually exclusive when one is already mapped. + // - If `geometry` is mapped and `point` is not, hide the `point` option. + // - If `point` is mapped and `geometry` is not, hide the `geometry` option. + // - If both are mapped or both are unmapped, leave both rows as-is. + const geometryRow = rows.find(r => r.field === 'geometry') + const pointRow = rows.find(r => r.field === 'point') + const geometryMapped = Boolean(geometryRow && geometryRow.isMapped && !geometryRow.userIgnored) + const pointMapped = Boolean(pointRow && pointRow.isMapped && !pointRow.userIgnored) + if (geometryMapped && !pointMapped) { + rows = rows.filter(r => r.field !== 'point') + } else if (pointMapped && !geometryMapped) { + rows = rows.filter(r => r.field !== 'geometry') + } + + return rows.sort((a, b) => { + const rank = (row) => { + if (row.isAutoMapped && row.isRequired) return 0 + if (row.isAutoMapped && !row.isRequired) return 1 + if (row.userDefined && row.isRequired) return 2 + if (!row.isMapped && row.isRequired) return 3 + if (row.userDefined && !row.isRequired) return 4 + return 5 + } + + if (rank(a) !== rank(b)) return rank(a) - rank(b) + return a.field.localeCompare(b.field) + }) +} + +export default ColumnMappingController diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index 15be56300..1775e0158 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -224,6 +224,27 @@ export async function setupTableParams (req, res, next) { req.locals.datasetTypology === 'geography' ? await responseDetails.getGeometries() : null + // Show column mapping if the requestParams contains a non-empty mapping object + const columnMapping = req.locals.requestParams?.column_mapping + const hasMappingObject = columnMapping && typeof columnMapping === 'object' && Object.keys(columnMapping).length > 0 + const columnMappingEnabled = columnMapping === true || hasMappingObject + req.locals.columnMappingEnabled = columnMappingEnabled + if (columnMappingEnabled) { + const requestData = req.locals.requestData + const columnMappingRows = requestData.getColumnFieldLog() + .filter(({ field, column, missing, mandatory }) => Boolean(field) && Boolean(column)) + .map(({ field, column, missing, mandatory }) => { + return { + field, + column: column || '' + } + }).map(({ field, column }) => ({ + key: { text: field }, + value: { text: String(column || '') } + })) + + req.locals.columnMappingRows = columnMappingRows + } // pagination is on the 'table' tab, so we want to ensure clicking those // links takes us to a page with the table tab *selected* const { pageNumber } = req.parsedParams @@ -278,9 +299,15 @@ export function filterOutInternalIssues (req, res, next) { export function addQualityCriteriaLevelsToIssues (req, res, next) { const { issues, issueTypes } = req + req.issues = addQualityCriteriaLevels(issues, issueTypes) + + next() +} + +export function addQualityCriteriaLevels (issues = [], issueTypes = []) { const issueTypeMap = new Map(issueTypes.map(it => [it.issue_type, it])) - req.issues = issues.map(issue => { + return issues.map(issue => { const issueType = issueTypeMap.get(issue['issue-type']) let qualityLevel = issueType ? issueType.quality_criteria_level : null @@ -294,8 +321,6 @@ export function addQualityCriteriaLevelsToIssues (req, res, next) { quality_criteria_level: qualityLevel } }) - - next() } /** diff --git a/src/controllers/statusController.js b/src/controllers/statusController.js index 3b902b93a..d768251cd 100644 --- a/src/controllers/statusController.js +++ b/src/controllers/statusController.js @@ -2,6 +2,11 @@ import PageController from './pageController.js' import { getRequestData } from '../services/asyncRequestApi.js' import { finishedProcessingStatuses } from '../utils/utils.js' import { headingTexts, messageTexts, buttonTexts, buttonAriaLabels } from '../content/statusPage.js' +import platformApi from '../services/platformApi.js' +import logger from '../utils/logger.js' +import { types } from '../utils/logging.js' +import { processSpecificationMiddlewares } from '../middleware/common.middleware.js' +import { shouldShowColumnMapping } from '../services/columnMappingDecider.js' /** * Attempts to infer how we ended up on this page. @@ -20,10 +25,58 @@ function getLastPage (req) { } class StatusController extends PageController { + middlewareSetup () { + super.middlewareSetup() + // Populate req.params and dataset, then run specification processing middlewares + this.use(async (req, res, next) => { + const requestData = await getRequestData(req.params.id) + const params = requestData?.getParams() ?? {} + try { + const { formattedData } = await platformApi.fetchDatasets({ dataset: params.dataset }) + // Bounds check TODO move to external bounds handling as in fetchOne + if (!formattedData || formattedData.length === 0) { + const error = new Error(`Dataset not found: ${params.dataset}`) + logger.warn('fetchDatasetPlatformInfo: no dataset returned', { type: types.App, dataset: params.dataset }) + return next(error) + } + const datasetInfo = formattedData[0] + req.dataset = { + collection: datasetInfo.collection, + name: datasetInfo.name, + dataset: datasetInfo.dataset, + typology: datasetInfo.typology + } + } catch (error) { + logger.warn('fetchDatasetPlatformInfo failed', { type: types.App, errorMessage: error.message, errorStack: error.stack }) + return next(error) + } + return next() + }) + // attach the standard specification processing middleware chain + processSpecificationMiddlewares.forEach(mw => this.use(mw)) + } + + async post (req, res, next) { + try { + const requestData = await getRequestData(req.params.id) + const uniqueDatasetFields = req.uniqueDatasetFields || [] + const nextStep = await shouldShowColumnMapping(requestData, uniqueDatasetFields) + ? `/check/column-mapping/${req.params.id}` + : `/check/results/${req.params.id}/1` + + res.redirect(nextStep) + } catch (error) { + next(error) + } + } + async locals (req, res, next) { try { req.form.options.data = await getRequestData(req.params.id) req.form.options.processingComplete = finishedProcessingStatuses.includes(req.form.options.data.status) + req.form.options.showColumnMapping = req.form.options.processingComplete + ? await shouldShowColumnMapping(req.form.options.data, req.uniqueDatasetFields || []) + : false req.form.options.headingTexts = headingTexts req.form.options.messageTexts = messageTexts req.form.options.buttonTexts = buttonTexts @@ -44,4 +97,6 @@ class StatusController extends PageController { } } +export { shouldShowColumnMapping } + export default StatusController diff --git a/src/models/requestData.js b/src/models/requestData.js index ce2e63fb7..43fa97a98 100644 --- a/src/models/requestData.js +++ b/src/models/requestData.js @@ -109,7 +109,7 @@ export default class ResultData { const columnMapping = this.response.data['column-mapping'] ?? [] const taskLog = this.response.data['task-log'] ?? [] - const log = columnMapping.map(({ field, column }) => ({ field, column, missing: false })) + const log = columnMapping.map(({ field, column, mandatory }) => ({ field, column, missing: false, mandatory })) for (const task of taskLog) { if (task['task-source'] === 'column-field') { @@ -121,7 +121,12 @@ export default class ResultData { continue } if (details.field) { - log.push({ field: details.field, missing: true }) + const logEntry = log.find(entry => entry.field === details.field) + if (logEntry) { + logEntry.missing = true + } else { + log.push({ field: details.field, column: null, missing: true, mandatory: true }) + } } } } diff --git a/src/routes/api.js b/src/routes/api.js index 143d77037..546830ed6 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -1,5 +1,6 @@ import express from 'express' import { getRequestData } from '../services/asyncRequestApi.js' +import { shouldShowColumnMapping } from '../services/columnMappingDecider.js' import { getBoundaryForLpa } from '../services/boundaryService.js' import { getOsMapAccessToken } from '../services/osMapService.js' @@ -16,11 +17,24 @@ const router = express.Router() */ router.get('/status/:result_id', async (req, res) => { res.set('Cache-Control', 'no-store') - const response = getRequestData(req.params.result_id) - .then(data => res.json(data)) - .catch(error => res.status(500).json({ error })) - - return response + try { + const resultData = await getRequestData(req.params.result_id) + // serialize the result data (plain properties) + const payload = { ...resultData } + // compute whether we should show column mapping when the request finished + if (typeof resultData.isComplete === 'function' && resultData.isComplete() && !resultData.isFailed?.()) { + try { + const show = await shouldShowColumnMapping(resultData, []) + payload.showColumnMapping = show + if (show) payload.columnMappingUrl = `/check/column-mapping/${resultData.id}` + } catch (e) { + // swallow and continue without the flag + } + } + return res.json(payload) + } catch (error) { + return res.status(500).json({ error }) + } }) /** diff --git a/src/routes/form-wizard/check/steps.js b/src/routes/form-wizard/check/steps.js index 9396ee3ec..fbf4c90fc 100644 --- a/src/routes/form-wizard/check/steps.js +++ b/src/routes/form-wizard/check/steps.js @@ -10,6 +10,7 @@ import shareResultsController from '../../../controllers/ShareResultsController. import issueDetailsController from '../../../controllers/issueDetailsController.js' import checkStartController from '../../../controllers/checkStartController.js' import checkConfirmationController from '../../../controllers/checkConfirmationController.js' +import columnMappingController from '../../../controllers/columnMappingController.js' const baseSettings = { controller: PageController, @@ -82,6 +83,14 @@ export default { entryPoint: true, next: (req, res) => `results/${req.params.id}/1` }, + '/column-mapping/:id': { + ...baseSettings, + template: 'check/column-mapping.html', + controller: columnMappingController, + checkJourney: false, + entryPoint: true, + next: (req, res) => `results/${req.params.id}/1` + }, '/results/:id/share': { ...baseSettings, template: 'check/results/shareResults.html', diff --git a/src/services/asyncRequestApi.js b/src/services/asyncRequestApi.js index 306347f60..21da16286 100644 --- a/src/services/asyncRequestApi.js +++ b/src/services/asyncRequestApi.js @@ -33,6 +33,10 @@ export const postUrlRequest = async (formData) => { }) } +export const postCheckRequest = async (params) => { + return await postRequest(params) +} + /** * POSTs a requeset to the 'publish' API. * diff --git a/src/services/columnMappingDecider.js b/src/services/columnMappingDecider.js new file mode 100644 index 000000000..9231d6d03 --- /dev/null +++ b/src/services/columnMappingDecider.js @@ -0,0 +1,110 @@ +import datasette from '../services/datasette.js' +import { isStatutoryDataset } from '../utils/redisLoader.js' +import { addQualityCriteriaLevels } from '../controllers/resultsController.js' + +export async function shouldShowColumnMapping (requestData, uniqueDatasetFields = []) { + if (!requestData?.isComplete?.() || requestData?.isFailed?.()) { + return false + } + + const params = requestData.getParams?.() ?? {} + + try { + if (await isStatutoryDataset({ + organisation: params.organisationName, + dataset: params.dataset + })) { + return false + } + if (await hasBlockingNonColumnMappingTasks(requestData)) return false + + let columnMapping = requestData.getColumnFieldLog?.() ?? [] + + if (uniqueDatasetFields.length > 0) { + columnMapping = columnMapping.filter(entry => uniqueDatasetFields.includes(entry?.field)) + } + let allFieldsInDataset = columnMapping.map(column => column?.field).filter(Boolean) + const geometryItem = columnMapping.find(column => column?.field?.toLowerCase() === 'geometry') ?? null + const pointFieldItem = columnMapping.find(column => column?.field?.toLowerCase() === 'point') ?? null + const geometryMapped = Boolean(geometryItem?.column) + const pointMapped = Boolean(pointFieldItem?.column) + if (geometryMapped && !pointMapped) { + allFieldsInDataset = allFieldsInDataset.filter(field => field.toLowerCase() !== 'point') + } else if (pointMapped && !geometryMapped) { + allFieldsInDataset = allFieldsInDataset.filter(field => field.toLowerCase() !== 'geometry') + } + if (allFieldsInDataset.length === 0) return false + + const userColumnMapping = params.column_mapping ?? {} + const hasUserColumnMapping = Object.values(userColumnMapping).some(value => Boolean(value)) + if (hasUserColumnMapping) return false + + const responseDetails = await requestData.fetchResponseDetails(0, 50) + const rows = responseDetails.getRows?.() ?? [] + const fieldsMappedByUser = buildMappedFields(columnMapping, userColumnMapping) + + const unmappedDatasetFields = new Set(allFieldsInDataset.filter(field => !fieldsMappedByUser.has(field))) + + const hasUnmappedExpectedFields = unmappedDatasetFields.size > 0 + if (!hasUnmappedExpectedFields) return false + + const spareUploadedColumns = buildSpareUploadedColumns(columnMapping, rows, userColumnMapping) + if (spareUploadedColumns.length === 0) return false + + const hasMissingMandatoryFields = columnMapping.some(column => column?.mandatory && !column?.column && !fieldsMappedByUser.has(column.field)) + if (hasMissingMandatoryFields) return true + + return hasUnmappedExpectedFields + } catch { + return false + } +} + +export async function hasBlockingNonColumnMappingTasks (requestData) { + const issueTasks = requestData.getIssueTasks?.() ?? [] + if (issueTasks.length === 0) return false + + const { formattedData: issueTypes } = await datasette.runQuery(` + select issue_type, quality_criteria_level + from issue_type + `) + + return addQualityCriteriaLevels(issueTasks, issueTypes).some(issue => + issue.responsibility !== 'internal' && + issue.quality_criteria_level === 2 && + !['missing column', 'missing-field'].includes(issue['issue-type']) + ) +} + +function buildMappedFields (columnMapping = [], userColumnMapping = {}) { + const fields = new Set() + + columnMapping.forEach(column => { + if (column?.field && column?.column && !column?.missing) fields.add(column.field) + }) + + Object.values(userColumnMapping).forEach(field => { + if (field && field !== 'IGNORE') fields.add(field) + }) + + return fields +} + +function buildSpareUploadedColumns (columnMapping = [], rows = [], userColumnMapping = {}) { + const mappedColumns = new Set(columnMapping.map(column => column?.column).filter(Boolean)) + Object.entries(userColumnMapping).forEach(([column, field]) => { + if (field) mappedColumns.add(column) + }) + + const uploadedColumns = new Set() + rows.forEach(row => { + Object.keys(row?.converted_row ?? {}).forEach(column => uploadedColumns.add(column)) + }) + + return [...uploadedColumns].filter(column => !mappedColumns.has(column)) +} + +export default { + shouldShowColumnMapping, + hasBlockingNonColumnMappingTasks +} diff --git a/src/services/jiraService.js b/src/services/jiraService.js index f9c40373e..619a376dc 100644 --- a/src/services/jiraService.js +++ b/src/services/jiraService.js @@ -90,3 +90,32 @@ export async function attachFileToIssue (issueKey, file, additionalComment = 'Pl return attachment } + +/** + * Adds an internal note (non-public comment) to an existing JIRA Service Desk issue. + * + * @doc https://developer.atlassian.com/cloud/jira/service-desk/rest/api-group-request/#api-rest-servicedeskapi-request-issueidorkey-comment-post + * @param {string} issueKey - The key of the issue to which the note will be added. + * @param {string} note - The internal note body text. + * @returns {Promise} - A promise that resolves to the response of the JIRA API. + * @throws {Error} - Throws an error if JIRA_URL, JIRA_API_KEY, or JIRA_SERVICE_DESK_ID are not set. + */ +export async function addInternalNoteToIssue (issueKey, note) { + const JIRA_URL = process.env.JIRA_URL + const JIRA_API_KEY = process.env.JIRA_API_KEY + const JIRA_SERVICE_DESK_ID = process.env.JIRA_SERVICE_DESK_ID + + if (!JIRA_URL || !JIRA_API_KEY || !JIRA_SERVICE_DESK_ID) { + throw new Error('JIRA_URL, JIRA_API_KEY and JIRA_SERVICE_DESK_ID must be set') + } + + return await axios.post(`${JIRA_URL}/rest/servicedeskapi/request/${issueKey}/comment`, { + body: note, + public: false + }, { + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${JIRA_API_KEY}` + } + }) +} diff --git a/src/utils/redisLoader.js b/src/utils/redisLoader.js index a848a474c..f83479273 100644 --- a/src/utils/redisLoader.js +++ b/src/utils/redisLoader.js @@ -2,6 +2,7 @@ import config from '../../config/index.js' import { createClient } from 'redis' import logger from '../utils/logger.js' +import datasette from '../services/datasette.js' let redisClient @@ -36,7 +37,66 @@ export async function getRedisClient () { return redisClient } -const CACHE_TTL = 300 // 5min +const CACHE_TTL = 60 * 60 * 6 // 6 hours + +function escapeSqlString (value) { + return String(value).replaceAll("'", "''") +} + +async function getCachedJson (key, logPrefix) { + const client = await getRedisClient() + if (!client) return undefined + + try { + const cached = await client.get(cacheKey(key)) + if (cached) return JSON.parse(cached) + } catch (err) { + logger.warn(`${logPrefix}/redis get error: ${err.message}`) + } + + return undefined +} + +async function setCachedJson (key, value, logPrefix, ttl = CACHE_TTL) { + const client = await getRedisClient() + if (!client) return + + try { + await client.setEx(cacheKey(key), ttl, JSON.stringify(value)) + } catch (err) { + logger.warn(`${logPrefix}/redis set error: ${err.message}`) + } +} + +export async function getProvisionReasonsForDataset ({ organisation, dataset }) { + if (!organisation || !dataset) return [] + + const key = `provision-reasons:${organisation}:${dataset}` + const cached = await getCachedJson(key, 'getProvisionReasonsForDataset') + if (cached) return cached + + const query = ` + select provision_reason from provision + where organisation = '${escapeSqlString(organisation)}' + and dataset = '${escapeSqlString(dataset)}' + and ( + end_date is null + or end_date = '') + ` + const response = await datasette.runQuery(query) + const provisionReasons = response.formattedData + .map(row => row?.provision_reason) + .filter(Boolean) + + await setCachedJson(key, provisionReasons, 'getProvisionReasonsForDataset') + + return provisionReasons +} + +export async function isStatutoryDataset ({ organisation, dataset }) { + const provisionReasons = await getProvisionReasonsForDataset({ organisation, dataset }) + return provisionReasons.includes('statutory') +} // TODO: future removal of this function in favour of using datasetNameSlug and datasetSubjectLoaded instead. export async function fetchDatasetNames (datasetKeys) { diff --git a/src/views/check/column-mapping.html b/src/views/check/column-mapping.html new file mode 100644 index 000000000..d8629bc39 --- /dev/null +++ b/src/views/check/column-mapping.html @@ -0,0 +1,66 @@ +{% extends "layouts/main.html" %} + +{% from 'govuk/components/back-link/macro.njk' import govukBackLink %} +{% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} +{% from '../components/dataset-banner.html' import datasetBanner %} +{% from './components/column-mapping.html' import columnMapping %} + +{% set serviceType = 'Check' %} +{% set pageName = 'Map your fields' %} + +{% block beforeContent %} + {{ govukBackLink({ + text: "Back", + href: options.lastPage + }) }} +{% endblock %} + +{% block content %} +
+
+ {{ datasetBanner(options.requestParams.organisationName | orgIdToName, options.requestParams.dataset) }} + + {% set columnMappingErrorKeys = options.columnMappingErrors | getkeys %} + {% if columnMappingErrorKeys | length > 0 %} + {% set errorList = [] %} + {% for row in options.mappingRows %} + {% if options.columnMappingErrors[row.field] %} + {% set _ = errorList.push({ + text: options.columnMappingErrors[row.field].text, + href: "#fieldMap-" + loop.index0 + }) %} + {% endif %} + {% endfor %} + {{ govukErrorSummary({ + titleText: "The following fields need to be mapped before you can check your data", + errorList: errorList + }) }} + {% endif %} + + + + + +

{{ pageName }}

+

We could not map all of the fields in your data to what is in the data standard.

+

For all expected fields, you need to select a field found in your data or 'not provided'.

+ +
+ + {{ columnMapping(options.mappingRows, options.uploadedColumns, options.columnMappingErrors,false) }} +
+
+ {% set datasetGuidanceUrl = options.requestParams.dataset | getDatasetGuidanceUrl %} + {% if datasetGuidanceUrl %} +

+ Find out what we expect in each field (opens in new tab) +

+ {% endif %} + + +
+ +
+
+
+{% endblock %} diff --git a/src/views/check/components/column-mapping.html b/src/views/check/components/column-mapping.html new file mode 100644 index 000000000..f7a15d2de --- /dev/null +++ b/src/views/check/components/column-mapping.html @@ -0,0 +1,99 @@ +{% from 'govuk/components/button/macro.njk' import govukButton %} +{% from "govuk/components/error-message/macro.njk" import govukErrorMessage %} +{% from "govuk/components/select/macro.njk" import govukSelect %} + + +{% macro columnMapping(mappingRows, uploadedColumns=[], errors={}, allowUnmapping=true) %} +
+
+ + + + + + + + + + + + + + + + + {% for row in mappingRows %} + {% set isEditable = row.isEditable%} + + + + + + + {% endfor %} + +
Expected fieldsFields we found in your dataStatus
+ {{ row.field }} + + {% if not isEditable %} + {{ row.column }} + {% else %} + {% set selectItems = [ + { + value: "", + text: "-- Select a field --", + selected: not row.column and not row.userIgnored + } + ] %} + {% set _ = selectItems.push({ + value: "na", + text: "Not provided", + selected: row.userIgnored + }) %} + {% for column in uploadedColumns %} + {% set _ = selectItems.push({ + value: column, + text: column, + selected: column == row.column + }) %} + {% endfor %} + {% if errors[row.field] %} + {{ govukErrorMessage({ + id: "fieldMap-" + loop.index0 + "-error", + text: errors[row.field].text + }) }} + {% endif %} + {{ govukSelect({ + id: "fieldMap-" + loop.index0, + name: "fieldMap[" + row.field + "]", + classes: "govuk-!-width-full govuk-select--error" if errors[row.field] else "govuk-!-width-full", + describedBy: "fieldMap-" + loop.index0 + "-error" if errors[row.field] else "", + label: { + text: "fields we found in your data", + classes: "govuk-visually-hidden" + }, + formGroup: { + classes: "govuk-!-margin-bottom-0" + }, + items: selectItems + }) }} + {% endif %} + + {% if not isEditable %} + {% if allowUnmapping %} + + {% else %} + Mapped + {% endif %} + {% else %} + Unmapped + {% endif %} +
+
+
+{% endmacro %} diff --git a/src/views/check/results/results.html b/src/views/check/results/results.html index 35838b0be..997064730 100644 --- a/src/views/check/results/results.html +++ b/src/views/check/results/results.html @@ -119,6 +119,31 @@

{{ govukSummaryList({ rows: rows }) }} + + {% if options.columnMappingEnabled and options.columnMappingRows and options.columnMappingRows | length > 0 %} +
+
+
+

Field mapping

+
+
+ Map again +
+
+ + {% set headerRows = [ + { + key: { text: 'Expected field' }, + value: { html: 'Field in your data' } + } + ] %} + + {% set summaryRows = headerRows.concat(options.columnMappingRows) %} + + {{ govukSummaryList({ rows: summaryRows }) }} +
+ + {% endif %} {% if options.passedChecks.length > 0 %}
diff --git a/src/views/check/statusPage/status.html b/src/views/check/statusPage/status.html index 8a3f28a4f..a429bca41 100644 --- a/src/views/check/statusPage/status.html +++ b/src/views/check/statusPage/status.html @@ -8,7 +8,11 @@ {% set serviceType = 'Check' %} {% set pageName = 'Status' %} -{% if options.processingComplete %} +{% if options.showColumnMapping %} + {% set pageContent = statusContent(options.headingTexts.columnMapping, options.messageTexts.columnMapping.primary, options.messageTexts.columnMapping.secondary) %} + {% set buttonText = buttonTexts.columnMapping %} + {% set buttonClasses = "" %} +{% elif options.processingComplete %} {% set pageContent = statusContent(options.headingTexts.checked, options.messageTexts.checked) %} {% set buttonText = buttonTexts.checked %} {% set buttonClasses = "" %} diff --git a/src/views/check/statusPage/statusContentMacro.html b/src/views/check/statusPage/statusContentMacro.html index 6b8441b36..e21da8c04 100644 --- a/src/views/check/statusPage/statusContentMacro.html +++ b/src/views/check/statusPage/statusContentMacro.html @@ -1,4 +1,5 @@ -{% macro statusContent(heading, message) %} +{% macro statusContent(heading, message, secondaryMessage = null) %}

{{heading}}

{{message}}

-{% endmacro %} \ No newline at end of file +

{{secondaryMessage}}

+{% endmacro %} diff --git a/test/unit/checkAnswersController.test.js b/test/unit/checkAnswersController.test.js index 81a2a1584..e2964db6b 100644 --- a/test/unit/checkAnswersController.test.js +++ b/test/unit/checkAnswersController.test.js @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { createCustomerRequest, attachFileToIssue } from '../../src/services/jiraService.js' +import { addInternalNoteToIssue, createCustomerRequest, attachFileToIssue } from '../../src/services/jiraService.js' import config from '../../config/index.js' import CheckAnswersController from '../../src/controllers/CheckAnswersController.js' import { getRequestData } from '../../src/services/asyncRequestApi.js' @@ -31,10 +31,11 @@ describe('CheckAnswersController', () => { form: { options: {} }, body: {} } - res = { redirect: vi.fn() } + res = { redirect: vi.fn(), json: vi.fn() } next = vi.fn() controller = new CheckAnswersController({ route: '/check-answers/:requestId' }) vi.clearAllMocks() + addInternalNoteToIssue.mockResolvedValue({ data: {} }) }) describe('locals', () => { @@ -52,6 +53,31 @@ describe('CheckAnswersController', () => { }) describe('POST to CheckAnswersController', () => { + it('should return the Manage Service link as JSON locally when Jira is not configured', async () => { + const originalEnvironment = config.environment + config.environment = 'local' + vi.stubEnv('JIRA_URL', '') + vi.stubEnv('JIRA_API_KEY', '') + vi.stubEnv('JIRA_SERVICE_DESK_ID', '') + req.sessionModel.get.mockImplementation((key) => sessionData[key]) + + try { + await controller.post(req, res, next) + + expect(createCustomerRequest).not.toHaveBeenCalled() + expect(res.json).toHaveBeenCalledWith({ + message: 'Jira is not configured for local development. Use this Manage Service link to add the data.', + manageServiceLink: expect.stringContaining(`${config.manageServiceUrl}/datamanager`) + }) + expect(res.json.mock.calls[0][0].manageServiceLink).toContain('requestId=existing-request-id') + expect(res.json.mock.calls[0][0].manageServiceLink).toContain('documentationUrl=http%3A%2F%2Fexample.com%2Fdoc') + expect(next).not.toHaveBeenCalled() + } finally { + config.environment = originalEnvironment + vi.unstubAllEnvs() + } + }) + it('should create a Jira issue and set session data on success', async () => { const issue = { issueKey: 'TEST-123' } vi.spyOn(controller, 'createJiraServiceRequest').mockResolvedValue(issue) @@ -157,6 +183,7 @@ describe('CheckAnswersController', () => { const response = { data: { issueKey: 'TEST-123' } } createCustomerRequest.mockResolvedValue(response) attachFileToIssue.mockResolvedValue({ data: {} }) + vi.spyOn(controller, 'attachFileToIssue').mockResolvedValue() const result = await controller.createJiraServiceRequest(req, res, next) diff --git a/test/unit/columnMappingController.test.js b/test/unit/columnMappingController.test.js new file mode 100644 index 000000000..20c40249a --- /dev/null +++ b/test/unit/columnMappingController.test.js @@ -0,0 +1,576 @@ +import { describe, expect, it } from 'vitest' +import { + applySubmittedFieldSelections, + applyDetectedGeometryColumnMapping, + buildExpectedFieldRows, + buildColumnMappingRows, + buildSpecFields, + buildSubmittedColumnMapping, + buildSelectableColumns, + detectGeometryColumnMapping, + getBracketFields, + validateColumnMapping +} from '../../src/controllers/columnMappingController.js' + +describe('columnMappingController helpers', () => { + it('builds rows from mapped, missing and unmapped columns', () => { + const rows = buildColumnMappingRows({ + columnFieldLog: [ + { column: 'Reference', field: 'reference', mandatory: true, missing: false }, + { column: 'Name', field: 'name', mandatory: false, missing: false }, + { field: 'start-date', mandatory: false, missing: true } + ], + userColumnMapping: { + Name: 'name' + } + }) + + expect(rows).toEqual([ + { + column: 'Reference', + field: 'reference', + isMapped: true, + isAutoMapped: true, + isMissing: false, + userDefined: false + }, + { + column: 'Name', + field: 'name', + isMapped: true, + isAutoMapped: false, + isMissing: false, + userDefined: true + }, + { + column: '', + field: 'start-date', + isMapped: false, + isMissing: true, + userDefined: false, + userIgnored: true + } + ]) + }) + + it('keeps missing required fields that have no uploaded column', () => { + const rows = buildColumnMappingRows({ + columnFieldLog: [ + { field: 'reference', mandatory: true, missing: true } + ], + userColumnMapping: {} + }) + + expect(rows).toEqual([ + { + column: '', + field: 'reference', + isMapped: false, + isMissing: true, + userDefined: false, + userIgnored: false + } + ]) + }) + + it('merges submitted mappings with existing mappings', () => { + const mapping = buildSubmittedColumnMapping({ + existingMapping: { + Old: 'notes', + Removed: 'name', + Reference: 'reference' + }, + body: { + map: { + Reference: 'IGNORE' + }, + fieldMap: { + reference: 'New reference', + notes: 'na' + } + } + }) + + expect(mapping).toEqual({ + Removed: 'name', + Reference: 'IGNORE', + 'New reference': 'reference', + na: 'IGNORE' + }) + }) + + it('supports literal bracketed form keys', () => { + const mapping = buildSubmittedColumnMapping({ + existingMapping: { + Old: 'notes' + }, + body: { + 'map[Old]': 'IGNORE', + 'fieldMap[reference]': 'Reference', + 'unmap[Removed]': 'yes' + } + }) + + expect(mapping).toEqual({ + Old: 'IGNORE', + Reference: 'reference' + }) + }) + + it('leaves existing mappings unchanged when fieldMap values are blank', () => { + const mapping = buildSubmittedColumnMapping({ + existingMapping: { + Description: 'description', + Geometry: 'geometry' + }, + body: { + fieldMap: { + description: '', + geometry: '', + notes: 'Notes' + }, + map: { + Description: '', + Geometry: '' + } + } + }) + + expect(mapping).toEqual({ + Description: 'description', + Geometry: 'geometry', + Notes: 'notes' + }) + }) + + it('builds spec fields from dataset fields without hardcoded defaults', () => { + expect(buildSpecFields(['notes', 'reference', 'name'])).toEqual(['name', 'notes', 'reference']) + expect(buildSpecFields([])).toEqual([]) + }) + + it('builds expected field rows and uploaded column options', () => { + const columnMappingRows = [ + { + column: 'Reference', + field: 'reference', + isMapped: true, + isAutoMapped: true, + userDefined: false, + userIgnored: false + }, + { + column: 'Name', + field: 'name', + isMapped: true, + userDefined: true, + userIgnored: false + } + ] + + expect(buildSelectableColumns(columnMappingRows, [{ converted_row: { Reference: 'abc', Name: 'Test name' } }])).toEqual(['Name']) + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['notes', 'reference', 'name'], + requiredFields: ['reference'] + })).toEqual([ + { + field: 'reference', + column: 'Reference', + isMapped: true, + isAutoMapped: true, + isEditable: false, + userDefined: false, + userIgnored: false, + isRequired: true + }, + { + field: 'name', + column: 'Name', + isMapped: true, + isAutoMapped: false, + isEditable: true, + userDefined: true, + userIgnored: false, + isRequired: false + }, + { + field: 'notes', + column: '', + isMapped: false, + isAutoMapped: false, + isEditable: true, + userDefined: false, + userIgnored: false, + isRequired: false + } + ]) + }) + + it('detects point geometry from the first data row', () => { + const columnFieldLog = [ + { field: 'point', column: null, missing: true, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ] + const rows = [ + { converted_row: { Reference: 'abc', WKT: 'POINT (-0.1 51.5)' } } + ] + + expect(detectGeometryColumnMapping(columnFieldLog, rows)).toEqual({ + WKT: 'point' + }) + expect(applyDetectedGeometryColumnMapping(columnFieldLog, rows)).toEqual([ + { field: 'point', column: 'WKT', detectedGeometryMapping: true, missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ]) + }) + + it('detects polygon geometry from the first data row', () => { + const columnFieldLog = [ + { field: 'geometry', column: null, missing: true, mandatory: true } + ] + const rows = [ + { converted_row: { Reference: 'abc', Shape: ' MULTIPOLYGON (((-0.1 51.5,-0.2 51.6)))' } } + ] + + expect(detectGeometryColumnMapping(columnFieldLog, rows)).toEqual({ + Shape: 'geometry' + }) + expect(applyDetectedGeometryColumnMapping(columnFieldLog, rows)).toEqual([ + { field: 'geometry', column: 'Shape', detectedGeometryMapping: true, missing: false, mandatory: true } + ]) + expect(detectGeometryColumnMapping(columnFieldLog, [ + { converted_row: { Boundary: 'POLYGON ((-0.1 51.5,-0.2 51.6))' } } + ])).toEqual({ + Boundary: 'geometry' + }) + }) + + it('does not detect geometry when point or geometry are not expected fields', () => { + expect(detectGeometryColumnMapping([ + { field: 'reference', column: null, missing: true, mandatory: true } + ], [ + { converted_row: { WKT: 'POINT (-0.1 51.5)' } } + ])).toEqual({}) + }) + + it('does not override an existing point or geometry mapping', () => { + expect(detectGeometryColumnMapping([ + { field: 'geometry', column: 'Geometry', missing: false, mandatory: true }, + { field: 'point', column: null, missing: true, mandatory: true } + ], [ + { converted_row: { WKT: 'POINT (-0.1 51.5)' } } + ])).toEqual({}) + }) + + it('hides the other geometry field when one is mapped by column mapping', () => { + expect(buildExpectedFieldRows({ + columnMappingRows: [ + { + column: 'WKT', + field: 'geometry', + isMapped: true, + isAutoMapped: false, + userDefined: true, + userIgnored: false + }, + { + column: '', + field: 'point', + isMapped: false, + userDefined: false, + userIgnored: false + } + ], + specFields: ['geometry', 'point'], + requiredFields: ['geometry', 'point'] + }).map(row => row.field)).toEqual(['geometry']) + }) + + it('locks detected geometry mappings as mapped field rows', () => { + const columnMappingRows = buildColumnMappingRows({ + columnFieldLog: applyDetectedGeometryColumnMapping([ + { field: 'geometry', column: null, missing: true, mandatory: true }, + { field: 'point', column: null, missing: true, mandatory: true } + ], [ + { converted_row: { WKT: 'POLYGON ((-0.1 51.5,-0.2 51.6))' } } + ]), + userColumnMapping: {} + }) + + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['geometry', 'point'], + requiredFields: ['geometry', 'point'] + })).toEqual([ + { + field: 'geometry', + column: 'WKT', + isMapped: true, + isAutoMapped: true, + isEditable: false, + userDefined: false, + userIgnored: false, + isRequired: true + } + ]) + }) + + it('keeps detected geometry mappings locked after they are persisted as column mappings', () => { + const columnMappingRows = buildColumnMappingRows({ + columnFieldLog: [ + { + field: 'geometry', + column: 'WKT', + detectedGeometryMapping: true, + missing: false, + mandatory: true + } + ], + userColumnMapping: { + WKT: 'geometry' + } + }) + + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['geometry'], + requiredFields: ['geometry'] + })).toEqual([ + { + field: 'geometry', + column: 'WKT', + isMapped: true, + isAutoMapped: true, + isEditable: false, + userDefined: false, + userIgnored: false, + isRequired: true + } + ]) + }) + + it('sorts expected field rows by automapped, required, then user-defined priority', () => { + const columnMappingRows = [ + { + column: 'Alpha column', + field: 'alpha', + isMapped: true, + isAutoMapped: true, + userDefined: false, + userIgnored: false + }, + { + column: 'Beta column', + field: 'beta', + isMapped: true, + isAutoMapped: true, + userDefined: false, + userIgnored: false + }, + { + column: '', + field: 'gamma', + isMapped: false, + userDefined: false, + userIgnored: false + }, + { + column: '', + field: 'delta', + isMapped: false, + userDefined: false, + userIgnored: false + }, + { + column: 'Epsilon column', + field: 'epsilon', + isMapped: true, + userDefined: true, + userIgnored: false + } + ] + + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['delta', 'beta', 'gamma', 'epsilon', 'alpha'], + requiredFields: ['alpha', 'gamma'] + }).map(row => row.field)).toEqual([ + 'alpha', + 'beta', + 'gamma', + 'epsilon', + 'delta' + ]) + }) + + it('marks missing expected field rows as required', () => { + const columnMappingRows = [ + { + column: '', + field: 'reference', + isMapped: false, + isMissing: true, + userDefined: false, + userIgnored: false + } + ] + + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['notes', 'reference'], + requiredFields: ['reference'] + })).toEqual([ + { + field: 'reference', + column: '', + isMapped: false, + isAutoMapped: false, + isEditable: true, + userDefined: false, + userIgnored: false, + isRequired: true + }, + { + field: 'notes', + column: '', + isMapped: false, + isAutoMapped: false, + isEditable: true, + userDefined: false, + userIgnored: false, + isRequired: false + } + ]) + }) + + it('preserves ignored fields when building expected rows', () => { + const columnMappingRows = [ + { + column: '', + field: 'notes', + isMapped: false, + userDefined: false, + userIgnored: true + } + ] + + expect(buildExpectedFieldRows({ + columnMappingRows, + specFields: ['notes'], + requiredFields: [] + })).toEqual([ + { + field: 'notes', + column: '', + isMapped: false, + isAutoMapped: false, + isEditable: true, + userDefined: false, + userIgnored: true, + isRequired: false + } + ]) + }) + + it('includes ignored and spare uploaded columns in dropdown options', () => { + const rows = buildColumnMappingRows({ + columnFieldLog: [ + { column: 'Reference', field: 'reference' } + ], + userColumnMapping: { + Extra: 'IGNORE' + } + }) + + expect(buildSelectableColumns(rows, [{ converted_row: { Reference: 'abc', Notes: 'note', Extra: 'extra' } }])).toEqual(['Extra', 'Notes']) + }) + + it('extracts nested and literal bracketed fields', () => { + expect(getBracketFields({ + map: { Column: 'field' }, + 'map[Other column]': 'other-field' + }, 'map')).toEqual({ + Column: 'field', + 'Other column': 'other-field' + }) + }) + + it('validates blank field mapping selections', () => { + expect(validateColumnMapping({ + fieldMap: { + notes: '', + reference: 'Reference', + geometry: 'na' + } + })).toEqual({ + notes: { + text: 'Select the notes field' + } + }) + }) + + it('validates not provided selections for required fields', () => { + expect(validateColumnMapping({ + fieldMap: { + reference: 'na', + notes: 'na' + } + }, [ + { field: 'reference', isRequired: true }, + { field: 'notes', isRequired: false } + ])).toEqual({}) + }) + + it('applies submitted selections to mapping rows for redisplay', () => { + const rows = [ + { field: 'notes', column: '', userIgnored: false }, + { field: 'reference', column: 'Reference', userIgnored: false } + ] + + applySubmittedFieldSelections(rows, { + fieldMap: { + notes: 'Notes' + } + }) + + expect(rows).toEqual([ + { field: 'notes', column: 'Notes', userIgnored: false }, + { field: 'reference', column: 'Reference', userIgnored: false } + ]) + }) + + it('marks na submitted selections as user ignored for redisplay', () => { + const rows = [ + { field: 'notes', column: 'Notes', userIgnored: false } + ] + + applySubmittedFieldSelections(rows, { + fieldMap: { + notes: 'na' + } + }) + + expect(rows).toEqual([ + { field: 'notes', column: '', userIgnored: true } + ]) + }) + + it('marks unselected spare uploaded columns as ignored on submit', () => { + const mapping = buildSubmittedColumnMapping({ + body: { + fieldMap: { + reference: 'Reference', + notes: 'na' + } + }, + spareUploadedColumns: ['Reference', 'Notes', 'Extra'] + }) + + expect(mapping).toEqual({ + Reference: 'reference', + Notes: 'IGNORE', + Extra: 'IGNORE', + na: 'IGNORE' + }) + }) +}) diff --git a/test/unit/redisLoader.test.js b/test/unit/redisLoader.test.js index 0cfc6045e..ca04522e7 100644 --- a/test/unit/redisLoader.test.js +++ b/test/unit/redisLoader.test.js @@ -92,7 +92,7 @@ describe('getDatasetNameMap', () => { expect(mockRedisClient.get).toHaveBeenCalledWith('deploy-123:dataset:example-dataset') expect(mockRedisClient.setEx).toHaveBeenCalledWith( 'deploy-123:dataset:example-dataset', - 300, + 21600, JSON.stringify({ 'example-dataset': 'Example dataset' }) ) } finally { @@ -103,4 +103,131 @@ describe('getDatasetNameMap', () => { vi.resetModules() } }) + + it('should fetch provision reasons with namespaced cache keys', async () => { + const originalDeployTime = process.env.DEPLOY_TIME + const mockRedisClient = { + isOpen: false, + connect: vi.fn().mockImplementation(async () => { + mockRedisClient.isOpen = true + }), + get: vi.fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(JSON.stringify(['statutory'])), + setEx: vi.fn().mockResolvedValue() + } + const mockDatasette = { + default: { + runQuery: vi.fn() + .mockResolvedValueOnce({ + formattedData: [{ provision_reason: 'statutory' }] + }) + } + } + + try { + process.env.DEPLOY_TIME = 'deploy-456' + + vi.resetModules() + vi.doMock('redis', () => ({ + createClient: vi.fn(() => mockRedisClient) + })) + vi.doMock('../../config/index.js', () => ({ + default: { + redis: { + secure: false, + host: 'localhost', + port: 6379 + }, + mainWebsiteUrl: config.mainWebsiteUrl + } + })) + vi.doMock('../../src/services/datasette.js', () => mockDatasette) + + const { + getProvisionReasonsForDataset, + isStatutoryDataset + } = await import('../../src/utils/redisLoader.js') + + await expect(getProvisionReasonsForDataset({ + organisation: 'local-authority:TST', + dataset: 'conservation-area' + })).resolves.toEqual(['statutory']) + await expect(isStatutoryDataset({ + organisation: 'local-authority:TST', + dataset: 'conservation-area' + })).resolves.toBe(true) + + expect(mockRedisClient.get).toHaveBeenNthCalledWith(1, 'deploy-456:provision-reasons:local-authority:TST:conservation-area') + expect(mockRedisClient.setEx).toHaveBeenNthCalledWith( + 1, + 'deploy-456:provision-reasons:local-authority:TST:conservation-area', + 21600, + JSON.stringify(['statutory']) + ) + expect(mockDatasette.default.runQuery).toHaveBeenCalledTimes(1) + } finally { + process.env.DEPLOY_TIME = originalDeployTime + vi.unmock('redis') + vi.unmock('../../config/index.js') + vi.unmock('../../src/services/datasette.js') + vi.resetModules() + } + }) + + it('should return cached provision reasons without querying Datasette', async () => { + const mockRedisClient = { + isOpen: false, + connect: vi.fn().mockImplementation(async () => { + mockRedisClient.isOpen = true + }), + get: vi.fn() + .mockResolvedValueOnce(JSON.stringify(['cached-field'])) + .mockResolvedValueOnce(JSON.stringify(['expected'])), + setEx: vi.fn().mockResolvedValue() + } + const mockDatasette = { + default: { + runQuery: vi.fn() + } + } + + try { + vi.resetModules() + vi.doMock('redis', () => ({ + createClient: vi.fn(() => mockRedisClient) + })) + vi.doMock('../../config/index.js', () => ({ + default: { + redis: { + secure: false, + host: 'localhost', + port: 6379 + }, + mainWebsiteUrl: config.mainWebsiteUrl + } + })) + vi.doMock('../../src/services/datasette.js', () => mockDatasette) + + const { + getProvisionReasonsForDataset, + isStatutoryDataset + } = await import('../../src/utils/redisLoader.js') + + await expect(getProvisionReasonsForDataset({ + organisation: 'local-authority:TST', + dataset: 'conservation-area' + })).resolves.toEqual(['cached-field']) + await expect(isStatutoryDataset({ + organisation: 'local-authority:TST', + dataset: 'conservation-area' + })).resolves.toBe(false) + expect(mockDatasette.default.runQuery).not.toHaveBeenCalled() + } finally { + vi.unmock('redis') + vi.unmock('../../config/index.js') + vi.unmock('../../src/services/datasette.js') + vi.resetModules() + } + }) }) diff --git a/test/unit/requestData.test.js b/test/unit/requestData.test.js index e4c8b84f1..72084d6e1 100644 --- a/test/unit/requestData.test.js +++ b/test/unit/requestData.test.js @@ -2,20 +2,20 @@ import RequestData from '../../src/models/requestData.js' import ResponseDetails from '../../src/models/responseDetails.js' import { describe, it, expect, vi } from 'vitest' import axios from 'axios' -import logger from '../../src/utils/logger.js' vi.mock('axios') -vi.mock('../utils/logger.js', () => { +vi.mock('../../src/utils/logger.js', () => { return { default: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), error: vi.fn() } } }) -vi.spyOn(logger, 'error') - // Tech Debt: we should write some more tests around the requestData.js file describe('RequestData', () => { describe('fetchResponseDetails', () => { @@ -31,7 +31,7 @@ describe('RequestData', () => { const response = { id: 1, - getColumnFieldLog: () => [] + getColumnMapping: () => [] } const requestData = new RequestData(response) @@ -60,7 +60,7 @@ describe('RequestData', () => { const response = { id: 1, - getColumnFieldLog: () => [] + getColumnMapping: () => [] } const requestData = new RequestData(response) @@ -89,7 +89,7 @@ describe('RequestData', () => { const response = { id: 1, - getColumnFieldLog: () => [] + getColumnMapping: () => [] } const requestData = new RequestData(response) @@ -260,8 +260,8 @@ describe('RequestData', () => { const requestData = new RequestData({ response }) expect(requestData.getColumnFieldLog()).toStrictEqual([ - { field: 'name', column: 'name', missing: false }, - { field: 'geometry', column: 'geom', missing: false } + { field: 'name', column: 'name', missing: false, mandatory: undefined }, + { field: 'geometry', column: 'geom', missing: false, mandatory: undefined } ]) }) @@ -288,8 +288,8 @@ describe('RequestData', () => { const requestData = new RequestData({ response }) expect(requestData.getColumnFieldLog()).toStrictEqual([ - { field: 'name', column: 'name', missing: false }, - { field: 'geometry', missing: true } + { field: 'name', column: 'name', missing: false, mandatory: undefined }, + { field: 'geometry', column: null, missing: true, mandatory: true } ]) }) diff --git a/test/unit/services/jiraService.test.js b/test/unit/services/jiraService.test.js index 391a58c70..3bcd985d2 100644 --- a/test/unit/services/jiraService.test.js +++ b/test/unit/services/jiraService.test.js @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import axios from 'axios' -import { attachFileToIssue, createCustomerRequest } from '../../../src/services/jiraService' +import { addInternalNoteToIssue, attachFileToIssue, createCustomerRequest } from '../../../src/services/jiraService' vi.mock('axios') @@ -252,4 +252,70 @@ describe('jiraService', () => { .toThrowError('Request failed with status code 500') }) }) + + describe('addInternalNoteToIssue', () => { + const issueKey = 'SUP-1' + const note = 'Internal triage note' + + beforeEach(() => { + process.env.JIRA_URL = 'https://jira.example.com' + process.env.JIRA_API_KEY = 'apiToken' + process.env.JIRA_SERVICE_DESK_ID = 'serviceDeskId' + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('should throw an error if JIRA_URL, JIRA_API_KEY, or JIRA_SERVICE_DESK_ID are not set', async () => { + delete process.env.JIRA_URL + + await expect(addInternalNoteToIssue(issueKey, note)) + .rejects + .toThrowError('JIRA_URL, JIRA_API_KEY and JIRA_SERVICE_DESK_ID must be set') + }) + + it('should call axios.post with correct parameters', async () => { + const mockResponse = { + data: { + id: '10000', + body: note, + public: false + } + } + axios.post.mockResolvedValue(mockResponse) + + const response = await addInternalNoteToIssue(issueKey, note) + + expect(axios.post).toHaveBeenCalledWith( + 'https://jira.example.com/rest/servicedeskapi/request/SUP-1/comment', + { + body: 'Internal triage note', + public: false + }, + { + headers: { + 'Content-Type': 'application/json', + Authorization: 'Bearer apiToken' + } + } + ) + expect(response).toBe(mockResponse) + }) + + it('should handle network errors gracefully', async () => { + axios.post.mockRejectedValue({ + response: { + status: 500, + statusText: 'Internal Server Error', + data: { error: 'Internal Server Error' } + }, + message: 'Request failed with status code 500' + }) + + await expect(addInternalNoteToIssue(issueKey, note)) + .rejects + .toThrowError('Request failed with status code 500') + }) + }) }) diff --git a/test/unit/statusController.test.js b/test/unit/statusController.test.js index d5ceaea50..029259868 100644 --- a/test/unit/statusController.test.js +++ b/test/unit/statusController.test.js @@ -1,14 +1,48 @@ -import StatusController from '../../src/controllers/statusController.js' import { describe, it, vi, expect, beforeEach } from 'vitest' +import StatusController, { shouldShowColumnMapping } from '../../src/controllers/statusController.js' +import { isStatutoryDataset } from '../../src/utils/redisLoader.js' +import datasette from '../../src/services/datasette.js' -describe('StatusController', () => { - vi.mock('@/services/asyncRequestApi.js') +vi.mock('@/services/asyncRequestApi.js') +vi.mock('../../src/utils/redisLoader.js') +vi.mock('../../src/services/datasette.js', () => ({ + default: { + runQuery: vi.fn() + } +})) + +const makeRequestData = ({ + params = { organisationName: 'local-authority:TST', dataset: 'test-dataset' }, + columnFieldLog = [], + rows = [], + issueTasks = [], + complete = true, + failed = false +} = {}) => ({ + isComplete: () => complete, + isFailed: () => failed, + getParams: () => params, + getColumnFieldLog: () => columnFieldLog, + fetchResponseDetails: vi.fn().mockResolvedValue({ + getRows: () => rows + }), + getIssueTasks: () => issueTasks +}) +describe('StatusController', () => { let asyncRequestApi let statusController beforeEach(async () => { asyncRequestApi = await import('@/services/asyncRequestApi') + vi.mocked(isStatutoryDataset).mockResolvedValue(false) + vi.mocked(datasette.runQuery).mockResolvedValue({ + formattedData: [ + { issue_type: 'invalid geometry', quality_criteria_level: 2 }, + { issue_type: 'missing-field', quality_criteria_level: 2 }, + { issue_type: 'minor formatting issue', quality_criteria_level: 3 } + ] + }) statusController = new StatusController({ route: '/status' @@ -24,7 +58,7 @@ describe('StatusController', () => { form: { options: {} }, - params: 'fake_id' + params: { id: 'fake_id' } } const res = {} @@ -40,4 +74,185 @@ describe('StatusController', () => { expect(req.form.options.data).toBe(mockResult) }) }) + + describe('post', () => { + it('redirects to column mapping when columns need mapping', async () => { + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Ref: 'abc' } }] + }) + }) + + const req = { params: { id: '123' } } + const res = { redirect: vi.fn() } + const next = vi.fn() + + await statusController.post(req, res, next) + + expect(res.redirect).toHaveBeenCalledWith('/check/column-mapping/123') + }) + + it('redirects to results when all columns are mapped', async () => { + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'name', column: 'Name', missing: false, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Name: 'Test' } }] + }) + }) + + const req = { params: { id: '123' } } + const res = { redirect: vi.fn() } + const next = vi.fn() + + await statusController.post(req, res, next) + + expect(res.redirect).toHaveBeenCalledWith('/check/results/123/1') + }) + }) + + describe('shouldShowColumnMapping', () => { + it('returns true when expected fields are unmapped and unmapped headings are available', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Ref: 'abc' } }] + }) + })).resolves.toBe(true) + }) + + it('returns false for statutory datasets', async () => { + vi.mocked(isStatutoryDataset).mockResolvedValue(true) + + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc' } }] + }) + })).resolves.toBe(false) + }) + + it('returns false when expected fields are unmapped but no unmapped headings are available', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'name', column: 'Name', missing: false, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Name: 'abc', Extra: 'extra' } }] + }) + })).resolves.toBe(false) + }) + + it('returns false when all expected fields are mapped', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'name', column: 'Name', missing: false, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Name: 'Name', Extra: 'extra' } }] + }) + })).resolves.toBe(false) + }) + + it('returns false when the user has already started mapping', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + params: { + organisationName: 'local-authority:TST', + dataset: 'test-dataset', + column_mapping: { + na: 'IGNORE' + } + }, + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Ref: 'abc' } }] + }) + })).resolves.toBe(false) + }) + + it('returns false when there are level 2 external issue tasks', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'description', column: null, missing: true, mandatory: false } + ], + rows: [{ converted_row: { Reference: 'abc', Notes: 'note', Extra: 'extra' } }], + issueTasks: [{ + severity: 'error', + responsibility: 'external', + 'issue-type': 'invalid geometry' + }] + }) + })).resolves.toBe(false) + }) + + it('does not block when external issue tasks are level 3', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'description', column: null, missing: true, mandatory: false } + ], + rows: [{ converted_row: { Reference: 'abc', Notes: 'note', Extra: 'extra' } }], + issueTasks: [{ + severity: 'error', + responsibility: 'external', + 'issue-type': 'minor formatting issue' + }] + }) + })).resolves.toBe(true) + }) + + it('does not block when issue-type is missing-field', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Ref: 'abc' } }], + issueTasks: [{ + severity: 'error', + responsibility: 'external', + 'issue-type': 'missing-field' + }] + }) + })).resolves.toBe(true) + }) + + it('does not block when issues are internal', async () => { + await expect(shouldShowColumnMapping({ + ...makeRequestData({ + columnFieldLog: [ + { field: 'reference', column: 'Reference', missing: false, mandatory: true }, + { field: 'geometry', column: null, missing: true, mandatory: true } + ], + rows: [{ converted_row: { Reference: 'abc', Ref: 'abc' } }], + issueTasks: [{ + severity: 'error', + responsibility: 'internal', + 'issue-type': 'invalid geometry' + }] + }) + })).resolves.toBe(true) + }) + }) }) diff --git a/test/unit/statusPage.test.js b/test/unit/statusPage.test.js index 422a9e052..287e1431d 100644 --- a/test/unit/statusPage.test.js +++ b/test/unit/statusPage.test.js @@ -6,6 +6,7 @@ describe('StatusPage', () => { let statusPage let mockHeading let mockMessage + let mockSecondaryMessage let mockButton beforeEach(async () => { @@ -34,6 +35,7 @@ describe('StatusPage', () => { mockHeading = { textContent: 'Checking File' } mockButton = { textContent: 'Check latest status', style: { display: 'block' } } mockMessage = { textContent: 'Please wait', style: { display: 'block' } } + mockSecondaryMessage = { textContent: '', style: { display: 'none' } } global.window = { addEventListener: vi.fn() } @@ -46,6 +48,8 @@ describe('StatusPage', () => { return mockButton case '#js-async-processing-message': return mockMessage + case '#js-async-processing-secondary-message': + return mockSecondaryMessage default: return null } @@ -77,6 +81,25 @@ describe('StatusPage', () => { expect(statusPage.heading.textContent).toBe(headingTexts.checked) expect(statusPage.continueButton.style.display).toBe('block') expect(statusPage.processingMessage.style.display).toBe('none') + expect(statusPage.secondaryProcessingMessage.style.display).toBe('none') + }) + + it('should update the page when column mapping is required', async () => { + const mockResponse = { status: 'COMPLETE', showColumnMapping: true } + global.fetch.mockResolvedValueOnce({ + json: () => Promise.resolve(mockResponse) + }) + + statusPage.beginPolling('http://test.com', '123') + await vi.advanceTimersByTimeAsync(1000) + await Promise.resolve() + + expect(statusPage.heading.textContent).toBe(headingTexts.columnMapping) + expect(statusPage.processingMessage.textContent).toBe(messageTexts.columnMapping.primary) + expect(statusPage.secondaryProcessingMessage.textContent).toBe(messageTexts.columnMapping.secondary) + expect(statusPage.secondaryProcessingMessage.style.display).toBe('block') + expect(statusPage.continueButton.textContent).toBe('Map your fields') + expect(statusPage.continueButton.style.display).toBe('block') }) it('should begin polling and update the page when the status is FAILED', async () => { diff --git a/test/unit/views/check/columnMapping.test.js b/test/unit/views/check/columnMapping.test.js new file mode 100644 index 000000000..8a635a576 --- /dev/null +++ b/test/unit/views/check/columnMapping.test.js @@ -0,0 +1,212 @@ +import { describe, expect, it } from 'vitest' +import { JSDOM } from 'jsdom' +import { setupNunjucks } from '../../../../src/serverSetup/nunjucks.js' + +const nunjucks = setupNunjucks({ datasetNameMapping: new Map() }) + +describe('check/column-mapping.html', () => { + it('renders mapping rows and submit actions', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'article-4-direction' + }, + mappingRows: [ + { + field: 'reference', + column: 'Reference', + isMapped: true, + isEditable: false, + userDefined: false + }, + { + field: 'notes', + column: '', + isMapped: false, + isEditable: true, + userDefined: false + } + ], + uploadedColumns: ['Name', 'Reference'], + columnMappingErrors: { + notes: { + text: 'Select the notes field' + } + } + } + }) + + const document = new JSDOM(html, { url: 'http://localhost' }).window.document + const notesSelect = document.querySelector('select[name="fieldMap[notes]"]') + const notesRow = notesSelect.closest('tr') + + expect(document.querySelector('th').textContent).toContain('Expected fields') + expect(document.querySelector('td strong').textContent).toBe('reference') + expect(document.querySelectorAll('td')[1].textContent).toContain('Reference') + expect(document.defaultView.getComputedStyle(document.querySelector('td')).verticalAlign).toBe('middle') + expect(document.querySelector('.govuk-error-summary').textContent).toContain('Select the notes field') + expect(document.querySelector('.govuk-error-summary a').getAttribute('href')).toBe('#fieldMap-1') + expect(document.querySelector('.govuk-error-message').textContent).toContain('Select the notes field') + expect(document.querySelector('.govuk-form-group').className).toContain('govuk-!-margin-bottom-0') + expect(notesRow.className).toContain('govuk-form-group--error') + expect(notesRow.textContent).toContain('notes') + expect(document.querySelector('.govuk-form-group').className).not.toContain('govuk-form-group--error') + const guidanceLink = [...document.querySelectorAll('a')].find(link => + link.textContent.includes('Find out what we expect in each field') + ) + expect(guidanceLink).not.toBeUndefined() + expect(guidanceLink.getAttribute('href')).toBe('/guidance/specifications/article-4-direction') + expect(guidanceLink.target).toBe('_blank') + expect(guidanceLink.rel).toBe('noopener noreferrer') + expect(document.querySelector('button[form="columnMappingForm"]').textContent).toContain('Check your data') + }) + + it('does not render an error summary when there are no mapping errors', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'conservation-area' + }, + mappingRows: [ + { + field: 'notes', + column: '', + isMapped: false, + userDefined: false + } + ], + uploadedColumns: ['Name', 'Reference'], + columnMappingErrors: {} + } + }) + + const document = new JSDOM(html, { url: 'http://localhost' }).window.document + + expect(document.querySelector('.govuk-error-summary')).toBeNull() + expect(document.querySelector('.govuk-error-message')).toBeNull() + }) + + it('shows not provided for required fields', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'conservation-area' + }, + mappingRows: [ + { + field: 'reference', + column: '', + isMapped: false, + isEditable: true, + userDefined: false, + isRequired: true + } + ], + uploadedColumns: ['Reference'], + columnMappingErrors: {} + } + }) + + const document = new JSDOM(html, { url: 'https://example.test/' }).window.document + + expect(document.querySelector('option[value="na"]')).not.toBeNull() + expect(document.querySelector('option[value="na"]').textContent).toContain('Not provided') + }) + + it('preselects user-defined mapped fields in the selector', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'conservation-area' + }, + mappingRows: [ + { + field: 'notes', + column: 'Shape__Area', + isMapped: true, + userDefined: true, + isEditable: true, + isRequired: false + } + ], + uploadedColumns: ['Shape__Area', 'Reference'], + columnMappingErrors: {} + } + }) + + const document = new JSDOM(html, { url: 'https://example.test/' }).window.document + const fieldSelect = document.querySelector('select[name="fieldMap[notes]"]') + + expect(fieldSelect).not.toBeNull() + expect(fieldSelect.value).toBe('Shape__Area') + expect(document.querySelectorAll('td')[2].textContent).toContain('Unmapped') + }) + + it('shows not provided for optional fields', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'conservation-area' + }, + mappingRows: [ + { + field: 'notes', + column: '', + isMapped: false, + userDefined: false, + userIgnored: false, + isEditable: true, + isRequired: false + } + ], + uploadedColumns: ['Notes'], + columnMappingErrors: {} + } + }) + + const document = new JSDOM(html).window.document + + expect(document.querySelector('option[value="na"]').textContent).toContain('Not provided') + }) + + it('preselects not provided when field is user ignored', () => { + const html = nunjucks.render('check/column-mapping.html', { + options: { + lastPage: '/check/status/123', + requestParams: { + organisationName: 'local-authority:ABC', + dataset: 'conservation-area' + }, + mappingRows: [ + { + field: 'notes', + column: '', + isMapped: false, + userDefined: false, + userIgnored: true, + isEditable: true, + isRequired: false + } + ], + uploadedColumns: ['Notes'], + columnMappingErrors: {} + } + }) + + const document = new JSDOM(html).window.document + const fieldSelect = document.querySelector('select[name="fieldMap[notes]"]') + + expect(fieldSelect).not.toBeNull() + expect(fieldSelect.value).toBe('na') + }) +}) diff --git a/vite.config.js b/vite.config.js index 3a9f37bde..a73c1ed1c 100644 --- a/vite.config.js +++ b/vite.config.js @@ -16,7 +16,7 @@ export default defineConfig({ // If you want a coverage reports even if your tests are failing, include the reportOnFailure option reportOnFailure: true, // ignore these files - exclude: ['**/node_modules/**', '**/test/**', '**/public/**', '**/coverage/**', '**/vite.config.js'] + exclude: ['**/node_modules/**', '**/test/**', '**/public/**', '**/coverage/**', '**/vite.config.js', '**/.history/**'] }, environmentOptions: { jsdom: {