-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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(db-connection-ui): Show Preferred DBs #14951
Changes from 25 commits
6d0c45f
b0cdaba
8d96434
3eea36d
b0ab2da
c8b148e
e7f0dbb
b57d090
271c6e9
ef63e3c
8907bc1
96a87d8
717c9ab
e299bfa
8cfa0d8
7c610d5
dcf8144
ef23e53
2602827
530150e
508a6b9
607e01a
b343ca4
0df0cab
0d3c31d
c21987f
9d2b36e
6c9fc9c
4aa2428
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ import Tabs from 'src/components/Tabs'; | |
import { Alert, Select } from 'src/common/components'; | ||
import Modal from 'src/components/Modal'; | ||
import Button from 'src/components/Button'; | ||
import IconButton from 'src/components/IconButton'; | ||
import withToasts from 'src/messageToasts/enhancers/withToasts'; | ||
import { | ||
testDatabaseConnection, | ||
|
@@ -60,6 +61,7 @@ import { | |
formStyles, | ||
StyledBasicTab, | ||
SelectDatabaseStyles, | ||
StyledFormHeader, | ||
} from './styles'; | ||
|
||
const DOCUMENTATION_LINK = | ||
|
@@ -299,6 +301,73 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |
} | ||
}; | ||
|
||
const setDatabaseModel = option => { | ||
const isDynamic = | ||
availableDbs?.databases.filter(db => db.engine === option)[0] | ||
.parameters !== undefined; | ||
setDB({ | ||
type: ActionType.dbSelected, | ||
payload: { | ||
configuration_method: isDynamic | ||
? CONFIGURATION_METHOD.DYNAMIC_FORM | ||
: CONFIGURATION_METHOD.SQLALCHEMY_URI, | ||
engine: option, | ||
}, | ||
}); | ||
console.log('isDynamic', isDynamic); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit remove |
||
}; | ||
|
||
const renderAvailableSelector = () => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we move step one into a separate file? |
||
<div className="available"> | ||
<span className="available-label"> | ||
Or choose from a list of other databases we support{' '} | ||
</span> | ||
<label className="label-available-select">supported databases</label> | ||
<Select style={{ width: '100%' }} onChange={setDatabaseModel}> | ||
{availableDbs?.databases?.map(database => ( | ||
<Select.Option value={database.engine} key={database.engine}> | ||
{database.name} | ||
</Select.Option> | ||
))} | ||
</Select> | ||
</div> | ||
); | ||
|
||
const renderPreferredSelector = () => ( | ||
<div className="preferred"> | ||
{availableDbs?.databases | ||
?.filter(db => db.preferred) | ||
.map(database => ( | ||
<IconButton | ||
className="preferred-item" | ||
onClick={() => setDatabaseModel(database.engine)} | ||
buttonText={database.name} | ||
/> | ||
))} | ||
</div> | ||
); | ||
|
||
const renderModalFooter = () => | ||
db // if db show back + connect | ||
? [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. db && ( would that work instead? |
||
<Button | ||
key="back" | ||
onClick={() => { | ||
setDB({ type: ActionType.reset }); | ||
}} | ||
> | ||
Back | ||
</Button>, | ||
!hasConnectedDb ? ( // if hasConnectedDb show back + finish | ||
<Button key="submit" type="primary" onClick={onSave}> | ||
Connect | ||
</Button> | ||
) : ( | ||
<Button onClick={onClose}>Finish</Button> | ||
), | ||
] | ||
: []; | ||
|
||
useEffect(() => { | ||
if (show) { | ||
setTabKey(DEFAULT_TAB_KEY); | ||
|
@@ -369,14 +438,18 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |
title={ | ||
<h4>{isEditMode ? t('Edit database') : t('Connect a database')}</h4> | ||
} | ||
footer={renderModalFooter()} | ||
> | ||
{isEditMode ? ( | ||
{isEditMode && ( | ||
<TabHeader> | ||
<EditHeaderTitle>{db?.backend}</EditHeaderTitle> | ||
<EditHeaderSubtitle>{dbName}</EditHeaderSubtitle> | ||
</TabHeader> | ||
) : ( | ||
)} | ||
{/* Show Legacy Header */} | ||
{useSqlAlchemyForm && ( | ||
<TabHeader> | ||
<p className="helper"> Step 2 of 2 </p> | ||
<CreateHeaderTitle>Enter Primary Credentials</CreateHeaderTitle> | ||
<CreateHeaderSubtitle> | ||
Need help? Learn how to connect your database{' '} | ||
|
@@ -391,6 +464,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |
</CreateHeaderSubtitle> | ||
</TabHeader> | ||
)} | ||
{/* Add styled header here when not in edit mode */} | ||
<hr /> | ||
<Tabs | ||
defaultActiveKey={DEFAULT_TAB_KEY} | ||
|
@@ -481,6 +555,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |
width="500px" | ||
show={show} | ||
title={<h4>{t('Connect a database')}</h4>} | ||
footer={renderModalFooter()} | ||
> | ||
{hasConnectedDb ? ( | ||
<ExtraOptions | ||
|
@@ -505,64 +580,65 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |
/> | ||
) : ( | ||
<> | ||
<DatabaseConnectionForm | ||
dbModel={dbModel} | ||
onParametersChange={({ target }: { target: HTMLInputElement }) => | ||
onChange(ActionType.parametersChange, { | ||
type: target.type, | ||
name: target.name, | ||
checked: target.checked, | ||
value: target.value, | ||
}) | ||
} | ||
onChange={({ target }: { target: HTMLInputElement }) => | ||
onChange(ActionType.textChange, { | ||
name: target.name, | ||
value: target.value, | ||
}) | ||
} | ||
getValidation={() => getValidation(db)} | ||
validationErrors={validationErrors} | ||
/> | ||
{/* Step 1 */} | ||
{!isLoading && !db && ( | ||
<SelectDatabaseStyles> | ||
<label className="label-select"> | ||
What database would you like to connect? | ||
</label> | ||
<Select | ||
style={{ width: '100%' }} | ||
onChange={option => { | ||
<StyledFormHeader> | ||
<div className="select-db"> | ||
<p className="helper"> Step 1 of 3 </p> | ||
<h4>Select a database to connect</h4> | ||
</div> | ||
</StyledFormHeader> | ||
{renderPreferredSelector()} | ||
{renderAvailableSelector()} | ||
</SelectDatabaseStyles> | ||
)} | ||
{/* Step 1 */} | ||
|
||
{/* Step 2 */} | ||
{!isLoading && db && ( | ||
<> | ||
<DatabaseConnectionForm | ||
dbModel={dbModel} | ||
onParametersChange={({ | ||
target, | ||
}: { | ||
target: HTMLInputElement; | ||
}) => | ||
onChange(ActionType.parametersChange, { | ||
type: target.type, | ||
name: target.name, | ||
checked: target.checked, | ||
value: target.value, | ||
}) | ||
} | ||
onChange={({ target }: { target: HTMLInputElement }) => | ||
onChange(ActionType.textChange, { | ||
name: target.name, | ||
value: target.value, | ||
}) | ||
} | ||
getValidation={() => getValidation(db)} | ||
validationErrors={validationErrors} | ||
/> | ||
|
||
<Button | ||
buttonStyle="link" | ||
onClick={() => | ||
setDB({ | ||
type: ActionType.dbSelected, | ||
type: ActionType.configMethodChange, | ||
payload: { | ||
configuration_method: CONFIGURATION_METHOD.DYNAMIC_FORM, | ||
engine: option, | ||
configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, | ||
}, | ||
}); | ||
}} | ||
}) | ||
} | ||
css={buttonLinkStyles} | ||
> | ||
{availableDbs?.databases?.map(database => ( | ||
<Select.Option value={database.engine} key={database.engine}> | ||
{database.name} | ||
</Select.Option> | ||
))} | ||
</Select> | ||
</SelectDatabaseStyles> | ||
Connect this database with a SQLAlchemy URI string instead | ||
</Button> | ||
{/* Step 2 */} | ||
</> | ||
)} | ||
<Button | ||
buttonStyle="link" | ||
onClick={() => | ||
setDB({ | ||
type: ActionType.configMethodChange, | ||
payload: { | ||
configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI, | ||
}, | ||
}) | ||
} | ||
css={buttonLinkStyles} | ||
> | ||
Connect this database with a SQLAlchemy URI string instead | ||
</Button> | ||
</> | ||
)} | ||
</Modal> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,15 @@ export const StyledFormHeader = styled.header` | |
font-weight: bold; | ||
font-size: ${({ theme }) => theme.typography.sizes.l}px; | ||
} | ||
|
||
.select-db { | ||
.helper { | ||
margin-top: 0; | ||
} | ||
h4 { | ||
margin: 0 0 29px; | ||
} | ||
} | ||
`; | ||
|
||
export const antdCollapseStyles = (theme: SupersetTheme) => css` | ||
|
@@ -313,6 +322,12 @@ export const TabHeader = styled.div` | |
padding: 0px; | ||
margin: 0 ${({ theme }) => theme.gridUnit * 4}px | ||
${({ theme }) => theme.gridUnit * 8}px; | ||
|
||
.helper { | ||
color: ${({ theme }) => theme.colors.grayscale.base}; | ||
font-size: ${({ theme }) => theme.typography.sizes.s - 1}px; | ||
margin: 0px; | ||
} | ||
`; | ||
|
||
export const CreateHeaderTitle = styled.div` | ||
|
@@ -340,5 +355,30 @@ export const EditHeaderSubtitle = styled.div` | |
`; | ||
|
||
export const SelectDatabaseStyles = styled.div` | ||
margin: ${({ theme }) => theme.gridUnit * 4}px; | ||
margin: 0 16px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use gridUnit |
||
|
||
.preferred { | ||
display: flex; | ||
flex-wrap: wrap; | ||
justify-content: space-between; | ||
margin-bottom: 32px; | ||
} | ||
|
||
.preferred-item { | ||
width: 133px; | ||
height: 133px; | ||
} | ||
|
||
.available { | ||
.available-label { | ||
font-weight: bold; | ||
margin-top: 32px; | ||
margin-bottom: 16px; | ||
} | ||
} | ||
|
||
.label-available-select { | ||
text-transform: uppercase; | ||
font-size: ${({ theme }) => theme.typography.sizes.s - 1}px; | ||
} | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1075,10 +1075,10 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( | |
# use the "engine_name" attribute of the corresponding DB engine spec | ||
# in `superset/db_engine_specs/`. | ||
PREFERRED_DATABASES: List[str] = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will remove before merging |
||
# "PostgreSQL", | ||
# "Presto", | ||
# "MySQL", | ||
# "SQLite", | ||
"PostgreSQL", | ||
"Presto", | ||
"MySQL", | ||
"SQLite", | ||
# etc. | ||
] | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the param be "engine" if it's just a string? Maybe a type on that would help, too.