Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zoom level type error in custom layer #605

Merged
merged 3 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
113 changes: 113 additions & 0 deletions public/components/layer_config/layer_basic_settings.test.tsx
Original file line number Diff line number Diff line change
@@ -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');
});
});
14 changes: 11 additions & 3 deletions public/components/layer_config/layer_basic_settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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' }}
Expand Down
154 changes: 154 additions & 0 deletions public/model/customLayerFunctions.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
11 changes: 9 additions & 2 deletions public/model/customLayerFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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],
});
}
};
Expand All @@ -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)
Expand Down
Loading