Fix zoom level type error in custom layer (#605) (#606)

* Fix zoom level type error in custom layer

Signed-off-by: Junqiu Lei <[email protected]>
(cherry picked from commit 4d21bf0)

Co-authored-by: Junqiu Lei <[email protected]>

CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Features
 ### Enhancements
 ### Bug Fixes
+* Fix zoom level type error in custom layer ([#605](https://github.com/opensearch-project/dashboards-maps/pull/605))
 ### Infrastructure
 ### Documentation
 ### Maintenance

public/components/layer_config/layer_basic_settings.test.tsx

@@ -0,0 +1,113 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import { LayerBasicSettings } from './layer_basic_settings';
+import { MapLayerSpecification } from '../../model/mapLayerType';
+import { DASHBOARDS_MAPS_LAYER_TYPE, MAX_LAYER_NAME_LIMIT } from '../../../common';
+import { shallow } from 'enzyme';
+import { EuiDualRange, EuiFieldText, EuiFormRow, EuiRange, EuiTextArea } from '@elastic/eui';
+
+describe('LayerBasicSettings', () => {
+  let wrapper: any;
+  const mockLayerConfig: MapLayerSpecification = {
+    name: 'Test Layer',
+    id: 'test-layer',
+    type: DASHBOARDS_MAPS_LAYER_TYPE.OPENSEARCH_MAP,
+    description: 'Some description',
+    zoomRange: [1, 20],
+    visibility: 'visible',
+    opacity: 80,
+  };
+
+  const mockProps = {
+    selectedLayerConfig: mockLayerConfig,
+    setSelectedLayerConfig: jest.fn(),
+    setIsUpdateDisabled: jest.fn(),
+    isLayerExists: jest.fn(),
+  };
+
+  beforeEach(() => {
+    wrapper = shallow(<LayerBasicSettings {...mockProps} />);
+  });
+
+  it('should render with correct props', () => {
+    expect(wrapper.find(EuiFieldText).prop('value')).toBe(mockLayerConfig.name);
+    expect(wrapper.find(EuiTextArea).prop('value')).toBe(mockLayerConfig.description);
+    expect(wrapper.find(EuiDualRange).prop('value')).toEqual(mockLayerConfig.zoomRange);
+    expect(wrapper.find(EuiRange).prop('value')).toEqual(mockLayerConfig.opacity);
+  });
+
+  it('should call setSelectedLayerConfig with updated zoom range on zoom level change', () => {
+    const newZoomRange = [2, 15];
+    wrapper.find(EuiDualRange).simulate('change', newZoomRange);
+
+    expect(mockProps.setSelectedLayerConfig).toHaveBeenCalledWith({
+      ...mockLayerConfig,
+      zoomRange: newZoomRange,
+    });
+  });
+
+  it('should call setSelectedLayerConfig with updated opacity on opacity change', () => {
+    const newOpacity = 50;
+    wrapper.find(EuiRange).simulate('change', { target: { value: newOpacity.toString() } });
+
+    expect(mockProps.setSelectedLayerConfig).toHaveBeenCalledWith({
+      ...mockLayerConfig,
+      opacity: newOpacity,
+    });
+  });
+
+  it('should call setSelectedLayerConfig with updated name on name change', () => {
+    const newName = 'Updated Test Layer';
+    wrapper.find(EuiFieldText).simulate('change', { target: { value: newName } });
+
+    expect(mockProps.setSelectedLayerConfig).toHaveBeenCalledWith({
+      ...mockLayerConfig,
+      name: newName,
+    });
+  });
+
+  it('should call setSelectedLayerConfig with updated description on description change', () => {
+    const newDescription = 'Updated description';
+    wrapper.find(EuiTextArea).simulate('change', { target: { value: newDescription } });
+
+    expect(mockProps.setSelectedLayerConfig).toHaveBeenCalledWith({
+      ...mockLayerConfig,
+      description: newDescription,
+    });
+  });
+
+  it('should display an error for an empty name', () => {
+    mockProps.isLayerExists.mockReturnValue(false); // Ensure this returns false for this test
+    const emptyName = '';
+    wrapper.find(EuiFieldText).simulate('change', { target: { value: emptyName } });
+
+    wrapper.update();
+
+    expect(wrapper.find(EuiFormRow).first().prop('error')).toContain('Name cannot be empty');
+  });
+
+  it('should display an error for a name exceeding the maximum length', () => {
+    const longName = 'a'.repeat(MAX_LAYER_NAME_LIMIT + 1); // Create a name longer than MAX_LAYER_NAME_LIMIT
+    wrapper.find(EuiFieldText).simulate('change', { target: { value: longName } });
+
+    wrapper.update();
+
+    expect(wrapper.find(EuiFormRow).first().prop('error')).toContain(
+      `Name should be less than ${MAX_LAYER_NAME_LIMIT} characters`
+    );
+  });
+
+  it('should display an error for a name that already exists', () => {
+    mockProps.isLayerExists.mockReturnValue(true); // Simulate that the name already exists
+    const existingName = 'Existing Layer Name';
+    wrapper.find(EuiFieldText).simulate('change', { target: { value: existingName } });
+
+    wrapper.update();
+
+    expect(wrapper.find(EuiFormRow).first().prop('error')).toContain('Name already exists');
+  });
+});

public/components/layer_config/layer_basic_settings.tsx

@@ -25,6 +25,7 @@ import {
   MAP_LAYER_DEFAULT_OPACITY_STEP,
   MAX_LAYER_NAME_LIMIT,
 } from '../../../common';
+import { ValueMember } from '../../../../../src/plugins/opensearch_dashboards_react/public/validated_range/validated_dual_range';
 
 interface Props {
   selectedLayerConfig: MapLayerSpecification;
@@ -41,6 +42,7 @@ export const LayerBasicSettings = ({
 }: Props) => {
   const [invalid, setInvalid] = useState<boolean>(selectedLayerConfig.name.length === 0);
   const [errors, setErrors] = useState<string[]>([]);
+  const [zoomLevel, setZoomLevel] = useState<[ValueMember, ValueMember]>(selectedLayerConfig.zoomRange as [ValueMember, ValueMember]);
 
   const validateName = (name: string) => {
     if (name?.length === 0) {
@@ -73,8 +75,14 @@ export const LayerBasicSettings = ({
     const newLayerConfig = { ...selectedLayerConfig, [key]: value };
     setSelectedLayerConfig(newLayerConfig);
   };
-  const onZoomChange = (value: number[]) => {
-    commonUpdate('zoomRange', value);
+
+  const onZoomChange = (value: [ValueMember, ValueMember]) => {
+    // if value is not empty string, then update the zoom level to layer config
+    if (value[0] !== '' && value[1] !== '') {
+      // change value to number type
+      commonUpdate('zoomRange', value.map((zoomLevel) => Number(zoomLevel)));
+    }
+    setZoomLevel(value);
   };
 
   const onOpacityChange = (e: any) => {
@@ -121,7 +129,7 @@ export const LayerBasicSettings = ({
           <EuiDualRange
             min={MAP_DEFAULT_MIN_ZOOM}
             max={MAP_DEFAULT_MAX_ZOOM}
-            value={selectedLayerConfig.zoomRange}
+            value={zoomLevel}
             showInput
             minInputProps={{ 'aria-label': 'Min value' }}
             maxInputProps={{ 'aria-label': 'Max value' }}

public/model/customLayerFunctions.test.ts

@@ -0,0 +1,154 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { applyZoomRangeToLayer, CustomLayerFunctions } from './customLayerFunctions';
+import { MaplibreRef } from './layersFunctions';
+import { Map as MapLibre } from 'maplibre-gl';
+import { CustomLayerSpecification } from './mapLayerType';
+import { DASHBOARDS_CUSTOM_MAPS_LAYER_TYPE } from '../../common';
+
+const mockStyle = {
+  layers: [
+    {
+      id: 'existing-layer',
+      type: 'raster',
+      source: 'existing-layer',
+      layout: {
+        visibility: 'visible',
+      },
+    },
+  ],
+  sources: {
+    'existing-layer': {
+      type: 'raster',
+      tiles: ['https://example.com/tiles/{z}/{x}/{y}.png'],
+    },
+  },
+};
+
+jest.mock('maplibre-gl', () => ({
+  Map: jest.fn(() => ({
+    on: jest.fn(),
+    off: jest.fn(),
+    getStyle: jest.fn(() => mockStyle),
+    setPaintProperty: jest.fn(),
+    setLayerZoomRange: jest.fn(),
+    addSource: jest.fn(),
+    addLayer: jest.fn(),
+    // @ts-ignore
+    getSource: jest.fn().mockImplementation((id) => mockStyle.sources[id]),
+    getLayer: jest
+      .fn()
+      .mockImplementation((id) => mockStyle.layers.find((layer) => layer.id === id)),
+    triggerRepaint: jest.fn(),
+    _controls: [],
+    style: {
+      sourceCaches: {},
+    },
+  })),
+}));
+
+describe('CustomLayerFunctions', () => {
+  let map: MapLibre;
+  let maplibreRef: MaplibreRef;
+
+  beforeEach(() => {
+    map = new MapLibre({
+      container: document.createElement('div'),
+      style: 'mock-style-url',
+    });
+
+    // Initialize sourceCaches with a mock function for each layer that might be accessed
+    // @ts-ignore
+    map.style.sourceCaches['existing-layer'] = {
+      clearTiles: jest.fn(),
+      update: jest.fn(),
+    };
+
+    maplibreRef = {
+      current: map,
+    };
+  });
+
+  it('should add a new layer if it does not exist', () => {
+    const newLayerConfig: CustomLayerSpecification = {
+      id: 'new-layer',
+      source: {
+        // @ts-ignore
+        type: DASHBOARDS_CUSTOM_MAPS_LAYER_TYPE.TMS,
+        tiles: ['https://newtiles.example.com/{z}/{x}/{y}.png'],
+        attribution: 'Test Attribution',
+      },
+      opacity: 80,
+      zoomRange: [0, 22],
+      visibility: 'visible',
+    };
+
+    CustomLayerFunctions.render(maplibreRef, newLayerConfig);
+
+    expect(map.addSource).toHaveBeenCalledWith(newLayerConfig.id, expect.any(Object));
+    expect(map.addLayer).toHaveBeenCalledWith(expect.any(Object));
+  });
+
+  it('should update an existing layer', () => {
+    const updatedLayerConfig: CustomLayerSpecification = {
+      id: 'existing-layer',
+      source: {
+        // @ts-ignore
+        type: DASHBOARDS_CUSTOM_MAPS_LAYER_TYPE.TMS,
+        tiles: ['https://updatedtiles.example.com/{z}/{x}/{y}.png'],
+        attribution: 'Updated Test Attribution',
+      },
+      opacity: 50,
+      zoomRange: [0, 15],
+      visibility: 'visible',
+    };
+
+    CustomLayerFunctions.render(maplibreRef, updatedLayerConfig);
+
+    expect(map.setPaintProperty).toHaveBeenCalledWith(
+      updatedLayerConfig.id,
+      'raster-opacity',
+      updatedLayerConfig.opacity / 100
+    );
+    expect(map.setLayerZoomRange).toHaveBeenCalledWith(
+      updatedLayerConfig.id,
+      updatedLayerConfig.zoomRange[0],
+      updatedLayerConfig.zoomRange[1]
+    );
+  });
+
+  it('should convert zoomRange to a numeric array', () => {
+    const layerConfig = {
+      id: 'test-layer',
+      // Assuming zoomRange might be provided as strings from old versions
+      zoomRange: ['0', '10'],
+    };
+
+    // @ts-ignore
+    const result = applyZoomRangeToLayer(layerConfig);
+
+    // Expected result should be a numeric array
+    const expectedResult = [0, 10];
+
+    // Verify the result matches the expected numeric array
+    expect(result).toEqual(expectedResult);
+  });
+
+  it('should handle mixed types in zoomRange', () => {
+    // Define layerConfig with zoomRange as a mix of numbers and strings
+    const layerConfig = {
+      id: 'mixed-type-layer',
+      zoomRange: [1, '15'],
+    };
+
+    // @ts-ignore
+    const result = applyZoomRangeToLayer(layerConfig);
+
+    const expectedResult = [1, 15];
+
+    expect(result).toEqual(expectedResult);
+  });
+});

public/model/customLayerFunctions.ts

@@ -51,6 +51,8 @@ const addNewLayer = (layerConfig: CustomLayerSpecification, maplibreRef: Maplibr
       tileSize: 256,
       attribution: layerSource?.attribution,
     });
+    // Convert zoomRange to number[] to avoid type error for backport versions
+    const zoomRange: number[] = applyZoomRangeToLayer(layerConfig);
     maplibreInstance.addLayer({
       id: layerConfig.id,
       type: 'raster',
@@ -61,8 +63,8 @@ const addNewLayer = (layerConfig: CustomLayerSpecification, maplibreRef: Maplibr
       layout: {
         visibility: layerConfig.visibility === 'visible' ? 'visible' : 'none',
       },
-      minzoom: layerConfig.zoomRange[0],
-      maxzoom: layerConfig.zoomRange[1],
+      minzoom: zoomRange[0],
+      maxzoom: zoomRange[1],
     });
   }
 };
@@ -79,6 +81,11 @@ const getCustomMapURL = (layerConfig: CustomLayerSpecification) => {
   }
 };
 
+export const applyZoomRangeToLayer = (layerConfig: CustomLayerSpecification) => {
+  // Convert zoomRange to number[] to avoid type error for backport versions
+  return layerConfig.zoomRange.map((zoom) => Number(zoom));
+};
+
 export const CustomLayerFunctions = {
   render: (maplibreRef: MaplibreRef, layerConfig: CustomLayerSpecification) => {
     return hasLayer(maplibreRef.current!, layerConfig.id)