From 635ebc5dd26a5130d9f1b1ea8823c73f660075c2 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Wed, 29 Mar 2017 16:06:31 -0700 Subject: [PATCH] Refactor some notifications to use consolidated dispatcher in Admin and Dashboards (#1116) * Separate notification dispatchers from actions, share constants, simplify, clean up * Replace 'addFlashMessage' with 'notify' pattern throughout Admin and Dashboards, clean up * Remove semicolons * Notify success upon AJAX success --- ui/src/admin/actions/index.js | 60 ++++++++++--------- ui/src/admin/apis/index.js | 28 ++------- ui/src/admin/constants/index.js | 3 - ui/src/admin/containers/AdminPage.js | 15 +++-- .../admin/containers/DatabaseManagerPage.js | 7 ++- ui/src/admin/containers/QueriesPage.js | 11 ++-- ui/src/dashboards/actions/index.js | 7 +-- .../dashboards/containers/DashboardsPage.js | 1 - ui/src/shared/actions/notifications.js | 6 -- ui/src/shared/constants/index.js | 4 +- ui/src/shared/dispatchers/index.js | 18 +++++- 11 files changed, 81 insertions(+), 79 deletions(-) diff --git a/ui/src/admin/actions/index.js b/ui/src/admin/actions/index.js index dca4516939..f69705d4b4 100644 --- a/ui/src/admin/actions/index.js +++ b/ui/src/admin/actions/index.js @@ -20,8 +20,10 @@ import { killQuery as killQueryProxy, } from 'shared/apis/metaQuery' -import {publishNotification} from 'src/shared/actions/notifications' -import {ADMIN_NOTIFICATION_DELAY} from 'src/admin/constants' +import {publishNotification} from 'shared/actions/notifications' +import {publishAutoDismissingNotification} from 'shared/dispatchers' + +import {REVERT_STATE_DELAY} from 'shared/constants' export const loadUsers = ({users}) => ({ type: 'LOAD_USERS', @@ -241,24 +243,24 @@ export const loadDBsAndRPsAsync = (url) => async (dispatch) => { export const createUserAsync = (url, user) => async (dispatch) => { try { const {data} = await createUserAJAX(url, user) - dispatch(publishNotification('success', 'User created successfully')) + dispatch(publishAutoDismissingNotification('success', 'User created successfully')) dispatch(syncUser(user, data)) } catch (error) { // undo optimistic update dispatch(publishNotification('error', `Failed to create user: ${error.data.message}`)) - setTimeout(() => dispatch(deleteUser(user)), ADMIN_NOTIFICATION_DELAY) + setTimeout(() => dispatch(deleteUser(user)), REVERT_STATE_DELAY) } } export const createRoleAsync = (url, role) => async (dispatch) => { try { const {data} = await createRoleAJAX(url, role) - dispatch(publishNotification('success', 'Role created successfully')) + dispatch(publishAutoDismissingNotification('success', 'Role created successfully')) dispatch(syncRole(role, data)) } catch (error) { // undo optimistic update dispatch(publishNotification('error', `Failed to create role: ${error.data.message}`)) - setTimeout(() => dispatch(deleteRole(role)), ADMIN_NOTIFICATION_DELAY) + setTimeout(() => dispatch(deleteRole(role)), REVERT_STATE_DELAY) } } @@ -266,23 +268,23 @@ export const createDatabaseAsync = (url, database) => async (dispatch) => { try { const {data} = await createDatabaseAJAX(url, database) dispatch(syncDatabase(database, data)) - dispatch(publishNotification('success', 'Database created successfully')) + dispatch(publishAutoDismissingNotification('success', 'Database created successfully')) } catch (error) { // undo optimistic update dispatch(publishNotification('error', `Failed to create database: ${error.data.message}`)) - setTimeout(() => dispatch(removeDatabase(database)), ADMIN_NOTIFICATION_DELAY) + setTimeout(() => dispatch(removeDatabase(database)), REVERT_STATE_DELAY) } } export const createRetentionPolicyAsync = (database, retentionPolicy) => async (dispatch) => { try { const {data} = await createRetentionPolicyAJAX(database.links.retentionPolicies, retentionPolicy) - dispatch(publishNotification('success', 'Retention policy created successfully')) + dispatch(publishAutoDismissingNotification('success', 'Retention policy created successfully')) dispatch(syncRetentionPolicy(database, retentionPolicy, data)) } catch (error) { // undo optimistic update dispatch(publishNotification('error', `Failed to create retention policy: ${error.data.message}`)) - setTimeout(() => dispatch(removeRetentionPolicy(database, retentionPolicy)), ADMIN_NOTIFICATION_DELAY) + setTimeout(() => dispatch(removeRetentionPolicy(database, retentionPolicy)), REVERT_STATE_DELAY) } } @@ -290,7 +292,7 @@ export const updateRetentionPolicyAsync = (database, retentionPolicy, updates) = try { dispatch(editRetentionPolicy(database, retentionPolicy, updates)) const {data} = await updateRetentionPolicyAJAX(retentionPolicy.links.self, updates) - dispatch(publishNotification('success', 'Retention policy updated successfully')) + dispatch(publishAutoDismissingNotification('success', 'Retention policy updated successfully')) dispatch(syncRetentionPolicy(database, retentionPolicy, data)) } catch (error) { dispatch(publishNotification('error', `Failed to update retention policy: ${error.data.message}`)) @@ -306,27 +308,31 @@ export const killQueryAsync = (source, queryID) => (dispatch) => { killQueryProxy(source, queryID) } -export const deleteRoleAsync = (role, addFlashMessage) => (dispatch) => { - // optimistic update +export const deleteRoleAsync = (role) => async (dispatch) => { dispatch(deleteRole(role)) - - // delete role on server - deleteRoleAJAX(role.links.self, addFlashMessage, role.name) + try { + await deleteRoleAJAX(role.links.self) + dispatch(publishAutoDismissingNotification('success', 'Role deleted')) + } catch (error) { + dispatch(publishNotification('error', `Failed to delete role: ${error.data.message}`)) + } } -export const deleteUserAsync = (user, addFlashMessage) => (dispatch) => { - // optimistic update +export const deleteUserAsync = (user) => async (dispatch) => { dispatch(deleteUser(user)) - - // delete user on server - deleteUserAJAX(user.links.self, addFlashMessage, user.name) + try { + await deleteUserAJAX(user.links.self) + dispatch(publishAutoDismissingNotification('success', 'User deleted')) + } catch (error) { + dispatch(publishNotification('error', `Failed to delete user: ${error.data.message}`)) + } } export const deleteDatabaseAsync = (database) => async (dispatch) => { dispatch(removeDatabase(database)) - dispatch(publishNotification('success', 'Database deleted')) try { await deleteDatabaseAJAX(database.links.self) + dispatch(publishAutoDismissingNotification('success', 'Database deleted')) } catch (error) { dispatch(publishNotification('error', `Failed to delete database: ${error.data.message}`)) } @@ -334,9 +340,9 @@ export const deleteDatabaseAsync = (database) => async (dispatch) => { export const deleteRetentionPolicyAsync = (database, retentionPolicy) => async (dispatch) => { dispatch(removeRetentionPolicy(database, retentionPolicy)) - dispatch(publishNotification('success', `Retention policy ${retentionPolicy.name} deleted`)) try { await deleteRetentionPolicyAJAX(retentionPolicy.links.self) + dispatch(publishAutoDismissingNotification('success', `Retention policy ${retentionPolicy.name} deleted`)) } catch (error) { dispatch(publishNotification('error', `Failed to delete retentionPolicy: ${error.data.message}`)) } @@ -345,7 +351,7 @@ export const deleteRetentionPolicyAsync = (database, retentionPolicy) => async ( export const updateRoleUsersAsync = (role, users) => async (dispatch) => { try { const {data} = await updateRoleAJAX(role.links.self, users, role.permissions) - dispatch(publishNotification('success', 'Role users updated')) + dispatch(publishAutoDismissingNotification('success', 'Role users updated')) dispatch(syncRole(role, data)) } catch (error) { dispatch(publishNotification('error', `Failed to update role: ${error.data.message}`)) @@ -355,7 +361,7 @@ export const updateRoleUsersAsync = (role, users) => async (dispatch) => { export const updateRolePermissionsAsync = (role, permissions) => async (dispatch) => { try { const {data} = await updateRoleAJAX(role.links.self, role.users, permissions) - dispatch(publishNotification('success', 'Role permissions updated')) + dispatch(publishAutoDismissingNotification('success', 'Role permissions updated')) dispatch(syncRole(role, data)) } catch (error) { dispatch(publishNotification('error', `Failed to updated role: ${error.data.message}`)) @@ -365,7 +371,7 @@ export const updateRolePermissionsAsync = (role, permissions) => async (dispatch export const updateUserPermissionsAsync = (user, permissions) => async (dispatch) => { try { const {data} = await updateUserAJAX(user.links.self, user.roles, permissions) - dispatch(publishNotification('success', 'User permissions updated')) + dispatch(publishAutoDismissingNotification('success', 'User permissions updated')) dispatch(syncUser(user, data)) } catch (error) { dispatch(publishNotification('error', `Failed to updated user: ${error.data.message}`)) @@ -375,7 +381,7 @@ export const updateUserPermissionsAsync = (user, permissions) => async (dispatch export const updateUserRolesAsync = (user, roles) => async (dispatch) => { try { const {data} = await updateUserAJAX(user.links.self, roles, user.permissions) - dispatch(publishNotification('success', 'User roles updated')) + dispatch(publishAutoDismissingNotification('success', 'User roles updated')) dispatch(syncUser(user, data)) } catch (error) { dispatch(publishNotification('error', `Failed to updated user: ${error.data.message}`)) diff --git a/ui/src/admin/apis/index.js b/ui/src/admin/apis/index.js index 7c411ba0f2..7ef4ab11e0 100644 --- a/ui/src/admin/apis/index.js +++ b/ui/src/admin/apis/index.js @@ -107,43 +107,27 @@ export const deleteRetentionPolicy = async (url) => { } } -export const deleteRole = async (url, addFlashMessage, rolename) => { +export const deleteRole = async (url) => { try { - const response = await AJAX({ + return await AJAX({ method: 'DELETE', url, }) - addFlashMessage({ - type: 'success', - text: `${rolename} successfully deleted.`, - }) - return response } catch (error) { console.error(error) - addFlashMessage({ - type: 'error', - text: `Error deleting: ${rolename}.`, - }) + throw error } } -export const deleteUser = async (url, addFlashMessage, username) => { +export const deleteUser = async (url) => { try { - const response = await AJAX({ + return await AJAX({ method: 'DELETE', url, }) - addFlashMessage({ - type: 'success', - text: `${username} successfully deleted.`, - }) - return response } catch (error) { console.error(error) - addFlashMessage({ - type: 'error', - text: `Error deleting: ${username}.`, - }) + throw error } } diff --git a/ui/src/admin/constants/index.js b/ui/src/admin/constants/index.js index 1b9c35b9d9..67d33448ac 100644 --- a/ui/src/admin/constants/index.js +++ b/ui/src/admin/constants/index.js @@ -8,8 +8,6 @@ export const TIMES = [ {test: /^\d*h\d*m\d*s/, magnitude: 5}, ] -export const ADMIN_NOTIFICATION_DELAY = 1500 // milliseconds - export const NEW_DEFAULT_USER = { name: '', password: '', @@ -49,4 +47,3 @@ export const NEW_DEFAULT_DATABASE = { retentionPolicies: [NEW_DEFAULT_RP], links: {self: ''}, } - diff --git a/ui/src/admin/containers/AdminPage.js b/ui/src/admin/containers/AdminPage.js index 3ec74b6350..3c02260558 100644 --- a/ui/src/admin/containers/AdminPage.js +++ b/ui/src/admin/containers/AdminPage.js @@ -25,6 +25,8 @@ import { import AdminTabs from 'src/admin/components/AdminTabs' +import {publishAutoDismissingNotification} from 'shared/dispatchers' + const isValidUser = (user) => { const minLen = 3 return (user.name.length >= minLen && user.password.length >= minLen) @@ -81,8 +83,9 @@ class AdminPage extends Component { } async handleSaveUser(user) { + const {notify} = this.props if (!isValidUser(user)) { - this.props.addFlashMessage({type: 'error', text: 'Username and/or password too short'}) + notify('error', 'Username and/or password too short') return } if (user.isNew) { @@ -93,8 +96,9 @@ class AdminPage extends Component { } async handleSaveRole(role) { + const {notify} = this.props if (!isValidRole(role)) { - this.props.addFlashMessage({type: 'error', text: 'Role name too short'}) + notify('error', 'Role name too short') return } if (role.isNew) { @@ -114,11 +118,11 @@ class AdminPage extends Component { } handleDeleteRole(role) { - this.props.deleteRole(role, this.props.addFlashMessage) + this.props.deleteRole(role) } handleDeleteUser(user) { - this.props.deleteUser(user, this.props.addFlashMessage) + this.props.deleteUser(user) } handleUpdateRoleUsers(role, users) { @@ -223,13 +227,13 @@ AdminPage.propTypes = { createRole: func, deleteRole: func, deleteUser: func, - addFlashMessage: func, filterRoles: func, filterUsers: func, updateRoleUsers: func, updateRolePermissions: func, updateUserPermissions: func, updateUserRoles: func, + notify: func, } const mapStateToProps = ({admin: {users, roles, permissions}}) => ({ @@ -258,6 +262,7 @@ const mapDispatchToProps = (dispatch) => ({ updateRolePermissions: bindActionCreators(updateRolePermissionsAsync, dispatch), updateUserPermissions: bindActionCreators(updateUserPermissionsAsync, dispatch), updateUserRoles: bindActionCreators(updateUserRolesAsync, dispatch), + notify: bindActionCreators(publishAutoDismissingNotification, dispatch), }) export default connect(mapStateToProps, mapDispatchToProps)(AdminPage) diff --git a/ui/src/admin/containers/DatabaseManagerPage.js b/ui/src/admin/containers/DatabaseManagerPage.js index 7cc5959f62..f04f37f1df 100644 --- a/ui/src/admin/containers/DatabaseManagerPage.js +++ b/ui/src/admin/containers/DatabaseManagerPage.js @@ -2,9 +2,10 @@ import React, {PropTypes, Component} from 'react' import {connect} from 'react-redux' import {bindActionCreators} from 'redux' -import * as adminActionCreators from 'src/admin/actions' import DatabaseManager from 'src/admin/components/DatabaseManager' -import {publishNotification} from 'src/shared/actions/notifications' + +import * as adminActionCreators from 'src/admin/actions' +import {publishAutoDismissingNotification} from 'shared/dispatchers' class DatabaseManagerPage extends Component { constructor(props) { @@ -141,7 +142,7 @@ const mapStateToProps = ({admin: {databases, retentionPolicies}}) => ({ const mapDispatchToProps = (dispatch) => ({ actions: bindActionCreators(adminActionCreators, dispatch), - notify: bindActionCreators(publishNotification, dispatch), + notify: bindActionCreators(publishAutoDismissingNotification, dispatch), }) export default connect(mapStateToProps, mapDispatchToProps)(DatabaseManagerPage) diff --git a/ui/src/admin/containers/QueriesPage.js b/ui/src/admin/containers/QueriesPage.js index dce54506bb..a3a517fca3 100644 --- a/ui/src/admin/containers/QueriesPage.js +++ b/ui/src/admin/containers/QueriesPage.js @@ -20,6 +20,8 @@ import { killQueryAsync, } from 'src/admin/actions' +import {publishAutoDismissingNotification} from 'shared/dispatchers' + class QueriesPage extends Component { constructor(props) { super(props) @@ -47,11 +49,11 @@ class QueriesPage extends Component { } updateQueries() { - const {source, addFlashMessage, loadQueries} = this.props + const {source, notify, loadQueries} = this.props showDatabases(source.links.proxy).then((resp) => { const {databases, errors} = showDatabasesParser(resp.data) if (errors.length) { - errors.forEach((message) => addFlashMessage({type: 'error', text: message})) + errors.forEach((message) => notify('error', message)) return } @@ -62,7 +64,7 @@ class QueriesPage extends Component { queryResponses.forEach((queryResponse) => { const result = showQueriesParser(queryResponse.data) if (result.errors.length) { - result.erorrs.forEach((message) => this.props.addFlashMessage({type: 'error', text: message})) + result.errors.forEach((message) => notify('error', message)) } allQueries.push(...result.queries) @@ -113,11 +115,11 @@ QueriesPage.propTypes = { }), }), queries: arrayOf(shape()), - addFlashMessage: func, loadQueries: func, queryIDToKill: string, setQueryToKill: func, killQuery: func, + notify: func, } const mapStateToProps = ({admin: {queries, queryIDToKill}}) => ({ @@ -129,6 +131,7 @@ const mapDispatchToProps = (dispatch) => ({ loadQueries: bindActionCreators(loadQueriesAction, dispatch), setQueryToKill: bindActionCreators(setQueryToKillAction, dispatch), killQuery: bindActionCreators(killQueryAsync, dispatch), + notify: bindActionCreators(publishAutoDismissingNotification, dispatch), }) export default connect(mapStateToProps, mapDispatchToProps)(QueriesPage) diff --git a/ui/src/dashboards/actions/index.js b/ui/src/dashboards/actions/index.js index 8312da819b..ae8a0a8caf 100644 --- a/ui/src/dashboards/actions/index.js +++ b/ui/src/dashboards/actions/index.js @@ -7,9 +7,9 @@ import { deleteDashboardCell as deleteDashboardCellAJAX, } from 'src/dashboards/apis' -import {publishNotification, delayDismissNotification} from 'src/shared/actions/notifications' +import {publishNotification} from 'shared/actions/notifications' +import {publishAutoDismissingNotification} from 'shared/dispatchers' -import {SHORT_NOTIFICATION_DISAPPEARING_DELAY} from 'shared/constants' import {NEW_DEFAULT_DASHBOARD_CELL} from 'src/dashboards/constants' export const loadDashboards = (dashboards, dashboardID) => ({ @@ -135,8 +135,7 @@ export const deleteDashboardAsync = (dashboard) => async (dispatch) => { dispatch(deleteDashboard(dashboard)) try { await deleteDashboardAJAX(dashboard) - dispatch(publishNotification('success', 'Dashboard deleted successfully.')) - dispatch(delayDismissNotification('success', SHORT_NOTIFICATION_DISAPPEARING_DELAY)) + dispatch(publishAutoDismissingNotification('success', 'Dashboard deleted successfully.')) } catch (error) { dispatch(deleteDashboardFailed(dashboard)) dispatch(publishNotification('error', `Failed to delete dashboard: ${error.data.message}.`)) diff --git a/ui/src/dashboards/containers/DashboardsPage.js b/ui/src/dashboards/containers/DashboardsPage.js index bcb998df2c..63ec591bd0 100644 --- a/ui/src/dashboards/containers/DashboardsPage.js +++ b/ui/src/dashboards/containers/DashboardsPage.js @@ -32,7 +32,6 @@ const DashboardsPage = React.createClass({ router: shape({ push: func.isRequired, }).isRequired, - addFlashMessage: func, handleGetDashboards: func.isRequired, handleDeleteDashboard: func.isRequired, dashboards: arrayOf(shape()), diff --git a/ui/src/shared/actions/notifications.js b/ui/src/shared/actions/notifications.js index e0ce00edde..52dfcf4e51 100644 --- a/ui/src/shared/actions/notifications.js +++ b/ui/src/shared/actions/notifications.js @@ -17,12 +17,6 @@ export function dismissNotification(type) { } } -export function delayDismissNotification(type, wait) { - return (dispatch) => { - setTimeout(() => dispatch(dismissNotification(type)), wait) - } -} - export function dismissAllNotifications() { return { type: 'ALL_NOTIFICATIONS_DISMISSED', diff --git a/ui/src/shared/constants/index.js b/ui/src/shared/constants/index.js index c103677b26..7ae12f8669 100644 --- a/ui/src/shared/constants/index.js +++ b/ui/src/shared/constants/index.js @@ -471,7 +471,9 @@ 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 SHORT_NOTIFICATION_DISMISS_DELAY = 1500 // in milliseconds + +export const REVERT_STATE_DELAY = 1500 // ms export const RES_UNAUTHORIZED = 401 diff --git a/ui/src/shared/dispatchers/index.js b/ui/src/shared/dispatchers/index.js index a7771a9ff9..04833eddd2 100644 --- a/ui/src/shared/dispatchers/index.js +++ b/ui/src/shared/dispatchers/index.js @@ -1,9 +1,21 @@ +import {publishNotification, dismissNotification} from 'shared/actions/notifications' import {delayEnablePresentationMode} from 'shared/actions/app' -import {publishNotification, delayDismissNotification} from 'shared/actions/notifications' + import {PRESENTATION_MODE_NOTIFICATION_DELAY} from 'shared/constants' +import {SHORT_NOTIFICATION_DISMISS_DELAY} from 'shared/constants' + +export function delayDismissNotification(type, delay) { + return (dispatch) => { + setTimeout(() => dispatch(dismissNotification(type)), delay) + } +} + +export const publishAutoDismissingNotification = (type, message, delay = SHORT_NOTIFICATION_DISMISS_DELAY) => (dispatch) => { + dispatch(publishNotification(type, message)) + dispatch(delayDismissNotification(type, delay)) +} export const presentationButtonDispatcher = (dispatch) => () => { dispatch(delayEnablePresentationMode()) - dispatch(publishNotification('success', 'Press ESC to disable presentation mode.')) - dispatch(delayDismissNotification('success', PRESENTATION_MODE_NOTIFICATION_DELAY)) + dispatch(publishAutoDismissingNotification('success', 'Press ESC to disable presentation mode.', PRESENTATION_MODE_NOTIFICATION_DELAY)) }