From e82902356a787f45e15d53bfe165c17194663ba1 Mon Sep 17 00:00:00 2001 From: nayij Date: Wed, 7 Oct 2026 22:22:30 +0100 Subject: [PATCH 1/3] DF-1497: Add failing tests for stale grid references after update `getFeaturesManager.updateFeature` assigns both grid reference properties before it applies the new geometry, and both are derived from `feature.geometry`, so an updated point, line or shape keeps the grid reference of its previous position while its coordinates move. Adds unit tests covering add and update for each geometry type. The three grid reference assertions for `updateFeature` fail until the ordering is corrected; `getFeaturesManager` is exported so they can be driven directly. No behaviour change. --- src/client/javascripts/geospatial-map.js | 2 +- src/client/javascripts/geospatial-map.test.js | 191 ++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 src/client/javascripts/geospatial-map.test.js diff --git a/src/client/javascripts/geospatial-map.js b/src/client/javascripts/geospatial-map.js index 5414237a1..8e165e092 100644 --- a/src/client/javascripts/geospatial-map.js +++ b/src/client/javascripts/geospatial-map.js @@ -490,7 +490,7 @@ function prepareGeometry(feature) { * @param {GeoJSON} geojson * @returns {FeaturesManager} */ -function getFeaturesManager(geojson) { +export function getFeaturesManager(geojson) { /** * Get a feature from the geojson by id * @type {GetFeatures} diff --git a/src/client/javascripts/geospatial-map.test.js b/src/client/javascripts/geospatial-map.test.js new file mode 100644 index 000000000..035fa44c5 --- /dev/null +++ b/src/client/javascripts/geospatial-map.test.js @@ -0,0 +1,191 @@ +import { getFeaturesManager } from '~/src/client/javascripts/geospatial-map.js' + +/** + * @param {string} id + * @param {Geometry} geometry + * @returns {Feature} + */ +function buildFeature(id, geometry) { + return { + id, + type: 'Feature', + properties: { + description: 'Survey location', + coordinateGridReference: '', + centroidGridReference: '' + }, + geometry + } +} + +/* + * DF-1497: `updateFeature` writes both grid references before it applies the + * new geometry, so after a feature is updated the grid references still + * describe where the feature used to be. The three `updateFeature` grid + * reference tests below fail until that ordering is corrected. + */ +describe('getFeaturesManager', () => { + /** @type {GeoJSON} */ + let geojson + + beforeEach(() => { + geojson = { type: 'FeatureCollection', features: [] } + }) + + describe('addFeature', () => { + it('stores grid references for the added geometry', () => { + const { addFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'Point', + coordinates: [-0.1329571, 51.5027075] + }) + ) + + expect(geojson.features[0].properties).toEqual( + expect.objectContaining({ + coordinateGridReference: 'TQ 29684 79849', + centroidGridReference: 'TQ 29684 79849' + }) + ) + }) + }) + + describe('updateFeature', () => { + it('recalculates the grid references of an updated point', () => { + const { addFeature, updateFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'Point', + coordinates: [-0.1329571, 51.5027075] + }) + ) + + updateFeature('1', { + type: 'Point', + coordinates: [-0.1497151, 51.5025274] + }) + + expect(geojson.features[0].properties).toEqual( + expect.objectContaining({ + coordinateGridReference: 'TQ 28521 79799', + centroidGridReference: 'TQ 28521 79799' + }) + ) + }) + + it('recalculates the grid references of an updated line', () => { + const { addFeature, updateFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'LineString', + coordinates: [ + [-0.1329571, 51.5027075], + [-0.1324662, 51.5006911] + ] + }) + ) + + updateFeature('1', { + type: 'LineString', + coordinates: [ + [-0.1497151, 51.5025274], + [-0.1404592, 51.5022288] + ] + }) + + const { properties } = geojson.features[0] + + // First point of the new line + expect(properties.coordinateGridReference).toBe('TQ 28521 79799') + // Midpoint of the new line (-0.14508715, 51.5023781) + expect(properties.centroidGridReference).toBe('TQ 28843 79791') + }) + + it('recalculates the grid references of an updated shape', () => { + const { addFeature, updateFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'Polygon', + coordinates: [ + [ + [-0.1430274, 51.5012331], + [-0.1424662, 51.5006911], + [-0.1416921, 51.5010163], + [-0.1430274, 51.5012331] + ] + ] + }) + ) + + updateFeature('1', { + type: 'Polygon', + coordinates: [ + [ + [-0.1497151, 51.5025274], + [-0.1404592, 51.5022288], + [-0.1400755, 51.5020198], + [-0.1497151, 51.5025274] + ] + ] + }) + + const { properties } = geojson.features[0] + + // First corner of the new shape + expect(properties.coordinateGridReference).toBe('TQ 28521 79799') + // Centroid of the three distinct corners (-0.1434166, 51.5022587) + expect(properties.centroidGridReference).toBe('TQ 28959 79780') + }) + + it('reduces the precision of the updated geometry to 7 decimal places', () => { + const { addFeature, updateFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'Point', + coordinates: [-0.1329571, 51.5027075] + }) + ) + + updateFeature('1', { + type: 'Point', + coordinates: [-0.14971510866865856, 51.50252738875241] + }) + + expect(geojson.features[0].geometry.coordinates).toEqual([ + -0.1497151, 51.5025274 + ]) + }) + + it('returns undefined and changes nothing when the feature is unknown', () => { + const { addFeature, updateFeature } = getFeaturesManager(geojson) + + addFeature( + buildFeature('1', { + type: 'Point', + coordinates: [-0.1329571, 51.5027075] + }) + ) + + const result = updateFeature('unknown', { + type: 'Point', + coordinates: [-0.1497151, 51.5025274] + }) + + expect(result).toBeUndefined() + expect(geojson.features[0].properties.coordinateGridReference).toBe( + 'TQ 29684 79849' + ) + }) + }) +}) + +/** + * @import { GeoJSON } from '~/src/client/javascripts/geospatial-map.js' + * @import { Feature, Geometry } from '~/src/server/plugins/engine/types.js' + */ From ececaf8d0e3b3db30ee1f2ce06090180d63781e7 Mon Sep 17 00:00:00 2001 From: David Stone Date: Fri, 9 Oct 2026 13:09:04 +0100 Subject: [PATCH 2/3] Correct geometry assignment in getFeaturesManager function --- src/client/javascripts/geospatial-map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/javascripts/geospatial-map.js b/src/client/javascripts/geospatial-map.js index 8e165e092..b85d1a16b 100644 --- a/src/client/javascripts/geospatial-map.js +++ b/src/client/javascripts/geospatial-map.js @@ -528,9 +528,9 @@ export function getFeaturesManager(geojson) { // Ensure the feature exists in the geojson if (feature) { + feature.geometry = geometry feature.properties.coordinateGridReference = getCoordinateGridRef(feature) feature.properties.centroidGridReference = getCentroidGridRef(feature) - feature.geometry = geometry prepareGeometry(feature) } From 7ac40308853898d4a404d716d2601d5f26dac17f Mon Sep 17 00:00:00 2001 From: David Stone Date: Fri, 9 Oct 2026 16:00:48 +0100 Subject: [PATCH 3/3] Update coordinates and grid references in map tests for accuracy --- test/client/javascripts/map.test.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/client/javascripts/map.test.js b/test/client/javascripts/map.test.js index 3952f7611..95f8b3edf 100644 --- a/test/client/javascripts/map.test.js +++ b/test/client/javascripts/map.test.js @@ -1127,16 +1127,18 @@ describe('Maps Client JS', () => { type: 'Polygon', coordinates: [ [ - [0, 0], - [1, 1], - [2, 2] + [-0.1406682, 51.5021389], + [-0.1395772, 51.5002887], + [-0.1293066, 51.5013894], + [-0.1305105, 51.5060496], + [-0.1406682, 51.5021389] ] ] }, properties: { description: "St James' Park", - coordinateGridReference: 'TQ 29200 79762', - centroidGridReference: 'TQ 29560 79824' + coordinateGridReference: 'TQ 29150 79772', + centroidGridReference: 'TQ 29541 79818' } } @@ -1247,14 +1249,14 @@ describe('Maps Client JS', () => { // Manually invoke interactMarkerChange callback with a new point const updatedPointArgs = { - coords: [0, 0] + coords: [-1.3260764, 53.2366094] } interactMarkerChange(updatedPointArgs) expect(mockAddMarker).toHaveBeenLastCalledWith( '6d67810c-7228-4f71-b6ec-0d16b132fcd7', - [0, 0] + [-1.3260764, 53.2366094] ) expect(geospatialInput.value).toBe( @@ -1262,9 +1264,14 @@ describe('Maps Client JS', () => { [ { ...features[0], + properties: { + ...features[0].properties, + coordinateGridReference: 'SK 45077 71275', + centroidGridReference: 'SK 45077 71275' + }, geometry: { ...features[0].geometry, - coordinates: [0, 0] + coordinates: [-1.3260764, 53.2366094] } }, ...features.splice(1)