Skip to content

Commit

Permalink
Merge commit '70f35d8b2040f09ef86b6204fac1a66798935031' into 681-dash…
Browse files Browse the repository at this point in the history
…board-fetch-data-from-all-active-resources
  • Loading branch information
GeorgeGoodall committed Dec 12, 2024
2 parents cbe168e + 70f35d8 commit fd0e5c0
Show file tree
Hide file tree
Showing 14 changed files with 142 additions and 17 deletions.
3 changes: 2 additions & 1 deletion config/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ aws: {
s3ForcePathStyle: false,
}
# ToDo: url and name might need updating
url: 'https://check-planning.data.gov.uk'
url: 'https://submit.planning.data.gov.uk'
mainWebsiteUrl: https://www.planning.data.gov.uk
# NOTE: this property is deprecated, use serviceNames instead
serviceName: 'Submit and update your planning data'
# NOTE: the keys in this map are sometimes referred to as "serviceType" in the templates
Expand Down
1 change: 1 addition & 0 deletions config/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const ConfigSchema = v.object({
})
),
url: v.url(),
mainWebsiteUrl: v.url(),
serviceName: NonEmptyString,
serviceNames: v.object({
check: NonEmptyString,
Expand Down
7 changes: 4 additions & 3 deletions src/controllers/deepLinkController.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { NonEmptyString } from '../routes/schemas.js'

const QueryParams = v.object({
dataset: NonEmptyString,
orgName: NonEmptyString
orgName: NonEmptyString,
orgId: NonEmptyString
})

/**
Expand Down Expand Up @@ -44,7 +45,7 @@ class DeepLinkController extends PageController {
get (req, res, next) {
// if the query params don't contain what we need, redirect to the "get started" page,
// this way the user can still proceed (but need to fill the dataset+orgName themselves)
const { dataset, orgName } = req.query
const { dataset, orgName, orgId } = req.query
const validationResult = v.safeParse(QueryParams, req.query)
if (!(validationResult.success && datasets.has(dataset))) {
logger.info('DeepLinkController.get(): invalid params for deep link, redirecting to start page',
Expand All @@ -55,7 +56,7 @@ class DeepLinkController extends PageController {
req.sessionModel.set('dataset', dataset)
const datasetInfo = datasets.get(dataset) ?? { dataSubject: '', requiresGeometryTypeSelection: false }
req.sessionModel.set('data-subject', datasetInfo.dataSubject)
const sessionData = { 'data-subject': datasetInfo.dataSubject, orgName, dataset, datasetName: datasetInfo.text }
const sessionData = { 'data-subject': datasetInfo.dataSubject, orgName, orgId, dataset, datasetName: datasetInfo.text }
maybeSetReferrer(req, sessionData)
req.sessionModel.set(this.checkToolDeepLinkSessionKey, sessionData)

Expand Down
2 changes: 1 addition & 1 deletion src/filters/checkToolDeepLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
* @return {string}
*/
export function checkToolDeepLink (organisation, dataset) {
return `/check/link?dataset=${encodeURIComponent(dataset.dataset)}&orgName=${encodeURIComponent(organisation.name)}`
return `/check/link?dataset=${encodeURIComponent(dataset.dataset)}&orgName=${encodeURIComponent(organisation.name)}&orgId=${encodeURIComponent(organisation.organisation)}`
}
27 changes: 27 additions & 0 deletions src/routes/api.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
import express from 'express'
import { getRequestData } from '../services/asyncRequestApi.js'
import { getBoundaryForLpa } from '../services/boundaryService.js'

const router = express.Router()

/**
* Retrieves the status of a request by result ID.
*
* @route GET /status/:result_id
* @param {express.Request} req - The Express request object, with the `result_id` parameter specifying the ID of the request to retrieve.
* @param {express.Response} res - The Express response object.
* @returns {Promise<object>} Returns a JSON object with the request data if successful.
* If an error occurs, returns a JSON object with an error message and sets the HTTP status code to 500.
*/
router.get('/status/:result_id', async (req, res) => {
const response = getRequestData(req.params.result_id)
.then(data => res.json(data))
Expand All @@ -11,4 +21,21 @@ router.get('/status/:result_id', async (req, res) => {
return response
})

/**
* Retrieves the boundary data for a local planning authority (LPA) by boundary ID.
*
* @route GET /lpa-boundary/:boundaryId
* @param {express.Request} req - The Express request object, with the `boundaryId` parameter specifying the boundary ID in the format `datasetId:lpaId`.
* @param {express.Response} res - The Express response object.
* @returns {Promise<object>} A JSON response with the boundary data and a 200 status code if successful.
* If an error occurs, it returns a JSON response with an error message and a 500 status code.
*/
router.get('/lpa-boundary/:boundaryId', async (req, res) => {
const response = await getBoundaryForLpa(req.params.boundaryId)

return res
.status(response?.error ? 500 : 200)
.json(response)
})

export default router
32 changes: 32 additions & 0 deletions src/services/boundaryService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import axios from 'axios'
import config from '../../config/index.js'

export const getBoundaryForLpa = async (boundaryId) => {
try {
const [datasetId, lpaId] = boundaryId.split(':')
if (!datasetId || !lpaId) {
return {
error: 'Invalid boundary ID'
}
}

const response = await axios.get(`${config.mainWebsiteUrl}/entity.json?dataset=${datasetId}&reference=${lpaId}`)
const entity = response?.data?.entities?.[0] ?? null
if (!entity || !entity?.['local-planning-authority']) {
return {
error: `Failed to get boundary data for ${boundaryId}. Please ensure you are sending the correct parameters.`
}
}

const boundaryResponse = await axios.get(`${config.mainWebsiteUrl}/entity.geojson?reference=${entity['local-planning-authority']}`)
return boundaryResponse.data
} catch (error) {
return {
error: `Failed to get boundary data: ${
error.response?.status === 404
? 'LPA not found'
: `Error ${error.response?.status}: ${error.response?.statusText || 'Service temporarily unavailable'}`
}`
}
}
}
1 change: 1 addition & 0 deletions src/views/check/results/errors.html
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ <h3 class="govuk-heading-m">How to improve the data</h3>
window.serverContext = {
containerId: 'map',
geometries: {{ options.geometries | dump | safe }},
{% if options.deepLink.orgId %}boundaryGeoJsonUrl: "/api/lpa-boundary/{{ options.deepLink.orgId }}"{% endif %}
}
</script>
<script src="/public/js/map.bundle.js"></script>
Expand Down
3 changes: 2 additions & 1 deletion src/views/check/results/no-errors.html
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ <h3 class="govuk-heading-m">Before you continue</h3>
<link href='https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.css' rel='stylesheet' />
<script>
window.serverContext = {
containerId: 'map',
containerId: 'map',
geometries: {{ options.geometries | dump | safe }},
{% if options.deepLink.orgId %}boundaryGeoJsonUrl: "/api/lpa-boundary/{{ options.deepLink.orgId }}"{% endif %}
}
</script>
<script src="/public/js/map.bundle.js"></script>
Expand Down
2 changes: 1 addition & 1 deletion src/views/organisations/dataset-overview.html
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ <h2 class="govuk-heading-m">Dataset actions</h2>
window.serverContext = {
containerId: 'map',
geoJsonUrl: "https://www.planning.data.gov.uk/entity.geojson?dataset={{ dataset.dataset }}&geometry_curie=statistical-geography:{{ organisation.statistical_geography }}&limit=500",
boundaryGeoJsonUrl: "https://www.planning.data.gov.uk/entity.geojson?dataset=local-planning-authority&geometry_curie=statistical-geography:{{ organisation.statistical_geography }}"
boundaryGeoJsonUrl: "/api/lpa-boundary/{{ organisation.organisation }}"
}
</script>
<script src="/public/js/map.bundle.js"></script>
Expand Down
7 changes: 1 addition & 6 deletions src/views/organisations/datasetTaskList.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,7 @@ <h2 class="govuk-heading-l">
</h2>

{% if taskList.length == 0 %}
<p>There are no issues with {{ organisation.name }}'s {{ dataset.name }} dataset.</p>

<p><a
href="https://www.planning.data.gov.uk/entity/?dataset={{ dataset.dataset }}&geometry_curie=statistical-geography:{{ organisation.statistical_geography }}">View
the dataset on the Planning Data platform</a></p>

<p class="govuk-body">There are no issues with {{ organisation.name }}'s {{ dataset.name }} dataset.</p>
{% else %}


Expand Down
2 changes: 0 additions & 2 deletions test/unit/datasetTaskListPage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ describe(`Dataset Task List Page (seed: ${seed})`, () => {
})

const paragraphText = `There are no issues with ${organisation.name}'s ${dataset.name} dataset.`
const linkHref = `https://www.planning.data.gov.uk/entity/?dataset=${dataset.dataset}&geometry_curie=statistical-geography:${organisation.statistical_geography}`

expect(html).toContain(paragraphText)
expect(html).toContain(linkHref)
})
})
3 changes: 2 additions & 1 deletion test/unit/deepLinkController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ describe('DeepLinkController', () => {
})

it('should update session with deep link info', async () => {
const query = { dataset: 'conservation-area', orgName: 'Some Org' }
const query = { dataset: 'conservation-area', orgName: 'Some Org', orgId: 'some-org' }
const { req, res, next } = mockMiddlewareArgs({ query })

deepLinkController.get(req, res, next)

expect(req.sessionModel.get(deepLinkController.checkToolDeepLinkSessionKey)).toStrictEqual({
'data-subject': 'conservation-area',
orgName: 'Some Org',
orgId: 'some-org',
dataset: 'conservation-area',
datasetName: 'Conservation area'
})
Expand Down
67 changes: 67 additions & 0 deletions test/unit/services/boundaryService.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { describe, it, expect, vi } from 'vitest'
import axios from 'axios'
import { getBoundaryForLpa } from '../../../src/services/boundaryService.js'
import config from '../../../config/index.js'

vi.mock('axios')

describe('getBoundaryForLpa', () => {
const boundaryId = 'dataset1:lpa1'
const invalidBoundaryId = 'invalidBoundaryId'
const entityResponse = {
data: {
entities: [
{ 'local-planning-authority': 'lpa1' }
]
}
}
const boundaryResponse = {
data: { type: 'FeatureCollection', features: [] }
}

it('should return boundary data for valid boundary ID', async () => {
axios.get.mockResolvedValueOnce(entityResponse)
axios.get.mockResolvedValueOnce(boundaryResponse)

const result = await getBoundaryForLpa(boundaryId)

expect(result).toEqual(boundaryResponse.data)
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.json?dataset=dataset1&reference=lpa1`)
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.geojson?reference=lpa1`)
})

it('should return error for invalid boundary ID format', async () => {
const result = await getBoundaryForLpa(invalidBoundaryId)

expect(result).toEqual({ error: 'Invalid boundary ID' })
})

it('should return error if no entity found', async () => {
axios.get.mockResolvedValueOnce({ data: { entities: [] } })

const result = await getBoundaryForLpa(boundaryId)

expect(result).toEqual({ error: 'Failed to get boundary data for dataset1:lpa1. Please ensure you are sending the correct parameters.' })
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.json?dataset=dataset1&reference=lpa1`)
})

it('should return error if no local planning authority found', async () => {
axios.get.mockResolvedValueOnce({ data: { entities: [{}] } })

const result = await getBoundaryForLpa(boundaryId)

expect(result).toEqual({ error: 'Failed to get boundary data for dataset1:lpa1. Please ensure you are sending the correct parameters.' })
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.json?dataset=dataset1&reference=lpa1`)
})

it('should return error if failed to get boundary data', async () => {
axios.get.mockResolvedValueOnce(entityResponse)
axios.get.mockRejectedValueOnce(new Error('Network Error'))

const result = await getBoundaryForLpa(boundaryId)

expect(result).toEqual({ error: 'Failed to get boundary data: Error undefined: Service temporarily unavailable' })
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.json?dataset=dataset1&reference=lpa1`)
expect(axios.get).toHaveBeenCalledWith(`${config.mainWebsiteUrl}/entity.geojson?reference=lpa1`)
})
})
2 changes: 1 addition & 1 deletion test/unit/views/organisations/dataset-overview.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe('Dataset Overview Page', () => {
expect(header.textContent.trim()).toEqual('Dataset actions')

expect(links[0].textContent.trim()).toEqual('Check Article 4 direction area dataset')
expect(links[0].querySelector('.govuk-link').href).toEqual('/check/link?dataset=article-4-direction-area&orgName=Mock%20org')
expect(links[0].querySelector('.govuk-link').href).toEqual('/check/link?dataset=article-4-direction-area&orgName=Mock%20org&orgId=mock-org')

expect(links[1].textContent.trim()).toEqual('Article 4 direction area guidance')
expect(links[1].querySelector('.govuk-link').href).toEqual('/guidance/specifications/article-4-direction')
Expand Down

0 comments on commit fd0e5c0

Please sign in to comment.