Merge pull request #5926 from influxdata/feat/influxdb_new_create_role

feat(ui): improve InfluxDB role creation

CHANGELOG.md

@@ -6,6 +6,7 @@
 1. [#5921](https://github.com/influxdata/chronograf/pull/5921): Manage InfluxDB users including their database permissions.
 1. [#5923](https://github.com/influxdata/chronograf/pull/5923): Manage InfluxDB roles including their database permissions.
 1. [#5925](https://github.com/influxdata/chronograf/pull/5925): Improve InfluxDB user creation.
+1. [#5926](https://github.com/influxdata/chronograf/pull/5926): Improve InfluxDB role creation.
 
 ### Bug Fixes
 

ui/src/admin/actions/influxdb.js

@@ -84,10 +84,6 @@ export const loadDatabases = databases => ({
   },
 })
 
-export const addRole = () => ({
-  type: 'INFLUXDB_ADD_ROLE',
-})
-
 export const addDatabase = () => ({
   type: 'INFLUXDB_ADD_DATABASE',
 })
@@ -132,14 +128,6 @@ export const syncRetentionPolicy = (database, stale, synced) => ({
   },
 })
 
-export const editRole = (role, updates) => ({
-  type: 'INFLUXDB_EDIT_ROLE',
-  payload: {
-    role,
-    updates,
-  },
-})
-
 export const editDatabase = (database, updates) => ({
   type: 'INFLUXDB_EDIT_DATABASE',
   payload: {
@@ -318,8 +306,6 @@ export const createRoleAsync = (url, role) => async dispatch => {
     dispatch(syncRole(role, data))
   } catch (error) {
     dispatch(errorThrown(error, notifyRoleCreationFailed(error.data.message)))
-    // undo optimistic update
-    setTimeout(() => dispatch(deleteRole(role)), REVERT_STATE_DELAY)
   }
 }
 

ui/src/admin/components/RoleRow.tsx

@@ -1,6 +1,5 @@
 import React from 'react'
 
-import RoleRowEdit from 'src/admin/components/RoleRowEdit'
 import {ROLES_TABLE} from 'src/admin/constants/tableSizing'
 import {UserRole} from 'src/types/influxAdmin'
 import {Link} from 'react-router'
@@ -12,9 +11,6 @@ interface Props {
   page: string
   perDBPermissions: Array<Record<string, boolean>>
   showUsers: boolean
-  onCancel: (role: UserRole) => void
-  onEdit: (role: UserRole, updates: Partial<UserRole>) => void
-  onSave: (role: UserRole) => Promise<void>
 }
 
 const RoleRow = ({
@@ -23,22 +19,7 @@ const RoleRow = ({
   page,
   perDBPermissions,
   showUsers,
-  onEdit,
-  onSave,
-  onCancel,
 }: Props) => {
-  if (role.isEditing) {
-    return (
-      <RoleRowEdit
-        role={role}
-        onEdit={onEdit}
-        onSave={onSave}
-        onCancel={onCancel}
-        colSpan={1 + +showUsers + perDBPermissions.length}
-      />
-    )
-  }
-
   return (
     <tr data-test={`role-${role.name}--row`}>
       <td style={{width: `${ROLES_TABLE.colName}px`}}>

ui/src/admin/components/RoleRowEdit.tsx

@@ -1,56 +0,0 @@
-import React from 'react'
-
-import ConfirmOrCancel from 'src/shared/components/ConfirmOrCancel'
-import {UserRole} from 'src/types/influxAdmin'
-
-interface UserRowEditProps {
-  role: UserRole
-  onEdit: (role: UserRole, updates: Partial<UserRole>) => void
-  onSave: (role: UserRole) => Promise<void>
-  onCancel: (role: UserRole) => void
-  colSpan: number
-}
-
-const RoleRowEdit = ({
-  role,
-  onEdit,
-  onSave,
-  onCancel,
-  colSpan,
-}: UserRowEditProps) => {
-  const onKeyPress: React.KeyboardEventHandler = e => {
-    if (e.key === 'Enter') {
-      onSave(role)
-    }
-  }
-
-  return (
-    <tr className="admin-table--edit-row">
-      <td colSpan={colSpan} style={{padding: '5px 0 5px 5px'}}>
-        <div style={{display: 'flex', flexDirection: 'row', columnGap: '5px'}}>
-          <input
-            className="form-control input-xs"
-            name="name"
-            type="text"
-            value={role.name || ''}
-            placeholder="Role name"
-            onChange={e => onEdit(role, {name: e.target.value})}
-            onKeyPress={onKeyPress}
-            autoFocus={true}
-            spellCheck={false}
-            autoComplete="false"
-            data-test="role-name--input"
-          />
-          <ConfirmOrCancel
-            item={role}
-            onConfirm={onSave}
-            onCancel={onCancel}
-            buttonSize="btn-xs"
-          />
-        </div>
-      </td>
-    </tr>
-  )
-}
-
-export default RoleRowEdit

ui/src/admin/components/influxdb/CreateRoleDialog.tsx

@@ -0,0 +1,67 @@
+import React, {useCallback, useState} from 'react'
+import {
+  ComponentStatus,
+  Form,
+  Input,
+  OverlayBody,
+  OverlayContainer,
+  OverlayHeading,
+  OverlayTechnology,
+} from 'src/reusable_ui'
+
+const minLen = 3
+export function validateRoleName(name: string): boolean {
+  return name?.length >= minLen
+}
+
+interface Props {
+  create: (role: {name: string}) => void
+  setVisible: (visible: boolean) => void
+  visible: boolean
+}
+const CreateRoleDialog = ({visible, setVisible, create}: Props) => {
+  const [name, setName] = useState('')
+  const cancel = useCallback(() => {
+    setName('')
+    setVisible(false)
+  }, [])
+  return (
+    <OverlayTechnology visible={visible}>
+      <OverlayContainer maxWidth={650}>
+        <OverlayHeading title="Create Role" onDismiss={cancel} />
+        <OverlayBody>
+          <Form>
+            <Form.Element label="Role Name">
+              <Input
+                value={name}
+                autoFocus={true}
+                onChange={e => setName(e.target.value)}
+                status={
+                  validateRoleName(name)
+                    ? ComponentStatus.Valid
+                    : ComponentStatus.Default
+                }
+              />
+            </Form.Element>
+            <Form.Footer>
+              <div className="form-group text-center form-group-submit col-xs-12">
+                <button className="btn btn-sm btn-default" onClick={cancel}>
+                  Cancel
+                </button>
+                <button
+                  className="btn btn-sm btn-success"
+                  disabled={!name}
+                  onClick={() => create({name})}
+                >
+                  Create
+                </button>
+              </div>
+            </Form.Footer>
+          </Form>
+        </OverlayBody>
+      </OverlayContainer>
+    </OverlayTechnology>
+  )
+}
+
+export default CreateRoleDialog

ui/src/admin/components/influxdb/CreateUserDialog.tsx

@@ -1,5 +1,6 @@
 import React, {useCallback, useState} from 'react'
 import {
+  ComponentStatus,
   Form,
   Input,
   InputType,
@@ -9,6 +10,12 @@ import {
   OverlayTechnology,
 } from 'src/reusable_ui'
 
+const minLen = 3
+export function validateUserName(name: string): boolean {
+  return name?.length >= minLen
+}
+export const validatePassword = validateUserName
+
 interface Props {
   create: (user: {name: string; password: string}) => void
   setVisible: (visible: boolean) => void
@@ -32,6 +39,12 @@ const CreateUserDialog = ({visible, setVisible, create}: Props) => {
               <Input
                 value={name}
                 onChange={e => setName(e.target.value)}
+                autoFocus={true}
+                status={
+                  validateUserName(name)
+                    ? ComponentStatus.Valid
+                    : ComponentStatus.Default
+                }
                 testId="username--input"
               />
             </Form.Element>
@@ -39,6 +52,11 @@ const CreateUserDialog = ({visible, setVisible, create}: Props) => {
               <Input
                 value={password}
                 type={InputType.Password}
+                status={
+                  validatePassword(password)
+                    ? ComponentStatus.Valid
+                    : ComponentStatus.Default
+                }
                 onChange={e => setPassword(e.target.value)}
                 testId="password--input"
               />

ui/src/admin/constants/index.js

@@ -1,11 +1,3 @@
-export const NEW_DEFAULT_ROLE = {
-  name: '',
-  permissions: [],
-  users: [],
-  links: {self: ''},
-  isNew: true,
-}
-
 export const NEW_DEFAULT_RP = {
   name: 'autogen',
   duration: '0',

ui/src/admin/containers/influxdb/RolePage.tsx

@@ -66,7 +66,7 @@ type Props = WithRouterProps<RouterParams> &
   ConnectedProps &
   ReduxDispatchProps
 
-const UserPage = ({
+const RolePage = ({
   users,
   databases,
   permissions: serverPermissions,
@@ -102,10 +102,10 @@ const UserPage = ({
         setRunning(true)
         try {
           await deleteAsync(r)
-          router.push(`/sources/${sourceID}/admin-influxdb/roles`)
         } finally {
           setRunning(false)
         }
+        router.push(`/sources/${sourceID}/admin-influxdb/roles`)
       },
     ]
   }, [source, roles, roleName])
@@ -249,7 +249,7 @@ const UserPage = ({
   const body =
     role === FAKE_ROLE ? (
       <div className="container-fluid">
-        User <span className="error-warning">{roleName}</span> not found!
+        Role <span className="error-warning">{roleName}</span> not found!
       </div>
     ) : (
       <div className="panel panel-solid influxdb-admin">
@@ -403,5 +403,5 @@ const UserPage = ({
 }
 
 export default withSource(
-  withRouter(connect(mapStateToProps, mapDispatchToProps)(UserPage))
+  withRouter(connect(mapStateToProps, mapDispatchToProps)(RolePage))
 )

ui/src/admin/containers/influxdb/RolesPage.tsx

@@ -1,17 +1,18 @@
 import React, {useMemo, useState} from 'react'
 import {connect, ResolveThunks} from 'react-redux'
 import {withSource} from 'src/CheckSources'
-import {Source} from 'src/types'
+import {withRouter, WithRouterProps} from 'react-router'
+import {Source, NotificationAction} from 'src/types'
 import {UserRole, User, Database} from 'src/types/influxAdmin'
 import {notify as notifyAction} from 'src/shared/actions/notifications'
 import {
-  addRole as addRoleActionCreator,
-  editRole as editRoleActionCreator,
-  deleteRole as deleteRoleAction,
   createRoleAsync,
   filterRoles as filterRolesAction,
 } from 'src/admin/actions/influxdb'
-import {notifyRoleNameInvalid} from 'src/shared/copy/notifications'
+import {
+  notifyRoleNameExists,
+  notifyRoleNameInvalid,
+} from 'src/shared/copy/notifications'
 import AdminInfluxDBTabbedPage, {
   hasRoleManagement,
   isConnectedToLDAP,
@@ -25,10 +26,19 @@ import computeEffectiveDBPermissions from './util/computeEffectiveDBPermissions'
 import useDebounce from 'src/utils/useDebounce'
 import useChangeEffect from 'src/utils/useChangeEffect'
 import {ComponentSize, MultiSelectDropdown, SlideToggle} from 'src/reusable_ui'
+import CreateRoleDialog, {
+  validateRoleName,
+} from 'src/admin/components/influxdb/CreateRoleDialog'
 
-const isValidRole = (role: UserRole): boolean => {
-  const minLen = 3
-  return role.name.length >= minLen
+const validateRole = (
+  role: Pick<UserRole, 'name'>,
+  notify: NotificationAction
+) => {
+  if (!validateRoleName(role.name)) {
+    notify(notifyRoleNameInvalid())
+    return false
+  }
+  return true
 }
 
 const mapStateToProps = ({adminInfluxDB: {databases, users, roles}}) => ({
@@ -40,9 +50,6 @@ const mapStateToProps = ({adminInfluxDB: {databases, users, roles}}) => ({
 const mapDispatchToProps = {
   filterRoles: filterRolesAction,
   createRole: createRoleAsync,
-  removeRole: deleteRoleAction,
-  addRole: addRoleActionCreator,
-  editRole: editRoleActionCreator,
   notify: notifyAction,
 }
 
@@ -57,34 +64,18 @@ interface ConnectedProps {
 
 type ReduxDispatchProps = ResolveThunks<typeof mapDispatchToProps>
 
-type Props = OwnProps & ConnectedProps & ReduxDispatchProps
+type Props = WithRouterProps & OwnProps & ConnectedProps & ReduxDispatchProps
 
 const RolesPage = ({
   source,
   users,
   roles,
   databases,
-  addRole,
+  router,
   filterRoles,
-  editRole,
-  removeRole,
   createRole,
   notify,
 }: Props) => {
-  const handleSaveRole = useCallback(
-    async (role: UserRole) => {
-      if (!isValidRole(role)) {
-        notify(notifyRoleNameInvalid())
-        return
-      }
-      if (role.isNew) {
-        createRole(source.links.roles, role)
-      }
-    },
-    [source, createRole]
-  )
-  const isEditing = useMemo(() => roles.some(r => r.isEditing), [roles])
-
   const rolesPage = useMemo(
     () => `/sources/${source.id}/admin-influxdb/roles`,
     [source]
@@ -129,8 +120,33 @@ const RolesPage = ({
     setShowUsers,
   ])
 
+  const [createVisible, setCreateVisible] = useState(false)
+  const createNew = useCallback(
+    async (role: {name: string}) => {
+      if (roles.some(x => x.name === role.name)) {
+        notify(notifyRoleNameExists())
+        return
+      }
+      if (!validateRole(role, notify)) {
+        return
+      }
+      await createRole(source.links.roles, role)
+      router.push(
+        `/sources/${source.id}/admin-influxdb/roles/${encodeURIComponent(
+          role.name
+        )}`
+      )
+    },
+    [roles, router, source, notify]
+  )
+
   return (
     <AdminInfluxDBTabbedPage activeTab="roles" source={source}>
+      <CreateRoleDialog
+        visible={createVisible}
+        setVisible={setCreateVisible}
+        create={createNew}
+      />
       <div className="panel panel-solid influxdb-admin">
         <div className="panel-heading">
           <div className="search-widget">
@@ -182,8 +198,8 @@ const RolesPage = ({
           <div className="panel-heading--right">
             <button
               className="btn btn-sm btn-primary"
-              disabled={isEditing}
-              onClick={addRole}
+              onClick={() => setCreateVisible(true)}
+              data-test="create-role--button"
             >
               <span className="icon plus" /> Create Role
             </button>
@@ -223,9 +239,6 @@ const RolesPage = ({
                       perDBPermissions={perDBPermissions[roleIndex]}
                       allUsers={users}
                       showUsers={showUsers}
-                      onEdit={editRole}
-                      onSave={handleSaveRole}
-                      onCancel={removeRole}
                     />
                   ))
                 ) : (
@@ -268,5 +281,5 @@ const RolesPageAvailable = (props: Props) => {
 }
 
 export default withSource(
-  connect(mapStateToProps, mapDispatchToProps)(RolesPageAvailable)
+  withRouter(connect(mapStateToProps, mapDispatchToProps)(RolesPageAvailable))
 )

ui/src/admin/containers/influxdb/UsersPage.tsx

@@ -26,19 +26,21 @@ import MultiSelectDropdown from 'src/reusable_ui/components/dropdowns/MultiSelec
 import {ComponentSize, SlideToggle} from 'src/reusable_ui'
 import computeEffectiveDBPermissions from './util/computeEffectiveDBPermissions'
 import allOrParticularSelection from './util/allOrParticularSelection'
-import CreateUserDialog from '../../components/influxdb/CreateUserDialog'
+import CreateUserDialog, {
+  validatePassword,
+  validateUserName,
+} from '../../components/influxdb/CreateUserDialog'
 import {withRouter, WithRouterProps} from 'react-router'
 
-const minLen = 3
 const validateUser = (
   user: Pick<User, 'name' | 'password'>,
   notify: NotificationAction
 ) => {
-  if (user.name.length < minLen) {
+  if (!validateUserName(user.name)) {
     notify(notifyDBUserNameInvalid())
     return false
   }
-  if (user.password.length < minLen) {
+  if (!validatePassword(user.password)) {
     notify(notifyDBPasswordInvalid())
     return false
   }

ui/src/admin/reducers/influxdb.js

@@ -1,9 +1,5 @@
 import reject from 'lodash/reject'
-import {
-  NEW_DEFAULT_ROLE,
-  NEW_DEFAULT_DATABASE,
-  NEW_EMPTY_RP,
-} from 'src/admin/constants'
+import {NEW_DEFAULT_DATABASE, NEW_EMPTY_RP} from 'src/admin/constants'
 import uuid from 'uuid'
 import {parseDuration, compareDurations} from 'src/utils/influxDuration'
 
@@ -66,14 +62,6 @@ const adminInfluxDB = (state = initialState, action) => {
       return {...state, ...action.payload}
     }
 
-    case 'INFLUXDB_ADD_ROLE': {
-      const newRole = {...NEW_DEFAULT_ROLE, isEditing: true}
-      return {
-        ...state,
-        roles: [newRole, ...state.roles],
-      }
-    }
-
     case 'INFLUXDB_ADD_DATABASE': {
       const newDatabase = {
         ...NEW_DEFAULT_DATABASE,
@@ -118,11 +106,13 @@ const adminInfluxDB = (state = initialState, action) => {
 
     case 'INFLUXDB_SYNC_ROLE': {
       const {staleRole, syncedRole} = action.payload
-      const newState = {
-        roles: state.roles.map(r =>
-          r.links.self === staleRole.links.self ? {...syncedRole} : r
-        ),
-      }
+      const newState = staleRole.links
+        ? {
+            roles: state.roles.map(r =>
+              r.links.self === staleRole.links.self ? {...syncedRole} : r
+            ),
+          }
+        : {roles: [{...syncedRole}, ...state.roles]}
       return {...state, ...newState}
     }
 
@@ -155,16 +145,6 @@ const adminInfluxDB = (state = initialState, action) => {
       return {...state, ...newState}
     }
 
-    case 'INFLUXDB_EDIT_ROLE': {
-      const {role, updates} = action.payload
-      const newState = {
-        roles: state.roles.map(r =>
-          r.links.self === role.links.self ? {...r, ...updates} : r
-        ),
-      }
-      return {...state, ...newState}
-    }
-
     case 'INFLUXDB_EDIT_DATABASE': {
       const {database, updates} = action.payload
       const newState = {
@@ -231,11 +211,13 @@ const adminInfluxDB = (state = initialState, action) => {
 
     case 'INFLUXDB_DELETE_ROLE': {
       const {role} = action.payload
-      const newState = {
-        roles: state.roles.filter(r => r.links.self !== role.links.self),
+      if (role.links) {
+        const newState = {
+          roles: state.roles.filter(r => r.links.self !== role.links.self),
+        }
+        return {...state, ...newState}
       }
-
-      return {...state, ...newState}
+      return state
     }
 
     case 'INFLUXDB_REMOVE_DATABASE': {

ui/src/reusable_ui/components/inputs/Input.tsx

@@ -4,6 +4,7 @@ import React, {
   CSSProperties,
   ChangeEvent,
   KeyboardEvent,
+  RefObject,
 } from 'react'
 import classnames from 'classnames'
 
@@ -52,6 +53,27 @@ class Input extends Component<Props> {
     type: InputType.Text,
   }
 
+  private inputRef: RefObject<HTMLInputElement>
+  private timeoutHandle: ReturnType<typeof setTimeout> | undefined
+
+  constructor(props: Props) {
+    super(props)
+    this.inputRef = React.createRef()
+  }
+
+  public componentDidMount() {
+    if (this.props.autoFocus) {
+      this.timeoutHandle = setTimeout(() => {
+        this.inputRef.current.focus()
+        this.timeoutHandle = undefined
+      }, 50)
+    }
+  }
+  public componentWillUnmount() {
+    if (this.timeoutHandle) {
+      clearTimeout(this.timeoutHandle)
+    }
+  }
   public render() {
     const {
       status,
@@ -68,7 +90,6 @@ class Input extends Component<Props> {
       onKeyDown,
       testId,
     } = this.props
-
     return (
       <div className={this.className} style={this.containerStyle}>
         <input
@@ -87,6 +108,7 @@ class Input extends Component<Props> {
           className="input-field"
           data-test={testId}
           disabled={status === ComponentStatus.Disabled}
+          ref={this.inputRef}
         />
         {this.icon}
         {this.statusIndicator}

ui/src/shared/copy/notifications.ts

@@ -439,6 +439,11 @@ export const notifyRoleNameInvalid = (): Notification => ({
   message: 'Role name is too short.',
 })
 
+export const notifyRoleNameExists = (): Notification => ({
+  ...defaultErrorNotification,
+  message: 'Role name already exists.',
+})
+
 export const notifyDatabaseNameInvalid = (): Notification => ({
   ...defaultErrorNotification,
   message: 'Database name cannot be blank.',

ui/src/utils/useChangeEffect.ts

@@ -14,7 +14,7 @@ function useChangeEffect(
       first.current = false
       return
     }
-    effect()
+    return effect()
   }, deps)
 }
 

ui/test/admin/reducers/influxdb.test.js

@@ -1,12 +1,10 @@
 import reducer from 'src/admin/reducers/influxdb'
 
 import {
-  addRole,
   addDatabase,
   addRetentionPolicy,
   syncUser,
   syncRole,
-  editRole,
   editDatabase,
   editRetentionPolicyRequested,
   loadRoles,
@@ -23,11 +21,7 @@ import {
   setQueriesSort,
 } from 'src/admin/actions/influxdb'
 
-import {
-  NEW_DEFAULT_ROLE,
-  NEW_DEFAULT_DATABASE,
-  NEW_EMPTY_RP,
-} from 'src/admin/constants'
+import {NEW_DEFAULT_DATABASE, NEW_EMPTY_RP} from 'src/admin/constants'
 
 // Users
 const u1 = {
@@ -239,19 +233,6 @@ describe('Admin.InfluxDB.Reducers', () => {
     expect(actual.users).toEqual(expected.users)
   })
 
-  it('it can add a role', () => {
-    state = {
-      roles: [r1],
-    }
-
-    const actual = reducer(state, addRole())
-    const expected = {
-      roles: [{...NEW_DEFAULT_ROLE, isEditing: true}, r1],
-    }
-
-    expect(actual.roles).toEqual(expected.roles)
-  })
-
   it('it can sync a stale role', () => {
     const staleRole = {...r1, permissions: []}
     state = {roles: [r2, staleRole]}
@@ -263,16 +244,13 @@ describe('Admin.InfluxDB.Reducers', () => {
 
     expect(actual.roles).toEqual(expected.roles)
   })
+  it('it can sync a new role', () => {
+    const staleRole = {name: 'new-role'}
+    state = {roles: [r2]}
 
-  it('it can edit a role', () => {
-    const updates = {name: 'onecool'}
-    state = {
-      roles: [r2, r1],
-    }
-
-    const actual = reducer(state, editRole(r2, updates))
+    const actual = reducer(state, syncRole(staleRole, r1))
     const expected = {
-      roles: [{...r2, ...updates}, r1],
+      roles: [r1, r2],
     }
 
     expect(actual.roles).toEqual(expected.roles)
@@ -287,6 +265,19 @@ describe('Admin.InfluxDB.Reducers', () => {
     expect(actual.roles).toEqual(expected.roles)
   })
 
+  it('it can delete a non-existing role', () => {
+    state = {
+      roles: [r1],
+    }
+
+    const actual = reducer(state, deleteRole({}))
+    const expected = {
+      roles: [r1],
+    }
+
+    expect(actual.roles).toEqual(expected.roles)
+  })
+
   it('it can delete a role', () => {
     state = {
       roles: [r1],