From e68e1903c5d3af2e266f38c6b0e6e14702f34ef0 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 17:51:34 -0700 Subject: [PATCH 1/9] DashboardsPage can now delete a dashboard (tho not reflected in UI except on refresh) --- ui/src/dashboards/actions/index.js | 28 +++++++++++++++++++ ui/src/dashboards/apis/index.js | 12 ++++++++ .../dashboards/containers/DashboardsPage.js | 22 +++++++++++++-- ui/src/dashboards/reducers/ui.js | 20 +++++++++++++ 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index df70da1f57..1a337b0980 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -1,11 +1,14 @@ import { getDashboards as getDashboardsAJAX, updateDashboard as updateDashboardAJAX, + deleteDashboard as deleteDashboardAJAX, updateDashboardCell as updateDashboardCellAJAX, addDashboardCell as addDashboardCellAJAX, deleteDashboardCell as deleteDashboardCellAJAX, } from 'src/dashboards/apis' +import {publishNotification} from 'src/shared/actions/notifications'; + import {NEW_DEFAULT_DASHBOARD_CELL} from 'src/dashboards/constants' export const loadDashboards = (dashboards, dashboardID) => ({ @@ -37,6 +40,20 @@ export const updateDashboard = (dashboard) => ({ }, }) +export const deleteDashboard = (dashboard) => ({ + type: 'DELETE_DASHBOARD', + payload: { + dashboard, + }, +}) + +export const undoDeleteDashboard = (dashboard) => ({ + type: 'UNDO_DELETE_DASHBOARD', + payload: { + dashboard, + }, +}) + export const updateDashboardCells = (cells) => ({ type: 'UPDATE_DASHBOARD_CELLS', payload: { @@ -109,6 +126,17 @@ export const updateDashboardCell = (cell) => (dispatch) => { }) } +export const deleteDashboardAsync = (dashboard) => async (dispatch) => { + dispatch(deleteDashboard(dashboard)) + try { + await deleteDashboardAJAX(dashboard) + dispatch(publishNotification('success', 'Dashboard deleted successfully.')) + } catch (error) { + dispatch(undoDeleteDashboard(dashboard)) // undo optimistic update + dispatch(publishNotification('error', `Failed to delete dashboard: ${error.data.message}.`)) + } +} + export const addDashboardCellAsync = (dashboard) => async (dispatch) => { try { const {data} = await addDashboardCellAJAX(dashboard, NEW_DEFAULT_DASHBOARD_CELL) diff --git a/ui/src/dashboards/apis/index.js b/ui/src/dashboards/apis/index.js index 2abf5db8fc..fb2570f218 100644 --- a/ui/src/dashboards/apis/index.js +++ b/ui/src/dashboards/apis/index.js @@ -36,6 +36,18 @@ export const createDashboard = async (dashboard) => { } } +export const deleteDashboard = async (dashboard) => { + try { + return await AJAX({ + method: 'DELETE', + url: dashboard.links.self, + }) + } catch (error) { + console.error(error) + throw error + } +} + export const addDashboardCell = async (dashboard, cell) => { try { return await AJAX({ diff --git a/ui/src/dashboards/containers/DashboardsPage.js b/ui/src/dashboards/containers/DashboardsPage.js index 7987ce5c81..5ea6a05083 100644 --- a/ui/src/dashboards/containers/DashboardsPage.js +++ b/ui/src/dashboards/containers/DashboardsPage.js @@ -1,8 +1,14 @@ import React, {PropTypes} from 'react' import {Link, withRouter} from 'react-router' -import SourceIndicator from '../../shared/components/SourceIndicator' +import {connect} from 'react-redux' +import {bindActionCreators} from 'redux' + +import SourceIndicator from 'shared/components/SourceIndicator' +import DeleteConfirmTableCell from 'shared/components/DeleteConfirmTableCell' import {getDashboards, createDashboard} from '../apis' +import {deleteDashboardAsync} from 'src/dashboards/actions' + import {NEW_DASHBOARD} from 'src/dashboards/constants' const { @@ -26,6 +32,7 @@ const DashboardsPage = React.createClass({ push: func.isRequired, }).isRequired, addFlashMessage: func, + handleDeleteDashboard: func.isRequired, }, getInitialState() { @@ -50,6 +57,10 @@ const DashboardsPage = React.createClass({ push(`/sources/${id}/dashboards/${data.id}`) }, + handleDeleteDashboard(dashboard) { + this.props.handleDeleteDashboard(dashboard) + }, + render() { const dashboardLink = `/sources/${this.props.source.id}` let tableHeader @@ -95,12 +106,13 @@ const DashboardsPage = React.createClass({ { this.state.dashboards.map((dashboard) => { return ( - + {dashboard.name} + ); }) @@ -125,4 +137,8 @@ const DashboardsPage = React.createClass({ }, }) -export default withRouter(DashboardsPage) +const mapDispatchToProps = (dispatch) => ({ + handleDeleteDashboard: bindActionCreators(deleteDashboardAsync, dispatch), +}) + +export default connect(null, mapDispatchToProps)(withRouter(DashboardsPage)) diff --git a/ui/src/dashboards/reducers/ui.js b/ui/src/dashboards/reducers/ui.js index e265589110..020bc2dbab 100644 --- a/ui/src/dashboards/reducers/ui.js +++ b/ui/src/dashboards/reducers/ui.js @@ -48,6 +48,26 @@ export default function ui(state = initialState, action) { return {...state, ...newState} } + case 'DELETE_DASHBOARD': { + const {dashboard} = action.payload + const newState = { + dashboards: state.dashboards.filter((d) => d.id !== dashboard.id), + } + + return {...state, ...newState} + } + + case 'UNDO_DELETE_DASHBOARD': { + const {dashboard} = action.payload + const newState = { + dashboards: [ + _.cloneDeep(dashboard), + ...state.dashboards, + ], + } + return {...state, ...newState} + } + case 'UPDATE_DASHBOARD_CELLS': { const {cells} = action.payload const {dashboard} = state From 7439f19053c60b1bad71b1b80004882d8099c512 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 18:30:34 -0700 Subject: [PATCH 2/9] Refactor DashboardsPage to load dashboards via action creator and to use mapStateToProps so delete is Reactive --- ui/src/dashboards/actions/index.js | 7 ++-- ui/src/dashboards/containers/DashboardPage.js | 6 +-- .../dashboards/containers/DashboardsPage.js | 42 +++++++++---------- ui/src/dashboards/reducers/ui.js | 2 +- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index 1a337b0980..a817a46ac4 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -106,10 +106,9 @@ export const deleteDashboardCell = (cell) => ({ // Async Action Creators -export const getDashboards = (dashboardID) => (dispatch) => { - getDashboardsAJAX().then(({data: {dashboards}}) => { - dispatch(loadDashboards(dashboards, dashboardID)) - }) +export const getDashboardsAsync = (dashboardID) => async (dispatch) => { + const {data: {dashboards}} = await getDashboardsAJAX() + dispatch(loadDashboards(dashboards, dashboardID)) } export const putDashboard = () => (dispatch, getState) => { diff --git a/ui/src/dashboards/containers/DashboardPage.js b/ui/src/dashboards/containers/DashboardPage.js index 8f2be10f09..bd994fc93e 100644 --- a/ui/src/dashboards/containers/DashboardPage.js +++ b/ui/src/dashboards/containers/DashboardPage.js @@ -39,7 +39,7 @@ const DashboardPage = React.createClass({ }).isRequired, dashboardActions: shape({ putDashboard: func.isRequired, - getDashboards: func.isRequired, + getDashboardsAsync: func.isRequired, setDashboard: func.isRequired, setTimeRange: func.isRequired, addDashboardCellAsync: func.isRequired, @@ -84,10 +84,10 @@ const DashboardPage = React.createClass({ componentDidMount() { const { params: {dashboardID}, - dashboardActions: {getDashboards}, + dashboardActions: {getDashboardsAsync}, } = this.props; - getDashboards(dashboardID) + getDashboardsAsync(dashboardID) }, componentWillReceiveProps(nextProps) { diff --git a/ui/src/dashboards/containers/DashboardsPage.js b/ui/src/dashboards/containers/DashboardsPage.js index 5ea6a05083..75de18b396 100644 --- a/ui/src/dashboards/containers/DashboardsPage.js +++ b/ui/src/dashboards/containers/DashboardsPage.js @@ -6,12 +6,13 @@ import {bindActionCreators} from 'redux' import SourceIndicator from 'shared/components/SourceIndicator' import DeleteConfirmTableCell from 'shared/components/DeleteConfirmTableCell' -import {getDashboards, createDashboard} from '../apis' -import {deleteDashboardAsync} from 'src/dashboards/actions' +import {createDashboard} from 'src/dashboards/apis' +import {getDashboardsAsync, deleteDashboardAsync} from 'src/dashboards/actions' import {NEW_DASHBOARD} from 'src/dashboards/constants' const { + arrayOf, func, string, shape, @@ -32,23 +33,13 @@ const DashboardsPage = React.createClass({ push: func.isRequired, }).isRequired, addFlashMessage: func, + handleGetDashboards: func.isRequired, handleDeleteDashboard: func.isRequired, - }, - - getInitialState() { - return { - dashboards: [], - waiting: true, - }; + dashboards: arrayOf(shape()), }, componentDidMount() { - getDashboards().then((resp) => { - this.setState({ - dashboards: resp.data.dashboards, - waiting: false, - }); - }); + this.props.handleGetDashboards() }, async handleCreateDashbord() { @@ -62,14 +53,15 @@ const DashboardsPage = React.createClass({ }, render() { + const {dashboards: dashboards} = this.props const dashboardLink = `/sources/${this.props.source.id}` let tableHeader - if (this.state.waiting) { + if (dashboards === null) { tableHeader = "Loading Dashboards..." - } else if (this.state.dashboards.length === 0) { + } else if (dashboards.length === 0) { tableHeader = "1 Dashboard" } else { - tableHeader = `${this.state.dashboards.length + 1} Dashboards` + tableHeader = `${dashboards.length + 1} Dashboards` } return ( @@ -104,7 +96,8 @@ const DashboardsPage = React.createClass({ { - this.state.dashboards.map((dashboard) => { + dashboards && dashboards.length ? + dashboards.map((dashboard) => { return ( @@ -115,7 +108,8 @@ const DashboardsPage = React.createClass({ ); - }) + }) : + null } @@ -137,8 +131,14 @@ const DashboardsPage = React.createClass({ }, }) +const mapStateToProps = ({dashboardUI: {dashboards, dashboard}}) => ({ + dashboards, + dashboard, +}) + const mapDispatchToProps = (dispatch) => ({ + handleGetDashboards: bindActionCreators(getDashboardsAsync, dispatch), handleDeleteDashboard: bindActionCreators(deleteDashboardAsync, dispatch), }) -export default connect(null, mapDispatchToProps)(withRouter(DashboardsPage)) +export default connect(mapStateToProps, mapDispatchToProps)(withRouter(DashboardsPage)) diff --git a/ui/src/dashboards/reducers/ui.js b/ui/src/dashboards/reducers/ui.js index 020bc2dbab..f9850b3290 100644 --- a/ui/src/dashboards/reducers/ui.js +++ b/ui/src/dashboards/reducers/ui.js @@ -5,7 +5,7 @@ import timeRanges from 'hson!../../shared/data/timeRanges.hson'; const {lower, upper} = timeRanges[1] const initialState = { - dashboards: [], + dashboards: null, dashboard: EMPTY_DASHBOARD, timeRange: {lower, upper}, isEditMode: false, From f905ad36ed139ec5833b7b4d34155c86ee0e5537 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 19:03:15 -0700 Subject: [PATCH 3/9] Fix DashboardPage to handle null values for and until loaded --- ui/src/dashboards/containers/DashboardPage.js | 64 +++++++++++-------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/ui/src/dashboards/containers/DashboardPage.js b/ui/src/dashboards/containers/DashboardPage.js index bd994fc93e..5fe9e18a3c 100644 --- a/ui/src/dashboards/containers/DashboardPage.js +++ b/ui/src/dashboards/containers/DashboardPage.js @@ -24,10 +24,10 @@ const { const DashboardPage = React.createClass({ propTypes: { - source: PropTypes.shape({ - links: PropTypes.shape({ - proxy: PropTypes.string, - self: PropTypes.string, + source: shape({ + links: shape({ + proxy: string, + self: string, }), }), params: shape({ @@ -49,11 +49,11 @@ const DashboardPage = React.createClass({ dashboards: arrayOf(shape({ id: number.isRequired, cells: arrayOf(shape({})).isRequired, - })).isRequired, + })), dashboard: shape({ id: number.isRequired, cells: arrayOf(shape({})).isRequired, - }).isRequired, + }), handleChooseAutoRefresh: func.isRequired, autoRefresh: number.isRequired, timeRange: shape({}).isRequired, @@ -224,30 +224,38 @@ const DashboardPage = React.createClass({ onAddCell={this.handleAddCell} onEditDashboard={this.handleEditDashboard} > - {(dashboards).map((d, i) => { - return ( -
  • - - {d.name} - -
    • - ); - })} + { + dashboards ? + dashboards.map((d, i) => { + return ( +
  • + + {d.name} + +
    • + ); + }) : + null + } } - + { + dashboard ? + : + null + } ); }, From 889c55efa730e2b709efc57c45566d40b16c490b Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 19:37:35 -0700 Subject: [PATCH 4/9] Rename action on delete dashboard failed to follow effect style rather than affect --- ui/src/dashboards/actions/index.js | 6 +++--- ui/src/dashboards/reducers/ui.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index a817a46ac4..baaa4cf71b 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -47,8 +47,8 @@ export const deleteDashboard = (dashboard) => ({ }, }) -export const undoDeleteDashboard = (dashboard) => ({ - type: 'UNDO_DELETE_DASHBOARD', +export const deleteDashboardFailed = (dashboard) => ({ + type: 'DELETE_DASHBOARD_FAILED', payload: { dashboard, }, @@ -131,7 +131,7 @@ export const deleteDashboardAsync = (dashboard) => async (dispatch) => { await deleteDashboardAJAX(dashboard) dispatch(publishNotification('success', 'Dashboard deleted successfully.')) } catch (error) { - dispatch(undoDeleteDashboard(dashboard)) // undo optimistic update + dispatch(deleteDashboardFailed(dashboard)) dispatch(publishNotification('error', `Failed to delete dashboard: ${error.data.message}.`)) } } diff --git a/ui/src/dashboards/reducers/ui.js b/ui/src/dashboards/reducers/ui.js index f9850b3290..c2f015868a 100644 --- a/ui/src/dashboards/reducers/ui.js +++ b/ui/src/dashboards/reducers/ui.js @@ -57,7 +57,7 @@ export default function ui(state = initialState, action) { return {...state, ...newState} } - case 'UNDO_DELETE_DASHBOARD': { + case 'DELETE_DASHBOARD_FAILED': { const {dashboard} = action.payload const newState = { dashboards: [ From 012c0fbbdd6eeac96cffb7ef18a0ab56a1fadb8e Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 20:03:33 -0700 Subject: [PATCH 5/9] Add tests for deleteDashboard and deleteDashboardFailed --- ui/spec/dashboards/reducers/uiSpec.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ui/spec/dashboards/reducers/uiSpec.js b/ui/spec/dashboards/reducers/uiSpec.js index 5ed3de6c37..dc8897f954 100644 --- a/ui/spec/dashboards/reducers/uiSpec.js +++ b/ui/spec/dashboards/reducers/uiSpec.js @@ -1,9 +1,13 @@ +import _ from 'lodash' + import reducer from 'src/dashboards/reducers/ui' import timeRanges from 'hson!src/shared/data/timeRanges.hson'; import { loadDashboards, setDashboard, + deleteDashboard, + deleteDashboardFailed, setTimeRange, updateDashboardCells, editDashboardCell, @@ -50,6 +54,25 @@ describe('DataExplorer.Reducers.UI', () => { expect(actual.dashboard).to.deep.equal(d2) }) + it('can handle a successful dashboard deletion', () => { + const loadedState = reducer(state, loadDashboards(dashboards)) + const expected = [d1] + const actual = reducer(loadedState, deleteDashboard(d2)) + + expect(actual.dashboards).to.deep.equal(expected) + }) + + it('can handle a failed dashboard deletion', () => { + const loadedState = reducer(state, loadDashboards([d1])) + const actual = reducer(loadedState, deleteDashboardFailed(d2)) + const actualFirst = _.first(actual.dashboards) + + expect(actual.dashboards).to.have.length(2) + _.forOwn(d2, (v, k) => { + expect(actualFirst[k]).to.deep.equal(v) + }) + }) + it('can set the time range', () => { const expected = {upper: null, lower: 'now() - 1h'} const actual = reducer(state, setTimeRange(expected)) From da6e1c6a87e52b885715d7ef9424cbb8f611bb91 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Tue, 28 Mar 2017 20:09:12 -0700 Subject: [PATCH 6/9] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6284cca7d7..dd05f637e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ 1. [#1104](https://github.com/influxdata/chronograf/pull/1104): Fix windows hosts on host list ### Features + 1. [#1112](https://github.com/influxdata/chronograf/pull/1112): Add ability to delete a dashboard + ### UI Improvements 1. [#1101](https://github.com/influxdata/chronograf/pull/1101): Compress InfluxQL responses with gzip From a72b3d996d448d233df174cb9f7882d0d432a9c3 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Wed, 29 Mar 2017 10:53:44 -0700 Subject: [PATCH 7/9] Use simpler destructuring shorthand --- ui/src/dashboards/containers/DashboardsPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/dashboards/containers/DashboardsPage.js b/ui/src/dashboards/containers/DashboardsPage.js index 75de18b396..908cf0e6ee 100644 --- a/ui/src/dashboards/containers/DashboardsPage.js +++ b/ui/src/dashboards/containers/DashboardsPage.js @@ -53,7 +53,7 @@ const DashboardsPage = React.createClass({ }, render() { - const {dashboards: dashboards} = this.props + const {dashboards} = this.props const dashboardLink = `/sources/${this.props.source.id}` let tableHeader if (dashboards === null) { From 4a5eb775d51570327ba40797277d978ee2f473a2 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Wed, 29 Mar 2017 10:55:16 -0700 Subject: [PATCH 8/9] Use try/catch with async/await --- ui/src/dashboards/actions/index.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index baaa4cf71b..3eeca56d75 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -107,8 +107,13 @@ export const deleteDashboardCell = (cell) => ({ // Async Action Creators export const getDashboardsAsync = (dashboardID) => async (dispatch) => { - const {data: {dashboards}} = await getDashboardsAJAX() - dispatch(loadDashboards(dashboards, dashboardID)) + try { + const {data: {dashboards}} = await getDashboardsAJAX() + dispatch(loadDashboards(dashboards, dashboardID)) + } catch (error) { + console.error(error) + throw error + } } export const putDashboard = () => (dispatch, getState) => { From fb6a0e14c2119f1cb326b465712ab9ec01d03b99 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Wed, 29 Mar 2017 11:23:46 -0700 Subject: [PATCH 9/9] Auto-dismiss notification --- ui/src/dashboards/actions/index.js | 4 +++- ui/src/shared/constants/index.js | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index 3eeca56d75..8312da819b 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -7,8 +7,9 @@ import { deleteDashboardCell as deleteDashboardCellAJAX, } from 'src/dashboards/apis' -import {publishNotification} from 'src/shared/actions/notifications'; +import {publishNotification, delayDismissNotification} from 'src/shared/actions/notifications' +import {SHORT_NOTIFICATION_DISAPPEARING_DELAY} from 'shared/constants' import {NEW_DEFAULT_DASHBOARD_CELL} from 'src/dashboards/constants' export const loadDashboards = (dashboards, dashboardID) => ({ @@ -135,6 +136,7 @@ export const deleteDashboardAsync = (dashboard) => async (dispatch) => { try { await deleteDashboardAJAX(dashboard) dispatch(publishNotification('success', 'Dashboard deleted successfully.')) + dispatch(delayDismissNotification('success', SHORT_NOTIFICATION_DISAPPEARING_DELAY)) } catch (error) { dispatch(deleteDashboardFailed(dashboard)) dispatch(publishNotification('error', `Failed to delete dashboard: ${error.data.message}.`)) diff --git a/ui/src/shared/constants/index.js b/ui/src/shared/constants/index.js index d238e7bb7a..4ebcac2dc8 100644 --- a/ui/src/shared/constants/index.js +++ b/ui/src/shared/constants/index.js @@ -471,6 +471,8 @@ export const STROKE_WIDTH = { export const PRESENTATION_MODE_ANIMATION_DELAY = 0 // In milliseconds. export const PRESENTATION_MODE_NOTIFICATION_DELAY = 2000 // In milliseconds. +export const SHORT_NOTIFICATION_DISAPPEARING_DELAY = 1500 // in milliseconds + export const RES_UNAUTHORIZED = 401 export const AUTOREFRESH_DEFAULT = 15000 // in milliseconds