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

Soft delete of jobs and datasets. Style dialog component #2343

Merged
merged 8 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
4 changes: 2 additions & 2 deletions web/src/__tests__/components/Dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ describe('Dialog Component', () => {
const mockProps = {
dialogIsOpen: true,
dialogToggle: dialogToggle,
ignoreWarning: ignoreWarning
ignoreWarning: ignoreWarning,
editWarningField: 'Description of dialog...'
}

const ignoreWarning = () => {
Expand All @@ -30,5 +31,4 @@ describe('Dialog Component', () => {
it('renders a snapshot that matches previous', () => {
expect(wrapper).toMatchSnapshot()
})

})
62 changes: 37 additions & 25 deletions web/src/__tests__/components/__snapshots__/Dialog.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -1,29 +1,41 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Dialog Component renders a snapshot that matches previous 1`] = `
<div>
<WithStyles(ForwardRef(Dialog))
open={true}
>
<WithStyles(ForwardRef(DialogTitle)) />
<WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogContentText)) />
</WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogActions))>
<WithStyles(ForwardRef(Button))
className="dialogButton"
color="secondary"
>
Continue
</WithStyles(ForwardRef(Button))>
<WithStyles(ForwardRef(Button))
className="dialogButton"
color="primary"
onClick={[Function]}
>
Cancel
</WithStyles(ForwardRef(Button))>
</WithStyles(ForwardRef(DialogActions))>
</WithStyles(ForwardRef(Dialog))>
</div>
<WithStyles(ForwardRef(Dialog))
open={true}
>
<WithStyles(ForwardRef(DialogTitle)) />
<WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogContentText))>
Description of dialog...
</WithStyles(ForwardRef(DialogContentText))>
</WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogContentText))>
Description of dialog...
</WithStyles(ForwardRef(DialogContentText))>
</WithStyles(ForwardRef(DialogContent))>
<WithStyles(ForwardRef(DialogActions))>
<WithStyles(ForwardRef(Button))
className="dialogButton"
color="primary"
onClick={[Function]}
style={
Object {
"backgroundColor": "#ee7b7b",
"color": "#fff",
}
}
>
Cancel
</WithStyles(ForwardRef(Button))>
<WithStyles(ForwardRef(Button))
className="dialogButton"
color="primary"
variant="outlined"
>
Continue
</WithStyles(ForwardRef(Button))>
</WithStyles(ForwardRef(DialogActions))>
</WithStyles(ForwardRef(Dialog))>
`;
7 changes: 6 additions & 1 deletion web/src/__tests__/reducers/datasets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ describe('datasets reducer', () => {
datasets: datasets
}
}
expect(datasetsReducer(initialState, action)).toStrictEqual({init: true, isLoading: false, result: datasets})
expect(datasetsReducer(initialState, action)).toStrictEqual({
init: true,
isLoading: false,
result: datasets,
deletedDatasetName: ''
})
})

})
2 changes: 1 addition & 1 deletion web/src/__tests__/reducers/jobs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('jobs reducer', () => {
jobs: jobs
}
}
expect(jobsReducer(initialState, action)).toStrictEqual({ isLoading: false, result: jobs, init: true })
expect(jobsReducer(initialState, action)).toStrictEqual({ isLoading: false, result: jobs, init: true, deletedJobName: '' })
})
})

Expand Down
48 changes: 32 additions & 16 deletions web/src/components/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// SPDX-License-Identifier: Apache-2.0

import * as React from 'react'
import { dialogToggle } from '../store/actionCreators'
import { theme } from '../helpers/theme'
import Button from '@material-ui/core/Button'
import Dialog from '@material-ui/core/Dialog'
import DialogActions from '@material-ui/core/DialogActions'
import DialogContent from '@material-ui/core/DialogContent'
import DialogContentText from '@material-ui/core/DialogContentText'
import DialogTitle from '@material-ui/core/DialogTitle'
import React, { FunctionComponent } from 'react'

interface IProps {
dialogIsOpen: boolean
Expand All @@ -17,27 +18,42 @@ interface IProps {
title?: string
}

export default function AlertDialog(props: IProps) {
function handleClose() {
const AlertDialog: FunctionComponent<IProps> = props => {
const handleClose = () => {
props.dialogToggle('')
}

return (
<div>
<Dialog open={props.dialogIsOpen}>
<DialogTitle>{props.title}</DialogTitle>
<Dialog open={props.dialogIsOpen}>
<DialogTitle>{props.title}</DialogTitle>
{props.editWarningField && (
<DialogContent>
<DialogContentText>{props.editWarningField}</DialogContentText>
</DialogContent>
<DialogActions>
<Button className='dialogButton' color='secondary' onClick={props.ignoreWarning}>
Continue
</Button>
<Button className='dialogButton' color='primary' onClick={handleClose}>
Cancel
</Button>
</DialogActions>
</Dialog>
</div>
)}
<DialogContent>
<DialogContentText>{props.editWarningField}</DialogContentText>
</DialogContent>
<DialogActions>
<Button
className='dialogButton'
color='primary'
onClick={handleClose}
style={{ backgroundColor: theme.palette.error.main, color: theme.palette.common.white }}
>
Cancel
</Button>
<Button
className='dialogButton'
color='primary'
variant='outlined'
onClick={props.ignoreWarning}
>
Continue
</Button>
</DialogActions>
</Dialog>
)
}

export default AlertDialog
67 changes: 60 additions & 7 deletions web/src/components/datasets/DatasetDetailPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: Apache-2.0

import * as Redux from 'redux'
import { Box, Chip, Tab, Tabs } from '@material-ui/core'
import { Box, Button, Chip, Tab, Tabs } from '@material-ui/core'
import { DatasetVersion } from '../../types/api'
import { IState } from '../../store/reducers'
import {
Expand All @@ -11,19 +11,24 @@ import {
withStyles
} from '@material-ui/core/styles'
import { LineageDataset } from '../lineage/types'
import { alpha } from '@material-ui/core/styles'
import { bindActionCreators } from 'redux'
import { connect } from 'react-redux'
import {
deleteDataset,
dialogToggle,
fetchDatasetVersions,
resetDataset,
resetDatasetVersions
} from '../../store/actionCreators'
import { theme } from '../../helpers/theme'
import { useHistory } from 'react-router-dom'
import CircularProgress from '@material-ui/core/CircularProgress/CircularProgress'
import CloseIcon from '@material-ui/icons/Close'
import DatasetColumnLineage from './DatasetColumnLineage'
import DatasetInfo from './DatasetInfo'
import DatasetVersions from './DatasetVersions'
import Dialog from '../Dialog'
import IconButton from '@material-ui/core/IconButton'
import MqText from '../core/text/MqText'
import React, { ChangeEvent, FunctionComponent, SetStateAction, useEffect } from 'react'
Expand All @@ -44,6 +49,14 @@ const styles = ({ spacing }: ITheme) => {
'&:not(:last-of-type)': {
marginRight: spacing(1)
}
},
buttonDelete: {
borderColor: theme.palette.error.main,
color: theme.palette.error.main,
'&:hover': {
borderColor: alpha(theme.palette.error.main, 0.3),
backgroundColor: alpha(theme.palette.error.main, 0.3)
}
}
})
}
Expand All @@ -52,12 +65,16 @@ interface StateProps {
lineageDataset: LineageDataset
versions: DatasetVersion[]
versionsLoading: boolean
datasets: IState['datasets']
display: IState['display']
}

interface DispatchProps {
fetchDatasetVersions: typeof fetchDatasetVersions
resetDatasetVersions: typeof resetDatasetVersions
resetDataset: typeof resetDataset
deleteDataset: typeof deleteDataset
dialogToggle: typeof dialogToggle
}

type IProps = IWithStyles<typeof styles> & StateProps & DispatchProps
Expand All @@ -72,11 +89,16 @@ function a11yProps(index: number) {
const DatasetDetailPage: FunctionComponent<IProps> = props => {
const {
classes,
datasets,
display,
fetchDatasetVersions,
resetDataset,
resetDatasetVersions,
deleteDataset,
dialogToggle,
versions,
versionsLoading
versionsLoading,
lineageDataset
} = props
const { root } = classes
const history = useHistory()
Expand All @@ -86,6 +108,12 @@ const DatasetDetailPage: FunctionComponent<IProps> = props => {
fetchDatasetVersions(props.lineageDataset.namespace, props.lineageDataset.name)
}, [props.lineageDataset.name])

useEffect(() => {
if (datasets.deletedDatasetName) {
history.push('/datasets')
}
}, [datasets.deletedDatasetName])

// unmounting
useEffect(
() => () => {
Expand Down Expand Up @@ -147,9 +175,31 @@ const DatasetDetailPage: FunctionComponent<IProps> = props => {
/>
</Tabs>
</Box>
<IconButton onClick={() => history.push('/datasets')}>
<CloseIcon />
</IconButton>
<Box display={'flex'} alignItems={'center'}>
<Box mr={1}>
<Button
variant='outlined'
className={classes.buttonDelete}
onClick={() => {
props.dialogToggle('')
}}
>
{i18next.t('datasets.dialog_delete')}
</Button>
<Dialog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two instances of this dialog... Can the dialog itself just be a reusable component.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is reusable and located in "components/Dialog.tsx", and use it with different props

dialogIsOpen={display.dialogIsOpen}
dialogToggle={dialogToggle}
title={i18next.t('jobs.dialog_confirmation_title')}
ignoreWarning={() => {
deleteDataset(lineageDataset.name, lineageDataset.namespace)
props.dialogToggle('')
}}
/>
</Box>
<IconButton onClick={() => history.push('/datasets')}>
<CloseIcon />
</IconButton>
</Box>
</Box>
<MqText heading font={'mono'}>
{name}
Expand All @@ -172,7 +222,8 @@ const DatasetDetailPage: FunctionComponent<IProps> = props => {
}

const mapStateToProps = (state: IState) => ({
datasets: state.datasets.result,
datasets: state.datasets,
display: state.display,
versions: state.datasetVersions.result.versions,
versionsLoading: state.datasetVersions.isLoading
})
Expand All @@ -182,7 +233,9 @@ const mapDispatchToProps = (dispatch: Redux.Dispatch) =>
{
fetchDatasetVersions: fetchDatasetVersions,
resetDatasetVersions: resetDatasetVersions,
resetDataset: resetDataset
resetDataset: resetDataset,
deleteDataset: deleteDataset,
dialogToggle: dialogToggle
},
dispatch
)
Expand Down
Loading