From fbe6f1605230cfc0a390b8fe52f3c96e35dea6a6 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Mon, 24 May 2021 10:13:56 -0700 Subject: [PATCH] database modal should close on connect with tab layout (#14771) --- .../database/DatabaseModal/SqlAlchemyForm.tsx | 2 + .../database/DatabaseModal/index.test.jsx | 536 +++++++++--------- .../data/database/DatabaseModal/index.tsx | 25 +- 3 files changed, 278 insertions(+), 285 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx index cf442b4a5..57105ebe8 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/SqlAlchemyForm.tsx @@ -44,6 +44,7 @@ const SqlAlchemyTab = ({ { - afterEach(fetchMock.reset); - describe('enzyme', () => { - let wrapper; - let spyOnUseSelector; - beforeAll(() => { - spyOnUseSelector = jest.spyOn(redux, 'useSelector'); - spyOnUseSelector.mockReturnValue(initialState.common.conf); + afterEach(fetchMock.restore); + describe('initial load', () => { + it('hides the forms from the db when not selected', () => { + render(, { useRedux: true }); + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + // Select SQL Lab tab + const sqlLabSettingsTab = screen.getByRole('tab', { + name: /sql lab/i, + }); + userEvent.click(sqlLabSettingsTab); + + const exposeInSqlLab = screen.getByText('Expose in SQL Lab'); + const exposeChoicesForm = exposeInSqlLab.parentElement.nextSibling; + const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; + expect(exposeChoicesForm).not.toHaveClass('open'); + expect(schemaField).not.toHaveClass('open'); }); + }); + it('renders all settings when "Expose in SQL Lab" is checked', () => { + render(, { useRedux: true }); + + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab + const sqlLabSettingsTab = screen.getByRole('tab', { + name: /sql lab/i, + }); + + userEvent.click(sqlLabSettingsTab); + + // Grab all SQL Lab settings by their labels + // const exposeInSqlLab = screen.getByText('Expose in SQL Lab'); + const exposeInSqlLab = screen.getByRole('checkbox', { + name: /expose in sql lab/i, + }); + + expect(exposeInSqlLab).not.toBeChecked(); + userEvent.click(exposeInSqlLab); + + // While checked make sure all checkboxes are showing + expect(exposeInSqlLab).toBeChecked(); + const checkboxes = screen + .getAllByRole('checkbox') + .filter(checkbox => !checkbox.checked); + + expect(checkboxes.length).toEqual(4); + }); + + it('renders the schema field when allowCTAS is checked', () => { + render(, { useRedux: true }); + + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab + const sqlLabSettingsTab = screen.getByRole('tab', { + name: /sql lab/i, + }); + userEvent.click(sqlLabSettingsTab); + // Grab CTAS & schema field by their labels + const allowCTAS = screen.getByLabelText('Allow CREATE TABLE AS'); + const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; + + // While CTAS & CVAS are unchecked, schema field is not visible + expect(schemaField).not.toHaveClass('open'); + + // Check "Allow CTAS" to reveal schema field + userEvent.click(allowCTAS); + expect(schemaField).toHaveClass('open'); + + // Uncheck "Allow CTAS" to hide schema field again + userEvent.click(allowCTAS); + expect(schemaField).not.toHaveClass('open'); + }); + + it('renders the schema field when allowCVAS is checked', () => { + render(, { useRedux: true }); + + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab + const sqlLabSettingsTab = screen.getByRole('tab', { + name: /sql lab/i, + }); + userEvent.click(sqlLabSettingsTab); + // Grab CVAS by it's label & schema field + const allowCVAS = screen.getByText('Allow CREATE VIEW AS'); + const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; + + // While CTAS & CVAS are unchecked, schema field is not visible + expect(schemaField).not.toHaveClass('open'); + + // Check "Allow CVAS" to reveal schema field + userEvent.click(allowCVAS); + expect(schemaField).toHaveClass('open'); + + // Uncheck "Allow CVAS" to hide schema field again + userEvent.click(allowCVAS); + expect(schemaField).not.toHaveClass('open'); + }); + + it('renders the schema field when both allowCTAS and allowCVAS are checked', () => { + render(, { useRedux: true }); + + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab + const sqlLabSettingsTab = screen.getByRole('tab', { + name: /sql lab/i, + }); + userEvent.click(sqlLabSettingsTab); + // Grab CTAS and CVAS by their labels, & schema field + const allowCTAS = screen.getByText('Allow CREATE TABLE AS'); + const allowCVAS = screen.getByText('Allow CREATE VIEW AS'); + const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; + + // While CTAS & CVAS are unchecked, schema field is not visible + expect(schemaField).not.toHaveClass('open'); + + // Check both "Allow CTAS" and "Allow CVAS" to reveal schema field + userEvent.click(allowCTAS); + userEvent.click(allowCVAS); + expect(schemaField).toHaveClass('open'); + // Uncheck both "Allow CTAS" and "Allow CVAS" to hide schema field again + userEvent.click(allowCTAS); + userEvent.click(allowCVAS); + + // Both checkboxes go unchecked, so the field should no longer render + expect(schemaField).not.toHaveClass('open'); + }); + + describe('create database', () => { beforeEach(() => { - wrapper = mount( - - - , - ); + fetchMock.post(DATABASE_POST_ENDPOINT, { + id: 10, + }); + fetchMock.mock(AVAILABLE_DB_ENDPOINT, { + databases: [ + { + engine: 'mysql', + name: 'MySQL', + preferred: false, + }, + ], + }); }); - afterEach(() => { - wrapper.unmount(); + const props = { + ...dbProps, + databaseId: null, + database_name: null, + sqlalchemy_uri: null, + }; + it('should show a form when dynamic_form is selected', async () => { + render(, { useRedux: true }); + // it should have the correct header text + const headerText = screen.getByText(/connect a database/i); + expect(headerText).toBeVisible(); + + await screen.findByText(/display name/i); + + // it does not fetch any databases if no id is passed in + expect(fetchMock.calls(DATABASE_FETCH_ENDPOINT).length).toEqual(0); + + // todo we haven't hooked this up to load dynamically yet so + // we can't currently test it }); - it('renders', () => { - expect(wrapper.find(DatabaseModal)).toExist(); - }); - it('renders a Modal', () => { - expect(wrapper.find(Modal)).toExist(); - }); - it('renders "Connect a database" header when no database is included', () => { - expect(wrapper.find('h4').text()).toEqual('Connect a database'); - }); - it('renders "Edit database" header when database prop is included', () => { - const editWrapper = mount(); - waitForComponentToPaint(editWrapper); - expect(editWrapper.find('h4').text()).toEqual('Edit database'); - editWrapper.unmount(); - }); - it('renders a Tabs menu', () => { - expect(wrapper.find(Tabs)).toExist(); - }); - it('renders two TabPanes', () => { - expect(wrapper.find('.ant-tabs-tab')).toExist(); - expect(wrapper.find('.ant-tabs-tab')).toHaveLength(2); - }); - it('renders input elements for Connection section', () => { - expect(wrapper.find('input[name="database_name"]')).toExist(); - expect(wrapper.find('input[name="sqlalchemy_uri"]')).toExist(); + it('should close the modal on save if using the sqlalchemy form', async () => { + const onHideMock = jest.fn(); + render(, { + useRedux: true, + }); + // button should be disabled by default + const submitButton = screen.getByTestId('modal-confirm-button'); + expect(submitButton).toBeDisabled(); + + const displayName = screen.getByTestId('database-name-input'); + userEvent.type(displayName, 'MyTestDB'); + expect(displayName.value).toBe('MyTestDB'); + const sqlalchemyInput = screen.getByTestId('sqlalchemy-uri-input'); + userEvent.type(sqlalchemyInput, 'some_url'); + expect(sqlalchemyInput.value).toBe('some_url'); + + // button should not be disabled now + expect(submitButton).toBeEnabled(); + + await waitFor(() => { + userEvent.click(submitButton); + }); + expect(fetchMock.calls(DATABASE_POST_ENDPOINT)).toHaveLength(1); + expect(onHideMock).toHaveBeenCalled(); }); }); - describe('RTL', () => { - describe('initial load', () => { - it('hides the forms from the db when not selected', () => { - render(, { useRedux: true }); - // Select Advanced tab - const advancedTab = screen.getByRole('tab', { - name: /advanced/i, - }); - userEvent.click(advancedTab); - // Select SQL Lab tab - const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab/i, - }); - userEvent.click(sqlLabSettingsTab); - - const exposeInSqlLab = screen.getByText('Expose in SQL Lab'); - const exposeChoicesForm = exposeInSqlLab.parentElement.nextSibling; - const schemaField = screen.getByText('CTAS & CVAS SCHEMA') - .parentElement; - expect(exposeChoicesForm).not.toHaveClass('open'); - expect(schemaField).not.toHaveClass('open'); - }); - }); - it('renders all settings when "Expose in SQL Lab" is checked', () => { - render(, { useRedux: true }); - - // Select Advanced tab - const advancedTab = screen.getByRole('tab', { - name: /advanced/i, - }); - userEvent.click(advancedTab); - - // Select SQL Lab tab - const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab/i, - }); - - userEvent.click(sqlLabSettingsTab); - - // Grab all SQL Lab settings by their labels - // const exposeInSqlLab = screen.getByText('Expose in SQL Lab'); - const exposeInSqlLab = screen.getByRole('checkbox', { - name: /expose in sql lab/i, - }); - - expect(exposeInSqlLab).not.toBeChecked(); - userEvent.click(exposeInSqlLab); - - // While checked make sure all checkboxes are showing - expect(exposeInSqlLab).toBeChecked(); - const checkboxes = screen - .getAllByRole('checkbox') - .filter(checkbox => !checkbox.checked); - - expect(checkboxes.length).toEqual(4); - }); - - it('renders the schema field when allowCTAS is checked', () => { - render(, { useRedux: true }); - - // Select Advanced tab - const advancedTab = screen.getByRole('tab', { - name: /advanced/i, - }); - userEvent.click(advancedTab); - - // Select SQL Lab tab - const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab/i, - }); - userEvent.click(sqlLabSettingsTab); - // Grab CTAS & schema field by their labels - const allowCTAS = screen.getByLabelText('Allow CREATE TABLE AS'); - const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; - - // While CTAS & CVAS are unchecked, schema field is not visible - expect(schemaField).not.toHaveClass('open'); - - // Check "Allow CTAS" to reveal schema field - userEvent.click(allowCTAS); - expect(schemaField).toHaveClass('open'); - - // Uncheck "Allow CTAS" to hide schema field again - userEvent.click(allowCTAS); - expect(schemaField).not.toHaveClass('open'); - }); - - it('renders the schema field when allowCVAS is checked', () => { - render(, { useRedux: true }); - - // Select Advanced tab - const advancedTab = screen.getByRole('tab', { - name: /advanced/i, - }); - userEvent.click(advancedTab); - - // Select SQL Lab tab - const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab/i, - }); - userEvent.click(sqlLabSettingsTab); - // Grab CVAS by it's label & schema field - const allowCVAS = screen.getByText('Allow CREATE VIEW AS'); - const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; - - // While CTAS & CVAS are unchecked, schema field is not visible - expect(schemaField).not.toHaveClass('open'); - - // Check "Allow CVAS" to reveal schema field - userEvent.click(allowCVAS); - expect(schemaField).toHaveClass('open'); - - // Uncheck "Allow CVAS" to hide schema field again - userEvent.click(allowCVAS); - expect(schemaField).not.toHaveClass('open'); - }); - - it('renders the schema field when both allowCTAS and allowCVAS are checked', () => { - render(, { useRedux: true }); - - // Select Advanced tab - const advancedTab = screen.getByRole('tab', { - name: /advanced/i, - }); - userEvent.click(advancedTab); - - // Select SQL Lab tab - const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab/i, - }); - userEvent.click(sqlLabSettingsTab); - // Grab CTAS and CVAS by their labels, & schema field - const allowCTAS = screen.getByText('Allow CREATE TABLE AS'); - const allowCVAS = screen.getByText('Allow CREATE VIEW AS'); - const schemaField = screen.getByText('CTAS & CVAS SCHEMA').parentElement; - - // While CTAS & CVAS are unchecked, schema field is not visible - expect(schemaField).not.toHaveClass('open'); - - // Check both "Allow CTAS" and "Allow CVAS" to reveal schema field - userEvent.click(allowCTAS); - userEvent.click(allowCVAS); - expect(schemaField).toHaveClass('open'); - // Uncheck both "Allow CTAS" and "Allow CVAS" to hide schema field again - userEvent.click(allowCTAS); - userEvent.click(allowCVAS); - - // Both checkboxes go unchecked, so the field should no longer render - expect(schemaField).not.toHaveClass('open'); - }); - - describe('create database', () => { - it('should show a form when dynamic_form is selected', async () => { - const props = { - ...dbProps, - databaseId: null, - database_name: null, - sqlalchemy_uri: null, - }; - render(, { useRedux: true }); - // it should have the correct header text - const headerText = screen.getByText(/connect a database/i); - expect(headerText).toBeVisible(); - - await screen.findByText(/display name/i); - - // it does not fetch any databases if no id is passed in - expect(fetchMock.calls().length).toEqual(0); - - // todo we haven't hooked this up to load dynamically yet so - // we can't currently test it - }); - }); - - describe('edit database', () => { - it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { - render(, { useRedux: true }); - - // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - expect(tabs[0]).toHaveTextContent('Basic'); - expect(tabs[1]).toHaveTextContent('Advanced'); - - // it should have the correct header text - const headerText = screen.getByText(/edit database/i); - expect(headerText).toBeVisible(); - - // todo add more when this form is built out - }); - it('renders the dynamic form when the dynamic_form configuration method is set', async () => { - fetchMock.get(DATABASE_ENDPOINT, { - result: { - id: 10, - database_name: 'my database', - expose_in_sqllab: false, - allow_ctas: false, - allow_cvas: false, - configuration_method: 'dynamic_form', - parameters: { - database: 'mydatabase', - }, + describe('edit database', () => { + beforeEach(() => { + fetchMock.mock(AVAILABLE_DB_ENDPOINT, { + databases: [ + { + engine: 'mysql', + name: 'MySQL', + preferred: false, }, - }); - render(, { useRedux: true }); - - await screen.findByText(/todo/i); - - // // it should have tabs - const tabs = screen.getAllByRole('tab'); - expect(tabs.length).toEqual(2); - - // it should show a TODO for now - const todoText = screen.getAllByText(/todo/i); - expect(todoText[0]).toBeVisible(); + ], }); }); + it('renders the sqlalchemy form when the sqlalchemy_form configuration method is set', async () => { + render(, { useRedux: true }); + + // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + expect(tabs[0]).toHaveTextContent('Basic'); + expect(tabs[1]).toHaveTextContent('Advanced'); + + // it should have the correct header text + const headerText = screen.getByText(/edit database/i); + expect(headerText).toBeVisible(); + + // todo add more when this form is built out + }); + it('renders the dynamic form when the dynamic_form configuration method is set', async () => { + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'dynamic_form', + parameters: { + database: 'mydatabase', + }, + }, + }); + render(, { useRedux: true }); + + await screen.findByText(/todo/i); + + // // it should have tabs + const tabs = screen.getAllByRole('tab'); + expect(tabs.length).toEqual(2); + + // it should show a TODO for now + const todoText = screen.getAllByText(/todo/i); + expect(todoText[0]).toBeVisible(); + }); }); }); diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index fb3f0c5d9..545cba00a 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -202,7 +202,7 @@ const DatabaseModal: FunctionComponent = ({ const isEditMode = !!databaseId; const useSqlAlchemyForm = db?.configuration_method === CONFIGURATION_METHOD.SQLALCHEMY_URI; - + const useTabLayout = isEditMode || useSqlAlchemyForm; // Database fetch logic const { state: { loading: dbLoading, resource: dbFetched }, @@ -240,7 +240,7 @@ const DatabaseModal: FunctionComponent = ({ onHide(); }; - const onSave = () => { + const onSave = async () => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { id, ...update } = db || {}; if (db?.id) { @@ -258,14 +258,18 @@ const DatabaseModal: FunctionComponent = ({ }); } else if (db) { // Create - createResource(update as DatabaseObject).then(dbId => { - if (dbId) { - setHasConnectedDb(true); - if (onDatabaseAdd) { - onDatabaseAdd(); - } + const dbId = await createResource(update as DatabaseObject); + if (dbId) { + setHasConnectedDb(true); + if (onDatabaseAdd) { + onDatabaseAdd(); } - }); + if (useTabLayout) { + // tab layout only has one step + // so it should close immediately on save + onClose(); + } + } } }; @@ -336,7 +340,7 @@ const DatabaseModal: FunctionComponent = ({ FALSY_FORM_VALUES.includes(db?.parameters?.[field]), ).length); - return isEditMode || useSqlAlchemyForm ? ( + return useTabLayout ? ( [ antDTabsStyles, @@ -346,6 +350,7 @@ const DatabaseModal: FunctionComponent = ({ ]} name="database" disablePrimaryButton={disableSave} + data-test="database-modal" height="600px" onHandledPrimaryAction={onSave} onHide={onClose}