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.

Sign up for GitHub

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

Jump to bottom

feat(explore): metrics and filters controls redesign #12095

Merged
merged 6 commits into from
Dec 17, 2020
Merged

feat(explore): metrics and filters controls redesign #12095

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9dec3b4
Redesign metrics control
kgabryje Dec 16, 2020
2ef052d
Redesign filters control
kgabryje Dec 16, 2020
a5d73dc
Bugfixes
kgabryje Dec 17, 2020
b20d83b
Fix unit tests
kgabryje Dec 17, 2020
3a174c4
Fix tests
kgabryje Dec 17, 2020
197f5da
Code review fixes
kgabryje Dec 17, 2020
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
21 changes: 21 additions & 0 deletions superset-frontend/images/icons/fx.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
48 changes: 14 additions & 34 deletions superset-frontend/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';

import Select from 'src/components/Select';
import AdhocFilter, {
EXPRESSION_TYPES,
CLAUSES,
} from 'src/explore/AdhocFilter';
import AdhocFilterControl from 'src/explore/components/controls/AdhocFilterControl';
import AdhocMetric from 'src/explore/AdhocMetric';
import { AGGREGATES, OPERATORS } from 'src/explore/constants';
import { supersetTheme } from '@superset-ui/core';
import { LabelsContainer } from '../../../../src/explore/components/OptionControls';
Copy link
Member

@villebro villebro Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
import { LabelsContainer } from '../../../../src/explore/components/OptionControls';
import { LabelsContainer } from 'src/explore/components/OptionControls';


const simpleAdhocFilter = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
Expand Down Expand Up @@ -66,22 +67,23 @@ function setup(overrides) {
columns,
savedMetrics: [savedMetric],
formData,
theme: supersetTheme,
...overrides,
};
const wrapper = shallow(<AdhocFilterControl {...props} />);
return { wrapper, onChange };
const component = wrapper.dive().shallow();
return { wrapper, component, onChange };
}

describe('AdhocFilterControl', () => {
it('renders Select', () => {
const { wrapper } = setup();
expect(wrapper.find(Select)).toExist();
it('renders LabelsContainer', () => {
const { component } = setup();
expect(component.find(LabelsContainer)).toExist();
});

it('handles saved metrics being selected to filter on', () => {
const { wrapper, onChange } = setup({ value: [] });
const select = wrapper.find(Select);
select.simulate('change', [{ saved_metric_name: 'sum__value' }]);
const { component, onChange } = setup({ value: [] });
component.instance().onNewFilter({ saved_metric_name: 'sum__value' });

const adhocFilter = onChange.lastCall.args[0][0];
expect(adhocFilter instanceof AdhocFilter).toBe(true);
Expand All @@ -99,9 +101,8 @@ describe('AdhocFilterControl', () => {
});

it('handles adhoc metrics being selected to filter on', () => {
const { wrapper, onChange } = setup({ value: [] });
const select = wrapper.find(Select);
select.simulate('change', [sumValueAdhocMetric]);
const { component, onChange } = setup({ value: [] });
component.instance().onNewFilter(sumValueAdhocMetric);

const adhocFilter = onChange.lastCall.args[0][0];
expect(adhocFilter instanceof AdhocFilter).toBe(true);
Expand All @@ -118,30 +119,9 @@ describe('AdhocFilterControl', () => {
).toBe(true);
});

it('handles columns being selected to filter on', () => {
const { wrapper, onChange } = setup({ value: [] });
const select = wrapper.find(Select);
select.simulate('change', [columns[0]]);

const adhocFilter = onChange.lastCall.args[0][0];
expect(adhocFilter instanceof AdhocFilter).toBe(true);
expect(
adhocFilter.equals(
new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: columns[0].column_name,
operator: OPERATORS['=='],
comparator: '',
clause: CLAUSES.WHERE,
}),
),
).toBe(true);
});

it('persists existing filters even when new filters are added', () => {
const { wrapper, onChange } = setup();
const select = wrapper.find(Select);
select.simulate('change', [simpleAdhocFilter, columns[0]]);
const { component, onChange } = setup();
component.instance().onNewFilter(columns[0]);

const existingAdhocFilter = onChange.lastCall.args[0][0];
expect(existingAdhocFilter instanceof AdhocFilter).toBe(true);
Expand Down
16 changes: 5 additions & 11 deletions superset-frontend/spec/javascripts/explore/components/AdhocFilterOption_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import sinon from 'sinon';
import { shallow } from 'enzyme';
import Popover from 'src/common/components/Popover';

import Label from 'src/components/Label';
import AdhocFilter, {
EXPRESSION_TYPES,
CLAUSES,
Expand Down Expand Up @@ -53,15 +52,10 @@ function setup(overrides) {
describe('AdhocFilterOption', () => {
it('renders an overlay trigger wrapper for the label', () => {
const { wrapper } = setup();
const overlay = wrapper.find(Popover);
expect(overlay).toHaveLength(1);
expect(overlay.props().defaultVisible).toBe(false);
expect(wrapper.find(Label)).toExist();
});
it('should open new filter popup by default', () => {
const { wrapper } = setup({
adhocFilter: simpleAdhocFilter.duplicateWith({ isNew: true }),
});
expect(wrapper.find(Popover).props().defaultVisible).toBe(true);
const overlay = wrapper.find('AdhocFilterPopoverTrigger').shallow();
const popover = overlay.find(Popover);
expect(popover).toHaveLength(1);
expect(popover.props().defaultVisible).toBe(false);
expect(overlay.find('OptionControlLabel')).toExist();
});
});
3 changes: 1 addition & 2 deletions superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import sinon from 'sinon';
import { shallow } from 'enzyme';

import Popover from 'src/common/components/Popover';
import Label from 'src/components/Label';
import AdhocMetric from 'src/explore/AdhocMetric';
import AdhocMetricOption from 'src/explore/components/AdhocMetricOption';
import { AGGREGATES } from 'src/explore/constants';
Expand Down Expand Up @@ -54,7 +53,7 @@ describe('AdhocMetricOption', () => {
it('renders an overlay trigger wrapper for the label', () => {
const { wrapper } = setup();
expect(wrapper.find(Popover)).toExist();
expect(wrapper.find(Label)).toExist();
expect(wrapper.find('OptionControlLabel')).toExist();
});

it('overlay should open if metric is new', () => {
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('MetricDefinitionValue', () => {
const wrapper = shallow(
<MetricDefinitionValue option={{ metric_name: 'a_saved_metric' }} />,
);
expect(wrapper.find(MetricOption)).toExist();
expect(wrapper.find('OptionControlLabel')).toExist();
});

it('renders an AdhocMetricOption given an adhoc metric', () => {
Expand Down
Loading