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

fix(ui): sort out table state #15944

Merged
merged 7 commits into from
Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from all 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 ui/src/dashboards/actions/views.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Dashboards.Actions.getViewForTimeMachine', () => {
)

expect(mocked(getView)).toHaveBeenCalledTimes(1)
expect(mockedDispatch).toHaveBeenCalledTimes(2)
expect(mockedDispatch).toHaveBeenCalledTimes(3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because now we populate the queries


const [
setViewDispatchArguments,
Expand Down Expand Up @@ -103,7 +103,7 @@ describe('Dashboards.Actions.getViewForTimeMachine', () => {
)

expect(mocked(getView)).toHaveBeenCalledTimes(0)
expect(mockedDispatch).toHaveBeenCalledTimes(1)
expect(mockedDispatch).toHaveBeenCalledTimes(2)
expect(mockedDispatch).toHaveBeenCalledWith({
type: 'SET_ACTIVE_TIME_MACHINE',
payload: {
Expand Down
13 changes: 4 additions & 9 deletions ui/src/dashboards/actions/views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,14 @@ import {
import * as copy from 'src/shared/copy/notifications'

// Actions
import {
notify,
Action as NotificationAction,
} from 'src/shared/actions/notifications'
import {notify} from 'src/shared/actions/notifications'
import {setActiveTimeMachine} from 'src/timeMachine/actions'
import {executeQueries} from 'src/timeMachine/actions/queries'

// Types
import {RemoteDataState, QueryView, GetState} from 'src/types'
import {Dispatch} from 'redux'
import {View} from 'src/types'
import {Action as TimeMachineAction} from 'src/timeMachine/actions'
import {TimeMachineID} from 'src/types'

export type Action = SetViewAction | SetViewsAction | ResetViewsAction
Expand Down Expand Up @@ -103,10 +100,7 @@ export const getViewForTimeMachine = (
dashboardID: string,
cellID: string,
timeMachineID: TimeMachineID
) => async (
dispatch: Dispatch<Action | TimeMachineAction | NotificationAction>,
getState: GetState
): Promise<void> => {
) => async (dispatch, getState: GetState): Promise<void> => {
try {
const state = getState()
let view = getViewFromState(state, cellID) as QueryView
Expand All @@ -117,6 +111,7 @@ export const getViewForTimeMachine = (
}

dispatch(setActiveTimeMachine(timeMachineID, {view}))
dispatch(executeQueries(dashboardID))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i intended this to mean "whenever a timemachine loads a view, run the queries to populate its data"

} catch (e) {
dispatch(notify(copy.getViewFailed(e.message)))
dispatch(setView(cellID, null, RemoteDataState.Error))
Expand Down
8 changes: 2 additions & 6 deletions ui/src/dashboards/components/EditVEO.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import VEOHeader from 'src/dashboards/components/VEOHeader'
import {setName} from 'src/timeMachine/actions'
import {saveVEOView} from 'src/dashboards/actions'
import {getViewForTimeMachine} from 'src/dashboards/actions/views'
import {executeQueries} from 'src/timeMachine/actions/queries'

// Utils
import {getActiveTimeMachine} from 'src/timeMachine/selectors'
Expand All @@ -24,7 +23,6 @@ import {AppState, RemoteDataState, QueryView, TimeMachineID} from 'src/types'
interface DispatchProps {
onSetName: typeof setName
onSaveView: typeof saveVEOView
onExecuteQueries: typeof executeQueries
getViewForTimeMachine: typeof getViewForTimeMachine
}

Expand All @@ -38,7 +36,6 @@ type Props = DispatchProps & StateProps & WithRouterProps
const EditViewVEO: FunctionComponent<Props> = ({
getViewForTimeMachine,
activeTimeMachineID,
onExecuteQueries,
onSaveView,
onSetName,
params: {orgID, cellID, dashboardID},
Expand All @@ -47,8 +44,7 @@ const EditViewVEO: FunctionComponent<Props> = ({
}) => {
useEffect(() => {
getViewForTimeMachine(dashboardID, cellID, 'veo')
onExecuteQueries(dashboardID)
}, [cellID, dashboardID])
}, [])

const handleClose = () => {
router.push(`/orgs/${orgID}/dashboards/${dashboardID}`)
Expand Down Expand Up @@ -95,13 +91,13 @@ const mstp = (state: AppState): StateProps => {
const {activeTimeMachineID} = state.timeMachines

const {view} = getActiveTimeMachine(state)

return {view, activeTimeMachineID}
}

const mdtp: DispatchProps = {
onSetName: setName,
onSaveView: saveVEOView,
onExecuteQueries: executeQueries,
getViewForTimeMachine: getViewForTimeMachine,
}

Expand Down
39 changes: 1 addition & 38 deletions ui/src/shared/components/tables/TableGraphs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ import {setFieldOptions as setFieldOptionsAction} from 'src/timeMachine/actions'

// Utils
import {getDeep} from 'src/utils/wrappers'
import {findTableNameHeaders} from 'src/dashboards/utils/tableGraph'

// Types
import {TableViewProperties, FieldOption, FluxTable, TimeZone} from 'src/types'
import {TableViewProperties, FluxTable, TimeZone} from 'src/types'

interface PassedProps {
tables: FluxTable[]
Expand All @@ -41,25 +40,6 @@ class TableGraphs extends PureComponent<Props, State> {
selectedTableName: getDeep<string>(this, 'props.tables[0].name', null),
}

public componentDidMount() {
this.updateFieldOptions()
}

public componentDidUpdate(prevProps: Props, prevState: State) {
const prevHeaders = findTableNameHeaders(
prevProps.tables,
prevState.selectedTableName
)
const rawHeaders = findTableNameHeaders(
this.props.tables,
this.state.selectedTableName
)

if (!_.isEqual(rawHeaders, prevHeaders)) {
this.updateFieldOptions()
}
}

public render() {
const {tables, properties, timeZone} = this.props

Expand Down Expand Up @@ -107,7 +87,6 @@ class TableGraphs extends PureComponent<Props, State> {

private handleSelectTable = (selectedTableName: string): void => {
this.setState({selectedTableName})
this.updateFieldOptions()
}

private get showSidebar(): boolean {
Expand All @@ -127,22 +106,6 @@ class TableGraphs extends PureComponent<Props, State> {
const {tables} = this.props
return tables.find(t => t.name === this.nameOfSelectedTable)
}

private updateFieldOptions() {
this.props.setFieldOptions(this.extractFieldOptions())
}

private extractFieldOptions(): FieldOption[] {
return this.headers.map(h => ({
internalName: h,
displayName: h,
visible: true,
}))
}

private get headers(): string[] {
return getDeep(this.selectedTable, 'data.0', [])
}
}

const mdtp: DispatchProps = {
Expand Down
15 changes: 15 additions & 0 deletions ui/src/timeMachine/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export type Action =
| EditActiveQueryWithBuilderAction
| UpdateActiveQueryNameAction
| SetFieldOptionsAction
| UpdateFieldOptionAction
| SetTableOptionsAction
| SetTimeFormatAction
| SetXColumnAction
Expand Down Expand Up @@ -465,6 +466,20 @@ export const setFieldOptions = (
payload: {fieldOptions},
})

interface UpdateFieldOptionAction {
type: 'UPDATE_FIELD_OPTION'
payload: {
option: FieldOption
}
}

export const updateFieldOption = (
option: FieldOption
): UpdateFieldOptionAction => ({
type: 'UPDATE_FIELD_OPTION',
payload: {option},
})

interface SetTableOptionsAction {
type: 'SET_TABLE_OPTIONS'
payload: {
Expand Down
10 changes: 4 additions & 6 deletions ui/src/timeMachine/components/view_options/TableOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
setDecimalPlaces,
setColors,
setFieldOptions,
updateFieldOption,
setTableOptions,
setTimeFormat,
} from 'src/timeMachine/actions'
Expand Down Expand Up @@ -52,6 +53,7 @@ interface DispatchProps {
onSetColors: typeof setColors
onSetTimeFormat: typeof setTimeFormat
onSetFieldOptions: typeof setFieldOptions
onUpdateFieldOption: typeof updateFieldOption
onSetTableOptions: typeof setTableOptions
onSetDecimalPlaces: typeof setDecimalPlaces
}
Expand Down Expand Up @@ -158,12 +160,7 @@ export class TableOptions extends Component<Props, {}> {
}

private handleUpdateColumn = (fieldOption: FieldOption) => {
const {internalName} = fieldOption
const fieldOptions = this.props.fieldOptions.map(fopt =>
fopt.internalName === internalName ? fieldOption : fopt
)

this.props.onSetFieldOptions(fieldOptions)
this.props.onUpdateFieldOption(fieldOption)
}

private handleToggleFixFirstColumn = () => {
Expand All @@ -190,6 +187,7 @@ const mdtp: DispatchProps = {
onSetDecimalPlaces: setDecimalPlaces,
onSetColors: setColors,
onSetFieldOptions: setFieldOptions,
onUpdateFieldOption: updateFieldOption,
onSetTableOptions: setTableOptions,
onSetTimeFormat: setTimeFormat,
}
Expand Down
91 changes: 0 additions & 91 deletions ui/src/timeMachine/reducers/index.test.ts

This file was deleted.

Loading