From 8de1161e47b7b47e31ea7b2a4846cdc930ac8343 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 13:02:26 +0100 Subject: [PATCH 1/2] IM-401 addressed some Sonar Issues --- .../src/adapters/esri/esriLayerAdapter.js | 10 ++++---- .../adapters/maplibre/maplibreLayerAdapter.js | 6 +++-- plugins/datasets/src/api/getStyle.js | 2 +- plugins/datasets/src/api/setData.js | 2 +- plugins/datasets/src/components/Key/Key.jsx | 1 - .../components/LayersMenu/LayersMenuRadio.jsx | 2 +- .../LayersMenu/LayersRadioGroupWrapper.jsx | 7 +++--- .../src/initialise/initialiseDatasets.js | 1 - .../src/reducers/__data__/demoDatasets.js | 23 +++++++++---------- .../src/registry/datasetDefinitionCache.js | 4 ++-- .../datasets/src/registry/datasetRegistry.js | 2 +- plugins/datasets/src/utils/bbox.js | 8 +++---- 12 files changed, 33 insertions(+), 35 deletions(-) diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js index 3cc304b5e..6be8f18b7 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js @@ -6,11 +6,10 @@ import { EsriDataset } from './registry/esriDataset.js' import { logger } from '../../../../../src/services/logger.js' export default class EsriLayerAdapter extends LayerAdapter { - constructor (mapProvider, symbolRegistry, patternRegistry) { + constructor (mapProvider) { super() this._mapProvider = mapProvider this._map = mapProvider.map - // TODO: Implement symbolRegistry and patternRegistry usage in the adapter // _vectorTileLayers is a map of datasetId to VectorTileLayer instances // it includes stand alone vectorTileLayers and vectorTileLayers that are part of a groupLayer @@ -33,7 +32,7 @@ export default class EsriLayerAdapter extends LayerAdapter { async init () { const topLevelDatasets = datasetRegistry.topLevelDatasets() // ensure the datasets are added in order - for await (const registryDataset of topLevelDatasets) { + for (const registryDataset of topLevelDatasets) { await this._addLayers(registryDataset) } @@ -111,7 +110,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } // If the group layer has no more sublayers, we need to also remove the group layer from the map - if (groupLayer && groupLayer.layers.length === 0) { + if (groupLayer?.layers.length === 0) { this._map.remove(groupLayer) delete this._groupLayers[esriGroupId] } @@ -127,7 +126,8 @@ export default class EsriLayerAdapter extends LayerAdapter { this._applyStyleLayerVisibility(registryDataset, vectorTileLayer) // Don't apply the visibility change to the parent, since the parent may have other sublayers that are visible return - } else if (visible) { + } + if (visible) { // No need to apply style layer visibility for datasets that are hidden registryDataset.sublayers.forEach(sublayer => this._applyStyleLayerVisibility(sublayer, vectorTileLayer)) } diff --git a/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index dc214ad45..d33011fca 100644 --- a/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -125,7 +125,9 @@ export default class MaplibreLayerAdapter extends LayerAdapter { if (imageId) { this._map.setLayoutProperty(symbolLayerId, 'icon-image', imageId) } - } else if (fillLayerId && this._map.getLayer(fillLayerId)) { + return + } + if (fillLayerId && this._map.getLayer(fillLayerId)) { const imageId = this._patternRegistry.getPatternImageId(registryDataset.style, mapStyle.id, this._pixelRatio) if (imageId) { this._map.setPaintProperty(fillLayerId, 'fill-pattern', imageId) @@ -163,7 +165,7 @@ export default class MaplibreLayerAdapter extends LayerAdapter { // Remove source if no other dataset is using it const sourceIsShared = datasetRegistry.topLevelDatasets() - .filter(registryDataset => registryDataset.id !== datasetId && registryDataset.sourceId === sourceId) + .filter(dataset => dataset.id !== datasetId && dataset.sourceId === sourceId) .length > 0 if (!sourceIsShared && this._map.getSource(sourceId)) { diff --git a/plugins/datasets/src/api/getStyle.js b/plugins/datasets/src/api/getStyle.js index 6ecfc39af..0f26d3401 100644 --- a/plugins/datasets/src/api/getStyle.js +++ b/plugins/datasets/src/api/getStyle.js @@ -1,7 +1,7 @@ import { logger } from '../../../../src/services/logger.js' import { datasetRegistry } from '../registry/datasetRegistry.js' -export const getStyle = ({ pluginState }, { datasetId, sublayerId } = {}) => { +export const getStyle = ({ _pluginState }, { datasetId, sublayerId } = {}) => { datasetId = sublayerId ? `${datasetId}-${sublayerId}` : datasetId const registryDataset = datasetRegistry.getDataset(datasetId) if (!registryDataset) { diff --git a/plugins/datasets/src/api/setData.js b/plugins/datasets/src/api/setData.js index f5485fc87..a63f4f640 100644 --- a/plugins/datasets/src/api/setData.js +++ b/plugins/datasets/src/api/setData.js @@ -2,7 +2,7 @@ import { logger } from '../../../../src/services/logger.js' import { datasetRegistry } from '../registry/datasetRegistry.js' import { layerAdapter } from '../adapters/loadLayerAdapter.js' -export const setData = ({ pluginState }, geojson, { datasetId }) => { +export const setData = ({ _pluginState }, geojson, { datasetId }) => { const registryDataset = datasetRegistry.getDataset(datasetId) if (!registryDataset) { logger.warn(`setData: Dataset with id ${datasetId} not found`) diff --git a/plugins/datasets/src/components/Key/Key.jsx b/plugins/datasets/src/components/Key/Key.jsx index 40d9d3ff6..2b3582d5f 100755 --- a/plugins/datasets/src/components/Key/Key.jsx +++ b/plugins/datasets/src/components/Key/Key.jsx @@ -7,7 +7,6 @@ import { datasetRegistry } from '../../registry/datasetRegistry.js' export const Key = ({ pluginConfig: { noKeyItemText }, mapState: { mapStyle }, - pluginState: { mappedDatasets }, services: { symbolRegistry, patternRegistry } }) => { const { items: keyGroups, hasGroups } = datasetRegistry.keyItems() diff --git a/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx b/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx index 2b74090ea..0ffefb89c 100644 --- a/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx +++ b/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx @@ -1,6 +1,6 @@ import { isVisibleWhen } from '../../registry/isVisibleWhen.js' -export const LayersMenuRadio = ({ menuState, menuGroupItem, checked, name, onChange }) => { +export const LayersMenuRadio = ({ menuGroupItem, name, checked, onChange }) => { const itemClass = 'im-c-datasets-layers__item govuk-radios govuk-radios--small"' const { visibleWhen } = menuGroupItem const visible = visibleWhen ? isVisibleWhen(visibleWhen) : true diff --git a/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx b/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx index 493d51430..9f12b17b6 100644 --- a/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx +++ b/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react' +import React from 'react' import { isVisibleWhen } from '../../registry/isVisibleWhen.js' import { LayersMenuRadio } from './LayersMenuRadio.jsx' @@ -10,9 +10,8 @@ export const LayersRadioGroupWrapper = ({ pluginState, menuGroup }) => { } const { menuState, dispatch } = pluginState - const [value, setValue] = useState(menuState[id]) + const value = menuState[id] const handleChange = (event) => { - setValue(event.target.value) dispatch({ type: 'UPDATE_MENU_STATE', payload: { [id]: event.target.value } }) } @@ -23,7 +22,7 @@ export const LayersRadioGroupWrapper = ({ pluginState, menuGroup }) => { {menuGroup.label} -
+
{items.map((menuGroupItem) => { - const existingDefinition = this.idToDefinitionMap.get(id) - this.definitionToInstanceMap.delete(existingDefinition) + const definition = this.idToDefinitionMap.get(id) + this.definitionToInstanceMap.delete(definition) this.idToDefinitionMap.delete(id) }) } diff --git a/plugins/datasets/src/registry/datasetRegistry.js b/plugins/datasets/src/registry/datasetRegistry.js index aebdc2d4f..8b9a2e88f 100644 --- a/plugins/datasets/src/registry/datasetRegistry.js +++ b/plugins/datasets/src/registry/datasetRegistry.js @@ -25,7 +25,7 @@ const datasetRegistry = { // createDataset defaults to a generic dataset factory function, but can be overridden by calling // attachCreateDataset, which allows the layer adapter to provide its own createDataset function, - attachCreateDataset (createDataset) { this._createDataset = createDataset }, + attachCreateDataset (newCreateDatasetFunction) { this._createDataset = newCreateDatasetFunction }, _createDataset: (datasetDefinition) => createDataset(datasetDefinition), attachMapStyle (mapStyle) { diff --git a/plugins/datasets/src/utils/bbox.js b/plugins/datasets/src/utils/bbox.js index bcb0d55e7..7fa337638 100755 --- a/plugins/datasets/src/utils/bbox.js +++ b/plugins/datasets/src/utils/bbox.js @@ -103,10 +103,10 @@ export const getGeometryBbox = (geometry) => { case 'GeometryCollection': geometry.geometries.forEach(g => { const b = getGeometryBbox(g) - minX = Math.min(minX, b[0]) - minY = Math.min(minY, b[1]) - maxX = Math.max(maxX, b[2]) - maxY = Math.max(maxY, b[3]) + minX = Math.min(minX, b[0]) // west + minY = Math.min(minY, b[1]) // south + maxX = Math.max(maxX, b[2]) // east + maxY = Math.max(maxY, b[3]) // NOSONAR north }) break default: From dda7db2fa9d45b6c61ac7cf4ee6b31e0bdc51407 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 13:41:48 +0100 Subject: [PATCH 2/2] IM-401 more sonar fixes --- .../src/reducers/__data__/demoDatasets.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/plugins/datasets/src/reducers/__data__/demoDatasets.js b/plugins/datasets/src/reducers/__data__/demoDatasets.js index aeddec4d4..64cf922f8 100644 --- a/plugins/datasets/src/reducers/__data__/demoDatasets.js +++ b/plugins/datasets/src/reducers/__data__/demoDatasets.js @@ -3,16 +3,16 @@ const pointData = { features: [{ type: 'Feature', properties: { category: 'prehistoric' }, - geometry: { coordinates: [-2.4558622, 54.5617135], type: 'Point' } + geometry: { coordinates: [-2.4558622, 54.5617135], type: 'Point' } // NOSONAR }, { type: 'Feature', properties: { category: 'roman' }, - geometry: { coordinates: [-2.439823, 54.5525437], type: 'Point' } + geometry: { coordinates: [-2.439823, 54.5525437], type: 'Point' } // NOSONAR }, { type: 'Feature', properties: { category: 'medieval' }, - geometry: { coordinates: [-2.4481939, 54.5575261], type: 'Point' } + geometry: { coordinates: [-2.4481939, 54.5575261], type: 'Point' } // NOSONAR }] } export const datasets = [ @@ -25,7 +25,7 @@ export const datasets = [ transformRequest: (url) => url + 'TRANSFORMED', // Required maxFeatures: 50000 // Optional: evict distant features when exceeded }, - hiddenFeatures: [42], + hiddenFeatures: [42], // NOSONAR query: {}, maxFeatures: 50000, // Optional: evict distant features when exceeded minZoom: 10, @@ -219,14 +219,8 @@ export const expectedDatasetsMenuConfig = [ } ] -const landCovers = datasets[0] -const existingFields = datasets[1] -const historicMonuments = datasets[2] -const hedgeControl = datasets[3] -const landCoversMenuItem = expectedDatasetsMenuConfig[0] -const existingFieldsMenuItem = expectedDatasetsMenuConfig[1] -const historicMonumentsMenuItem = expectedDatasetsMenuConfig[2] -const hedgeControlMenuItem = expectedDatasetsMenuConfig[3] +const [landCovers, existingFields, historicMonuments, hedgeControl] = datasets +const [landCoversMenuItem, existingFieldsMenuItem, historicMonumentsMenuItem, hedgeControlMenuItem] = expectedDatasetsMenuConfig const testGroupLabel = 'Test group' export const datasetsWithGroups = [