Skip to content

Commit

Permalink
feat(filter): adding inputs to Numerical Range Filter (apache#31726)
Browse files Browse the repository at this point in the history
Co-authored-by: Diego Pucci <diegopucci.me@gmail.com>
Co-authored-by: Mehmet Salih Yavuz <salih.yavuz@proton.me>
  • Loading branch information
3 people authored Feb 28, 2025
1 parent 789049d commit 4d6b4f8
Show file tree
Hide file tree
Showing 8 changed files with 405 additions and 280 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/**
*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
Expand Down Expand Up @@ -192,23 +193,34 @@ describe('Native filters', () => {
testItems.filterNumericalColumn,
);
saveNativeFilterSettings([]);
// assertions
cy.get(nativeFilters.slider.slider).should('be.visible').click('center');

// Assertions
cy.get('[data-test="range-filter-from-input"]')
.should('be.visible')
.click();

cy.get('[data-test="range-filter-from-input"]').type('{selectall}5');

cy.get('[data-test="range-filter-to-input"]')
.should('be.visible')
.click();

cy.get('[data-test="range-filter-to-input"]').type('{selectall}50');
cy.get(nativeFilters.applyFilter).click();
// assert that the url contains 'native_filters' in the url

// Assert that the URL contains 'native_filters'
cy.url().then(u => {
const ur = new URL(u);
expect(ur.search).to.include('native_filters');
// assert that the start handle has a value
cy.get(nativeFilters.slider.startHandle)
.invoke('attr', 'aria-valuenow')
.should('exist');
// assert that the end handle has a value
cy.get(nativeFilters.slider.endHandle)
.invoke('attr', 'aria-valuenow')
.should('exist');
// assert slider text matches what we should have
cy.get(nativeFilters.slider.sliderText).should('have.text', '49');

cy.get('[data-test="range-filter-from-input"]')
.invoke('val')
.should('equal', '5');

// Assert that the "To" input has the correct value
cy.get('[data-test="range-filter-to-input"]')
.invoke('val')
.should('equal', '50');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ export function inputNativeFilterDefaultValue(
) {
if (!multiple) {
cy.contains('Filter has default value').click();
cy.contains('Default value is required').should('be.visible');
cy.contains('Please choose a valid value').should('be.visible');
cy.get(nativeFilters.modal.container).within(() => {
cy.get(
nativeFilters.filterConfigurationSections.filterPlaceholder,
Expand Down
47 changes: 47 additions & 0 deletions superset-frontend/src/components/Metadata/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { styled } from '@superset-ui/core';

const MetadataWrapper = styled.div`
display: flex;
width: 100%;
position: absolute;
left: 0;
top: 100%;
margin-top: ${({ theme }) => theme.gridUnit}px;
`;

const MetadataText = styled.span`
font-size: ${({ theme }) => theme.typography.sizes.xs}px;
color: ${({ theme }) => theme.colors.grayscale.light1};
font-weight: ${({ theme }) => theme.typography.weights.medium};
`;

export type MetadataProps = {
value: string;
};

const Metadata: React.FC<MetadataProps> = ({ value }) => (
<MetadataWrapper>
<MetadataText>{value}</MetadataText>
</MetadataWrapper>
);

export default Metadata;
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ const FilterValue: FC<FilterControlProps> = ({

const filterState = useMemo(
() => ({
...filter.dataMask?.filterState,
validateStatus,
...filter.dataMask?.filterState,
}),
[filter.dataMask?.filterState, validateStatus],
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,19 @@ export const checkIsMissingRequiredValue = (
);
};

export const checkIsValidateError = (dataMask: DataMaskStateWithId) => {
const values = Object.values(dataMask);
return values.every(value => value.filterState?.validateStatus !== 'error');
};

export const checkIsApplyDisabled = (
dataMaskSelected: DataMaskStateWithId,
dataMaskApplied: DataMaskStateWithId,
filters: Filter[],
) => {
if (!checkIsValidateError(dataMaskSelected)) {
return true;
}
const dataSelectedValues = Object.values(dataMaskSelected);
const dataAppliedValues = Object.values(dataMaskApplied);
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ const FiltersConfigForm = (
return [...prevErroredFilters, filterId];
});
return Promise.reject(
new Error(t('Default value is required')),
new Error(t('Please choose a valid value')),
);
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { AppSection, GenericDataType } from '@superset-ui/core';
import { render, screen } from 'spec/helpers/testing-library';
import { fireEvent, render, screen } from 'spec/helpers/testing-library';
import RangeFilterPlugin from './RangeFilterPlugin';
import { SingleValueType } from './SingleValueType';
import transformProps from './transformProps';
Expand Down Expand Up @@ -83,14 +83,15 @@ const rangeProps = {

describe('RangeFilterPlugin', () => {
const setDataMask = jest.fn();
const getWrapper = (props = {}) =>
const getWrapper = (props: any = {}) =>
render(
// @ts-ignore
<RangeFilterPlugin
// @ts-ignore
{...transformProps({
...rangeProps,
formData: { ...rangeProps.formData, ...props },
...props,
formData: { ...rangeProps.formData, ...props.formData },
})}
setDataMask={setDataMask}
/>,
Expand All @@ -100,11 +101,50 @@ describe('RangeFilterPlugin', () => {
jest.clearAllMocks();
});

it('should render two numerical inputs', () => {
getWrapper();

const inputs = screen.getAllByRole('spinbutton');
expect(inputs).toHaveLength(2);

expect(inputs[0]).toHaveValue('10');
expect(inputs[1]).toHaveValue('70');
});

it('should set the data mask to error when the range is incorrect', () => {
getWrapper({ filterState: { value: [null, null] } });

const inputs = screen.getAllByRole('spinbutton');
const fromInput = inputs[0];
const toInput = inputs[1];

fireEvent.change(fromInput, { target: { value: 20 } });

fireEvent.change(toInput, { target: { value: 10 } });

fireEvent.blur(toInput);

expect(setDataMask).toHaveBeenCalledWith({
extraFormData: {},
filterState: {
label: '',
validateMessage: 'Please provide a valid range',
validateStatus: 'error',
value: null,
},
});
});

it('should call setDataMask with correct filter', () => {
getWrapper();
expect(setDataMask).toHaveBeenCalledWith({
extraFormData: {
filters: [
{
col: 'SP_POP_TOTL',
op: '>=',
val: 10,
},
{
col: 'SP_POP_TOTL',
op: '<=',
Expand All @@ -113,16 +153,21 @@ describe('RangeFilterPlugin', () => {
],
},
filterState: {
label: 'x ≤ 70',
label: '10 ≤ x ≤ 70',
value: [10, 70],
validateMessage: '',
validateStatus: undefined,
},
});
});

it('should call setDataMask with correct greater than filter', () => {
getWrapper({
enableSingleValue: SingleValueType.Minimum,
defaultValue: [20, 60],
filterState: { value: [20, null] },
formData: {
enableSingleValue: SingleValueType.Minimum,
defaultValue: undefined,
},
});
expect(setDataMask).toHaveBeenCalledWith({
extraFormData: {
Expand All @@ -135,17 +180,22 @@ describe('RangeFilterPlugin', () => {
],
},
filterState: {
validateStatus: undefined,
validateMessage: '',
label: 'x ≥ 20',
value: [20, 100],
value: [20, null],
},
});
expect(screen.getByRole('slider')).toHaveAttribute('aria-valuenow', '20');
const input = screen.getByRole('spinbutton');
expect(input).toHaveValue('20');
});

it('should call setDataMask with correct less than filter', () => {
getWrapper({
enableSingleValue: SingleValueType.Maximum,
defaultValue: [20, 60],
filterState: { value: [null, 60] },
formData: {
enableSingleValue: SingleValueType.Maximum,
},
});
expect(setDataMask).toHaveBeenCalledWith({
extraFormData: {
Expand All @@ -159,14 +209,22 @@ describe('RangeFilterPlugin', () => {
},
filterState: {
label: 'x ≤ 60',
value: [10, 60],
value: [null, 60],
validateMessage: '',
validateStatus: undefined,
},
});
expect(screen.getByRole('slider')).toHaveAttribute('aria-valuenow', '60');
const input = screen.getByRole('spinbutton');
expect(input).toHaveValue('60');
});

it('should call setDataMask with correct exact filter', () => {
getWrapper({ enableSingleValue: SingleValueType.Exact });
getWrapper({
formData: {
enableSingleValue: SingleValueType.Exact,
},
filterState: { value: [10, 10] },
});
expect(setDataMask).toHaveBeenCalledWith({
extraFormData: {
filters: [
Expand All @@ -180,6 +238,8 @@ describe('RangeFilterPlugin', () => {
filterState: {
label: 'x = 10',
value: [10, 10],
validateStatus: undefined,
validateMessage: '',
},
});
});
Expand Down
Loading

0 comments on commit 4d6b4f8

Please # to comment.