Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/dataset slug name map #124

Merged
merged 33 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
bd53c91
get slug mapping from datasette on startup
GeorgeGoodall Jul 15, 2024
751813d
linting
GeorgeGoodall Jul 15, 2024
f3f221c
update package-lock
GeorgeGoodall Jul 15, 2024
8958b66
refactored a large amount of code to get tests working
GeorgeGoodall Jul 15, 2024
faff771
Revert "update package-lock"
GeorgeGoodall Jul 15, 2024
ea403a3
fixing webpack config as we need polyfills
GeorgeGoodall Jul 16, 2024
2055b7d
linting
GeorgeGoodall Jul 16, 2024
1431930
configuration
GeorgeGoodall Jul 16, 2024
fe84278
rewrite fetch local auth to use new datasette service
GeorgeGoodall Jul 16, 2024
9eee879
added todo comments
GeorgeGoodall Jul 17, 2024
cc13343
Merge remote-tracking branch 'origin/main' into feat/datasetSlugNameMap
GeorgeGoodall Jul 31, 2024
00fcf5c
Merge branch 'staging' into feat/datasetSlugNameMap
GeorgeGoodall Jul 31, 2024
9644b06
update ports
GeorgeGoodall Jul 31, 2024
8489812
linting
GeorgeGoodall Jul 31, 2024
5e4a514
add tests for datasette
GeorgeGoodall Jul 31, 2024
477e207
linting
GeorgeGoodall Jul 31, 2024
f65080f
updated tests for fetchLocalAuthorities
GeorgeGoodall Jul 31, 2024
59eae2a
added unit tests for getDatasetSlugNameMapping
GeorgeGoodall Jul 31, 2024
2b76c87
Merge remote-tracking branch 'origin/main' into feat/datasetSlugNameMap
GeorgeGoodall Jul 31, 2024
3ad1f62
added tests for getFullServiceName
GeorgeGoodall Jul 31, 2024
2e010a0
Merge remote-tracking branch 'origin/main' into feat/datasetSlugNameMap
GeorgeGoodall Jul 31, 2024
a589082
Merge branch 'staging' into feat/datasetSlugNameMap
GeorgeGoodall Jul 31, 2024
5874721
update workflow
GeorgeGoodall Jul 31, 2024
9a821e3
added unit tests for makeDatasetSlugToReadableNameFilter
GeorgeGoodall Jul 31, 2024
d7b20d7
added tests for the performanceDbApi
GeorgeGoodall Jul 31, 2024
fd5a0fd
Merge pull request #192 from digital-land/staging
GeorgeGoodall Aug 1, 2024
1fe4b0f
fix bug
GeorgeGoodall Aug 1, 2024
d9d5d74
Merge pull request #196 from digital-land/fix/loggerWarning
GeorgeGoodall Aug 1, 2024
5684274
Merge remote-tracking branch 'origin/main' into feat/datasetSlugNameMap
GeorgeGoodall Aug 1, 2024
960f303
revert feature deploy change
GeorgeGoodall Aug 1, 2024
cc86b73
fix test
GeorgeGoodall Aug 1, 2024
f6b351d
change expect statement
GeorgeGoodall Aug 1, 2024
ef146a4
updated tests to remove unneeded condition from performancedbapi
GeorgeGoodall Aug 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ import { setupErrorHandlers } from './src/serverSetup/errorHandlers.js'
import { setupSession } from './src/serverSetup/session.js'
import { setupNunjucks } from './src/serverSetup/nunjucks.js'
import { setupSentry } from './src/serverSetup/sentry.js'

import { dataSubjects } from './src/utils/utils.js'
import { getDatasetSlugNameMapping } from './src/utils/datasetteQueries/getDatasetSlugNameMapping.js'

dotenv.config()

const app = express()

setupMiddlewares(app)
setupSession(app)
setupNunjucks({ app, dataSubjects })
setupNunjucks({
app,
datasetNameMapping: await getDatasetSlugNameMapping()
})
setupRoutes(app)
setupSentry(app)
setupErrorHandlers(app)
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/lpaDetailsController.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import PageController from './pageController.js'
import { fetchLocalAuthorities } from '../services/fetchLocalAuthorities.js'
import { fetchLocalAuthorities } from '../utils/datasetteQueries/fetchLocalAuthorities.js'

class LpaDetailsController extends PageController {
async locals (req, res, next) {
Expand Down
5 changes: 2 additions & 3 deletions src/filters/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import validationMessageLookup from './validationMessageLookup.js'
import toErrorList from './toErrorList.js'
import prettifyColumnName from './prettifyColumnName.js'
import getFullServiceName from './getFullServiceName.js'
import { makeDatasetSlugToReadableNameFilter, createDatasetMapping } from './makeDatasetSlugToReadableNameFilter.js'
import { makeDatasetSlugToReadableNameFilter } from './makeDatasetSlugToReadableNameFilter.js'

const { govukMarkdown } = xGovFilters

const addFilters = (nunjucksEnv, { dataSubjects }) => {
const datasetNameMapping = createDatasetMapping(dataSubjects)
const addFilters = (nunjucksEnv, { datasetNameMapping }) => {
const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetNameMapping)
nunjucksEnv.addFilter('datasetSlugToReadableName', datasetSlugToReadableName)

Expand Down
11 changes: 7 additions & 4 deletions src/filters/getFullServiceName.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import config from '../../config/index.js'

export default (service) => {
const serviceName = config.serviceName

return serviceName.replace('Provide', service)
const getFullServiceName = (service) => {
if (!service || typeof service !== 'string') {
throw new Error('Service name must be a non-empty string')
}
return config.serviceName.replace('Provide', service)
}

export default getFullServiceName
Comment on lines +3 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is so we can correctly name each of the sub services.

i.e.

provide planning and housing data
submit planning and housing data
manage ...
etc

17 changes: 1 addition & 16 deletions src/filters/makeDatasetSlugToReadableNameFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,9 @@ export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => {
if (!name) {
// throw new Error(`Can't find a name for ${slug}`)
// ToDo: work out what to do here? potentially update it with data from datasette
logger.warning(`can't find a name for ${slug}`)
logger.warn(`can't find a name for ${slug}`)
return slug
}
return name
}
}

/**
*
* @param {*} dataSubjects
* @returns {Map<string,string>}
*/
export const createDatasetMapping = (dataSubjects) => {
const mapping = new Map()
for (const data of Object.values(dataSubjects)) {
for (const dataset of data.dataSets) {
mapping.set(dataset.value, dataset.text)
}
}
return mapping
}
4 changes: 2 additions & 2 deletions src/serverSetup/nunjucks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import nunjucks from 'nunjucks'
import config from '../../config/index.js'
import addFilters from '../filters/filters.js'

export function setupNunjucks ({ app, dataSubjects }) {
export function setupNunjucks ({ app, datasetNameMapping }) {
if (app) {
app.set('view engine', 'html')
}
Expand Down Expand Up @@ -33,7 +33,7 @@ export function setupNunjucks ({ app, dataSubjects }) {
Object.keys(globalValues).forEach((key) => {
nunjucksEnv.addGlobal(key, globalValues[key])
})
addFilters(nunjucksEnv, { dataSubjects })
addFilters(nunjucksEnv, { datasetNameMapping })

return nunjucks
}
31 changes: 30 additions & 1 deletion src/services/datasette.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,44 @@ const datasetteUrl = 'https://datasette.planning.data.gov.uk'
const database = 'digital-land'

export default {
/**
* Executes a SQL query on the Datasette instance and returns the results.
*
* @param {string} query - The SQL query to execute.
* @returns {Promise<{data: object, formattedData: object}>} - A promise that resolves to an object with the following properties:
* - `data`: The raw data returned by Datasette.
* - `formattedData`: The formatted data, with columns and rows parsed into a usable format.
* @throws {Error} If the query fails or there is an error communicating with Datasette.
*/
runQuery: async (query) => {
const encodedQuery = encodeURIComponent(query)
const url = `${datasetteUrl}/${database}.json?sql=${encodedQuery}`
try {
const response = await axios.get(url)
return response.data
return {
...response.data,
formattedData: formatData(response.data.columns, response.data.rows)
}
} catch (error) {
logger.warn(error)
throw error
}
}
}

/**
* Formats an array of rows into an easier to access format, where each row is an object with column names as keys.
*
* @param {string[]} columns - An array of column names
* @param {any[][]} rows - A 2D array of row data, where each inner array represents a row
* @returns {object[]} - An array of objects, where each object represents a row with column names as keys
*/
export function formatData (columns, rows) {
// convert the rows into an easier to access format
return rows.map((row) => {
return row.reduce((acc, val, index) => {
acc[columns[index]] = val
return acc
}, {})
})
}
17 changes: 4 additions & 13 deletions src/services/performanceDbApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,10 @@ ORDER BY

const result = await datasette.runQuery(query)

// convert the rows into an easier to access format
const columns = result.columns
const rows = result.rows.map((row) => {
return row.reduce((acc, val, index) => {
acc[columns[index]] = val
return acc
}, {})
})

const datasets = rows.reduce((accumulator, row) => {
const datasets = result.formattedData.reduce((accumulator, row) => {
let error
if (row.http_status !== '200' || row.exception !== '') {
error = row.exception !== '' ? row.exception : `endpoint returned with a status of ${row.http_status}`
if ((row.http_status !== '200' && row.http_status !== 200) || row.exception) {
GeorgeGoodall marked this conversation as resolved.
Show resolved Hide resolved
error = row.exception ? row.exception : `endpoint returned with a status of ${row.http_status}`
}

let issue
Expand All @@ -115,7 +106,7 @@ ORDER BY
}, {})

return {
name: result.rows[0][1],
name: result.formattedData[0].name,
datasets
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import axios from 'axios'
import logger from '../../src/utils/logger.js'
import datasette from '../../services/datasette.js'
import logger from '../logger.js'

/**
* Fetches a list of local authority names from a specified dataset.
Expand All @@ -24,15 +24,14 @@ export const fetchLocalAuthorities = async () => {
order by
provision.organisation`

const url = `https://datasette.planning.data.gov.uk/digital-land.json?sql=${encodeURIComponent(sql)}`
try {
const response = await axios.get(url)
const names = response.data.rows.map(row => {
if (row[1] === null) {
const response = await datasette.runQuery(sql)
const names = response.formattedData.map(row => {
if (row.name == null) {
logger.debug('Null value found in response:', row)
return null
} else {
return row[1]
return row.name
}
}).filter(name => name !== null) // Filter out null values
return names
Expand Down
11 changes: 11 additions & 0 deletions src/utils/datasetteQueries/getDatasetSlugNameMapping.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import datasette from '../../services/datasette.js'

export const getDatasetSlugNameMapping = async () => {
const datasetSlugNameTable = await datasette.runQuery('select dataset, name from dataset')

const datasetMapping = new Map()
datasetSlugNameTable.rows.forEach(([slug, name]) => {
datasetMapping.set(slug, name)
})
return datasetMapping
}
5 changes: 2 additions & 3 deletions test/unit/check-answers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { setupNunjucks } from '../../src/serverSetup/nunjucks.js'
import { runGenericPageTests } from './generic-page.js'
import config from '../../config/index.js'
import { stripWhitespace } from '../utils/stripWhiteSpace.js'
import { mockDataSubjects } from './data.js'

describe('check-answers View', async () => {
const params = {
Expand All @@ -19,7 +18,7 @@ describe('check-answers View', async () => {
hasLicence: 'true'
}
}
const nunjucks = setupNunjucks({ dataSubjects: mockDataSubjects })
const nunjucks = setupNunjucks({ datasetNameMapping: new Map() })
const html = stripWhitespace(nunjucks.render('check-answers.html', params))

runGenericPageTests(html, {
Expand All @@ -43,7 +42,7 @@ describe('check-answers View', async () => {
})

it('should render the dataset entered', () => {
const datasetRegex = new RegExp('<div class="govuk-summary-list__row">.*Dataset.*A Mock dataset.*Change.*</div>', 'g')
const datasetRegex = new RegExp('<div class="govuk-summary-list__row">.*Dataset.*mockDataset.*Change.*</div>', 'g')
expect(html).toMatch(datasetRegex)
})

Expand Down
7 changes: 3 additions & 4 deletions test/unit/check/confirmationPage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import { setupNunjucks } from '../../../src/serverSetup/nunjucks.js'
import { runGenericPageTests } from '../generic-page.js'
import config from '../../../config/index.js'
import { stripWhitespace } from '../../utils/stripWhiteSpace.js'
import { mockDataSubjects } from '../data.js'

const nunjucks = setupNunjucks({ dataSubjects: mockDataSubjects })
const nunjucks = setupNunjucks({ datasetNameMapping: new Map() })

describe('Check confirmation View', () => {
const params = {
Expand All @@ -18,12 +17,12 @@ describe('Check confirmation View', () => {
const html = stripWhitespace(nunjucks.render('submit/confirmation.html', params))

runGenericPageTests(html, {
pageTitle: 'A Mock dataset submitted - Submit planning and housing data for England',
pageTitle: 'mockDataset submitted - Submit planning and housing data for England',
serviceName: config.serviceName
})

it('should render the gov uk panel', () => {
const regex = new RegExp('<h1 class="govuk-panel__title".*A Mock dataset submitted.*</h1>', 'g')
const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset submitted.*</h1>', 'g')
expect(html).toMatch(regex)
})
})
2 changes: 1 addition & 1 deletion test/unit/choose-datasetPage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { runGenericPageTests } from './generic-page.js'
import config from '../../config/index.js'
import { testValidationErrorMessage } from './validation-tests.js'

const nunjucks = setupNunjucks({ dataSubjects: {} })
const nunjucks = setupNunjucks({ datasetNameMapping: new Map() })

describe('choose dataset View', () => {
const params = {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/dataset-details.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { stripWhitespace } from '../utils/stripWhiteSpace.js'
import { testValidationErrorMessage } from './validation-tests.js'
import { mockDataSubjects } from './data.js'

const nunjucks = setupNunjucks({ dataSubjects: mockDataSubjects })
const nunjucks = setupNunjucks({ datasetNameMapping: new Map() })

function errorTestFn ({
params,
Expand Down Expand Up @@ -41,7 +41,7 @@ describe('dataset details View', () => {
errors: {}
}
const html = stripWhitespace(nunjucks.render('dataset-details.html', params))
const datasetName = mockDataSubjects.mockDataset.dataSets[0].text
const datasetName = mockDataSubjects.mockDataset.dataSets[0].value
runGenericPageTests(html, {
pageTitle: `Enter ${datasetName.toLowerCase()} details - Submit planning and housing data for England`,
serviceName: config.serviceName
Expand Down
52 changes: 52 additions & 0 deletions test/unit/datasette.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import datasette, { formatData } from '../../src/services/datasette.js'
import axios from 'axios'
import { vi, describe, beforeEach, afterEach, it, expect } from 'vitest'

describe('datasette', () => {
beforeEach(() => {
vi.spyOn(axios, 'get').mockResolvedValue({
data: {
columns: ['column1', 'column2'],
rows: [
['value1', 'value2'],
['value3', 'value4']
]
}
})
})

afterEach(() => {
vi.mocked(axios.get).mockReset()
})

it('runs a SQL query and returns the results', async () => {
const query = 'SELECT * FROM table_name'
const result = await datasette.runQuery(query)

expect(result).toHaveProperty('columns')
GeorgeGoodall marked this conversation as resolved.
Show resolved Hide resolved
expect(result).toHaveProperty('formattedData')
expect(result).toHaveProperty('rows')
expect(result.formattedData).toHaveLength(2)
expect(result.formattedData[0]).toEqual({ column1: 'value1', column2: 'value2' })
expect(result.formattedData[1]).toEqual({ column1: 'value3', column2: 'value4' })
expect(result.columns).toEqual(['column1', 'column2'])
expect(result.rows[0]).toEqual(['value1', 'value2'])
expect(result.rows[1]).toEqual(['value3', 'value4'])
})

it('throws an error if the query fails', async () => {
vi.spyOn(axios, 'get').mockRejectedValue(new Error('Query failed'))

await expect(datasette.runQuery('SELECT * FROM table_name')).rejects.toThrowError('Query failed')
})

it('formats data correctly', () => {
const columns = ['column1', 'column2']
const rows = [['value1', 'value2'], ['value3', 'value4']]
const formattedData = formatData(columns, rows)

expect(formattedData).toHaveLength(2)
expect(formattedData[0]).toEqual({ column1: 'value1', column2: 'value2' })
expect(formattedData[1]).toEqual({ column1: 'value3', column2: 'value4' })
})
})
7 changes: 3 additions & 4 deletions test/unit/endpointSubmissionForm/confirmationPage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import { setupNunjucks } from '../../../src/serverSetup/nunjucks.js'
import { runGenericPageTests } from '../generic-page.js'
import config from '../../../config/index.js'
import { stripWhitespace } from '../../utils/stripWhiteSpace.js'
import { mockDataSubjects } from '../data.js'

const nunjucks = setupNunjucks({ dataSubjects: mockDataSubjects })
const nunjucks = setupNunjucks({ datasetNameMapping: new Map() })

describe('Submit confirmation View', () => {
const params = {
Expand All @@ -18,12 +17,12 @@ describe('Submit confirmation View', () => {
const html = stripWhitespace(nunjucks.render('submit/confirmation.html', params))

runGenericPageTests(html, {
pageTitle: 'A Mock dataset submitted - Submit planning and housing data for England',
pageTitle: 'mockDataset submitted - Submit planning and housing data for England',
serviceName: config.serviceName
})

it('should render the gov uk panel', () => {
const regex = new RegExp('<h1 class="govuk-panel__title".*A Mock dataset submitted.*</h1>', 'g')
const regex = new RegExp('<h1 class="govuk-panel__title".*mockDataset submitted.*</h1>', 'g')
expect(html).toMatch(regex)
})
})
Loading
Loading