Skip to content

Commit

Permalink
fix: A newly connected database doesn't appear in the databases list …
Browse files Browse the repository at this point in the history 
…if user connected database using the 'plus' button (#19967)

* fix: A newly connected database doesn't appear in the databases list if user connected database using the 'plus' button

* include onDatabaseAdd on successful import
  • Loading branch information
@diegomedina248
diegomedina248 authored Jun 12, 2022
1 parent 86368dd commit 8345eb4
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 21 deletions.
13 changes: 13 additions & 0 deletions superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { SupersetClient, t, styled } from '@superset-ui/core';
import React, { useState, useMemo, useEffect } from 'react';
import rison from 'rison';
import { useSelector } from 'react-redux';
import { useQueryParams, BooleanParam } from 'use-query-params';

import Loading from 'src/components/Loading';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { useListViewResource } from 'src/views/CRUD/hooks';
Expand Down Expand Up @@ -91,6 +93,10 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
state => state.user,
);

const [query, setQuery] = useQueryParams({
databaseAdded: BooleanParam,
});

const [databaseModalOpen, setDatabaseModalOpen] = useState<boolean>(false);
const [databaseCurrentlyDeleting, setDatabaseCurrentlyDeleting] =
useState<DatabaseDeleteObject | null>(null);
Expand All @@ -110,6 +116,13 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
ALLOWED_EXTENSIONS,
} = useSelector<any, ExtentionConfigs>(state => state.common.conf);

useEffect(() => {
if (query?.databaseAdded) {
setQuery({ databaseAdded: undefined });
refreshData();
}
}, [query, setQuery, refreshData]);

const openDatabaseDeleteModal = (database: DatabaseObject) =>
SupersetClient.get({
endpoint: `/api/v1/database/${database.id}/related_objects/`,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setImportingModal(false);
setPasswords({});
setConfirmedOverwrite(false);
if (onDatabaseAdd) onDatabaseAdd();
onHide();
};

Expand Down Expand Up @@ -652,6 +651,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
confirmedOverwrite,
);
if (dbId) {
if (onDatabaseAdd) onDatabaseAdd();
onClose();
addSuccessToast(t('Database connected'));
}
Expand Down
43 changes: 23 additions & 20 deletions superset-frontend/src/views/components/Menu.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,16 @@ beforeEach(() => {

test('should render', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
const { container } = render(<Menu {...mockedProps} />, { useRedux: true });
const { container } = render(<Menu {...mockedProps} />, {
useRedux: true,
useQueryParams: true,
});
expect(container).toBeInTheDocument();
});

test('should render the navigation', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
expect(screen.getByRole('navigation')).toBeInTheDocument();
});

Expand All @@ -265,7 +268,7 @@ test('should render the brand', () => {
brand: { alt, icon },
},
} = mockedProps;
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
const image = screen.getByAltText(alt);
expect(image).toHaveAttribute('src', icon);
});
Expand All @@ -275,7 +278,7 @@ test('should render all the top navbar menu items', () => {
const {
data: { menu },
} = mockedProps;
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
menu.forEach(item => {
expect(screen.getByText(item.label)).toBeInTheDocument();
});
Expand All @@ -286,7 +289,7 @@ test('should render the top navbar child menu items', async () => {
const {
data: { menu },
} = mockedProps;
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
const sources = screen.getByText('Sources');
userEvent.hover(sources);
const datasets = await screen.findByText('Datasets');
Expand All @@ -300,7 +303,7 @@ test('should render the top navbar child menu items', async () => {

test('should render the dropdown items', async () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...notanonProps} />, { useRedux: true });
render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
const dropdown = screen.getByTestId('new-dropdown-icon');
userEvent.hover(dropdown);
// todo (philip): test data submenu
Expand All @@ -326,14 +329,14 @@ test('should render the dropdown items', async () => {

test('should render the Settings', async () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
const settings = await screen.findByText('Settings');
expect(settings).toBeInTheDocument();
});

test('should render the Settings menu item', async () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
userEvent.hover(screen.getByText('Settings'));
const label = await screen.findByText('Security');
expect(label).toBeInTheDocument();
Expand All @@ -344,21 +347,21 @@ test('should render the Settings dropdown child menu items', async () => {
const {
data: { settings },
} = mockedProps;
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
userEvent.hover(screen.getByText('Settings'));
const listUsers = await screen.findByText('List Users');
expect(listUsers).toHaveAttribute('href', settings[0].childs[0].url);
});

test('should render the plus menu (+) when user is not anonymous', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...notanonProps} />, { useRedux: true });
render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
expect(screen.getByTestId('new-dropdown')).toBeInTheDocument();
});

test('should NOT render the plus menu (+) when user is anonymous', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
});

Expand All @@ -370,7 +373,7 @@ test('should render the user actions when user is not anonymous', async () => {
},
} = mockedProps;

render(<Menu {...notanonProps} />, { useRedux: true });
render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
userEvent.hover(screen.getByText('Settings'));
const user = await screen.findByText('User');
expect(user).toBeInTheDocument();
Expand All @@ -384,7 +387,7 @@ test('should render the user actions when user is not anonymous', async () => {

test('should NOT render the user actions when user is anonymous', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
expect(screen.queryByText('User')).not.toBeInTheDocument();
});

Expand All @@ -396,7 +399,7 @@ test('should render the Profile link when available', async () => {
},
} = mockedProps;

render(<Menu {...notanonProps} />, { useRedux: true });
render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });

userEvent.hover(screen.getByText('Settings'));
const profile = await screen.findByText('Profile');
Expand All @@ -411,7 +414,7 @@ test('should render the About section and version_string, sha or build_number wh
},
} = mockedProps;

render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
userEvent.hover(screen.getByText('Settings'));
const about = await screen.findByText('About');
const version = await screen.findByText(`Version: ${version_string}`);
Expand All @@ -430,7 +433,7 @@ test('should render the Documentation link when available', async () => {
navbar_right: { documentation_url },
},
} = mockedProps;
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
userEvent.hover(screen.getByText('Settings'));
const doc = await screen.findByTitle('Documentation');
expect(doc).toHaveAttribute('href', documentation_url);
Expand All @@ -444,7 +447,7 @@ test('should render the Bug Report link when available', async () => {
},
} = mockedProps;

render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
const bugReport = await screen.findByTitle('Report a bug');
expect(bugReport).toHaveAttribute('href', bug_report_url);
});
Expand All @@ -457,19 +460,19 @@ test('should render the Login link when user is anonymous', () => {
},
} = mockedProps;

render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
const login = screen.getByText('Login');
expect(login).toHaveAttribute('href', user_login_url);
});

test('should render the Language Picker', () => {
useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
expect(screen.getByLabelText('Languages')).toBeInTheDocument();
});

test('should hide create button without proper roles', () => {
useSelectorMock.mockReturnValue({ roles: [] });
render(<Menu {...mockedProps} />, { useRedux: true });
render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
});
9 changes: 9 additions & 0 deletions superset-frontend/src/views/components/MenuRight.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import React, { Fragment, useState, useEffect } from 'react';
import rison from 'rison';
import { useSelector } from 'react-redux';
import { Link } from 'react-router-dom';
import { useQueryParams, BooleanParam } from 'use-query-params';

import {
t,
styled,
Expand Down Expand Up @@ -94,6 +96,10 @@ const RightMenu = ({
state => state.dashboardInfo?.id,
);

const [, setQuery] = useQueryParams({
databaseAdded: BooleanParam,
});

const { roles } = user;
const {
CSV_EXTENSIONS,
Expand Down Expand Up @@ -250,13 +256,16 @@ const RightMenu = ({
return null;
};

const handleDatabaseAdd = () => setQuery({ databaseAdded: true });

return (
<StyledDiv align={align}>
{canDatabase && (
<DatabaseModal
onHide={handleOnHideModal}
show={showModal}
dbEngine={engine}
onDatabaseAdd={handleDatabaseAdd}
/>
)}
<Menu
Expand Down

0 comments on commit 8345eb4

Please sign in to comment.