From ff5483d66535d5af2f4ded0d2c00f1bdbf9f62a2 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Wed, 14 Sep 2022 08:28:42 +0200 Subject: [PATCH 1/3] update interviewer ref --- network-canvas | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/network-canvas b/network-canvas index 75ce4a3eb..d010e684f 160000 --- a/network-canvas +++ b/network-canvas @@ -1 +1 @@ -Subproject commit 75ce4a3eb26d61cadf010100d01eb5bb01fb414b +Subproject commit d010e684f62e40a6cfcea07d34b75cfc9d4c631e From 5ba841527e779164996a37908f9e721776319190 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Wed, 14 Sep 2022 15:52:50 +0200 Subject: [PATCH 2/3] Fix inability to remove field validation Complex issue that revealed a series of unrelated bugs that have also been fixed. Root cause was that setting the validation field to `null` was causing it to be removed. The `updateVariable` action was being called with `merge: true`. Basic fix was to fetch the existing variable definition (which required refactoring several selectors), and handle the merging manually. This allows us to completely remove any property that is set to `null`. Further bugs/fixes: - Additional investigation revealed that ego variables were not being validated. Issued a fix in protocol-validation for this. - This in turn revealed that entity-based filtering of validation options was not being applied correctly (for example, `unique` is not valid for `ego` variables). This was resolved by correctly passing stage entity into the ValidationSection component. - This in turn revealed that the TypeEditor contained a great deal of obsolete code, including a VariableEditor component that was not being used. --- network-canvas | 2 +- src/components/Codebook/EntityType.js | 40 ++-- .../Fields/VariablePicker/VariablePill.js | 6 +- .../Screens/VariableEditorScreen.js | 12 - src/components/Screens/screenIndex.js | 2 - src/components/TypeEditor/Options.js | 166 -------------- src/components/TypeEditor/TypeEditor.js | 29 +-- src/components/TypeEditor/Validations.js | 216 ------------------ src/components/TypeEditor/VariableFields.js | 96 -------- src/components/TypeEditor/VariablePreview.js | 14 -- src/components/TypeEditor/Variables.js | 64 ------ .../TypeEditor/__tests__/convert.test.js | 42 ---- src/components/TypeEditor/convert.js | 39 ---- src/components/Validations/Validations.js | 20 +- src/components/Validations/withStoreState.js | 1 - .../VariableEditor/VariableEditor.js | 51 ----- src/components/VariableEditor/index.js | 1 - src/components/sections/Form/FieldFields.js | 147 ++++++------ .../sections/Form/withFormHandlers.js | 25 +- src/components/sections/ValidationSection.js | 8 +- src/ducks/modules/protocol/codebook.js | 10 +- src/protocol-validation | 2 +- src/selectors/codebook/index.js | 54 +++-- 23 files changed, 189 insertions(+), 858 deletions(-) delete mode 100644 src/components/Screens/VariableEditorScreen.js delete mode 100644 src/components/TypeEditor/Options.js delete mode 100644 src/components/TypeEditor/Validations.js delete mode 100644 src/components/TypeEditor/VariableFields.js delete mode 100644 src/components/TypeEditor/VariablePreview.js delete mode 100644 src/components/TypeEditor/Variables.js delete mode 100644 src/components/VariableEditor/VariableEditor.js delete mode 100644 src/components/VariableEditor/index.js diff --git a/network-canvas b/network-canvas index d010e684f..69345ca5d 160000 --- a/network-canvas +++ b/network-canvas @@ -1 +1 @@ -Subproject commit d010e684f62e40a6cfcea07d34b75cfc9d4c631e +Subproject commit 69345ca5d88a278920d359ed96aff90a9772bd27 diff --git a/src/components/Codebook/EntityType.js b/src/components/Codebook/EntityType.js index 3eeaf126a..5cd8f1732 100644 --- a/src/components/Codebook/EntityType.js +++ b/src/components/Codebook/EntityType.js @@ -40,14 +40,14 @@ const EntityType = ({
- { !inUse && not in use } - { inUse && ( - <> - used in: - {' '} - {stages} - - ) } + {!inUse && not in use} + {inUse && ( + <> + used in: + {' '} + {stages} + + )}
- { variables.length > 0 + {variables.length > 0 && ( -
-

Variables:

- -
+
+

Variables:

+ +
)} ); @@ -97,8 +97,8 @@ EntityType.propTypes = { EntityType.defaultProps = { variables: [], inUse: true, // Don't allow delete unless we explicitly say so - handleDelete: () => {}, - handleEdit: () => {}, + handleDelete: () => { }, + handleEdit: () => { }, }; const mapStateToProps = (state, { entity, type }) => { @@ -116,7 +116,7 @@ const withEntityHandlers = compose( }), withHandlers({ handleEdit: ({ openScreen, entity, type }) => () => { - openScreen('type', { entity, type, metaOnly: true }); + openScreen('type', { entity, type }); }, handleDelete: ({ deleteType, openDialog, entity, type, name, inUse, diff --git a/src/components/Form/Fields/VariablePicker/VariablePill.js b/src/components/Form/Fields/VariablePicker/VariablePill.js index d01d051c3..a66d80c4f 100644 --- a/src/components/Form/Fields/VariablePicker/VariablePill.js +++ b/src/components/Form/Fields/VariablePicker/VariablePill.js @@ -14,7 +14,7 @@ import { useDispatch, useSelector } from 'react-redux'; import { actionCreators as codebookActions } from '@modules/protocol/codebook'; import { required as requiredValidation, uniqueByList, allowedVariableName } from '@app/utils/validations'; import TextInput from '@codaco/ui/lib/components/Fields/Text'; -import { makeGetVariableFromUUID, getVariablesForSubject } from '../../../../selectors/codebook'; +import { makeGetVariableWithEntity, getVariablesForSubject } from '../../../../selectors/codebook'; import { getColorForType, getIconForType } from '../../../../config/variables'; const EDIT_COMPLETE_BUTTON_ID = 'editCompleteButton'; @@ -72,7 +72,7 @@ const EditableVariablePill = ({ uuid }) => { const { name, type, entity, entityType, - } = useSelector(makeGetVariableFromUUID(uuid)); + } = useSelector(makeGetVariableWithEntity(uuid)); const [newName, setNewName] = useState(name); @@ -131,7 +131,7 @@ const EditableVariablePill = ({ uuid }) => { <> - { editing ? ( + {editing ? ( ({ - editor: VariableEditor, - form: formName, - })), -)(EditorScreen); - -export default VariableEditorScreen; diff --git a/src/components/Screens/screenIndex.js b/src/components/Screens/screenIndex.js index c957ce2dd..736f7a093 100644 --- a/src/components/Screens/screenIndex.js +++ b/src/components/Screens/screenIndex.js @@ -3,7 +3,6 @@ import { get } from 'lodash'; import StageEditorScreen from './StageEditorScreen'; import Codebook from './CodebookScreen'; import TypeEditorScreen from './TypeEditorScreen'; -import VariableEditorScreen from './VariableEditorScreen'; import NewStageScreen from './NewStageScreen'; import AssetsScreen from './AssetsScreen'; @@ -13,7 +12,6 @@ const SCREEN_INDEX = { stage: StageEditorScreen, codebook: Codebook, type: TypeEditorScreen, - variable: VariableEditorScreen, newStage: NewStageScreen, assets: AssetsScreen, }; diff --git a/src/components/TypeEditor/Options.js b/src/components/TypeEditor/Options.js deleted file mode 100644 index 055a25da7..000000000 --- a/src/components/TypeEditor/Options.js +++ /dev/null @@ -1,166 +0,0 @@ -import React from 'react'; -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; -import PropTypes from 'prop-types'; -import { compose, defaultProps, withProps } from 'recompose'; -import { toNumber } from 'lodash'; -import { SortableElement, SortableHandle, SortableContainer } from 'react-sortable-hoc'; -import { Field, FieldArray } from 'redux-form'; -import { Icon, Button } from '@codaco/ui'; -import * as Fields from '@codaco/ui/lib/components/Fields'; -import { actionCreators as dialogsActions } from '../../ducks/modules/dialogs'; - -const isNumberLike = (value) => - parseInt(value, 10) == value; // eslint-disable-line - -const ItemHandle = compose( - SortableHandle, -)( - () => ( -
- -
- ), -); - -const ItemDelete = (props) => ( -
- -
-); - -const AddItem = (props) => ( - -); - -const mapDispatchToItemProps = (dispatch) => ({ - openDialog: bindActionCreators(dialogsActions.openDialog, dispatch), -}); - -const Item = compose( - connect(null, mapDispatchToItemProps), - withProps( - ({ fields, openDialog, index }) => ({ - handleDelete: () => { - openDialog({ - type: 'Warning', - title: 'Remove item', - message: 'Are you sure you want to remove this item?', - onConfirm: () => { fields.remove(index); }, - confirmLabel: 'Remove item', - }); - }, - }), - ), - SortableElement, -)( - ({ field, handleDelete }) => ( -
-
- -
-
-
-
Label
- -
-
-
Value
- (isNumberLike(value) ? toNumber(value) : value)} placeholder="value" /> -
-
-
- -
-
- ), -); - -Item.propTypes = { - // eslint-disable-next-line react/forbid-prop-types - fields: PropTypes.object.isRequired, - field: PropTypes.string.isRequired, - index: PropTypes.number.isRequired, -}; - -const Items = compose( - defaultProps({ - lockAxis: 'y', - useDragHandle: true, - }), - withProps( - ({ fields }) => ({ - onSortEnd: ({ oldIndex, newIndex }) => fields.move(oldIndex, newIndex), - }), - ), - SortableContainer, -)( - ({ fields, ...rest }) => ( -
-
-
- { - fields.map((field, index) => ( - - )) - } -
-
- - fields.push({})} /> -
- ), -); - -Items.propTypes = { - fields: PropTypes.shape({ - map: PropTypes.func.isRequired, - }).isRequired, -}; - -const Options = ({ - name, - label, - ...rest -}) => ( -
- { label - &&

{label}

} - -
-); - -Options.propTypes = { - name: PropTypes.string.isRequired, - label: PropTypes.string, -}; - -Options.defaultProps = { - label: '', -}; - -export default Options; diff --git a/src/components/TypeEditor/TypeEditor.js b/src/components/TypeEditor/TypeEditor.js index c59734c46..5a557a068 100644 --- a/src/components/TypeEditor/TypeEditor.js +++ b/src/components/TypeEditor/TypeEditor.js @@ -11,7 +11,6 @@ import { getCodebook } from '@selectors/protocol'; import ColorPicker from '../Form/Fields/ColorPicker'; import IconOption from './IconOption'; import getPalette from './getPalette'; -import Variables from './Variables'; const ICON_OPTIONS = [ 'add-a-person', @@ -23,8 +22,6 @@ const TypeEditor = ({ entity, type, existingTypes, - isNew, - metaOnly, }) => { const dispatch = useDispatch(); const getFormValue = formValueSelector(form); @@ -43,7 +40,7 @@ const TypeEditor = ({ <>
-

{ type ? `Edit ${entity}` : `Create ${entity}` }

+

{type ? `Edit ${entity}` : `Create ${entity}`}
@@ -57,8 +54,8 @@ const TypeEditor = ({ {' '} type. This name will be used to identify this type in the codebook, and in your data exports. - { entity === 'node' && ' Some examples might be "Person", "Place", or "Organization".' } - { entity === 'edge' && ' Some examples might be "Friends" or "Works With".' } + {entity === 'node' && ' Some examples might be "Person", "Place", or "Organization".'} + {entity === 'edge' && ' Some examples might be "Friends" or "Works With".'}

- { entity === 'node' + {entity === 'node' && (
)} - {(!isNew && !metaOnly) - && ( -
- -
- )} ); @@ -136,14 +119,10 @@ TypeEditor.propTypes = { form: PropTypes.string.isRequired, // eslint-disable-next-line react/forbid-prop-types existingTypes: PropTypes.array.isRequired, - isNew: PropTypes.bool, - metaOnly: PropTypes.bool, }; TypeEditor.defaultProps = { type: null, - isNew: false, - metaOnly: false, }; const mapStateToProps = (state, { type, isNew }) => { diff --git a/src/components/TypeEditor/Validations.js b/src/components/TypeEditor/Validations.js deleted file mode 100644 index f198e83d6..000000000 --- a/src/components/TypeEditor/Validations.js +++ /dev/null @@ -1,216 +0,0 @@ -import React from 'react'; -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; -import PropTypes from 'prop-types'; -import { compose, withHandlers } from 'recompose'; -import { get, map } from 'lodash'; -import { Field, FieldArray } from 'redux-form'; -import { Icon, Button } from '@codaco/ui'; -import * as Fields from '@codaco/ui/lib/components/Fields'; -import NativeSelect from '../Form/Fields/NativeSelect'; -import ValidatedField from '../Form/ValidatedField'; -import { actionCreators as dialogsActions } from '../../ducks/modules/dialogs'; - -const VALIDATION_TYPES = { - text: [ - 'required', - 'minLength', - 'maxLength', - ], - number: [ - 'required', - 'minValue', - 'maxValue', - ], - datetime: [ - 'required', - ], - boolean: [ - 'required', - ], - ordinal: [ - 'required', - ], - categorical: [ - 'required', - 'minSelected', - 'maxSelected', - ], -}; - -const getValidationTypesForVariable = (variableType) => get(VALIDATION_TYPES, variableType, []); - -const renderValidationOptions = ({ field, validationType }) => { - switch (validationType) { - case 'minLength': - case 'maxLength': - case 'minValue': - case 'maxValue': - case 'minSelected': - case 'maxSelected': - return ( - parseInt(value, 10)} - /> - ); - default: - return null; - } -}; - -renderValidationOptions.propTypes = { - field: PropTypes.string.isRequired, - validationType: PropTypes.string.isRequired, -}; - -const ItemDelete = (props) => ( -
- -
-); - -const AddItem = (props) => ( - -); - -const mapStateToItemProps = (state, { fields, index }) => ({ - rowValues: fields.get(index), - allValues: fields.getAll(), -}); - -const mapDispatchToItemProps = (dispatch) => ({ - openDialog: bindActionCreators(dialogsActions.openDialog, dispatch), -}); - -const Item = compose( - connect(mapStateToItemProps, mapDispatchToItemProps), - withHandlers(({ fields, openDialog, index }) => ({ - handleDelete: () => { - openDialog({ - type: 'Warning', - title: 'Remove validation', - message: 'Are you sure you want to remove this rule?', - onConfirm: () => { fields.remove(index); }, - confirmLabel: 'Remove validation', - }); - }, - })), -)( - ({ - field, variableType, rowValues, allValues, handleDelete, - }) => ( -
-
-
- ( - { - value: validation, - label: validation, - disabled: map(allValues, 'type').includes(validation), - } - ), - )} - /> -
-
- { - renderValidationOptions({ - field, - validationType: get(rowValues, 'type'), - }) - } -
-
-
- -
-
- ), -); - -const Items = ({ fields, variableType, ...rest }) => ( -
-
-
- { - fields.map((field, index) => ( - - )) - } -
-
- - { (fields.length < getValidationTypesForVariable(variableType).length) - && fields.push({})} />} -
-); - -Items.propTypes = { - // eslint-disable-next-line react/forbid-prop-types - fields: PropTypes.array.isRequired, - variableType: PropTypes.string.isRequired, -}; - -const Validations = ({ - name, - label, - variableType, - ...rest -}) => { - if (getValidationTypesForVariable(variableType).length === 0) { return null; } - - return ( -
- { label - &&

{label}

} - -
- ); -}; - -Validations.propTypes = { - name: PropTypes.string.isRequired, - variableType: PropTypes.string.isRequired, - label: PropTypes.string, -}; - -Validations.defaultProps = { - label: '', -}; - -export default Validations; diff --git a/src/components/TypeEditor/VariableFields.js b/src/components/TypeEditor/VariableFields.js deleted file mode 100644 index a286846f7..000000000 --- a/src/components/TypeEditor/VariableFields.js +++ /dev/null @@ -1,96 +0,0 @@ -import React, { Component } from 'react'; -import PropTypes from 'prop-types'; -import { formValueSelector, autofill } from 'redux-form'; -import { connect } from 'react-redux'; -import * as Fields from '@codaco/ui/lib/components/Fields'; -import { VARIABLE_TYPES, VARIABLE_TYPES_WITH_OPTIONS } from '@app/config/variables'; -import { getFieldId } from '@app/utils/issues'; -import safeName from '@app/utils/safeName'; -import { Row } from '@components/OrderedList'; -import ValidatedField from '@components/Form/ValidatedField'; -import NativeSelect from '../Form/Fields/NativeSelect'; -import Options from './Options'; -import ValidationSection from '../sections/ValidationSection'; - -const variableTypes = Object.values(VARIABLE_TYPES); - -class VariableFields extends Component { - handleNormalizeName = (value) => safeName(value); - - render() { - const { - form, - variableType, - resetOptions, - } = this.props; - - return ( - <> - -
- - - -
- - - - { VARIABLE_TYPES_WITH_OPTIONS.includes(variableType) - && ( - - )} - - - - ); - } -} - -VariableFields.propTypes = { - form: PropTypes.oneOfType([PropTypes.object, PropTypes.string]), - variableType: PropTypes.string, - resetOptions: PropTypes.func, -}; - -VariableFields.defaultProps = { - form: null, - variableType: null, - resetOptions: null, -}; - -const mapStateToProps = (state, { form }) => { - const variableType = formValueSelector(form)(state, 'type'); - - return { - variableType, - }; -}; - -const mapDispatchToProps = (dispatch, { form }) => ({ - autofill: (field, value) => dispatch(autofill(form, field, value)), -}); - -export default connect(mapStateToProps, mapDispatchToProps)(VariableFields); diff --git a/src/components/TypeEditor/VariablePreview.js b/src/components/TypeEditor/VariablePreview.js deleted file mode 100644 index c218d28c5..000000000 --- a/src/components/TypeEditor/VariablePreview.js +++ /dev/null @@ -1,14 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -const VariablePreview = ({ name }) => ( - <> - {name} - -); - -VariablePreview.propTypes = { - name: PropTypes.string.isRequired, -}; - -export default VariablePreview; diff --git a/src/components/TypeEditor/Variables.js b/src/components/TypeEditor/Variables.js deleted file mode 100644 index 8fb8e5f60..000000000 --- a/src/components/TypeEditor/Variables.js +++ /dev/null @@ -1,64 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import Fuse from 'fuse.js'; -import { orderBy } from 'lodash'; -import VariablePreview from './VariablePreview'; -import VariableFields from './VariableFields'; -import EditableList from '../EditableList'; - -const fuseOptions = { - shouldSort: true, - threshold: 0.6, - location: 0, - distance: 10, - maxPatternLength: 32, - minMatchCharLength: 1, - keys: [ - 'name', - 'label', - 'description', - ], -}; - -const search = (list, query) => { - if (!query) { return list; } - const fuse = new Fuse(list, fuseOptions); - const result = fuse.search(query); - return result; -}; - -const sort = (list, sortOrder) => { - if (!sortOrder) { return list; } - return orderBy(list, sortOrder.property, sortOrder.direction); -}; - -const filter = (list, { query, sortOrder }) => sort(search(list, query), sortOrder); - -const Variables = ({ - form, - name, - ...rest -}) => ( - -); - -Variables.propTypes = { - form: PropTypes.string.isRequired, - name: PropTypes.string.isRequired, - // eslint-disable-next-line react/forbid-prop-types - sortableProperties: PropTypes.array.isRequired, - // eslint-disable-next-line react/forbid-prop-types - initialSortOrder: PropTypes.object.isRequired, -}; - -export default Variables; diff --git a/src/components/TypeEditor/__tests__/convert.test.js b/src/components/TypeEditor/__tests__/convert.test.js index 591011000..3320dbd6d 100644 --- a/src/components/TypeEditor/__tests__/convert.test.js +++ b/src/components/TypeEditor/__tests__/convert.test.js @@ -6,54 +6,12 @@ const mockConfiguration = { label: 'Person', color: 'coral', iconVariant: 'add-a-person', - variables: { - 1234: { - name: 'closenessLayout', - label: 'Closeness layout', - description: 'Earthling readable description', - type: 'layout', - validation: {}, - }, - 5678: { - name: 'name', - label: 'Name', - description: 'Human readable description', - type: 'text', - validation: { - required: true, - minLength: 1, - maxLength: 24, - }, - }, - }, }; const mockFormConfiguration = { label: 'Person', color: 'coral', iconVariant: 'add-a-person', - variables: [ - { - id: '1234', - name: 'closenessLayout', - label: 'Closeness layout', - description: 'Earthling readable description', - type: 'layout', - validation: [], - }, - { - id: '5678', - name: 'name', - label: 'Name', - description: 'Human readable description', - type: 'text', - validation: [ - { type: 'required' }, - { type: 'minLength', value: 1 }, - { type: 'maxLength', value: 24 }, - ], - }, - ], }; describe('convert', () => { diff --git a/src/components/TypeEditor/convert.js b/src/components/TypeEditor/convert.js index 5c61cb761..ff4aa75c3 100644 --- a/src/components/TypeEditor/convert.js +++ b/src/components/TypeEditor/convert.js @@ -1,50 +1,11 @@ -import { reduce, get, omit } from 'lodash'; - -const formatValidations = (validation) => reduce( - validation, - (memo, value, key) => [...memo, { type: key, value }], - [], -); - // convert protocol format into redux-form compatible format const format = (configuration) => ({ ...configuration, - variables: reduce( - get(configuration, 'variables', {}), - (memo, variable, key) => [ - ...memo, - { - ...variable, - id: key, - validation: formatValidations(get(variable, 'validation', {})), - }, - ], - [], - ), }); -const parseValidations = (validation) => reduce( - validation, - (memo, { type, value }) => ({ ...memo, [type]: value || true }), - {}, -); - // convert redux-form format into protocol format const parse = (configuration) => ({ ...configuration, - variables: reduce( - configuration.variables, - (memo, variable) => ({ - ...memo, - [variable.id]: { - ...omit(variable, ['id', 'options']), - // Ideally we'd remove this at source, but reduxForm doesn't seem to let us? - ...(get(variable, 'options') ? { options: get(variable, 'options') } : {}), - validation: parseValidations(get(variable, 'validation', [])), - }, - }), - {}, - ), }); export { diff --git a/src/components/Validations/Validations.js b/src/components/Validations/Validations.js index 990301533..70616553b 100644 --- a/src/components/Validations/Validations.js +++ b/src/components/Validations/Validations.js @@ -58,7 +58,7 @@ const ValidationsField = ({ return (
- { input.value.map(([key, value]) => ( + {input.value.map(([key, value]) => ( - )) } + ))} {children}
@@ -123,18 +123,18 @@ const Validations = ({ onDelete={handleDelete} validate={validate} > - { addNew + {addNew && ( - setAddNew(false)} - options={availableOptions} - existingVariables={existingVariables} - /> + setAddNew(false)} + options={availableOptions} + existingVariables={existingVariables} + /> )} - { !isFull + {!isFull && setAddNew(true)} />}
); diff --git a/src/components/Validations/withStoreState.js b/src/components/Validations/withStoreState.js index 329f310c2..7e25b022a 100644 --- a/src/components/Validations/withStoreState.js +++ b/src/components/Validations/withStoreState.js @@ -8,7 +8,6 @@ const mapStateToProps = (state, { form, name, variableType, entity, }) => { const validationOptions = getValidationOptionsForVariableType(variableType, entity); - return { validationOptions, value: formValueSelector(form)(state, name), diff --git a/src/components/VariableEditor/VariableEditor.js b/src/components/VariableEditor/VariableEditor.js deleted file mode 100644 index d063cef44..000000000 --- a/src/components/VariableEditor/VariableEditor.js +++ /dev/null @@ -1,51 +0,0 @@ -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; -import { compose, withProps } from 'recompose'; -import { get } from 'lodash'; -import Editor from '../Editor'; -import VariableFields from '../TypeEditor/VariableFields'; -import { getCodebook } from '../../selectors/protocol'; -import { actionCreators as actions } from '../../ducks/modules/protocol/codebook'; - -export const formName = 'codebook'; - -const mapStateToProps = (state, { entity, type, ...props }) => { - if (!props.id) { return {}; } - - const codebook = getCodebook(state); - const variable = get(codebook, [entity, type, 'variables'], []) - .find(({ id }) => id === props.id); - - return { - initialValues: variable, - }; -}; - -const mapDispatchToProps = (dispatch) => ({ - updateVariable: bindActionCreators(actions.updateVariable, dispatch), - createVariable: bindActionCreators(actions.createVariable, dispatch), -}); - -/** - * An extended , with preconfigured props (form, onSubmit, component). - */ -const VariableEditor = compose( - connect(mapStateToProps, mapDispatchToProps), - withProps(({ - entity, type, onComplete, createVariable, updateVariable, - }) => ({ - form: formName, - component: VariableFields, - onSubmit: (options) => { - if (!options.id) { - return createVariable(entity, type, options) - .then(onComplete); - } - const { id } = options; - return updateVariable(entity, type, id, options) - .then(() => onComplete({ entity, type, variable: id })); - }, - })), -)(Editor); - -export default VariableEditor; diff --git a/src/components/VariableEditor/index.js b/src/components/VariableEditor/index.js deleted file mode 100644 index 57cc6ef89..000000000 --- a/src/components/VariableEditor/index.js +++ /dev/null @@ -1 +0,0 @@ -export { default, formName } from './VariableEditor'; diff --git a/src/components/sections/Form/FieldFields.js b/src/components/sections/Form/FieldFields.js index 1b1b99887..c122f6331 100644 --- a/src/components/sections/Form/FieldFields.js +++ b/src/components/sections/Form/FieldFields.js @@ -37,14 +37,14 @@ const PromptFields = ({ <>
- { variable && !isNewVariable + {variable && !isNewVariable && ( - -

- When selecting an existing variable, changes you make to the input control or - validation options will also change other uses of this variable. -

- + +

+ When selecting an existing variable, changes you make to the input control or + validation options will also change other uses of this variable. +

+ )} - { isNewVariable && variableType - && ( - -

- The selected input control will cause this variable to be defined as - type - {' '} - {variableType} - . Once set, this cannot be changed - (although you may change the input control within this type). -

- - )} - { !isNewVariable && variableType + {isNewVariable && variableType && ( - -
+

- A pre-existing variable is currently selected. You cannot change a variable - type after it has been created, so only + The selected input control will cause this variable to be defined as + type {' '} {variableType} - {' '} - compatible - input controls can be selected above. If you would like to use a different - input control type, you will need to create a new variable. + . Once set, this cannot be changed + (although you may change the input control within this type).

-
- + + )} + {!isNewVariable && variableType + && ( + +
+

+ A pre-existing variable is currently selected. You cannot change a variable + type after it has been created, so only + {' '} + {variableType} + {' '} + compatible + input controls can be selected above. If you would like to use a different + input control type, you will need to create a new variable. +

+
+ )} - { variableType - && ( - -

Preview

- - - )} + {variableType + && ( + +

Preview

+ + + )}
- { isOrdinalOrCategoricalType(variableType) + {isOrdinalOrCategoricalType(variableType) && ( -
- The input type you selected indicates that this is a categorical or ordinal variable. - Next, please create a minimum of two possible values for the participant to choose - between. -

- )} - > - - - -
+
+ The input type you selected indicates that this is a categorical or ordinal variable. + Next, please create a minimum of two possible values for the participant to choose + between. +

+ )} + > + + + +
)} - { isBooleanWithOptions(component) + {isBooleanWithOptions(component) && (
@@ -172,22 +172,23 @@ const PromptFields = ({
)} - { isVariableTypeWithParameters(variableType) + {isVariableTypeWithParameters(variableType) && ( -
- - - -
+
+ + + +
)} diff --git a/src/components/sections/Form/withFormHandlers.js b/src/components/sections/Form/withFormHandlers.js index ace2ba101..895d7fbb9 100644 --- a/src/components/sections/Form/withFormHandlers.js +++ b/src/components/sections/Form/withFormHandlers.js @@ -4,6 +4,7 @@ import { change, SubmissionError } from 'redux-form'; import { getTypeForComponent } from '@app/config/variables'; import { actionCreators as codebookActions } from '@modules/protocol/codebook'; import { getCodebookProperties } from './helpers'; +import { makeGetVariable } from '../../../selectors/codebook'; const formHandlers = withHandlers({ handleChangeFields: ({ @@ -13,10 +14,12 @@ const formHandlers = withHandlers({ entity, changeForm, form, + getVariable, }) => async (values) => { const { variable, component, _createNewVariable, ...rest } = values; + const variableType = getTypeForComponent(component); // prune properties that are not part of the codebook: const codebookProperties = getCodebookProperties(rest); @@ -31,9 +34,23 @@ const formHandlers = withHandlers({ changeForm(form, '_modified', new Date().getTime()); if (!_createNewVariable) { + const current = getVariable(variable); + + if (!current) { + throw new SubmissionError({ + _error: 'Variable not found', + }); + } + + const baseProps = { + component: current.component, + type: current.type, + name: current.name, + }; + // Merge is set to false below so that properties that were removed, such // as 'options: []' and 'parameters: {}' get deleted. - await updateVariable(entity, type, variable, configuration, true); + await updateVariable(entity, type, variable, { ...baseProps, ...configuration }, false); return { variable, @@ -63,7 +80,11 @@ const mapDispatchToProps = { createVariable: codebookActions.createVariable, }; -const formState = connect(null, mapDispatchToProps); +const mapStateToProps = (state) => ({ + getVariable: (uuid) => makeGetVariable(uuid)(state), +}); + +const formState = connect(mapStateToProps, mapDispatchToProps); export { formState, diff --git a/src/components/sections/ValidationSection.js b/src/components/sections/ValidationSection.js index 0891a39d6..631087a50 100644 --- a/src/components/sections/ValidationSection.js +++ b/src/components/sections/ValidationSection.js @@ -10,12 +10,16 @@ import { getFieldId } from '../../utils/issues'; const ValidationSection = ({ disabled, form, + entity, variableType, existingVariables, }) => { const dispatch = useDispatch(); const getFormValue = formValueSelector(form); - const hasValidation = useSelector((state) => getFormValue(state, 'validation')); + const hasValidation = useSelector((state) => { + const validation = getFormValue(state, 'validation'); + return validation && Object.keys(pickBy(validation)).length > 0; + }); const handleToggleValidation = (nextState) => { if (nextState === false) { @@ -45,6 +49,7 @@ const ValidationSection = ({ form={form} name="validation" variableType={variableType} + entity={entity} existingVariables={existingVariablesForType} /> @@ -61,6 +66,7 @@ ValidationSection.propTypes = { name: PropTypes.string.isRequired, }), ).isRequired, + entity: PropTypes.string.isRequired, }; ValidationSection.defaultProps = { diff --git a/src/ducks/modules/protocol/codebook.js b/src/ducks/modules/protocol/codebook.js index 9e52dee54..edd90e4e3 100644 --- a/src/ducks/modules/protocol/codebook.js +++ b/src/ducks/modules/protocol/codebook.js @@ -3,7 +3,7 @@ import { omit, get, has, isEmpty, find, } from 'lodash'; import prune from '@app/utils/prune'; -import { getAllVariableUUIDs, getVariablesForSubject } from '../../../selectors/codebook'; +import { getAllVariableUUIDsByEntity, getVariablesForSubject } from '../../../selectors/codebook'; import { makeGetUsageForType } from '../../../selectors/usage'; import { makeGetIsUsed } from '../../../selectors/codebook/isUsed'; import { getNextCategoryColor } from './utils/helpers'; @@ -64,15 +64,15 @@ const createVariable = (entity, type, variable, configuration) => prune({ entity, variable, }, - configuration, + configuration: prune(configuration), }); -const updateVariable = (variable, configuration, merge = false) => prune({ +const updateVariable = (variable, configuration, merge = false) => ({ type: UPDATE_VARIABLE, meta: { variable, }, - configuration, + configuration: prune(configuration), merge, }); @@ -301,7 +301,7 @@ export default function reducer(state = initialState, action = {}) { action.merge, ); case UPDATE_VARIABLE: { - const variables = getAllVariableUUIDs(state); + const variables = getAllVariableUUIDsByEntity(state); const { entity, entityType } = find(variables, ['uuid', action.meta.variable]); return getStateWithUpdatedVariable( diff --git a/src/protocol-validation b/src/protocol-validation index d70014442..6f5967ffa 160000 --- a/src/protocol-validation +++ b/src/protocol-validation @@ -1 +1 @@ -Subproject commit d700144422fef1a0a1052a88ae207cca8d5ff364 +Subproject commit 6f5967ffa8ce8b5d3fa411f4c9553565403dfd24 diff --git a/src/selectors/codebook/index.js b/src/selectors/codebook/index.js index 589aba779..dfc5adfb0 100644 --- a/src/selectors/codebook/index.js +++ b/src/selectors/codebook/index.js @@ -1,5 +1,3 @@ -/* eslint-disable import/prefer-default-export */ - import { get, find } from 'lodash'; import { getCodebook } from '../protocol'; import { asOptions } from '../utils'; @@ -25,8 +23,29 @@ const getType = (state, subject) => { */ const getVariablesForSubject = (state, subject) => get(getType(state, subject), 'variables', {}); -// Get all variables for all subjects in the codebook -const getAllVariableUUIDs = ({ node: nodeTypes = {}, edge: edgeTypes = {}, ego = {} }) => { +const getAllVariablesByUUID = ({ node: nodeTypes = {}, edge: edgeTypes = {}, ego = {} }) => { + const flattenedVariables = {}; + + const addVariables = (variables) => { + Object.keys(variables).forEach((variable) => { + flattenedVariables[variable] = variables[variable]; + }); + }; + + Object.values(nodeTypes).forEach((nodeType) => { + addVariables(nodeType.variables); + }); + + Object.values(edgeTypes).forEach((edgeType) => { + addVariables(edgeType.variables); + }); + + addVariables(ego.variables); + return flattenedVariables; +}; + +// Get all variables for all subjects in the codebook, adding the entity and type +const getAllVariableUUIDsByEntity = ({ node: nodeTypes = {}, edge: edgeTypes = {}, ego = {} }) => { const variables = new Set(); // Nodes @@ -72,20 +91,27 @@ const getAllVariableUUIDs = ({ node: nodeTypes = {}, edge: edgeTypes = {}, ego = return [...variables]; // Spread converts Set to Array }; -export const makeGetVariableFromUUID = (uuid) => (state) => { +const makeGetVariableWithEntity = (uuid) => (state) => { const codebook = getCodebook(state); - const variables = getAllVariableUUIDs(codebook); + const variables = getAllVariableUUIDsByEntity(codebook); const found = find(variables, { uuid }); return found; }; +const makeGetVariable = (uuid) => (state) => { + const codebook = getCodebook(state); + const variables = getAllVariablesByUUID(codebook); + const found = get(variables, uuid, {}); + return found; +}; + /** - * Given `subject` return a list of options (`{ label, value, ...}`) - * for matching entity - * - * @param {object} state redux state - * @param {object} subject subject object in format `{ entity, type }` - */ + * Given `subject` return a list of options (`{ label, value, ...}`) + * for matching entity + * + * @param {object} state redux state + * @param {object} subject subject object in format `{ entity, type }` + */ const getVariableOptionsForSubject = (state, subject, isUsedOptions = {}) => { const variables = getVariablesForSubject(state, subject); const options = asOptions(variables); @@ -115,5 +141,7 @@ export { getVariablesForSubject, getVariableOptionsForSubject, getOptionsForVariable, - getAllVariableUUIDs, + getAllVariableUUIDsByEntity, + makeGetVariableWithEntity, + makeGetVariable, }; From 42ab9909f54c6b182200570ec5ac4438eb95f3b0 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Thu, 15 Sep 2022 13:45:37 +0200 Subject: [PATCH 3/3] make getAllVariableUUIDsByEntity more robust --- .../sections/Form/withFieldsHandlers.js | 3 +- src/selectors/codebook/index.js | 30 +++++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/components/sections/Form/withFieldsHandlers.js b/src/components/sections/Form/withFieldsHandlers.js index 956f63671..16ea20484 100644 --- a/src/components/sections/Form/withFieldsHandlers.js +++ b/src/components/sections/Form/withFieldsHandlers.js @@ -21,7 +21,6 @@ const mapStateToProps = (state, { form, entity, type }) => { const component = formSelector(state, 'component'); const createNewVariable = formSelector(state, '_createNewVariable'); const isNewVariable = !!createNewVariable; - const existingVariables = getVariablesForSubject(state, { entity, type }); const variableOptions = getVariableOptionsForSubject(state, { entity, type }) // If not a variable with corresponding component, we can't use it here. @@ -74,7 +73,7 @@ const fieldsHandlers = withHandlers({ if (variableType !== typeForComponent) { changeField(form, 'validation', null); changeField(form, 'options', null); - // Special case for boolean, where BooleanChoice has options but Toggle doesn't + // Special case for boolean, where BooleanChoice has options but Toggle doesn't } else if (variableType === 'boolean') { changeField(form, 'options', null); } diff --git a/src/selectors/codebook/index.js b/src/selectors/codebook/index.js index dfc5adfb0..737f3c6f8 100644 --- a/src/selectors/codebook/index.js +++ b/src/selectors/codebook/index.js @@ -1,4 +1,4 @@ -import { get, find } from 'lodash'; +import { get, find, isObject } from 'lodash'; import { getCodebook } from '../protocol'; import { asOptions } from '../utils'; import { makeOptionsWithIsUsed } from './isUsed'; @@ -23,24 +23,36 @@ const getType = (state, subject) => { */ const getVariablesForSubject = (state, subject) => get(getType(state, subject), 'variables', {}); -const getAllVariablesByUUID = ({ node: nodeTypes = {}, edge: edgeTypes = {}, ego = {} }) => { +const getAllVariablesByUUID = (codebook) => { + if (!codebook) { throw new Error('Codebook not found'); } + + const { node: nodeTypes = {}, edge: edgeTypes = {}, ego = {} } = codebook; const flattenedVariables = {}; const addVariables = (variables) => { + if (!variables) { return; } + if (!isObject(variables)) { throw new Error('Variables must be an object'); } + Object.keys(variables).forEach((variable) => { flattenedVariables[variable] = variables[variable]; }); }; - Object.values(nodeTypes).forEach((nodeType) => { - addVariables(nodeType.variables); - }); + if (nodeTypes && nodeTypes.variables) { + Object.values(nodeTypes).forEach((nodeType) => { + addVariables(nodeType.variables); + }); + } - Object.values(edgeTypes).forEach((edgeType) => { - addVariables(edgeType.variables); - }); + if (edgeTypes && edgeTypes.variables) { + Object.values(edgeTypes).forEach((edgeType) => { + addVariables(edgeType.variables); + }); + } - addVariables(ego.variables); + if (ego.variables) { + addVariables(ego.variables); + } return flattenedVariables; };