You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2022/12/09 14:00:25 UTC
[superset] branch master updated: feat(dashboard): Add edit button to dashboard native filters filter cards (#22364)
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3b45ad8b97 feat(dashboard): Add edit button to dashboard native filters filter cards (#22364)
3b45ad8b97 is described below
commit 3b45ad8b97ff7a72ac4d57cfbd2289bf38022cfc
Author: Cody Leff <co...@preset.io>
AuthorDate: Fri Dec 9 07:00:15 2022 -0700
feat(dashboard): Add edit button to dashboard native filters filter cards (#22364)
Co-authored-by: Herbert Gainor <he...@preset.io>
---
.../FilterBar/FilterConfigurationLink/index.tsx | 10 +-
.../nativeFilters/FilterCard/DependenciesRow.tsx | 1 +
.../nativeFilters/FilterCard/FilterCard.test.tsx | 171 ++++++++++++---------
.../nativeFilters/FilterCard/FilterCardContent.tsx | 12 +-
.../nativeFilters/FilterCard/NameRow.tsx | 53 +++++--
.../components/nativeFilters/FilterCard/Styles.ts | 5 +
.../components/nativeFilters/FilterCard/index.tsx | 7 +-
7 files changed, 172 insertions(+), 87 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx
index 8319c3c9fb..c9e4b834d9 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx
@@ -27,6 +27,8 @@ import { getFilterBarTestId } from '../utils';
export interface FCBProps {
createNewOnOpen?: boolean;
dashboardId?: number;
+ initialFilterId?: string;
+ onClick?: () => void;
children?: React.ReactNode;
}
@@ -37,6 +39,8 @@ const HeaderButton = styled(Button)`
export const FilterConfigurationLink: React.FC<FCBProps> = ({
createNewOnOpen,
dashboardId,
+ initialFilterId,
+ onClick,
children,
}) => {
const dispatch = useDispatch();
@@ -56,7 +60,10 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
const handleClick = useCallback(() => {
setOpen(true);
- }, [setOpen]);
+ if (onClick) {
+ onClick();
+ }
+ }, [setOpen, onClick]);
return (
<>
@@ -73,6 +80,7 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
isOpen={isOpen}
onSave={submit}
onCancel={close}
+ initialFilterId={initialFilterId}
createNewOnOpen={createNewOnOpen}
key={`filters-for-${dashboardId}`}
/>
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx
index ccffdf4b56..6a846c012f 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx
@@ -106,6 +106,7 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => {
<RowValue ref={dependenciesRef}>
{dependencies.map((dependency, index) => (
<DependencyValue
+ key={dependency.id}
dependency={dependency}
hasSeparator={index !== 0}
/>
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx
index 970d494ebd..560fa1faf0 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx
@@ -27,6 +27,7 @@ import { SET_FOCUSED_NATIVE_FILTER } from 'src/dashboard/actions/nativeFilters';
import { FilterCardContent } from './FilterCardContent';
const baseInitialState = {
+ dashboardInfo: {},
nativeFilters: {
filters: {
'NATIVE_FILTER-1': {
@@ -204,6 +205,13 @@ jest.mock('@superset-ui/core', () => ({
}),
}));
+jest.mock(
+ 'src/components/Icons/Icon',
+ () =>
+ ({ fileName }: { fileName: string }) =>
+ <span role="img" aria-label={fileName.replace('_', '-')} />,
+);
+
// extract text from embedded html tags
// source: https://polvara.me/posts/five-things-you-didnt-know-about-testing-library
const getTextInHTMLTags =
@@ -217,90 +225,115 @@ const getTextInHTMLTags =
return nodeHasText && childrenDontHaveText;
};
+const hidePopover = jest.fn();
+
const renderContent = (filter = baseFilter, initialState = baseInitialState) =>
- render(<FilterCardContent filter={filter} />, {
+ render(<FilterCardContent filter={filter} hidePopover={hidePopover} />, {
useRedux: true,
initialState,
});
-describe('Filter Card', () => {
- it('Basic', () => {
- renderContent();
- expect(screen.getByText('Native filter 1')).toBeVisible();
- expect(screen.getByLabelText('filter-small')).toBeVisible();
+test('filter card title, type, scope, dependencies', () => {
+ renderContent();
+ expect(screen.getByText('Native filter 1')).toBeVisible();
+ expect(screen.getByLabelText('filter-small')).toBeVisible();
- expect(screen.getByText('Filter type')).toBeVisible();
- expect(screen.getByText('Select filter')).toBeVisible();
+ expect(screen.getByText('Filter type')).toBeVisible();
+ expect(screen.getByText('Select filter')).toBeVisible();
- expect(screen.getByText('Scope')).toBeVisible();
- expect(screen.getByText('All charts')).toBeVisible();
+ expect(screen.getByText('Scope')).toBeVisible();
+ expect(screen.getByText('All charts')).toBeVisible();
- expect(screen.queryByText('Dependencies')).not.toBeInTheDocument();
- });
+ expect(screen.queryByText('Dependencies')).not.toBeInTheDocument();
+});
+
+test('filter card scope with excluded', () => {
+ const filter = {
+ ...baseFilter,
+ scope: { rootPath: [DASHBOARD_ROOT_ID], excluded: [1, 4] },
+ };
+ renderContent(filter);
+ expect(screen.getByText('Scope')).toBeVisible();
+ expect(
+ screen.getByText(getTextInHTMLTags('Test chart 2, Test chart 3')),
+ ).toBeVisible();
+});
- describe('Scope', () => {
- it('Scope with excluded', () => {
- const filter = {
- ...baseFilter,
- scope: { rootPath: [DASHBOARD_ROOT_ID], excluded: [1, 4] },
- };
- renderContent(filter);
- expect(screen.getByText('Scope')).toBeVisible();
- expect(
- screen.getByText(getTextInHTMLTags('Test chart 2, Test chart 3')),
- ).toBeVisible();
- });
+test('filter card scope with top level tab as root', () => {
+ const filter = {
+ ...baseFilter,
+ scope: { rootPath: ['TAB-1', 'TAB-2'], excluded: [1, 2] },
+ };
+ renderContent(filter);
+ expect(screen.getByText('Scope')).toBeVisible();
+ expect(
+ screen.getByText(getTextInHTMLTags('Tab 2, Test chart 3')),
+ ).toBeVisible();
+});
- it('Scope with top level tab as root', () => {
- const filter = {
- ...baseFilter,
- scope: { rootPath: ['TAB-1', 'TAB-2'], excluded: [1, 2] },
- };
- renderContent(filter);
- expect(screen.getByText('Scope')).toBeVisible();
- expect(
- screen.getByText(getTextInHTMLTags('Tab 2, Test chart 3')),
- ).toBeVisible();
- });
+test('filter card empty scope', () => {
+ const filter = {
+ ...baseFilter,
+ scope: { rootPath: [], excluded: [1, 2, 3, 4] },
+ };
+ renderContent(filter);
+ expect(screen.getByText('Scope')).toBeVisible();
+ expect(screen.getByText('None')).toBeVisible();
+});
- it('Empty scope', () => {
- const filter = {
- ...baseFilter,
- scope: { rootPath: [], excluded: [1, 2, 3, 4] },
- };
- renderContent(filter);
- expect(screen.getByText('Scope')).toBeVisible();
- expect(screen.getByText('None')).toBeVisible();
- });
+test('filter card with dependency', () => {
+ const filter = {
+ ...baseFilter,
+ cascadeParentIds: ['NATIVE_FILTER-2'],
+ };
+ renderContent(filter);
+ expect(screen.getByText('Dependent on')).toBeVisible();
+ expect(screen.getByText('Native filter 2')).toBeVisible();
+});
+
+test('focus filter on filter card dependency click', () => {
+ const useDispatchMock = jest.spyOn(reactRedux, 'useDispatch');
+ const dummyDispatch = jest.fn();
+ useDispatchMock.mockReturnValue(dummyDispatch);
+
+ const filter = {
+ ...baseFilter,
+ cascadeParentIds: ['NATIVE_FILTER-2'],
+ };
+ renderContent(filter);
+
+ userEvent.click(screen.getByText('Native filter 2'));
+ expect(dummyDispatch).toHaveBeenCalledWith({
+ type: SET_FOCUSED_NATIVE_FILTER,
+ id: 'NATIVE_FILTER-2',
});
+});
- describe('Dependencies', () => {
- it('Has dependency', () => {
- const filter = {
- ...baseFilter,
- cascadeParentIds: ['NATIVE_FILTER-2'],
- };
- renderContent(filter);
- expect(screen.getByText('Dependent on')).toBeVisible();
- expect(screen.getByText('Native filter 2')).toBeVisible();
- });
+test('edit filter button for dashboard viewer', () => {
+ renderContent();
+ expect(
+ screen.queryByRole('button', { name: /edit/i }),
+ ).not.toBeInTheDocument();
+});
- it('Focus filter on dependency click', () => {
- const useDispatchMock = jest.spyOn(reactRedux, 'useDispatch');
- const dummyDispatch = jest.fn();
- useDispatchMock.mockReturnValue(dummyDispatch);
+test('edit filter button for dashboard editor', () => {
+ renderContent(baseFilter, {
+ ...baseInitialState,
+ dashboardInfo: { dash_edit_perm: true },
+ });
- const filter = {
- ...baseFilter,
- cascadeParentIds: ['NATIVE_FILTER-2'],
- };
- renderContent(filter);
+ expect(screen.getByRole('button', { name: /edit/i })).toBeVisible();
+});
- userEvent.click(screen.getByText('Native filter 2'));
- expect(dummyDispatch).toHaveBeenCalledWith({
- type: SET_FOCUSED_NATIVE_FILTER,
- id: 'NATIVE_FILTER-2',
- });
- });
+test('open modal on edit filter button click', async () => {
+ renderContent(baseFilter, {
+ ...baseInitialState,
+ dashboardInfo: { dash_edit_perm: true },
});
+
+ const editButton = screen.getByRole('button', { name: /edit/i });
+ userEvent.click(editButton);
+ expect(
+ await screen.findByRole('dialog', { name: /add and edit filters/i }),
+ ).toBeVisible();
});
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCardContent.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCardContent.tsx
index 41696ed98f..02b0d875a9 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCardContent.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCardContent.tsx
@@ -24,9 +24,17 @@ import { DependenciesRow } from './DependenciesRow';
import { NameRow } from './NameRow';
import { TypeRow } from './TypeRow';
-export const FilterCardContent = ({ filter }: { filter: Filter }) => (
+interface FilterCardContentProps {
+ filter: Filter;
+ hidePopover: () => void;
+}
+
+export const FilterCardContent = ({
+ filter,
+ hidePopover,
+}: FilterCardContentProps) => (
<div>
- <NameRow filter={filter} />
+ <NameRow filter={filter} hidePopover={hidePopover} />
<TypeRow filter={filter} />
<ScopeRow filter={filter} />
<DependenciesRow filter={filter} />
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx
index f6268296ef..6c7e82b15d 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx
@@ -17,34 +17,61 @@
* under the License.
*/
import React, { useRef } from 'react';
-import { css, SupersetTheme } from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import { css, SupersetTheme, useTheme } from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { useTruncation } from 'src/hooks/useTruncation';
-import { Row, FilterName } from './Styles';
+import { RootState } from 'src/dashboard/types';
+import { Row, FilterName, InternalRow } from './Styles';
import { FilterCardRowProps } from './types';
+import { FilterConfigurationLink } from '../FilterBar/FilterConfigurationLink';
import { TooltipWithTruncation } from './TooltipWithTruncation';
-export const NameRow = ({ filter }: FilterCardRowProps) => {
+export const NameRow = ({
+ filter,
+ hidePopover,
+}: FilterCardRowProps & { hidePopover: () => void }) => {
+ const theme = useTheme();
const filterNameRef = useRef<HTMLElement>(null);
const [elementsTruncated] = useTruncation(filterNameRef);
+ const dashboardId = useSelector<RootState, number>(
+ ({ dashboardInfo }) => dashboardInfo.id,
+ );
+
+ const canEdit = useSelector<RootState, boolean>(
+ ({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
+ );
+
return (
<Row
css={(theme: SupersetTheme) =>
css`
margin-bottom: ${theme.gridUnit * 3}px;
+ justify-content: space-between;
`
}
>
- <Icons.FilterSmall
- css={(theme: SupersetTheme) =>
- css`
- margin-right: ${theme.gridUnit}px;
- `
- }
- />
- <TooltipWithTruncation title={elementsTruncated ? filter.name : null}>
- <FilterName ref={filterNameRef}>{filter.name}</FilterName>
- </TooltipWithTruncation>
+ <InternalRow>
+ <Icons.FilterSmall
+ css={(theme: SupersetTheme) =>
+ css`
+ margin-right: ${theme.gridUnit}px;
+ `
+ }
+ />
+ <TooltipWithTruncation title={elementsTruncated ? filter.name : null}>
+ <FilterName ref={filterNameRef}>{filter.name}</FilterName>
+ </TooltipWithTruncation>
+ </InternalRow>
+ {canEdit && (
+ <FilterConfigurationLink
+ dashboardId={dashboardId}
+ onClick={hidePopover}
+ initialFilterId={filter.id}
+ >
+ <Icons.Edit iconSize="l" iconColor={theme.colors.grayscale.light1} />
+ </FilterConfigurationLink>
+ )}
</Row>
);
};
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts
index bda865063d..8090201f1c 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts
@@ -92,3 +92,8 @@ export const TooltipTrigger = styled.div`
display: inline-flex;
white-space: nowrap;
`;
+
+export const InternalRow = styled.div`
+ display: flex;
+ align-items: center;
+`;
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx
index 8d4b9051eb..bdbfcff8ae 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx
@@ -31,6 +31,10 @@ export const FilterCard = ({
}: FilterCardProps) => {
const [internalIsVisible, setInternalIsVisible] = useState(false);
+ const hidePopover = () => {
+ setInternalIsVisible(false);
+ };
+
useEffect(() => {
if (!externalIsVisible) {
setInternalIsVisible(false);
@@ -46,9 +50,8 @@ export const FilterCard = ({
setInternalIsVisible(externalIsVisible && visible);
}}
visible={externalIsVisible && internalIsVisible}
- content={<FilterCardContent filter={filter} />}
+ content={<FilterCardContent filter={filter} hidePopover={hidePopover} />}
getPopupContainer={getPopupContainer ?? (() => document.body)}
- destroyTooltipOnHide
>
{children}
</Popover>