From 14fcd57baeac48131b1222a4ad7fee2a8c98d672 Mon Sep 17 00:00:00 2001 From: soorajshankar Date: Thu, 4 Jun 2020 20:51:01 +0530 Subject: [PATCH] console: fix regression in editing permissions manually (fix #4683) (#4826) --- .../Data/TablePermissions/JSONEditor.tsx | 68 ++++++++++ .../TablePermissions/PermButtonsSection.tsx | 94 ++++++++++++++ .../Data/TablePermissions/Permissions.js | 118 +++--------------- .../Services/Data/TablePermissions/utils.tsx | 5 +- console/src/hooks/usePrevious.ts | 9 ++ 5 files changed, 190 insertions(+), 104 deletions(-) create mode 100644 console/src/components/Services/Data/TablePermissions/JSONEditor.tsx create mode 100644 console/src/components/Services/Data/TablePermissions/PermButtonsSection.tsx create mode 100644 console/src/hooks/usePrevious.ts diff --git a/console/src/components/Services/Data/TablePermissions/JSONEditor.tsx b/console/src/components/Services/Data/TablePermissions/JSONEditor.tsx new file mode 100644 index 0000000000000..9462221940f10 --- /dev/null +++ b/console/src/components/Services/Data/TablePermissions/JSONEditor.tsx @@ -0,0 +1,68 @@ +import React, { useState, useEffect, useCallback } from 'react'; +import AceEditor, { IAnnotation } from 'react-ace'; +import { isJsonString } from '../../../Common/utils/jsUtils'; +import { usePrevious } from '../../../../hooks/usePrevious'; + +export interface JSONEditorProps { + initData: string; + onChange: (v: string) => void; + data: string; +} + +const JSONEditor: React.FC = ({ + initData, + onChange, + data, +}) => { + const [value, setValue] = useState(initData || data || ''); + const [annotations, setAnnotations] = useState([]); + const prevData = usePrevious(data); + + useEffect(() => { + // if the data prop is changed do nothing + if (prevData !== data) return; + // when state gets new data, trigger parent callback + if (value !== data) onChange(value); + }, [value, data, prevData]); + + // check and set error message + useEffect(() => { + if (isJsonString(value)) { + setAnnotations([]); + } else { + setAnnotations([ + { row: 0, column: 0, text: 'Invalid JSON', type: 'error' }, + ]); + } + return () => { + setAnnotations([]); + }; + }, [value]); + + useEffect(() => { + // set data to editor only if the prop has a valid json string + // setting value from query editor will always have a valid json + // any invalid json means, the value is set from this component so no need to set that again + if (isJsonString(data)) setValue(data); + }, [data]); + + const onEditorValueChange = useCallback(newVal => setValue(newVal), [ + setValue, + ]); + + return ( + + ); +}; + +export default JSONEditor; diff --git a/console/src/components/Services/Data/TablePermissions/PermButtonsSection.tsx b/console/src/components/Services/Data/TablePermissions/PermButtonsSection.tsx new file mode 100644 index 0000000000000..2b9bb8275cdca --- /dev/null +++ b/console/src/components/Services/Data/TablePermissions/PermButtonsSection.tsx @@ -0,0 +1,94 @@ +import React, { useCallback } from 'react'; +import { ThunkDispatch } from 'redux-thunk'; +import { AnyAction } from 'redux'; +import Button from '../../../Common/Button'; +import { isJsonString, getConfirmation } from '../../../Common/utils/jsUtils'; +import { FilterState } from './utils'; +import { showErrorNotification } from '../../Common/Notification'; +import { permChangePermissions, permChangeTypes } from './Actions'; +import styles from '../../../Common/Permissions/PermissionStyles.scss'; + +interface PermButtonSectionProps { + readOnlyMode: string; + query: string; + localFilterString: FilterState; + dispatch: (d: ThunkDispatch<{}, {}, AnyAction>) => void; + permissionsState: FilterState; + permsChanged: string; + currQueryPermissions: string; +} + +const PermButtonSection: React.FC = ({ + readOnlyMode, + query, + localFilterString, + dispatch, + permissionsState, + permsChanged, + currQueryPermissions, +}) => { + if (readOnlyMode) { + return null; + } + + const dispatchSavePermissions = useCallback(() => { + const isInvalid = Object.values(localFilterString).some(val => { + if (val && !isJsonString(val)) { + return true; + } + return false; + }); + + if (isInvalid) { + dispatch( + showErrorNotification( + 'Saving permissions failed', + 'Row permission is not a valid JSON' + ) + ); + return; + } + + dispatch(permChangePermissions(permChangeTypes.save)); + }, [readOnlyMode, query, localFilterString, dispatch]); + + const dispatchRemoveAccess = useCallback(() => { + const confirmMessage = + 'This will permanently delete the currently set permissions'; + const isOk = getConfirmation(confirmMessage); + if (isOk) { + dispatch(permChangePermissions(permChangeTypes.delete)); + } + }, [dispatch]); + + return ( +
+ + +
+ ); +}; + +export default PermButtonSection; diff --git a/console/src/components/Services/Data/TablePermissions/Permissions.js b/console/src/components/Services/Data/TablePermissions/Permissions.js index ed77a2c8c0616..a9a2874c4c8a3 100644 --- a/console/src/components/Services/Data/TablePermissions/Permissions.js +++ b/console/src/components/Services/Data/TablePermissions/Permissions.js @@ -1,6 +1,6 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; -import AceEditor from 'react-ace'; +import JSONEditor from './JSONEditor'; import Tooltip from 'react-bootstrap/lib/Tooltip'; import InputGroup from 'react-bootstrap/lib/InputGroup'; import OverlayTrigger from 'react-bootstrap/es/OverlayTrigger'; @@ -9,7 +9,6 @@ import 'brace/theme/github'; import { RESET } from '../TableModify/ModifyActions'; import { - permChangeTypes, permOpenEdit, permSetFilter, permSetFilterSameAs, @@ -18,7 +17,6 @@ import { permAllowAll, permCloseEdit, permSetRoleName, - permChangePermissions, // permToggleAllowUpsert, permToggleAllowAggregation, permToggleModifyLimit, @@ -82,16 +80,15 @@ import { getTableSupportedQueries, QUERY_TYPES, } from '../../../Common/utils/pgUtils'; -import { showErrorNotification } from '../../Common/Notification'; import KnowMoreLink from '../../../Common/KnowMoreLink/KnowMoreLink'; import { getFilterQueries, replaceLegacyOperators, - getAllowedFilterKeys, updateFilterTypeLabel, getDefaultFilterType, getUpdateTooltip, } from './utils'; +import PermButtonSection from './PermButtonsSection'; class Permissions extends Component { constructor() { @@ -659,8 +656,8 @@ class Permissions extends Component { const dispatchFuncSetFilter = filter => { this.setState(prev => ({ - filterString: { - ...prev, + localFilterString: { + ...prev.localFilterString, [filterType]: filter, }, })); @@ -687,15 +684,9 @@ class Permissions extends Component { ); const selectedValue = ( - ); @@ -1741,93 +1732,6 @@ class Permissions extends Component { return clonePermissionsHtml; }; - const getButtonsSection = () => { - if (readOnlyMode) { - return null; - } - - const dispatchSavePermissions = () => { - const filterKeys = getAllowedFilterKeys(query); - const isInvalid = filterKeys.some(key => { - if ( - localFilterString[key] && - !isJsonString(localFilterString[key]) - ) { - return true; - } - return false; - }); - - if (isInvalid) { - dispatch( - showErrorNotification( - 'Saving permissions failed', - 'Row permission is not a valid JSON' - ) - ); - return; - } - - dispatch(permChangePermissions(permChangeTypes.save)); - }; - - const dispatchRemoveAccess = () => { - const confirmMessage = - 'This will permanently delete the currently set permissions'; - const isOk = getConfirmation(confirmMessage); - if (isOk) { - dispatch(permChangePermissions(permChangeTypes.delete)); - } - }; - - const getPermActionButton = ( - value, - color, - onClickFn, - disabled, - title - ) => ( - - ); - const applySameSelected = permissionsState.applySamePermissions.length; - - const disableSave = applySameSelected || !permsChanged; - const disableRemoveAccess = !currQueryPermissions; - - const saveButton = getPermActionButton( - 'Save Permissions', - 'yellow', - dispatchSavePermissions, - disableSave, - !permsChanged ? 'No changes made' : '' - ); - - const removeAccessButton = getPermActionButton( - 'Delete Permissions', - 'red', - dispatchRemoveAccess, - disableRemoveAccess, - disableRemoveAccess ? 'No permissions set' : '' - ); - - return ( -
- {saveButton} - {removeAccessButton} -
- ); - }; - const getBackendOnlySection = () => { if (!isQueryTypeBackendOnlyCompatible(permissionsState.query)) { return null; @@ -1901,7 +1805,15 @@ class Permissions extends Component { {getPresetsSection('insert')} {getPresetsSection('update')} {getBackendOnlySection()} - {getButtonsSection()} + {getClonePermsSection()} diff --git a/console/src/components/Services/Data/TablePermissions/utils.tsx b/console/src/components/Services/Data/TablePermissions/utils.tsx index 9e571153bae91..d32f4b9fb9ed9 100644 --- a/console/src/components/Services/Data/TablePermissions/utils.tsx +++ b/console/src/components/Services/Data/TablePermissions/utils.tsx @@ -7,7 +7,10 @@ import { escapeRegExp } from '../utils'; import { UNSAFE_keys } from '../../../Common/utils/tsUtils'; type FilterType = 'check' | 'filter'; -type BaseQueryType = 'select' | 'update' | 'insert' | 'delete'; +export type BaseQueryType = 'select' | 'update' | 'insert' | 'delete'; +export interface FilterState { + [key: string]: BaseQueryType; +} type DisplayQueryType = | Exclude diff --git a/console/src/hooks/usePrevious.ts b/console/src/hooks/usePrevious.ts new file mode 100644 index 0000000000000..ea1894b03f388 --- /dev/null +++ b/console/src/hooks/usePrevious.ts @@ -0,0 +1,9 @@ +import { useRef, useEffect } from 'react'; + +export const usePrevious = (value: T) => { + const ref = useRef(); + useEffect(() => { + ref.current = value; + }); + return ref.current; +};