Skip to content

Commit

Permalink
feat(bulkActionExecuting): add param to state tree if a bulk action i…
Browse files Browse the repository at this point in the history
…s executing

First, a bulk action executing may affect more than just the DatasetList in the future, it potentially adds a strain to the whole app and we may need to check if a bulk action is executing beyond the DatasetList.

Second, the way the remove modal is wired up currently violates our philosophy that all components should 'be responsible for themselves', and it is the only modal that needs outside actions to be passed in.

Third, we now have 2 diffferent ways to remove datasets, but both actions do essentially the same thing, with the same ui required before and after the action: toasts before and after to indicate that the action has happened, and a refresh to the datasets list to reflect the changes.

This commit also removes the `removeDatasetAndFetch` action in favor of the `removeDatasetsAndFetch`. It bypasses the `DatasetList`'s `BulkDatasetActions` function, and ensures all the proper toasts and ui is set. This also means if anything changes with remove datasets, we only have one place to refactor.

A future refactor should extend this to the pull bulk action.
  • Loading branch information
ramfox committed Oct 12, 2020
1 parent 28f25cf commit bb36b49
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 64 deletions.
44 changes: 12 additions & 32 deletions app/actions/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { selectWorkingDatasetBodyPageInfo,
selectMutationsDataset
} from '../selections'
import { CALL_API, ApiAction, ApiActionThunk, chainSuccess } from '../store/api'
import { openToast } from './ui'
import { openToast, setBulkActionExecuting } from './ui'
import { actionWithPagination } from '../utils/pagination'
import { getActionType } from '../utils/actionType'
import { datasetConvertStringToScriptBytes } from '../utils/datasetConvertStringToScriptBytes'
Expand Down Expand Up @@ -641,52 +641,32 @@ export function removeDataset (
}
}

let response: Action
try {
response = await dispatch(action)
dispatch(openToast('success', 'remove', `Removed ${username}/${name}`))
} catch (action) {
dispatch(openToast('error', 'remove', action.payload.err.message))
throw action
}

return response
}
}

// remove the specified dataset, then refresh the dataset list
export function removeDatasetAndFetch (username: string, name: string, isLinked: boolean, keepFiles: boolean): ApiActionThunk {
return async (dispatch, getState) => {
let response: Action

try {
response = await removeDataset(username, name, isLinked, keepFiles)(dispatch, getState)
} catch (action) {
if (!action.payload.err.message.contains('directory not empty')) {
throw action
}
}
// reset pagination
dispatch(push(pathToCollection()))
response = await fetchMyDatasets(-1)(dispatch, getState)
return response
return dispatch(action)
}
}

// remove the specified datasets, then refresh the dataset list
export function removeDatasetsAndFetch (refs: VersionInfo[], keepFiles: boolean): ApiActionThunk {
return async (dispatch, getState) => {
const datasetsString = refs.length === 1 ? `${refs[0].username}/${refs[0].name}` : `${refs.length} datasets`
try {
dispatch(setBulkActionExecuting(true))
dispatch(openToast('info', 'remove', `removing ${datasetsString}`))
await Promise.all(
refs.map(async (ref) => removeDataset(ref.username, ref.name, !!ref.fsiPath, keepFiles)(dispatch, getState))
)
dispatch(openToast('success', `remove-success`, `removed ${datasetsString}`))
// reset pagination
return fetchMyDatasets(-1)(dispatch, getState)
const res = await fetchMyDatasets(-1)(dispatch, getState)
dispatch(push(pathToCollection()))
dispatch(setBulkActionExecuting(false))
return res
} catch (action) {
if (!action.payload.err.message.contains('directory not empty')) {
throw action
}
dispatch(openToast('error', 'removeDatasetsAndFetch', action.payload.err.message))
dispatch(openToast('error', 'remove-error', `error removing ${datasetsString}: ${action.payload.err.message}`))
dispatch(setBulkActionExecuting(false))
return Promise.reject(action.payload.err.message)
}
}
Expand Down
10 changes: 9 additions & 1 deletion app/actions/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
UI_SET_DATASET_DIR_PATH,
UI_SET_EXPORT_PATH,
UI_SET_DETAILS_BAR,
UI_SET_BOOTUP_COMPONENT
UI_SET_BOOTUP_COMPONENT,
UI_BULK_ACTION_EXECUTING
} from '../reducers/ui'

import { ToastType, BootupComponentType } from '../models/store'
Expand Down Expand Up @@ -98,3 +99,10 @@ export const setBootupComponent = (component: BootupComponentType) => {
component
}
}

export const setBulkActionExecuting = (executing: boolean) => {
return {
type: UI_BULK_ACTION_EXECUTING,
executing
}
}
36 changes: 22 additions & 14 deletions app/components/collection/collectionHome/DatasetList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { connectComponentToPropsWithRouter } from '../../../utils/connectCompone
import { setFilter } from '../../../actions/myDatasets'
import { pullDatasets, fetchMyDatasets, removeDatasetsAndFetch } from '../../../actions/api'
import { setWorkingDataset } from '../../../actions/selections'
import { setModal, openToast } from '../../../actions/ui'
import { setModal, openToast, setBulkActionExecuting } from '../../../actions/ui'

import { selectSessionUsername, selectWorkingDataset } from '../../../selections'
import { selectSessionUsername, selectWorkingDataset, selectBulkActionExecuting } from '../../../selections'
import DatasetsTable from './DatasetsTable'

interface DatasetListProps extends RouteProps {
Expand All @@ -24,25 +24,37 @@ interface DatasetListProps extends RouteProps {
workingDataset: WorkingDataset
sessionUsername: string
showFSI: boolean
bulkActionExecuting: boolean
setFilter: (filter: string) => Action
setWorkingDataset: (username: string, name: string) => Action
fetchMyDatasets: (page: number, pageSize: number) => Promise<AnyAction>
pullDatasets: (refs: VersionInfo[]) => Promise<AnyAction>
removeDatasetsAndFetch: (refs: VersionInfo[], keepFiles: boolean) => Promise<AnyAction>
setModal: (modal: Modal) => void
openToast: (type: ToastType, name: string, message: string) => Action
setBulkActionExecuting: (executing: boolean) => Action
}

type DatasetActionType = 'pull' | 'remove'

export const DatasetListComponent: React.FC<DatasetListProps> = (props) => {
const { showFSI, setFilter, myDatasets, history, sessionUsername, pullDatasets, removeDatasetsAndFetch, openToast, setModal } = props
const {
showFSI,
setFilter,
myDatasets,
history,
sessionUsername,
bulkActionExecuting,
pullDatasets,
openToast,
setModal,
setBulkActionExecuting
} = props

const { filter, value: datasets } = myDatasets
const lowercasedFilterString = filter.toLowerCase()

const [selected, setSelected] = useState([] as VersionInfo[])
const [onlySessionUserDatasets, setOnlySessionUserDatasets] = useState(false)
const [bulkActionExecuting, setBulkActionExecuting] = useState(false)

const handleSetFilter = (value: string) => {
setFilter(value)
Expand Down Expand Up @@ -109,11 +121,6 @@ export const DatasetListComponent: React.FC<DatasetListProps> = (props) => {
return handleBulkActionForDatasets('pull', 'pulling', 'pulled', actionCallback, selected)
}

const handleBulkRemove = async (keepFiles: boolean) => {
const actionCallback = async () => removeDatasetsAndFetch(selected, keepFiles)
return handleBulkActionForDatasets('remove', 'removing', 'removed', actionCallback, selected)
}

const handleBulkActionForDatasets = async (actionType: DatasetActionType, actionGerund: string, actionPastTense: string, actionCallback: () => Promise<AnyAction>, refs: VersionInfo[]) => {
setBulkActionExecuting(true)
openToast('info', actionType, `${actionGerund} ${refs.length} ${refs.length === 1 ? 'dataset' : 'datasets'}`)
Expand All @@ -134,8 +141,7 @@ export const DatasetListComponent: React.FC<DatasetListProps> = (props) => {
setModal(
{
type: ModalType.RemoveDataset,
datasets: selected,
onSubmit: handleBulkRemove
datasets: selected
}
)
}
Expand Down Expand Up @@ -201,7 +207,8 @@ export default connectComponentToPropsWithRouter(
...ownProps,
myDatasets: state.myDatasets,
sessionUsername: selectSessionUsername(state),
setWorkingDataset: selectWorkingDataset(state)
setWorkingDataset: selectWorkingDataset(state),
bulkActionExecuting: selectBulkActionExecuting(state)
}),
{
setFilter,
Expand All @@ -210,6 +217,7 @@ export default connectComponentToPropsWithRouter(
setModal,
pullDatasets,
removeDatasetsAndFetch,
openToast
openToast,
setBulkActionExecuting
}
)
16 changes: 6 additions & 10 deletions app/components/collection/headerButtons/RemoveButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ import { selectFsiPath } from '../../../selections'
import { connectComponentToPropsWithRouter } from '../../../utils/connectComponentToProps'

import HeaderColumnButton from '../../chrome/HeaderColumnButton'
import { removeDatasetsAndFetch } from '../../../actions/api'
import { ApiAction } from '../../../store/api'
import { VersionInfo } from '../../../models/store'

interface RemoveButtonProps extends RouteProps {
qriRef: QriRef
fsiPath: string
showIcon: boolean
setModal: (modal: Modal) => void
removeDatasetsAndFetch: (datasets: VersionInfo[], keepfiles: boolean) => Promise<ApiAction>
size: 'sm' | 'md'
}

/**
Expand All @@ -31,8 +28,8 @@ export const RemoveButtonComponent: React.FunctionComponent<RemoveButtonProps> =
const {
qriRef,
fsiPath,
size = 'md',
showIcon = true,
removeDatasetsAndFetch,
setModal
} = props

Expand All @@ -42,15 +39,15 @@ export const RemoveButtonComponent: React.FunctionComponent<RemoveButtonProps> =
return null
}
return (<HeaderColumnButton
id='remove'
id='remove-button'
label='Remove'
tooltip='Copy the url of this dataset on the cloud to your clipboard'
icon={showIcon && faTrash }
size={size}
onClick={() => {
setModal({
type: ModalType.RemoveDataset,
datasets: [{ ...qriRef, fsiPath }],
onSubmit: async (keepfiles: boolean) => removeDatasetsAndFetch([{ ...qriRef, fsiPath }], keepfiles)
datasets: [{ ...qriRef, fsiPath }]
})
}}
/>)
Expand All @@ -66,7 +63,6 @@ export default connectComponentToPropsWithRouter(
fsiPath: selectFsiPath(state)
}
}, {
setModal,
removeDatasetsAndFetch
setModal
}
)
11 changes: 8 additions & 3 deletions app/components/modals/RemoveDataset.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,24 @@ import { RemoveDatasetModal } from '../../../app/models/modals'
import { connectComponentToProps } from '../../utils/connectComponentToProps'
import { dismissModal } from '../../actions/ui'
import { selectModal, selectSessionUsername } from '../../selections'
import { ApiAction } from '../../store/api'
import { removeDatasetsAndFetch } from '../../actions/api'

import CheckboxInput from '../form/CheckboxInput'
import Modal from './Modal'
import Error from './Error'
import Buttons from './Buttons'
import { VersionInfo } from '../../models/store'

interface RemoveDatasetProps {
modal: RemoveDatasetModal
sessionUsername: string
onDismissed: () => void
onSubmit: (refs: VersionInfo[], keepfiles: boolean) => Promise<ApiAction>
}

export const RemoveDatasetComponent: React.FC<RemoveDatasetProps> = (props: RemoveDatasetProps) => {
const { modal: { datasets, onSubmit }, sessionUsername, onDismissed } = props
const { modal: { datasets }, sessionUsername, onDismissed, onSubmit } = props

const [keepFiles, setKeepFiles] = useState(true)
const [dismissable, setDismissable] = useState(true)
Expand All @@ -38,7 +42,7 @@ export const RemoveDatasetComponent: React.FC<RemoveDatasetProps> = (props: Remo
if (!onSubmit) return

try {
await onSubmit(keepFiles)
await onSubmit(datasets, keepFiles)
setLoading(false)
setDismissable(true)
onDismissed()
Expand Down Expand Up @@ -104,6 +108,7 @@ export default connectComponentToProps(
}
},
{
onDismissed: dismissModal
onDismissed: dismissModal,
onSubmit: removeDatasetsAndFetch
}
)
3 changes: 0 additions & 3 deletions app/models/modals.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { AnyAction } from 'redux'

import { VersionInfo } from './store'

export enum ModalType {
Expand Down Expand Up @@ -55,7 +53,6 @@ export interface PublishDatasetModal {
export interface RemoveDatasetModal {
type: ModalType.RemoveDataset
datasets: VersionInfo[]
onSubmit: (keepFiles: boolean) => Promise<AnyAction>
}

export interface SearchModal {
Expand Down
1 change: 1 addition & 0 deletions app/models/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface UI {
importFileName: string
importFileSize: number
bootupComponent: BootupComponentType
bulkActionExecuting: boolean
}

export type BootupComponentType =
Expand Down
11 changes: 10 additions & 1 deletion app/reducers/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const UI_SET_DATASET_DIR_PATH = 'UI_SET_DATASET_DIR_PATH'
export const UI_SET_EXPORT_PATH = 'UI_SET_EXPORT_PATH'
export const UI_SET_DETAILS_BAR = 'UI_SET_DETAILS_BAR'
export const UI_SET_BOOTUP_COMPONENT = 'UI_SET_BOOTUP_COMPONENT'
export const UI_BULK_ACTION_EXECUTING = 'UI_BULK_ACTION_EXECUTING'

export const UNAUTHORIZED = 'UNAUTHORIZED'

Expand Down Expand Up @@ -62,7 +63,8 @@ const initialState = {
detailsBar: { type: DetailsType.NoDetails },
importFileName: '',
importFileSize: 0,
bootupComponent: 'loading'
bootupComponent: 'loading',
bulkActionExecuting: false
}

// send an event to electron to block menus on first load
Expand Down Expand Up @@ -203,6 +205,13 @@ export default (state = initialState, action: AnyAction) => {
...state,
bootupComponent: component
}

case UI_BULK_ACTION_EXECUTING:
const { executing } = action
return {
...state,
bulkActionExecuting: executing
}
default:
return state
}
Expand Down
4 changes: 4 additions & 0 deletions app/selections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ export function selectToast (state: Store): Toast {
export function selectBootupComponent (state: Store): BootupComponentType {
return state.ui.bootupComponent
}

export function selectBulkActionExecuting (state: Store): boolean {
return state.ui.bulkActionExecuting
}
/**
*
* WORKINGDATASET STATE TREE
Expand Down

0 comments on commit bb36b49

Please sign in to comment.