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

refactor: clean up codes #22002

Merged
merged 1 commit into from
Nov 2, 2022
Merged

refactor: clean up codes #22002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 8 additions & 3 deletions superset-frontend/src/addSlice/AddSliceContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
import React, { ReactNode } from 'react';
import rison from 'rison';
import querystring from 'query-string';
import { styled, t, SupersetClient, JsonResponse } from '@superset-ui/core';
import {
styled,
t,
SupersetClient,
JsonResponse,
isDefined,
} from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';
import { URL_PARAMS } from 'src/constants';
import { isNullish } from 'src/utils/common';
Copy link
Member Author

Choose a reason for hiding this comment

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

replace isNullish with isDefined

import { Link, withRouter, RouteComponentProps } from 'react-router-dom';
import Button from 'src/components/Button';
import { AsyncSelect, Steps } from 'src/components';
Expand Down Expand Up @@ -247,7 +252,7 @@ export class AddSliceContainer extends React.PureComponent<
exploreUrl() {
const dashboardId = getUrlParam(URL_PARAMS.dashboardId);
let url = `/explore/?viz_type=${this.state.vizType}&datasource=${this.state.datasource?.value}`;
if (!isNullish(dashboardId)) {
if (isDefined(dashboardId)) {
url += `&dashboard_id=${dashboardId}`;
}
return url;
Expand Down
6 changes: 3 additions & 3 deletions superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import {
DataMaskStateWithId,
styled,
t,
isDefined,
} from '@superset-ui/core';
import Button from 'src/components/Button';
import { isNullish } from 'src/utils/common';
import { OPEN_FILTER_BAR_WIDTH } from 'src/dashboard/constants';
import { rgba } from 'emotion-rgba';
import { getFilterBarTestId } from '../index';
Expand Down Expand Up @@ -97,9 +97,9 @@ export const ActionButtons = ({
() =>
Object.values(dataMaskApplied).some(
filter =>
!isNullish(dataMaskSelected[filter.id]?.filterState?.value) ||
isDefined(dataMaskSelected[filter.id]?.filterState?.value) ||
(!dataMaskSelected[filter.id] &&
!isNullish(filter.filterState?.value)),
isDefined(filter.filterState?.value)),
),
[dataMaskApplied, dataMaskSelected],
);
Expand Down
8 changes: 2 additions & 6 deletions superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { testWithId } from 'src/utils/testUtils';
import { FeatureFlag } from 'src/featureFlags';
import { Preset } from '@superset-ui/core';
import { TimeFilterPlugin, SelectFilterPlugin } from 'src/filters/components';
import { DATE_FILTER_CONTROL_TEST_ID } from 'src/explore/components/controls/DateFilterControl';
import { DATE_FILTER_TEST_KEY } from 'src/explore/components/controls/DateFilterControl';
import fetchMock from 'fetch-mock';
import { waitFor } from '@testing-library/react';
import FilterBar, { FILTER_BAR_TEST_ID } from '.';
Expand Down Expand Up @@ -71,10 +71,6 @@ fetchMock.get('glob:*/api/v1/dataset/7', {

const getTestId = testWithId<string>(FILTER_BAR_TEST_ID, true);
const getModalTestId = testWithId<string>(FILTERS_CONFIG_MODAL_TEST_ID, true);
const getDateControlTestId = testWithId<string>(
DATE_FILTER_CONTROL_TEST_ID,
true,
);

const FILTER_NAME = 'Time filter 1';
const FILTER_SET_NAME = 'New filter set';
Expand Down Expand Up @@ -121,7 +117,7 @@ const changeFilterValue = async () => {
userEvent.click(screen.getAllByText('No filter')[0]);
userEvent.click(screen.getByDisplayValue('Last day'));
expect(await screen.findByText(/2021-04-13/)).toBeInTheDocument();
userEvent.click(screen.getByTestId(getDateControlTestId('apply-button')));
userEvent.click(screen.getByTestId(DATE_FILTER_TEST_KEY.applyButton));
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep simple logic for a simple use case.

};

describe('FilterBar', () => {
Expand Down
11 changes: 2 additions & 9 deletions superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import React, { useState, useEffect, useMemo } from 'react';
import { css, styled, t, useTheme, NO_TIME_RANGE } from '@superset-ui/core';
import Button from 'src/components/Button';
import ControlHeader from 'src/explore/components/ControlHeader';
import Label, { Type } from 'src/components/Label';
import Label from 'src/components/Label';
import Modal from 'src/components/Modal';
import { Divider } from 'src/components';
import Icons from 'src/components/Icons';
Expand All @@ -36,7 +36,6 @@ import {
DATE_FILTER_TEST_KEY,
fetchTimeRange,
FRAME_OPTIONS,
getDateFilterControlTestId,
guessFrame,
useDefaultTimeFilter,
} from './utils';
Expand Down Expand Up @@ -124,7 +123,6 @@ const IconWrapper = styled.span`
export default function DateFilterLabel(props: DateFilterControlProps) {
const {
onChange,
type,
onOpenPopover = noOp,
onClosePopover = noOp,
overlayStyle = 'Popover',
Expand Down Expand Up @@ -174,11 +172,6 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
guessedFrame === 'No filter'
) {
setActualTimeRange(value);
setTooltipTitle(
type === ('error' as Type)
? t('Default value is required')
: actualRange || '',
);
Comment on lines -177 to -181
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the default value always gets from Store or constants, the value of the DateFilterControl will not be undefined.

So the type in the DateFilterControl is useless.

} else {
setActualTimeRange(actualRange || '');
setTooltipTitle(value || '');
Expand Down Expand Up @@ -302,7 +295,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
disabled={!validTimeRange}
key="apply"
onClick={onSave}
{...getDateFilterControlTestId('apply-button')}
data-test={DATE_FILTER_TEST_KEY.applyButton}
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep simple too.

>
{t('APPLY')}
</Button>
Expand Down
7 changes: 1 addition & 6 deletions superset-frontend/src/explore/components/controls/DateFilterControl/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,4 @@
* under the License.
*/
export { default } from './DateFilterLabel';
export {
DATE_FILTER_CONTROL_TEST_ID,
fetchTimeRange,
guessFrame,
DATE_FILTER_TEST_KEY,
} from './utils';
export * from './utils';
3 changes: 0 additions & 3 deletions superset-frontend/src/explore/components/controls/DateFilterControl/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Type } from 'src/components/Label';

export type SelectOptionType = {
value: string;
label: string;
Expand Down Expand Up @@ -96,7 +94,6 @@ export interface DateFilterControlProps {
name: string;
onChange: (timeRange: string) => void;
value?: string;
type?: Type;
onOpenPopover?: () => void;
onClosePopover?: () => void;
overlayStyle?: 'Modal' | 'Popover';
Expand Down
7 changes: 1 addition & 6 deletions superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
CommonRangeType,
CalendarRangeType,
} from 'src/explore/components/controls/DateFilterControl/types';
import { testWithId } from 'src/utils/testUtils';

export const FRAME_OPTIONS: SelectOptionType[] = [
{ value: 'Common', label: t('Last') },
Expand Down Expand Up @@ -133,15 +132,11 @@ export const LOCALE_MAPPING = {
nl: 'nl_NL',
};

export const DATE_FILTER_CONTROL_TEST_ID = 'date-filter-control';
export const getDateFilterControlTestId = testWithId(
DATE_FILTER_CONTROL_TEST_ID,
);

export enum DATE_FILTER_TEST_KEY {
commonFrame = 'common-frame',
modalOverlay = 'modal-overlay',
popoverOverlay = 'time-range-trigger',
noFilter = 'no-filter',
cancelButton = 'cancel-button',
applyButton = 'date-filter-control__apply-button',
}
2 changes: 0 additions & 2 deletions superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export default function TimeFilterPlugin(props: PluginFilterTimeProps) {
}, [filterState.value]);

return props.formData?.inView ? (
// @ts-ignore
<TimeFilterStyles width={width} height={height}>
<ControlContainer
tabIndex={-1}
Expand All @@ -103,7 +102,6 @@ export default function TimeFilterPlugin(props: PluginFilterTimeProps) {
value={filterState.value || NO_TIME_RANGE}
name="time_range"
onChange={handleTimeRangeChange}
type={filterState.validateStatus}
onOpenPopover={() => setFilterActive(true)}
onClosePopover={() => setFilterActive(false)}
/>
Expand Down
13 changes: 0 additions & 13 deletions superset-frontend/src/utils/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,6 @@ export const SHORT_TIME = 'h:m a';

const DATETIME_FORMATTER = getTimeFormatter(TimeFormats.DATABASE_DATETIME);

export function getParamFromQuery(query, param) {
const vars = query.split('&');
for (let i = 0; i < vars.length; i += 1) {
const pair = vars[i].split('=');
if (decodeURIComponent(pair[0]) === param) {
return decodeURIComponent(pair[1]);
}
}
return null;
}

export function storeQuery(query) {
return SupersetClient.post({
endpoint: '/kv/store/',
Expand Down Expand Up @@ -150,5 +139,3 @@ export const isSafari = () => {

return userAgent && /^((?!chrome|android).)*safari/i.test(userAgent);
};

export const isNullish = value => value === null || value === undefined;
2 changes: 1 addition & 1 deletion superset-frontend/src/visualizations/presets/MainPreset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import {
TimeColumnFilterPlugin,
TimeGrainFilterPlugin,
GroupByFilterPlugin,
} from 'src/filters/components/';
} from 'src/filters/components';
import { PivotTableChartPlugin as PivotTableChartPluginV2 } from '@superset-ui/plugin-chart-pivot-table';
import { HandlebarsChartPlugin } from '@superset-ui/plugin-chart-handlebars';
import FilterBoxChartPlugin from '../FilterBox/FilterBoxChartPlugin';
Expand Down