From 94a12e78d3c6f498a8d3c2d895eb5b373e123f93 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 23 Jan 2024 14:06:29 +0100 Subject: [PATCH 1/2] Add EmbeddedModal override and tests --- .../src/ui-overrides/types.ts | 10 + .../EmbeddedModal/EmbeddedModal.test.tsx | 187 ++++++++++++++++++ .../index.tsx} | 13 +- .../src/dashboard/components/Header/index.jsx | 2 +- 4 files changed, 208 insertions(+), 4 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx rename superset-frontend/src/dashboard/components/{DashboardEmbedControls.tsx => EmbeddedModal/index.tsx} (94%) diff --git a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts index 27646442de3d9..45ec06e90ed7d 100644 --- a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts @@ -135,11 +135,21 @@ export interface SliceHeaderExtension { dashboardId: number; } +/** + * Interface for extensions to Embed Modal + */ +export interface DashboardEmbedModalExtensions { + dashboardId: string; + show: boolean; + onHide: () => void; +} + export type Extensions = Partial<{ 'alertsreports.header.icon': React.ComponentType; 'embedded.documentation.configuration_details': React.ComponentType; 'embedded.documentation.description': ReturningDisplayable; 'embedded.documentation.url': string; + 'embedded.modal': React.ComponentType; 'dashboard.nav.right': React.ComponentType; 'navbar.right-menu.item.icon': React.ComponentType; 'navbar.right': React.ComponentType; diff --git a/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx new file mode 100644 index 0000000000000..7add60fc9abc5 --- /dev/null +++ b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx @@ -0,0 +1,187 @@ +/** + * 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 React from 'react'; +import { fireEvent, waitFor } from '@testing-library/react'; +import { render, screen } from 'spec/helpers/testing-library'; +import '@testing-library/jest-dom'; +import { + SupersetApiError, + getExtensionsRegistry, + makeApi, +} from '@superset-ui/core'; +import setupExtensions from 'src/setup/setupExtensions'; +import DashboardEmbedModal from './index'; + +const defaultResponse = { + result: { uuid: 'uuid', dashboard_id: '1', allowed_domains: ['example.com'] }, +}; + +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + makeApi: jest.fn(), +})); + +const mockOnHide = jest.fn(); +const defaultProps = { + dashboardId: '1', + show: true, + onHide: mockOnHide, +}; +const resetMockApi = () => { + (makeApi as any).mockReturnValue( + jest.fn().mockResolvedValue(defaultResponse), + ); +}; +const setMockApiNotFound = () => { + const notFound = new SupersetApiError({ message: 'Not found', status: 404 }); + (makeApi as any).mockReturnValue(jest.fn().mockRejectedValue(notFound)); +}; + +const setup = () => { + render(, { useRedux: true }); + resetMockApi(); +}; + +beforeEach(() => { + jest.clearAllMocks(); + resetMockApi(); +}); + +it('renders', async () => { + setup(); + await waitFor(() => { + expect(screen.getByText('Embed')).toBeInTheDocument(); + }); +}); + +it('displays loading state', async () => { + setup(); + await waitFor(() => { + expect(screen.getByRole('status', { name: 'Loading' })).toBeInTheDocument(); + }); +}); + +it('renders the modal default content', async () => { + render(, { useRedux: true }); + await waitFor(() => { + expect(screen.getByText('Settings')).toBeInTheDocument(); + expect( + screen.getByText(new RegExp(/Allowed Domains/, 'i')), + ).toBeInTheDocument(); + }); +}); + +it('renders the correct actions when dashboard is ready to embed', async () => { + setup(); + await waitFor(() => { + expect( + screen.getByRole('button', { name: 'Deactivate' }), + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Save changes' }), + ).toBeInTheDocument(); + }); +}); + +it('renders the correct actions when dashboard is not ready to embed', async () => { + setMockApiNotFound(); + setup(); + await waitFor(() => { + expect( + screen.getByRole('button', { name: 'Enable embedding' }), + ).toBeInTheDocument(); + }); +}); + +it('enables embedding', async () => { + setMockApiNotFound(); + setup(); + + await waitFor(() => { + const enableEmbed = screen.getByRole('button', { + name: 'Enable embedding', + }); + expect(enableEmbed).toBeInTheDocument(); + fireEvent.click(enableEmbed); + }); + + await waitFor(() => { + expect( + screen.getByRole('button', { name: 'Deactivate' }), + ).toBeInTheDocument(); + }); +}); + +it('shows and hides the confirmation modal on deactivation', async () => { + setup(); + + await waitFor(() => { + const deactivate = screen.getByRole('button', { name: 'Deactivate' }); + fireEvent.click(deactivate); + }); + + await waitFor(() => { + expect(screen.getByText('Disable embedding?')).toBeInTheDocument(); + expect( + screen.getByText('This will remove your current embed configuration.'), + ).toBeInTheDocument(); + }); + + const okBtn = screen.getByRole('button', { name: 'OK' }); + fireEvent.click(okBtn); + + await waitFor(() => { + expect(screen.queryByText('Disable embedding?')).not.toBeInTheDocument(); + }); +}); + +it('enables the "Save Changes" button', async () => { + setup(); + + await waitFor(() => { + const allowedDomainsInput = screen.getByLabelText( + new RegExp(/Allowed Domains/, 'i'), + ); + const saveChangesBtn = screen.getByRole('button', { name: 'Save changes' }); + + expect(saveChangesBtn).toBeDisabled(); + expect(allowedDomainsInput).toBeInTheDocument(); + + fireEvent.change(allowedDomainsInput, { target: { value: 'test.com' } }); + expect(saveChangesBtn).toBeEnabled(); + }); +}); + +it('adds extension to DashboardEmbedModal', async () => { + const extensionsRegistry = getExtensionsRegistry(); + + extensionsRegistry.set('embedded.modal', () => ( + <>dashboard.embed.modal.extension component + )); + + setupExtensions(); + setup(); + + await waitFor(() => { + expect( + screen.getByText('dashboard.embed.modal.extension component'), + ).toBeInTheDocument(); + }); +}); diff --git a/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx b/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx similarity index 94% rename from superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx rename to superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx index 8d7702edd4d3d..218a9d37a7b5c 100644 --- a/superset-frontend/src/dashboard/components/DashboardEmbedControls.tsx +++ b/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx @@ -31,7 +31,7 @@ import Button from 'src/components/Button'; import { Input } from 'src/components/Input'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import { FormItem } from 'src/components/Form'; -import { EmbeddedDashboard } from '../types'; +import { EmbeddedDashboard } from 'src/dashboard/types'; const extensionsRegistry = getExtensionsRegistry(); @@ -135,6 +135,7 @@ export const DashboardEmbedControls = ({ dashboardId, onHide }: Props) => { // 404 just means the dashboard isn't currently embedded return { result: null }; } + addDangerToast(t('Sorry, something went wrong. Please try again.')); throw err; }) .then(({ result }) => { @@ -199,6 +200,7 @@ export const DashboardEmbedControls = ({ dashboardId, onHide }: Props) => { setAllowedDomains(event.target.value)} @@ -237,12 +239,17 @@ export const DashboardEmbedControls = ({ dashboardId, onHide }: Props) => { ); }; -export const DashboardEmbedModal = (props: Props) => { +const DashboardEmbedModal = (props: Props) => { const { show, onHide } = props; + const DashboardEmbedModalExtension = extensionsRegistry.get('embedded.modal'); - return ( + return DashboardEmbedModalExtension ? ( + + ) : ( ); }; + +export default DashboardEmbedModal; diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 7d96ad6b54590..e94aa610a1557 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -55,7 +55,7 @@ import setPeriodicRunner, { stopPeriodicRender, } from 'src/dashboard/util/setPeriodicRunner'; import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions'; -import { DashboardEmbedModal } from '../DashboardEmbedControls'; +import DashboardEmbedModal from '../EmbeddedModal'; import OverwriteConfirm from '../OverwriteConfirm'; const extensionsRegistry = getExtensionsRegistry(); From c3e94cedd1348186a28a6b51324b173fe9bc5370 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 25 Jan 2024 15:41:08 +0100 Subject: [PATCH 2/2] Enhance tests --- .../EmbeddedModal/EmbeddedModal.test.tsx | 123 ++++++++---------- 1 file changed, 54 insertions(+), 69 deletions(-) diff --git a/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx index 7add60fc9abc5..a33115bf36c8f 100644 --- a/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx +++ b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx @@ -18,8 +18,12 @@ */ import React from 'react'; -import { fireEvent, waitFor } from '@testing-library/react'; -import { render, screen } from 'spec/helpers/testing-library'; +import { + render, + screen, + fireEvent, + waitFor, +} from 'spec/helpers/testing-library'; import '@testing-library/jest-dom'; import { SupersetApiError, @@ -64,85 +68,70 @@ beforeEach(() => { resetMockApi(); }); -it('renders', async () => { +test('renders', async () => { setup(); - await waitFor(() => { - expect(screen.getByText('Embed')).toBeInTheDocument(); - }); + expect(await screen.findByText('Embed')).toBeInTheDocument(); }); -it('displays loading state', async () => { +test('renders loading state', async () => { setup(); await waitFor(() => { expect(screen.getByRole('status', { name: 'Loading' })).toBeInTheDocument(); }); }); -it('renders the modal default content', async () => { +test('renders the modal default content', async () => { render(, { useRedux: true }); - await waitFor(() => { - expect(screen.getByText('Settings')).toBeInTheDocument(); - expect( - screen.getByText(new RegExp(/Allowed Domains/, 'i')), - ).toBeInTheDocument(); - }); + expect(await screen.findByText('Settings')).toBeInTheDocument(); + expect( + screen.getByText(new RegExp(/Allowed Domains/, 'i')), + ).toBeInTheDocument(); }); -it('renders the correct actions when dashboard is ready to embed', async () => { +test('renders the correct actions when dashboard is ready to embed', async () => { setup(); - await waitFor(() => { - expect( - screen.getByRole('button', { name: 'Deactivate' }), - ).toBeInTheDocument(); - expect( - screen.getByRole('button', { name: 'Save changes' }), - ).toBeInTheDocument(); - }); + expect( + await screen.findByRole('button', { name: 'Deactivate' }), + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Save changes' }), + ).toBeInTheDocument(); }); -it('renders the correct actions when dashboard is not ready to embed', async () => { +test('renders the correct actions when dashboard is not ready to embed', async () => { setMockApiNotFound(); setup(); - await waitFor(() => { - expect( - screen.getByRole('button', { name: 'Enable embedding' }), - ).toBeInTheDocument(); - }); + expect( + await screen.findByRole('button', { name: 'Enable embedding' }), + ).toBeInTheDocument(); }); -it('enables embedding', async () => { +test('enables embedding', async () => { setMockApiNotFound(); setup(); - await waitFor(() => { - const enableEmbed = screen.getByRole('button', { - name: 'Enable embedding', - }); - expect(enableEmbed).toBeInTheDocument(); - fireEvent.click(enableEmbed); + const enableEmbed = await screen.findByRole('button', { + name: 'Enable embedding', }); + expect(enableEmbed).toBeInTheDocument(); - await waitFor(() => { - expect( - screen.getByRole('button', { name: 'Deactivate' }), - ).toBeInTheDocument(); - }); + fireEvent.click(enableEmbed); + + expect( + await screen.findByRole('button', { name: 'Deactivate' }), + ).toBeInTheDocument(); }); -it('shows and hides the confirmation modal on deactivation', async () => { +test('shows and hides the confirmation modal on deactivation', async () => { setup(); - await waitFor(() => { - const deactivate = screen.getByRole('button', { name: 'Deactivate' }); - fireEvent.click(deactivate); - }); + const deactivate = await screen.findByRole('button', { name: 'Deactivate' }); + fireEvent.click(deactivate); - await waitFor(() => { - expect(screen.getByText('Disable embedding?')).toBeInTheDocument(); - expect( - screen.getByText('This will remove your current embed configuration.'), - ).toBeInTheDocument(); - }); + expect(await screen.findByText('Disable embedding?')).toBeInTheDocument(); + expect( + screen.getByText('This will remove your current embed configuration.'), + ).toBeInTheDocument(); const okBtn = screen.getByRole('button', { name: 'OK' }); fireEvent.click(okBtn); @@ -152,24 +141,22 @@ it('shows and hides the confirmation modal on deactivation', async () => { }); }); -it('enables the "Save Changes" button', async () => { +test('enables the "Save Changes" button', async () => { setup(); - await waitFor(() => { - const allowedDomainsInput = screen.getByLabelText( - new RegExp(/Allowed Domains/, 'i'), - ); - const saveChangesBtn = screen.getByRole('button', { name: 'Save changes' }); + const allowedDomainsInput = await screen.findByLabelText( + new RegExp(/Allowed Domains/, 'i'), + ); + const saveChangesBtn = screen.getByRole('button', { name: 'Save changes' }); - expect(saveChangesBtn).toBeDisabled(); - expect(allowedDomainsInput).toBeInTheDocument(); + expect(saveChangesBtn).toBeDisabled(); + expect(allowedDomainsInput).toBeInTheDocument(); - fireEvent.change(allowedDomainsInput, { target: { value: 'test.com' } }); - expect(saveChangesBtn).toBeEnabled(); - }); + fireEvent.change(allowedDomainsInput, { target: { value: 'test.com' } }); + expect(saveChangesBtn).toBeEnabled(); }); -it('adds extension to DashboardEmbedModal', async () => { +test('adds extension to DashboardEmbedModal', async () => { const extensionsRegistry = getExtensionsRegistry(); extensionsRegistry.set('embedded.modal', () => ( @@ -179,9 +166,7 @@ it('adds extension to DashboardEmbedModal', async () => { setupExtensions(); setup(); - await waitFor(() => { - expect( - screen.getByText('dashboard.embed.modal.extension component'), - ).toBeInTheDocument(); - }); + expect( + await screen.findByText('dashboard.embed.modal.extension component'), + ).toBeInTheDocument(); });