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

@@ -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}`))

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
   }
 }
 

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: ''},
 }
-

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)

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)

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)

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}.`))

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()),

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',

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
 

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))
 }