diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ea5bf93c..731d9d3d9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ui/src/admin/actions/influxdb.js b/ui/src/admin/actions/influxdb.js index b75897241..4f58406aa 100644 --- a/ui/src/admin/actions/influxdb.js +++ b/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) } } diff --git a/ui/src/admin/components/RoleRow.tsx b/ui/src/admin/components/RoleRow.tsx index 7e913333a..b65697425 100644 --- a/ui/src/admin/components/RoleRow.tsx +++ b/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> showUsers: boolean - onCancel: (role: UserRole) => void - onEdit: (role: UserRole, updates: Partial) => void - onSave: (role: UserRole) => Promise } const RoleRow = ({ @@ -23,22 +19,7 @@ const RoleRow = ({ page, perDBPermissions, showUsers, - onEdit, - onSave, - onCancel, }: Props) => { - if (role.isEditing) { - return ( - - ) - } - return ( diff --git a/ui/src/admin/components/RoleRowEdit.tsx b/ui/src/admin/components/RoleRowEdit.tsx deleted file mode 100644 index 1ea37f785..000000000 --- a/ui/src/admin/components/RoleRowEdit.tsx +++ /dev/null @@ -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) => void - onSave: (role: UserRole) => Promise - 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 ( - - -
- onEdit(role, {name: e.target.value})} - onKeyPress={onKeyPress} - autoFocus={true} - spellCheck={false} - autoComplete="false" - data-test="role-name--input" - /> - -
- - - ) -} - -export default RoleRowEdit diff --git a/ui/src/admin/components/influxdb/CreateRoleDialog.tsx b/ui/src/admin/components/influxdb/CreateRoleDialog.tsx new file mode 100644 index 000000000..07e44293a --- /dev/null +++ b/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 ( + + + + +
+ + setName(e.target.value)} + status={ + validateRoleName(name) + ? ComponentStatus.Valid + : ComponentStatus.Default + } + /> + + +
+ + +
+ +
+ + + + ) +} + +export default CreateRoleDialog diff --git a/ui/src/admin/components/influxdb/CreateUserDialog.tsx b/ui/src/admin/components/influxdb/CreateUserDialog.tsx index fb7352696..1b8bb18d8 100644 --- a/ui/src/admin/components/influxdb/CreateUserDialog.tsx +++ b/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) => { setName(e.target.value)} + autoFocus={true} + status={ + validateUserName(name) + ? ComponentStatus.Valid + : ComponentStatus.Default + } testId="username--input" /> @@ -39,6 +52,11 @@ const CreateUserDialog = ({visible, setVisible, create}: Props) => { setPassword(e.target.value)} testId="password--input" /> diff --git a/ui/src/admin/constants/index.js b/ui/src/admin/constants/index.js index 206eaec0e..6a07a587b 100644 --- a/ui/src/admin/constants/index.js +++ b/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', diff --git a/ui/src/admin/containers/influxdb/RolePage.tsx b/ui/src/admin/containers/influxdb/RolePage.tsx index 4818ff09a..c58a1cb76 100644 --- a/ui/src/admin/containers/influxdb/RolePage.tsx +++ b/ui/src/admin/containers/influxdb/RolePage.tsx @@ -66,7 +66,7 @@ type Props = WithRouterProps & 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 ? (
- User {roleName} not found! + Role {roleName} not found!
) : (
@@ -403,5 +403,5 @@ const UserPage = ({ } export default withSource( - withRouter(connect(mapStateToProps, mapDispatchToProps)(UserPage)) + withRouter(connect(mapStateToProps, mapDispatchToProps)(RolePage)) ) diff --git a/ui/src/admin/containers/influxdb/RolesPage.tsx b/ui/src/admin/containers/influxdb/RolesPage.tsx index 5a5b3793a..51a259cc5 100644 --- a/ui/src/admin/containers/influxdb/RolesPage.tsx +++ b/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, + 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 -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 ( +
@@ -182,8 +198,8 @@ const RolesPage = ({
@@ -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)) ) diff --git a/ui/src/admin/containers/influxdb/UsersPage.tsx b/ui/src/admin/containers/influxdb/UsersPage.tsx index 367812b44..40cd1f79c 100644 --- a/ui/src/admin/containers/influxdb/UsersPage.tsx +++ b/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, 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 } diff --git a/ui/src/admin/reducers/influxdb.js b/ui/src/admin/reducers/influxdb.js index 13b89d7bd..00326c49b 100644 --- a/ui/src/admin/reducers/influxdb.js +++ b/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': { diff --git a/ui/src/reusable_ui/components/inputs/Input.tsx b/ui/src/reusable_ui/components/inputs/Input.tsx index d9afc7da9..e12181f79 100644 --- a/ui/src/reusable_ui/components/inputs/Input.tsx +++ b/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 { type: InputType.Text, } + private inputRef: RefObject + private timeoutHandle: ReturnType | 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 { onKeyDown, testId, } = this.props - return (
{ className="input-field" data-test={testId} disabled={status === ComponentStatus.Disabled} + ref={this.inputRef} /> {this.icon} {this.statusIndicator} diff --git a/ui/src/shared/copy/notifications.ts b/ui/src/shared/copy/notifications.ts index 71eb871ad..0cbd96f15 100644 --- a/ui/src/shared/copy/notifications.ts +++ b/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.', diff --git a/ui/src/utils/useChangeEffect.ts b/ui/src/utils/useChangeEffect.ts index 080820a09..3d9a819b3 100644 --- a/ui/src/utils/useChangeEffect.ts +++ b/ui/src/utils/useChangeEffect.ts @@ -14,7 +14,7 @@ function useChangeEffect( first.current = false return } - effect() + return effect() }, deps) } diff --git a/ui/test/admin/reducers/influxdb.test.js b/ui/test/admin/reducers/influxdb.test.js index 9b96271a7..49dee7c5e 100644 --- a/ui/test/admin/reducers/influxdb.test.js +++ b/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],