From 213e33f9fe08ee9154dc76a1a1cd81320435fa97 Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 14:52:57 +0100 Subject: [PATCH 1/6] fix disambiguation result url --- server/routes/river-and-sea-levels.js | 3 +- server/views/river-and-sea-levels-list.html | 2 +- test/routes/river-and-sea-levels.js | 163 ++++++++++++++++++++ 3 files changed, 166 insertions(+), 2 deletions(-) diff --git a/server/routes/river-and-sea-levels.js b/server/routes/river-and-sea-levels.js index 46af2b90e..bb1b0c79b 100644 --- a/server/routes/river-and-sea-levels.js +++ b/server/routes/river-and-sea-levels.js @@ -76,7 +76,8 @@ async function locationQueryHandler (request, h) { const places = await findPlaces(location) if (places.length + rivers.length > 1) { - return h.view(`${route}-list`, { model: disambiguationModel(location, places, rivers) }) + const path = places.length ? (places[0].name.toLowerCase() === location.toLowerCase() ? `/${places[0].name}` : `?q=${places[0].name}`) : null + return h.view(`${route}-list`, { model: disambiguationModel(location, places, rivers), path }) } if (places.length === 0) { diff --git a/server/views/river-and-sea-levels-list.html b/server/views/river-and-sea-levels-list.html index c9972ff3a..850c034cb 100644 --- a/server/views/river-and-sea-levels-list.html +++ b/server/views/river-and-sea-levels-list.html @@ -40,7 +40,7 @@

Rivers

{% if model.place %}

Levels near

{% endif %}

Alternatively try searching again

diff --git a/test/routes/river-and-sea-levels.js b/test/routes/river-and-sea-levels.js index e6754245a..3e7a348f0 100644 --- a/test/routes/river-and-sea-levels.js +++ b/test/routes/river-and-sea-levels.js @@ -755,9 +755,172 @@ lab.experiment('Test - /river-and-sea-levels', () => { Code.expect(response.payload).to.contain('Levels near') Code.expect(response.payload).to.contain('Rivers') Code.expect(response.payload).to.contain('More than one match was found for your location.') + Code.expect(response.payload).to.contain('') Code.expect(response.statusCode).to.equal(200) }) + lab.test('GET /river-and-sea-levels?q=avon query parameter sets path to a location url', async () => { + stubs.getJson.callsFake(() => ( + { + authenticationResultCode: 'ValidCredentials', + brandLogoUri: 'http://dev.virtualearth.net/Branding/logo_powered_by.png', + copyright: 'Copyright', + resourceSets: [ + { + estimatedTotal: 1, + resources: [ + { + __type: 'Location:http://schemas.microsoft.com/search/local/ws/rest/v1', + bbox: [ + 52.16256332397461, + -1.7445210218429565, + 52.216796875, + -1.63857901096344 + ], + name: 'Avon', + point: { + type: 'Point', + coordinates: [ + 52.19157028, + -1.70698905 + ] + }, + address: { + adminDistrict: 'England', + adminDistrict2: 'Warwickshire', + countryRegion: 'United Kingdom', + formattedAddress: 'Stratford-upon-Avon, Warwickshire', + locality: 'Stratford-upon-Avon', + countryRegionIso2: 'GB' + }, + confidence: 'High', + entityType: 'PopulatedPlace', + geocodePoints: [ + { + type: 'Point', + coordinates: [ + 52.19157028, + -1.70698905 + ], + calculationMethod: 'Rooftop', + usageTypes: [ + 'Display' + ] + } + ], + matchCodes: [ + 'Good' + ] + } + ] + } + ], + statusCode: 200, + tatusDescription: 'OK', + traceId: 'trace-id' + } + )) + stubs.getStationsWithin.callsFake(() => [{ + river_id: 'river-alne', + river_name: 'River Alne', + river_qualified_name: 'River Alne', + navigable: true, + view_rank: 1, + rank: '1', + rloi_id: 2083, + up: null, + down: 2048, + telemetry_id: '2621', + region: 'Midlands', + catchment: 'Warwickshire Avon', + wiski_river_name: 'River Alne', + agency_name: 'Henley River', + external_name: 'Henley River', + station_type: 'S', + status: 'Active', + qualifier: 'u', + iswales: false, + value: '0.414', + value_timestamp: '2022-09-26T13:30:00.000Z', + value_erred: false, + percentile_5: '0.546', + percentile_95: '0.387', + centroid: '0101000020E6100000068A4FA62670FCBF9C9AE66602264A40', + lon: -1.77738060917966, + lat: 52.29694830188711, + day_total: null, + six_hr_total: null, + one_hr_total: null, + id: '610' + }]) + stubs.getIsEngland.callsFake(() => ({ is_england: true })) + stubs.getRiversByName.callsFake(() => [ + { + local_name: 'Little Avon River', + qualified_name: 'Little Avon River', + other_names: null, + river_id: 'little-avon-river' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Bristol)', + other_names: null, + river_id: 'river-avon-bristol' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Corsham)', + other_names: null, + river_id: 'river-avon-corsham' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Devon)', + other_names: null, + river_id: 'river-avon-devon' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Hampshire)', + other_names: null, + river_id: 'river-avon-hampshire' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Warwickshire)', + other_names: null, + river_id: 'river-avon-warwickshire' + }, + { + local_name: 'Sherston Avon', + qualified_name: 'Sherston Avon', + other_names: null, + river_id: 'sherston-avon' + }, + { + local_name: 'Tetbury Avon', + qualified_name: 'Tetbury Avon', + other_names: null, + river_id: 'tetbury-avon' + } + ]) + stubs.getStationsGeoJson.callsFake(() => data.cachedRainfallStation) + + // Set cached stationsGeojson + const floodService = require('../../server/services/flood') + floodService.stationsGeojson = await floodService.getStationsGeoJson() + + const options = { + method: 'GET', + url: '/river-and-sea-levels?q=avon' + } + + const response = await server.inject(options) + + Code.expect(response.statusCode).to.equal(200) + Code.expect(response.payload).to.contain('') + }) + lab.test('GET /river-and-sea-levels?q=river query parameter returns river stations', async () => { stubs.getJson.callsFake(() => { return { From d074262d75a181c201ec4242dd11632c9055fc5b Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 15:09:21 +0100 Subject: [PATCH 2/6] remove non english disambiguation options --- server/routes/river-and-sea-levels.js | 7 +- test/routes/river-and-sea-levels.js | 103 ++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/server/routes/river-and-sea-levels.js b/server/routes/river-and-sea-levels.js index bb1b0c79b..6e509bc86 100644 --- a/server/routes/river-and-sea-levels.js +++ b/server/routes/river-and-sea-levels.js @@ -76,8 +76,11 @@ async function locationQueryHandler (request, h) { const places = await findPlaces(location) if (places.length + rivers.length > 1) { - const path = places.length ? (places[0].name.toLowerCase() === location.toLowerCase() ? `/${places[0].name}` : `?q=${places[0].name}`) : null - return h.view(`${route}-list`, { model: disambiguationModel(location, places, rivers), path }) + const place = places[0] + const englandPlaces = place?.isUK && !place?.isScotlandOrNorthernIreland ? [place] : [] + + const path = englandPlaces.length ? (englandPlaces[0].name.toLowerCase() === location.toLowerCase() ? `/${englandPlaces[0].name}` : `?q=${englandPlaces[0].name}`) : null + return h.view(`${route}-list`, { model: disambiguationModel(location, englandPlaces, rivers), path }) } if (places.length === 0) { diff --git a/test/routes/river-and-sea-levels.js b/test/routes/river-and-sea-levels.js index 3e7a348f0..9d653f1b2 100644 --- a/test/routes/river-and-sea-levels.js +++ b/test/routes/river-and-sea-levels.js @@ -921,6 +921,109 @@ lab.experiment('Test - /river-and-sea-levels', () => { Code.expect(response.payload).to.contain('') }) + lab.test('GET /river-and-sea-levels?q=avon multiple choice location shows no places if outside of england', async () => { + stubs.getJson.callsFake(() => data.scotlandGetJson) + stubs.getStationsWithin.callsFake(() => [{ + river_id: 'river-alne', + river_name: 'River Alne', + river_qualified_name: 'River Alne', + navigable: true, + view_rank: 1, + rank: '1', + rloi_id: 2083, + up: null, + down: 2048, + telemetry_id: '2621', + region: 'Midlands', + catchment: 'Warwickshire Avon', + wiski_river_name: 'River Alne', + agency_name: 'Henley River', + external_name: 'Henley River', + station_type: 'S', + status: 'Active', + qualifier: 'u', + iswales: false, + value: '0.414', + value_timestamp: '2022-09-26T13:30:00.000Z', + value_erred: false, + percentile_5: '0.546', + percentile_95: '0.387', + centroid: '0101000020E6100000068A4FA62670FCBF9C9AE66602264A40', + lon: -1.77738060917966, + lat: 52.29694830188711, + day_total: null, + six_hr_total: null, + one_hr_total: null, + id: '610' + }]) + stubs.getIsEngland.callsFake(() => ({ is_england: true })) + stubs.getRiversByName.callsFake(() => [ + { + local_name: 'Little Avon River', + qualified_name: 'Little Avon River', + other_names: null, + river_id: 'little-avon-river' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Bristol)', + other_names: null, + river_id: 'river-avon-bristol' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Corsham)', + other_names: null, + river_id: 'river-avon-corsham' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Devon)', + other_names: null, + river_id: 'river-avon-devon' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Hampshire)', + other_names: null, + river_id: 'river-avon-hampshire' + }, + { + local_name: 'River Avon', + qualified_name: 'River Avon (Warwickshire)', + other_names: null, + river_id: 'river-avon-warwickshire' + }, + { + local_name: 'Sherston Avon', + qualified_name: 'Sherston Avon', + other_names: null, + river_id: 'sherston-avon' + }, + { + local_name: 'Tetbury Avon', + qualified_name: 'Tetbury Avon', + other_names: null, + river_id: 'tetbury-avon' + } + ]) + stubs.getStationsGeoJson.callsFake(() => data.cachedRainfallStation) + + // Set cached stationsGeojson + const floodService = require('../../server/services/flood') + floodService.stationsGeojson = await floodService.getStationsGeoJson() + + const options = { + method: 'GET', + url: '/river-and-sea-levels?q=avon' + } + + const response = await server.inject(options) + + Code.expect(response.statusCode).to.equal(200) + Code.expect(response.payload).to.not.contain('Levels near') + }) + lab.test('GET /river-and-sea-levels?q=river query parameter returns river stations', async () => { stubs.getJson.callsFake(() => { return { From 805f8d261e942e7a33f01466b5de53c1cb4b6c99 Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 18:30:03 +0100 Subject: [PATCH 3/6] add util fn --- server/routes/lib/utils.js | 8 +++++++- server/routes/river-and-sea-levels.js | 11 ++++++----- test/routes/river-and-sea-levels.js | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/server/routes/lib/utils.js b/server/routes/lib/utils.js index 5dac60f57..6771cd680 100644 --- a/server/routes/lib/utils.js +++ b/server/routes/lib/utils.js @@ -5,6 +5,11 @@ function slugify (text = '') { return text.replace(/,/g, '').replace(/ /g, '-').toLowerCase() } +function getDisambiguationPath (place, location) { + if (!place) return null + return place.name.toLowerCase() === location.toLowerCase() ? `/${place.name}` : `?q=${place.name}` +} + function isLocationEngland (location) { return location.match(/^england$/i) } @@ -53,5 +58,6 @@ module.exports = { isValidLocationSlug, renderNotFound, renderLocationNotFound, - createQueryParametersString + createQueryParametersString, + getDisambiguationPath } diff --git a/server/routes/river-and-sea-levels.js b/server/routes/river-and-sea-levels.js index 6e509bc86..e8cde7583 100644 --- a/server/routes/river-and-sea-levels.js +++ b/server/routes/river-and-sea-levels.js @@ -18,6 +18,7 @@ const { renderNotFound, renderLocationNotFound, createQueryParametersString, + getDisambiguationPath, isValidLocationSlug, isLocationEngland, isPlaceEngland @@ -73,14 +74,14 @@ async function locationQueryHandler (request, h) { } const rivers = await request.server.methods.flood.getRiversByName(location) - const places = await findPlaces(location) + let places = await findPlaces(location) if (places.length + rivers.length > 1) { - const place = places[0] - const englandPlaces = place?.isUK && !place?.isScotlandOrNorthernIreland ? [place] : [] + places = isPlaceEngland(places[0]) ? places : [] - const path = englandPlaces.length ? (englandPlaces[0].name.toLowerCase() === location.toLowerCase() ? `/${englandPlaces[0].name}` : `?q=${englandPlaces[0].name}`) : null - return h.view(`${route}-list`, { model: disambiguationModel(location, englandPlaces, rivers), path }) + const path = getDisambiguationPath(places[0], location) + + return h.view(`${route}-list`, { model: disambiguationModel(location, places, rivers), path }) } if (places.length === 0) { diff --git a/test/routes/river-and-sea-levels.js b/test/routes/river-and-sea-levels.js index 9d653f1b2..9e8341aa8 100644 --- a/test/routes/river-and-sea-levels.js +++ b/test/routes/river-and-sea-levels.js @@ -956,7 +956,7 @@ lab.experiment('Test - /river-and-sea-levels', () => { one_hr_total: null, id: '610' }]) - stubs.getIsEngland.callsFake(() => ({ is_england: true })) + stubs.getIsEngland.callsFake(() => ({ is_england: false })) stubs.getRiversByName.callsFake(() => [ { local_name: 'Little Avon River', From fbccbbdbf0ab51b78fba7e6ecbe4f9f90735f1a5 Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 18:33:31 +0100 Subject: [PATCH 4/6] sonar fix 1 --- server/routes/lib/utils.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/routes/lib/utils.js b/server/routes/lib/utils.js index 6771cd680..31ef3b2dd 100644 --- a/server/routes/lib/utils.js +++ b/server/routes/lib/utils.js @@ -6,7 +6,10 @@ function slugify (text = '') { } function getDisambiguationPath (place, location) { - if (!place) return null + if (!place) { + return null + } + return place.name.toLowerCase() === location.toLowerCase() ? `/${place.name}` : `?q=${place.name}` } From 013b15f43accb32ae4ca50940798591701b53113 Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 18:38:29 +0100 Subject: [PATCH 5/6] sonar fix 2 --- server/routes/lib/utils.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/routes/lib/utils.js b/server/routes/lib/utils.js index 31ef3b2dd..6cd3dc854 100644 --- a/server/routes/lib/utils.js +++ b/server/routes/lib/utils.js @@ -5,6 +5,10 @@ function slugify (text = '') { return text.replace(/,/g, '').replace(/ /g, '-').toLowerCase() } +function filterDisambiguationPlaces (places) { + return isPlaceEngland(places[0]) ? places : [] +} + function getDisambiguationPath (place, location) { if (!place) { return null @@ -62,5 +66,6 @@ module.exports = { renderNotFound, renderLocationNotFound, createQueryParametersString, - getDisambiguationPath + getDisambiguationPath, + filterDisambiguationPlaces } From ca55d1ec1d2bcafa6bc098b5afc32c15924f020e Mon Sep 17 00:00:00 2001 From: Ash Date: Tue, 16 Jul 2024 18:39:14 +0100 Subject: [PATCH 6/6] sonar fix 2 --- server/routes/river-and-sea-levels.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/routes/river-and-sea-levels.js b/server/routes/river-and-sea-levels.js index e8cde7583..633f76786 100644 --- a/server/routes/river-and-sea-levels.js +++ b/server/routes/river-and-sea-levels.js @@ -19,6 +19,7 @@ const { renderLocationNotFound, createQueryParametersString, getDisambiguationPath, + filterDisambiguationPlaces, isValidLocationSlug, isLocationEngland, isPlaceEngland @@ -77,7 +78,7 @@ async function locationQueryHandler (request, h) { let places = await findPlaces(location) if (places.length + rivers.length > 1) { - places = isPlaceEngland(places[0]) ? places : [] + places = filterDisambiguationPlaces(places) const path = getDisambiguationPath(places[0], location)