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.

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

feat: Dynamic Form for edit DB Modal #14845

Merged
merged 35 commits into from
Jun 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
05a1bb7
split db modal file
eschutho Apr 21, 2021
a8afce9
split db modal file
eschutho Apr 21, 2021
b69bf16
hook up available databases
eschutho Apr 28, 2021
6eae68c
add comment
eschutho May 18, 2021
e8cd4f4
split db modal file
eschutho Apr 21, 2021
95917e4
hook up available databases
eschutho Apr 28, 2021
e764072
use new validation component
eschutho May 21, 2021
842f856
first draft
AAfghahi May 25, 2021
7cb4098
use new validation component
eschutho May 21, 2021
253053b
first draft
AAfghahi May 26, 2021
a47d52d
first draft
AAfghahi May 26, 2021
8cec32b
merge
AAfghahi May 26, 2021
5a785d7
Merge branch 'master' of https://github.com/apache/superset into pexd…
hughhhh May 27, 2021
92d81b9
using paxdax feature branch
AAfghahi May 28, 2021
19a4c3a
get tests passing
AAfghahi Jun 1, 2021
e7f0dbb
split db modal file
eschutho Apr 21, 2021
b57d090
hook up available databases
eschutho Apr 28, 2021
271c6e9
use new validation component
eschutho May 21, 2021
ef63e3c
feat(db-connection-ui): Allow users to pick engine (#14884)
hughhhh Jun 1, 2021
013674b
Merge branch 'pexdax/db-connection-ui' into ch6732_editDBModal
AAfghahi Jun 1, 2021
9246710
revisions
AAfghahi Jun 1, 2021
0457bfe
fix conflicts
hughhhh Jun 1, 2021
502abec
fix package-lock.json
hughhhh Jun 1, 2021
25b435b
Merge branch 'pexdax/db-connection-ui' of ssh://github.com/apache/sup…
AAfghahi Jun 1, 2021
29a899d
# This is a combination of 6 commits.
eschutho Jun 1, 2021
f45cae2
fix test for db modal
hughhhh Jun 2, 2021
1aae834
feat(db-connection-ui): Allow users to pick engine (#14884)
hughhhh Jun 1, 2021
a380629
Merge branch 'master' into pexdax/db-connection-ui
hughhhh Jun 2, 2021
c4125fb
pulling feature branch
AAfghahi Jun 2, 2021
95c7f66
more revisions
AAfghahi Jun 2, 2021
e539834
used db.backend
AAfghahi Jun 2, 2021
47c30cb
added engine to model
AAfghahi Jun 2, 2021
9fac1eb
elizabeth revisions
AAfghahi Jun 2, 2021
78d7edb
elizabeth revisions
AAfghahi Jun 3, 2021
fc6d5d9
Merge branch 'pexdax/db-connection-ui' into ch6732_editDBModal
AAfghahi Jun 3, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
* under the License.
*/
import { DATABASE_LIST } from './helper';

// TODO: Add new tests with the modal
describe('Add database', () => {
beforeEach(() => {
cy.login();
});

it('should keep create modal open when error', () => {
xit('should keep create modal open when error', () => {
cy.visit(DATABASE_LIST);

// open modal
Expand Down Expand Up @@ -60,7 +60,7 @@ describe('Add database', () => {
cy.get('[data-test="database-modal"]').should('not.be.visible');
});

it('should keep update modal open when error', () => {
xit('should keep update modal open when error', () => {
// open modal
cy.get('[data-test="database-edit"]:last').click();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import { SupersetTheme, JsonObject } from '@superset-ui/core';
import { InputProps } from 'antd/lib/input';
import ValidatedInput from 'src/components/Form/LabeledErrorBoundInput';
import {
StyledFormHeader,
formScrollableStyles,
validatedFormStyles,
StyledFormHeader,
} from './styles';
import { DatabaseForm } from '../types';
import { DatabaseForm, DatabaseObject } from '../types';

export const FormFieldOrder = [
'host',
Expand All @@ -43,17 +43,20 @@ interface FieldPropTypes {
};
validationErrors: JsonObject | null;
getValidation: () => void;
db?: DatabaseObject;
}

const hostField = ({
required,
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="host"
name="host"
value={db?.parameters?.host || ''}
required={required}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.host}
Expand All @@ -68,11 +71,13 @@ const portField = ({
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="port"
name="port"
required={required}
value={db?.parameters?.port || ''}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.port}
placeholder="e.g. 5432"
Expand All @@ -86,29 +91,33 @@ const databaseField = ({
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="database"
name="database"
required={required}
value={db?.parameters?.database || ''}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.database}
placeholder="e.g. world_population"
label="Database name"
onChange={changeMethods.onParametersChange}
helpText="Copy the name of the PostgreSQL database you are trying to connect to."
helpText="Copy the name of the database you are trying to connect to."
/>
);
const usernameField = ({
required,
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="username"
name="username"
required={required}
value={db?.parameters?.username || ''}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.username}
placeholder="e.g. Analytics"
Expand All @@ -121,11 +130,14 @@ const passwordField = ({
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="password"
name="password"
required={required}
type="password"
value={db?.parameters?.password || ''}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.password}
placeholder="e.g. ********"
Expand All @@ -138,11 +150,13 @@ const displayField = ({
changeMethods,
getValidation,
validationErrors,
db,
}: FieldPropTypes) => (
<ValidatedInput
id="database_name"
name="database_name"
required={required}
value={db?.database_name || ''}
validationMethods={{ onBlur: getValidation }}
errorMessage={validationErrors?.database_name}
placeholder=""
Expand All @@ -167,8 +181,12 @@ const DatabaseConnectionForm = ({
onChange,
validationErrors,
getValidation,
db,
isEditMode = false,
}: {
isEditMode?: boolean;
dbModel: DatabaseForm;
db: Partial<DatabaseObject> | null;
onParametersChange: (
event: FormEvent<InputProps> | { target: HTMLInputElement },
) => void;
Expand Down Expand Up @@ -203,6 +221,7 @@ const DatabaseConnectionForm = ({
changeMethods: { onParametersChange, onChange },
validationErrors,
getValidation,
db,
key: field,
}),
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import React from 'react';
import fetchMock from 'fetch-mock';
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import { render, screen } from 'spec/helpers/testing-library';
import DatabaseModal from './index';

const dbProps = {
Expand All @@ -30,7 +30,7 @@ const dbProps = {
};

const DATABASE_FETCH_ENDPOINT = 'glob:*/api/v1/database/10';
const DATABASE_POST_ENDPOINT = 'glob:*/api/v1/database/';
// const DATABASE_POST_ENDPOINT = 'glob:*/api/v1/database/';
const AVAILABLE_DB_ENDPOINT = 'glob:*/api/v1/database/available*';
fetchMock.config.overwriteRoutes = true;
fetchMock.get(DATABASE_FETCH_ENDPOINT, {
Expand Down Expand Up @@ -203,68 +203,68 @@ describe('DatabaseModal', () => {
// Both checkboxes go unchecked, so the field should no longer render
expect(schemaField).not.toHaveClass('open');
});

describe('create database', () => {
beforeEach(() => {
fetchMock.post(DATABASE_POST_ENDPOINT, {
id: 10,
});
fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
databases: [
{
engine: 'mysql',
name: 'MySQL',
preferred: false,
},
],
});
});
const props = {
...dbProps,
databaseId: null,
database_name: null,
sqlalchemy_uri: null,
};
it('should show a form when dynamic_form is selected', async () => {
render(<DatabaseModal {...props} />, { 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('should close the modal on save if using the sqlalchemy form', async () => {
const onHideMock = jest.fn();
render(<DatabaseModal {...props} onHide={onHideMock} />, {
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();
});
});
// TODO: rewrite when Modal is complete
AAfghahi marked this conversation as resolved.
Show resolved Hide resolved
// describe('create database', () => {
// beforeEach(() => {
// fetchMock.post(DATABASE_POST_ENDPOINT, {
// id: 10,
// });
// fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
// databases: [
// {
// engine: 'mysql',
// name: 'MySQL',
// preferred: false,
// },
// ],
// });
// });
// const props = {
// ...dbProps,
// databaseId: null,
// database_name: null,
// sqlalchemy_uri: null,
// };
// it('should show a form when dynamic_form is selected', async () => {
// render(<DatabaseModal {...props} />, { 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('should close the modal on save if using the sqlalchemy form', async () => {
// const onHideMock = jest.fn();
// render(<DatabaseModal {...props} onHide={onHideMock} />, {
// 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('edit database', () => {
beforeEach(() => {
Expand All @@ -290,8 +290,6 @@ describe('DatabaseModal', () => {
// 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, {
Expand All @@ -309,15 +307,15 @@ describe('DatabaseModal', () => {
});
render(<DatabaseModal {...dbProps} />, { useRedux: true });

await screen.findByText(/todo/i);
await screen.findByText(/edit database/i);

// // it should have tabs
const tabs = screen.getAllByRole('tab');
expect(tabs.length).toEqual(2);

// it should show a TODO for now
AAfghahi marked this conversation as resolved.
Show resolved Hide resolved
const todoText = screen.getAllByText(/todo/i);
expect(todoText[0]).toBeVisible();
const headerText = screen.getByText(/edit database/i);
expect(headerText).toBeVisible();
});
});
});
Loading