IM-401 addressed some Sonar Issues

plugins/datasets/src/adapters/esri/esriLayerAdapter.js

@@ -6,11 +6,10 @@
 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 @@
   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 @@
     }
 
     // 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 @@
       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))
     }
@@ -189,13 +189,13 @@
       this._applyStyleLayerVisibility(registryDataset, vectorTileLayer)
       this._applyStyleLayerPaintProperties(registryDataset, vectorTileLayer)
     })
     // TODO - handle dynamic sources
   }
 
   // onMapSizeChange is not applicable to the esriLayerAdapter
   async onMapSizeChange () {}
 
   // Remaining methods are still todo
   async applyFeatureFilter (...args) {
     console.log('TODO: applyFeatureFilter', args)
   }

plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js

@@ -103,7 +103,7 @@
       this._applyFeatureFilter(registryDataset)
     })
 
     // TODO: check dynamicSources still work
     // Re-push cached data for dynamic sources
     this.dynamicSources.forEach(source => source.reapply())
   }
@@ -125,7 +125,9 @@
         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 @@
 
     // 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)) {

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) {

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`)

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()

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

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 }) => {
         <legend className='im-c-datasets-layers-group__legend'>
           {menuGroup.label}
         </legend>
-        <div class='govuk-radios govuk-radios--small' data-module='govuk-radios'>
+        <div className='govuk-radios govuk-radios--small' data-module='govuk-radios'>
           {items.map((menuGroupItem) =>
             <LayersMenuRadio
               key={menuGroupItem.id}

plugins/datasets/src/initialise/initialiseDatasets.js

@@ -10,7 +10,6 @@
 export const initialiseDatasets = ({
   adapter,
   pluginConfig,
-  pluginStateRef,
   mapStyle,
   mapProvider,
   events,
@@ -48,7 +47,7 @@
       dynamicSources.set(registryDataset.id, dynamicSource)
     })
     adapter.attachDynamicSources(dynamicSources)
     // TODO - apply dynamic source defaults here, and include in mappedDatasets
     dispatch({ type: 'SET_MENU', payload: { menu } })
     dispatch({ type: 'SET_DATASETS', payload: { datasets: processedDatasets, mappedDatasets, orderedDatasets } })
     eventBus.emit('datasets:ready')

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,
@@ -98,9 +98,8 @@ export const datasets = [
     }]
   },
   {
-    id: 'existing-fields',
+    id: 'existing-fields', // NOSONAR
     label: 'Existing fields',
-    // groupLabel: 'Test group',
     filter: ['all', ['==', ['get', 'sbi'], '106223377'], ['==', ['get', 'is_dominant_land_cover'], true]],
     tiles: 'https://farming-tiles-702a60f45633.herokuapp.com/field_parcels_with_hedges/{z}/{x}/{y}',
     sourceLayer: 'field_parcels_filtered',
@@ -157,9 +156,8 @@ export const datasets = [
       }
     }]
   }, {
-    id: 'hedge-control',
+    id: 'hedge-control', // NOSONAR
     label: 'Hedge control',
-    // groupLabel: 'Test group',
     tiles: ['https://farming-tiles-702a60f45633.herokuapp.com/field_parcels_with_hedges/{z}/{x}/{y}'],
     sourceLayer: 'hedge_control',
     minZoom: 10,
@@ -195,9 +193,9 @@ export const expectedDatasetsMenuConfig = [
   {
     visibleWhen: true,
     type: 'checkbox',
-    id: 'existing-fields',
+    id: 'existing-fields', // NOSONAR
     items: [
-      { id: 'existing-fields', label: 'Existing fields' }
+      { id: 'existing-fields', label: 'Existing fields' } // NOSONAR
     ]
   },
   {
@@ -214,36 +212,31 @@ export const expectedDatasetsMenuConfig = [
   {
     visibleWhen: true,
     type: 'checkbox',
-    id: 'hedge-control',
+    id: 'hedge-control', // NOSONAR
     items: [
-      { id: 'hedge-control', label: 'Hedge control' }
+      { id: 'hedge-control', label: 'Hedge control' } // NOSONAR
     ]
   }
 ]
 
-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 = [
   { ...landCovers },
-  { ...existingFields, groupLabel: 'Test group' },
+  { ...existingFields, groupLabel: testGroupLabel },
   { ...historicMonuments },
-  { ...hedgeControl, groupLabel: 'Test group', visible: true }
+  { ...hedgeControl, groupLabel: testGroupLabel, visible: true }
 ]
 
 export const expectedDatasetsMenuConfigWithGroups = [
   { ...landCoversMenuItem },
   {
     visibleWhen: true,
     type: 'checkbox',
-    groupLabel: 'Test group',
-    id: 'Test group',
+    groupLabel: testGroupLabel,
+    id: testGroupLabel,
     items: [
       ...existingFieldsMenuItem.items,
       ...hedgeControlMenuItem.items

plugins/datasets/src/registry/datasetDefinitionCache.js

@@ -13,8 +13,8 @@ export class DatasetDefinitionCache {
     const existingDefinition = this.idToDefinitionMap.get(registryDataset.id)
     const allIds = [existingDefinition.id, ...(existingDefinition?.sublayerIds || [])]
     allIds.forEach(id => {
-      const existingDefinition = this.idToDefinitionMap.get(id)
-      this.definitionToInstanceMap.delete(existingDefinition)
+      const definition = this.idToDefinitionMap.get(id)
+      this.definitionToInstanceMap.delete(definition)
       this.idToDefinitionMap.delete(id)
     })
   }

plugins/datasets/src/registry/datasetRegistry.js

@@ -25,7 +25,7 @@
 
   // 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) {
@@ -125,7 +125,7 @@
 
 Object.defineProperty(datasetRegistry, 'datasets', { get: () => datasetRegistry._datasets })
 
 // TODO remove this global reference once development is finished
 window.datasetRegistry = datasetRegistry
 
 export { datasetRegistry }

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: