From a0f60cc0b086dc28b48852ba4d18db4643553a68 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Fri, 9 Jun 2023 01:00:26 +0200 Subject: [PATCH 01/11] Fixes not able to close stage editor when filter disabled Redux form still validates fields that are not shown, even when their value is null. We set fields to invisible with a value of null when they are contained within a toggleable section, in order to "disable" them. Redux form has no concept of disabling a field. In this case, the hidden field representing the node panel filter was still being validated, and the validation function was returning a string indicating that the field was empty (because we set it to null in order to reset it!). This error was hidden from the user, because the field was hidden and deregistered. The fix is to change the validation function to return undefined if the value is null. Could also have been implemented as passing undefined as the validation prop to the field based on if the toggleable section is open or closed. --- src/components/Query/ruleValidator.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/Query/ruleValidator.js b/src/components/Query/ruleValidator.js index 057a89b07..b981625ca 100644 --- a/src/components/Query/ruleValidator.js +++ b/src/components/Query/ruleValidator.js @@ -1,14 +1,21 @@ import { get } from 'lodash'; const validateRules = (value) => { - const rules = get(value, 'rules', []); + // BUGFIX: If the section containing the filter is not expanded, we set + // the filter value to null. In this case, we don't want to + // validate the filter, because it will be invisible and will simply + // prevent the form from being submitted without an error. + if (!value) { + return undefined; + } + const rules = get(value, 'rules'); const join = get(value, 'join'); - if (rules.length > 1 && !join) { + if (rules && rules.length > 1 && !join) { return 'Please select a join type'; } - if (rules.length === 0) { + if (rules && rules.length === 0) { return 'Please create at least one rule'; } From 426ba66a6ce112535150d8815b3a171c6bc2bc9d Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Fri, 9 Jun 2023 12:12:56 +0200 Subject: [PATCH 02/11] update github actions to update apt cache --- .github/workflows/dist.yml | 3 +++ .github/workflows/main.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml index 6f1883a86..c0be8dadb 100644 --- a/.github/workflows/dist.yml +++ b/.github/workflows/dist.yml @@ -18,6 +18,9 @@ jobs: - uses: actions/setup-python@v4 with: python-version: '2.7.18' + # update apt cache + - name: Update apt cache + run: sudo apt-get update -y # Set node version - uses: actions/setup-node@v2 with: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bd1b01169..a043e5212 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,6 +18,9 @@ jobs: - uses: actions/setup-python@v4 with: python-version: '2.7.18' + # update apt cache + - name: Update apt cache + run: sudo apt-get update -y # Set node version - uses: actions/setup-node@v2 with: From bd474cb0bf56732f9b43e14c3dab0b411ad8531b Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Fri, 9 Jun 2023 12:32:26 +0200 Subject: [PATCH 03/11] also update build workflow action --- .github/workflows/main.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a043e5212..4e394b3f0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -58,6 +58,9 @@ jobs: - uses: actions/setup-python@v4 with: python-version: '2.7.18' + # update apt cache + - name: Update apt cache + run: sudo apt-get update -y # Set node version - uses: actions/setup-node@v2 with: From 58490c20c4df3d3e479b1194f0ed8e7074aded61 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Fri, 9 Jun 2023 15:47:17 +0200 Subject: [PATCH 04/11] add failing test for isUsed --- .../codebook/__tests__/variables.test.js | 210 ++++++++++++------ src/selectors/codebook/isUsed.js | 17 +- src/selectors/indexes.js | 12 +- src/selectors/reduxForm.js | 32 ++- 4 files changed, 187 insertions(+), 84 deletions(-) diff --git a/src/selectors/codebook/__tests__/variables.test.js b/src/selectors/codebook/__tests__/variables.test.js index ffd43f5ad..374ebd2d4 100644 --- a/src/selectors/codebook/__tests__/variables.test.js +++ b/src/selectors/codebook/__tests__/variables.test.js @@ -7,8 +7,8 @@ const variable2 = '1234-1234-1234-2'; const variable3 = '1234-1234-1234-3'; const variable4 = '1234-1234-1234-4'; -const mockCodebook = { - nodes: { +const mockCodebookWithoutUse = { + node: { person: { variables: { [variable1]: {}, @@ -20,21 +20,33 @@ const mockCodebook = { }, }; -describe('makeGetIsUsed', () => { - it('returns false when variables are not in use', () => { - const notUsedState = { - protocol: { - present: { - codebook: mockCodebook, - stages: [ - ], - }, +const mockProtocolWithoutUse = { + present: { + codebook: mockCodebookWithoutUse, + stages: [ + { + id: '1', }, - }; + ], + }, +}; + +const mockReduxFormsWithoutUse = { + 'edit-stage': { + values: {}, + }, +}; + +const mockStateWithoutUse = { + protocol: mockProtocolWithoutUse, + form: mockReduxFormsWithoutUse, +}; - const notUsedResult = makeGetIsUsed()(notUsedState); +describe('makeGetIsUsed', () => { + it('returns false when a variable is not present', () => { + const result = makeGetIsUsed()(mockStateWithoutUse); - expect(notUsedResult).toEqual({ + expect(result).toEqual({ [variable1]: false, [variable2]: false, [variable3]: false, @@ -42,20 +54,27 @@ describe('makeGetIsUsed', () => { }); }); - it('checks codebook for variable usage', () => { - const state = { + it('returns true when a variable is present in the protocol', () => { + const stateWithProtocolUse = { + ...mockStateWithoutUse, protocol: { + ...mockProtocolWithoutUse, present: { - codebook: mockCodebook, + ...mockProtocolWithoutUse.present, stages: [ - { [variable1]: 'foo' }, - { variableAsKey: variable2 }, { - nestedVariable: { - moreNested: { - nestedFurther: { - [variable3]: false, - }, + id: '1', + [variable1]: 'foo', + thing: { + variableAsKey: variable2, + nested: { + even: [ + { + moreNested: { + with: variable3, + }, + }, + ], }, }, }, @@ -64,7 +83,7 @@ describe('makeGetIsUsed', () => { }, }; - const result = makeGetIsUsed()(state); + const result = makeGetIsUsed()(stateWithProtocolUse); expect(result).toEqual({ [variable1]: true, @@ -74,68 +93,119 @@ describe('makeGetIsUsed', () => { }); }); - it('checks form for variable usage', () => { - const state = { - protocol: { - present: { - codebook: mockCodebook, - stages: [], - }, - }, + describe('redux forms', () => { + const stateWithFormUse = { + ...mockStateWithoutUse, form: { + ...mockReduxFormsWithoutUse, formName: { values: { - foo: variable1, + [variable1]: 'foo', + }, + }, + 'edit-stage': { + values: { + [variable2]: 'foo', + thing: { + foo: variable3, + }, + }, + }, + }, + }; + + describe('returns true when a variable is present in redux forms', () => { + it('returns from default form names only without parameters', () => { + const result = makeGetIsUsed()(stateWithFormUse); + + expect(result).toEqual({ + [variable1]: false, + [variable2]: true, + [variable3]: true, + [variable4]: false, + }); + }); + + it('allows the redux form name to be specified to return specific form', () => { + // Also check we can set form name + const result = makeGetIsUsed({ formNames: ['formName'] })(stateWithFormUse); + + expect(result).toEqual({ + [variable1]: true, + [variable2]: false, + [variable3]: false, + [variable4]: false, + }); + }); + }); + }); + + it('checks codebook for variable validation use', () => { + const stateWithCodebookUse = { + ...mockStateWithoutUse, + codebook: { + ...mockCodebookWithoutUse, + node: { + ...mockCodebookWithoutUse.node, + person: { + ...mockCodebookWithoutUse.node.person, + variables: { + ...mockCodebookWithoutUse.node.person.variables, + [variable1]: { + validation: { + sameAs: variable2, + }, + }, + }, }, }, }, }; - // Also check we can set form name - const result = makeGetIsUsed({ formNames: ['formName'] })(state); + const result = makeGetIsUsed()(stateWithCodebookUse); expect(result).toEqual({ - [variable1]: true, - [variable2]: false, + [variable1]: false, + [variable2]: true, [variable3]: false, [variable4]: false, }); }); -}); -describe('makeOptionsWithIsUsed', () => { - it('appends used state to options', () => { - const state = { - protocol: { - present: { - codebook: mockCodebook, - stages: [], + describe('makeOptionsWithIsUsed', () => { + it('appends used state to options', () => { + const state = { + protocol: { + present: { + codebook: mockCodebookWithoutUse, + stages: [], + }, }, - }, - form: { - formName: { - values: { - foo: variable1, + form: { + formName: { + values: { + foo: variable1, + }, }, }, - }, - }; - - const mockOptions = [ - { value: variable1, label: '1' }, - { value: variable2, label: '2' }, - { value: variable3, label: '3' }, - { value: variable4, label: '4' }, - ]; - - // Also check we can set form name - const result = makeOptionsWithIsUsed({ formNames: ['formName'] })(state, mockOptions); - - expect(result).toEqual([ - { value: variable1, label: '1', isUsed: true }, - { value: variable2, label: '2', isUsed: false }, - { value: variable3, label: '3', isUsed: false }, - { value: variable4, label: '4', isUsed: false }, - ]); + }; + + const mockOptions = [ + { value: variable1, label: '1' }, + { value: variable2, label: '2' }, + { value: variable3, label: '3' }, + { value: variable4, label: '4' }, + ]; + + // Also check we can set form name + const result = makeOptionsWithIsUsed({ formNames: ['formName'] })(state, mockOptions); + + expect(result).toEqual([ + { value: variable1, label: '1', isUsed: true }, + { value: variable2, label: '2', isUsed: false }, + { value: variable3, label: '3', isUsed: false }, + { value: variable4, label: '4', isUsed: false }, + ]); + }); }); }); diff --git a/src/selectors/codebook/isUsed.js b/src/selectors/codebook/isUsed.js index a4907418f..1e1d2723a 100644 --- a/src/selectors/codebook/isUsed.js +++ b/src/selectors/codebook/isUsed.js @@ -5,14 +5,23 @@ import { getIdsFromCodebook } from './helpers'; /** * Gets a key value object describing variables are - * in use (including in redux forms) - * @returns {object} in format: { [variableId]: boolean } + * in use (including in redux forms). + * + * Naive implementation: just checks if the variable id is in the flattened + * protocol, or any redux forms. + * + * @param {object} options - options object + * @param {Array} options.formNames - names of forms to check for variable usage + * @param {Array} options.excludePaths - paths to exclude from the check (e.g. 'stages') + * + * @returns {function} - selector function that returns a key value object + * describing variables are in use */ -export const makeGetIsUsed = (isUsedOptions = {}) => (state) => { +export const makeGetIsUsed = (options = {}) => (state) => { const { formNames = ['edit-stage', 'editable-list-form'], excludePaths = [], - } = isUsedOptions; + } = options; const protocol = getProtocol(state); const forms = getForms(formNames)(state); diff --git a/src/selectors/indexes.js b/src/selectors/indexes.js index 5ede588f1..c2972dd71 100644 --- a/src/selectors/indexes.js +++ b/src/selectors/indexes.js @@ -14,9 +14,13 @@ const mapAssetItems = ({ type, content }, path) => { }; /** - * Locations of referenceable entities in a standard protocol. + * Master list of paths where variables are used. + * + * ⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️ + * It is VITAL that this be updated when any new variable use occurs in the + * protocol schema! + * ⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️⚠️ * - * This is generally used to find which entities are used where. */ export const paths = { edges: [ @@ -53,6 +57,10 @@ export const paths = { 'stages[].presets[].groupVariable', 'stages[].presets[].edges.display[]', 'stages[].presets[].highlight[]', + // `sameAs` and `differentFrom` are variable references in these locations + 'codebook.ego.variables[].validation', + 'codebook.node.[].variables[].validation', + 'codebook.edge.[].variables[].validation', ], assets: [ 'stages[].panels[].dataSource', diff --git a/src/selectors/reduxForm.js b/src/selectors/reduxForm.js index 79c0a9c25..75084c1b4 100644 --- a/src/selectors/reduxForm.js +++ b/src/selectors/reduxForm.js @@ -1,9 +1,25 @@ -import { getFormValues } from 'redux-form'; +import { getFormValues, getFormNames } from 'redux-form'; -export const getForms = (formNames = []) => (state) => formNames.reduce( - (memo, formName) => ({ - ...memo, - [formName]: getFormValues(formName)(state), - }), - {}, -); +/** + * Returns the redux form values for the given form names. + * If no form names are given, returns all form values. + * @param {Array} formNames - names of forms to get values for + * @returns {function} - selector function that returns an object of form values + * keyed by form name + */ +export const getForms = (formNames) => (state) => { + const reduce = (names) => names.reduce( + (memo, formName) => ({ + ...memo, + [formName]: getFormValues(formName)(state), + }), + {}, + ); + + if (!formNames) { + const allFormNames = getFormNames(state); + return reduce(allFormNames); + } + + return reduce(formNames); +}; From 84bd9ece474b1eef3e6c04ec4b471126c12c66cb Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Mon, 12 Jun 2023 14:07:59 +0200 Subject: [PATCH 05/11] correctly add sameAs and differentFrom paths to paths used by getVariableIndex --- src/components/Codebook/Codebook.js | 87 ++++++++++--------- src/components/Codebook/EgoType.js | 18 ++-- src/components/Codebook/helpers.js | 2 + .../codebook/__tests__/variables.test.js | 65 +++++++++++--- src/selectors/codebook/helpers.js | 5 ++ src/selectors/codebook/isUsed.js | 47 +++++++++- src/selectors/indexes.js | 12 ++- 7 files changed, 165 insertions(+), 71 deletions(-) diff --git a/src/components/Codebook/Codebook.js b/src/components/Codebook/Codebook.js index de852602a..d70246d9d 100644 --- a/src/components/Codebook/Codebook.js +++ b/src/components/Codebook/Codebook.js @@ -21,60 +21,60 @@ const Codebook = ({ nodes, }) => (
- { !hasEgoVariables && !hasNodes && !hasEdges + {!hasEgoVariables && !hasNodes && !hasEdges && ( -

- There are currently no types or variables defined in this protocol. - When you have created some interview stages, the types and variables will be shown here. -

+

+ There are currently no types or variables defined in this protocol. + When you have created some interview stages, the types and variables will be shown here. +

)} - { hasEgoVariables + {hasEgoVariables && ( - - - + + + )} - { hasNodes + {/* {hasNodes && ( - - {nodes.map((node) => ( - - ))} - + + {nodes.map((node) => ( + + ))} + )} - { hasEdges + {hasEdges && ( - - {edges.map((edge) => ( - - ))} - + + {edges.map((edge) => ( + + ))} + )} - { hasNetworkAssets + {hasNetworkAssets && ( - - {networkAssets.map( - (networkAsset) => ( - - ), - )} - - )} + + {networkAssets.map( + (networkAsset) => ( + + ), + )} + + )} */}
); @@ -92,9 +92,10 @@ Codebook.propTypes = { nodes: PropTypes.array.isRequired, }; +// TODO: replace this with helpers getEntityProperties. This code was +// duplicated and needs to be reconciled. const getEntityWithUsage = (state, index, mergeProps) => { const search = utils.buildSearch([index]); - return (_, id) => { const inUse = search.has(id); diff --git a/src/components/Codebook/EgoType.js b/src/components/Codebook/EgoType.js index f9a0ab91e..b237ea55b 100644 --- a/src/components/Codebook/EgoType.js +++ b/src/components/Codebook/EgoType.js @@ -9,15 +9,15 @@ const EgoType = ({ variables, }) => (
- { variables.length > 0 + {variables.length > 0 && ( -
-

Variables:

- -
+
+

Variables:

+ +
)}
); @@ -33,7 +33,7 @@ EgoType.defaultProps = { const mapStateToProps = (state) => { const entityProperties = getEntityProperties(state, { entity: 'ego' }); - + console.log('entityProperties', entityProperties); return entityProperties; }; diff --git a/src/components/Codebook/helpers.js b/src/components/Codebook/helpers.js index 05941ecc8..d4133667a 100644 --- a/src/components/Codebook/helpers.js +++ b/src/components/Codebook/helpers.js @@ -72,6 +72,8 @@ export const getEntityProperties = (state, { entity, type }) => { const variableIndex = getVariableIndex(state); const isUsedIndex = getIsUsed(state); + console.log('isUsedIndex', isUsedIndex, variableIndex); + const variablesWithUsage = map( variables, (variable, id) => { diff --git a/src/selectors/codebook/__tests__/variables.test.js b/src/selectors/codebook/__tests__/variables.test.js index 374ebd2d4..87d20fa3c 100644 --- a/src/selectors/codebook/__tests__/variables.test.js +++ b/src/selectors/codebook/__tests__/variables.test.js @@ -6,8 +6,18 @@ const variable1 = '1234-1234-1234-1'; const variable2 = '1234-1234-1234-2'; const variable3 = '1234-1234-1234-3'; const variable4 = '1234-1234-1234-4'; +const variable5 = '1234-1234-1234-5'; +const variable6 = '1234-1234-1234-6'; +const variable7 = '1234-1234-1234-7'; +const variable8 = '1234-1234-1234-8'; const mockCodebookWithoutUse = { + ego: { + variables: { + [variable5]: {}, + [variable6]: {}, + }, + }, node: { person: { variables: { @@ -18,6 +28,14 @@ const mockCodebookWithoutUse = { }, }, }, + edge: { + friendship: { + variables: { + [variable7]: {}, + [variable8]: {}, + }, + }, + }, }; const mockProtocolWithoutUse = { @@ -51,6 +69,10 @@ describe('makeGetIsUsed', () => { [variable2]: false, [variable3]: false, [variable4]: false, + [variable5]: false, + [variable6]: false, + [variable7]: false, + [variable8]: false, }); }); @@ -90,6 +112,10 @@ describe('makeGetIsUsed', () => { [variable2]: true, [variable3]: true, [variable4]: false, + [variable5]: false, + [variable6]: false, + [variable7]: false, + [variable8]: false, }); }); @@ -123,6 +149,10 @@ describe('makeGetIsUsed', () => { [variable2]: true, [variable3]: true, [variable4]: false, + [variable5]: false, + [variable6]: false, + [variable7]: false, + [variable8]: false, }); }); @@ -135,6 +165,10 @@ describe('makeGetIsUsed', () => { [variable2]: false, [variable3]: false, [variable4]: false, + [variable5]: false, + [variable6]: false, + [variable7]: false, + [variable8]: false, }); }); }); @@ -143,17 +177,22 @@ describe('makeGetIsUsed', () => { it('checks codebook for variable validation use', () => { const stateWithCodebookUse = { ...mockStateWithoutUse, - codebook: { - ...mockCodebookWithoutUse, - node: { - ...mockCodebookWithoutUse.node, - person: { - ...mockCodebookWithoutUse.node.person, - variables: { - ...mockCodebookWithoutUse.node.person.variables, - [variable1]: { - validation: { - sameAs: variable2, + protocol: { + ...mockProtocolWithoutUse, + present: { + codebook: { + ...mockCodebookWithoutUse, + node: { + ...mockCodebookWithoutUse.node, + person: { + ...mockCodebookWithoutUse.node.person, + variables: { + ...mockCodebookWithoutUse.node.person.variables, + [variable1]: { + validation: { + sameAs: variable2, + }, + }, }, }, }, @@ -169,6 +208,10 @@ describe('makeGetIsUsed', () => { [variable2]: true, [variable3]: false, [variable4]: false, + [variable5]: false, + [variable6]: false, + [variable7]: false, + [variable8]: false, }); }); diff --git a/src/selectors/codebook/helpers.js b/src/selectors/codebook/helpers.js index b51c1180b..070acff52 100644 --- a/src/selectors/codebook/helpers.js +++ b/src/selectors/codebook/helpers.js @@ -4,6 +4,11 @@ import { flatMap } from 'lodash'; const getIdsFromEntity = (entity) => (entity.variables ? Object.keys(entity.variables) : []); +/** + * + * @param {*} codebook + * @returns + */ export const getIdsFromCodebook = (codebook) => flatMap( codebook, (entityOrEntities, type) => ( diff --git a/src/selectors/codebook/isUsed.js b/src/selectors/codebook/isUsed.js index 1e1d2723a..9433e0790 100644 --- a/src/selectors/codebook/isUsed.js +++ b/src/selectors/codebook/isUsed.js @@ -1,6 +1,6 @@ -import { get, omit, cloneDeep } from 'lodash'; +import { get, omit, cloneDeep, each } from 'lodash'; import { getForms } from '../reduxForm'; -import { getProtocol } from '../protocol'; +import { getCodebook, getProtocol } from '../protocol'; import { getIdsFromCodebook } from './helpers'; /** @@ -10,6 +10,11 @@ import { getIdsFromCodebook } from './helpers'; * Naive implementation: just checks if the variable id is in the flattened * protocol, or any redux forms. * + * JRM BUGFIX: This previously did not check for `sameAs` or `differentFrom` + * variable references that are contained within codebook variable definitions. + * This caused a bug where these variables were able to be removed, creating + * references to variables that no longer existed. + * * @param {object} options - options object * @param {Array} options.formNames - names of forms to check for variable usage * @param {Array} options.excludePaths - paths to exclude from the check (e.g. 'stages') @@ -26,10 +31,44 @@ export const makeGetIsUsed = (options = {}) => (state) => { const protocol = getProtocol(state); const forms = getForms(formNames)(state); const variableIds = getIdsFromCodebook(protocol.codebook); + const codebook = getCodebook(state); + + // Get all codebook[entityType][entityId].variables.validation references + const variableValidations = () => { + const validations = []; + const getEntityVariableValidations = (entityDefinition) => { + if (!entityDefinition.variables) { + return []; + } + + return Object.values(entityDefinition.variables).reduce((memo, variable) => { + if (variable.validation) { + memo.push(variable.validation); + } + return memo; + }, []); + }; + + Object.keys(codebook).forEach((entityType) => { + if (entityType === 'ego') { + validations.push(...getEntityVariableValidations(codebook[entityType])); + } + + Object.keys(codebook[entityType]).forEach((entityId) => { + validations.push(...getEntityVariableValidations(codebook[entityType][entityId])); + }); + }); + + return validations; + }; + + const searchLocations = { stages: protocol.stages, forms, validations: variableValidations() }; const data = excludePaths.length > 0 - ? omit(cloneDeep({ stages: protocol.stages, forms }), excludePaths) - : { stages: protocol.stages, forms }; + ? omit(cloneDeep( + searchLocations, + ), excludePaths) + : searchLocations; const flattenedData = JSON.stringify(data); diff --git a/src/selectors/indexes.js b/src/selectors/indexes.js index c2972dd71..eba0411d9 100644 --- a/src/selectors/indexes.js +++ b/src/selectors/indexes.js @@ -58,9 +58,10 @@ export const paths = { 'stages[].presets[].edges.display[]', 'stages[].presets[].highlight[]', // `sameAs` and `differentFrom` are variable references in these locations - 'codebook.ego.variables[].validation', - 'codebook.node.[].variables[].validation', - 'codebook.edge.[].variables[].validation', + 'codebook.ego.variables[].validation.sameAs', + 'codebook.ego.variables[].validation.differentFrom', + 'codebook.node.[].variables[].validation.sameAs', + 'codebook.edge.[].variables[].validation.differentFrom', ], assets: [ 'stages[].panels[].dataSource', @@ -94,7 +95,10 @@ const getNodeIndex = createSelector( */ const getVariableIndex = createSelector( getProtocol, - (protocol) => collectPaths(paths.variables, protocol), + (protocol) => { + console.log('getVariableIndex', collectPaths(paths.variables, protocol)); + return collectPaths(paths.variables, protocol); + }, ); /** From 38a3f0e902c7c12b1213515a0649bf118e929b1e Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Mon, 12 Jun 2023 20:06:36 +0200 Subject: [PATCH 06/11] update logic of validation onUpdate (withUpdateHandlers) to correctly reset validation value when the type is changed --- src/components/Codebook/Codebook.js | 5 ++- src/components/Codebook/helpers.js | 20 ++++++----- .../Validations/withUpdateHandlers.js | 33 +++++++++++++++++-- src/components/sections/ValidationSection.js | 2 +- src/selectors/indexes.js | 5 +-- 5 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/components/Codebook/Codebook.js b/src/components/Codebook/Codebook.js index d70246d9d..ccb711304 100644 --- a/src/components/Codebook/Codebook.js +++ b/src/components/Codebook/Codebook.js @@ -34,8 +34,7 @@ const Codebook = ({ )} - - {/* {hasNodes + {hasNodes && ( {nodes.map((node) => ( @@ -74,7 +73,7 @@ const Codebook = ({ ), )} - )} */} + )} ); diff --git a/src/components/Codebook/helpers.js b/src/components/Codebook/helpers.js index d4133667a..1407a0c85 100644 --- a/src/components/Codebook/helpers.js +++ b/src/components/Codebook/helpers.js @@ -72,23 +72,27 @@ export const getEntityProperties = (state, { entity, type }) => { const variableIndex = getVariableIndex(state); const isUsedIndex = getIsUsed(state); - console.log('isUsedIndex', isUsedIndex, variableIndex); - const variablesWithUsage = map( variables, (variable, id) => { const inUse = get(isUsedIndex, id, false); - const usage = inUse - ? getUsageAsStageMeta(state, getUsage(variableIndex, id)).sort(sortByLabel) - : []; + const baseProperties = { + ...variable, + id, + inUse, + }; + + if (!inUse) { + return (baseProperties); + } + const thing = getUsage(variableIndex, id); + const usage = getUsageAsStageMeta(state, thing).sort(sortByLabel); const usageString = usage.map(({ label }) => label).join(', ').toUpperCase(); return ({ - ...variable, - id, - inUse, + ...baseProperties, usage, usageString, }); diff --git a/src/components/Validations/withUpdateHandlers.js b/src/components/Validations/withUpdateHandlers.js index d57555f27..8a15e45a7 100644 --- a/src/components/Validations/withUpdateHandlers.js +++ b/src/components/Validations/withUpdateHandlers.js @@ -2,10 +2,37 @@ import { omit } from 'lodash'; import { withHandlers } from 'recompose'; import { isValidationWithListValue, isValidationWithNumberValue } from './options'; +/** + * Function called when a validation is added or updated. Returns a value + * based on the validation type, and the previous value (if any). + * + * @param {string} type - The validation type. + * @param {string} oldType - The previous validation type. + * @param {string} value - The current value. + * @returns {string} The new value. + */ +const getAutoValue = (type, oldType, value) => { + // Required is special - always return true. + if (type === 'required') { + return true; + } + + // If the new type and the old type are both numbers, keep the value + if (isValidationWithNumberValue(type) && isValidationWithNumberValue(oldType)) { + return value; + } + + // If the new type and the old type both reference variables, keep the value. + if (isValidationWithListValue(type) && isValidationWithListValue(oldType)) { + return value; + } + + // Otherwise, set an empty value to force the user to enter a value. + return null; +}; + const getUpdatedValue = (previousValue, key, value, oldKey = null) => { - const autoValue = isValidationWithNumberValue(key) || isValidationWithListValue(key) - ? value - : true; + const autoValue = getAutoValue(key, oldKey, value); if (!oldKey) { return { ...previousValue, [key]: autoValue }; } diff --git a/src/components/sections/ValidationSection.js b/src/components/sections/ValidationSection.js index 631087a50..84a9678eb 100644 --- a/src/components/sections/ValidationSection.js +++ b/src/components/sections/ValidationSection.js @@ -18,7 +18,7 @@ const ValidationSection = ({ const getFormValue = formValueSelector(form); const hasValidation = useSelector((state) => { const validation = getFormValue(state, 'validation'); - return validation && Object.keys(pickBy(validation)).length > 0; + return validation && Object.keys(validation).length > 0; }); const handleToggleValidation = (nextState) => { diff --git a/src/selectors/indexes.js b/src/selectors/indexes.js index eba0411d9..d741004fa 100644 --- a/src/selectors/indexes.js +++ b/src/selectors/indexes.js @@ -95,10 +95,7 @@ const getNodeIndex = createSelector( */ const getVariableIndex = createSelector( getProtocol, - (protocol) => { - console.log('getVariableIndex', collectPaths(paths.variables, protocol)); - return collectPaths(paths.variables, protocol); - }, + (protocol) => collectPaths(paths.variables, protocol), ); /** From 4459c2911af241e4467b73e8df1914dee4a2e910 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Mon, 12 Jun 2023 20:07:40 +0200 Subject: [PATCH 07/11] remove dialog confirmation when deleting validation rule from field --- src/components/Validations/withStoreState.js | 3 --- src/components/Validations/withUpdateHandlers.js | 11 ++--------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/components/Validations/withStoreState.js b/src/components/Validations/withStoreState.js index 7e25b022a..773049ff0 100644 --- a/src/components/Validations/withStoreState.js +++ b/src/components/Validations/withStoreState.js @@ -1,7 +1,5 @@ -import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; import { formValueSelector, change } from 'redux-form'; -import { actionCreators as dialogsActions } from '../../ducks/modules/dialogs'; import { getValidationOptionsForVariableType } from './options'; const mapStateToProps = (state, { @@ -15,7 +13,6 @@ const mapStateToProps = (state, { }; const mapDispatchToProps = (dispatch, { form, name }) => ({ - openDialog: bindActionCreators(dialogsActions.openDialog, dispatch), update: (value) => dispatch(change(form, name, value)), }); diff --git a/src/components/Validations/withUpdateHandlers.js b/src/components/Validations/withUpdateHandlers.js index 8a15e45a7..e3ea07af3 100644 --- a/src/components/Validations/withUpdateHandlers.js +++ b/src/components/Validations/withUpdateHandlers.js @@ -43,16 +43,9 @@ const getUpdatedValue = (previousValue, key, value, oldKey = null) => { }; const withUpdateHandlers = withHandlers({ - handleDelete: ({ openDialog, update, value: previousValue }) => (key) => { + handleDelete: ({ update, value: previousValue }) => (key) => { const newValue = omit(previousValue, key); - - openDialog({ - type: 'Warning', - title: 'Remove validation', - message: 'Are you sure you want to remove this rule?', - onConfirm: () => { update(newValue); }, - confirmLabel: 'Remove validation', - }); + update(newValue); }, handleChange: ({ update, value: previousValue }) => (key, value, oldKey) => { const newValue = getUpdatedValue(previousValue, key, value, oldKey); From cacc77814c9f9a99b6ecc6755218f1f56094c1c5 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Mon, 12 Jun 2023 21:55:09 +0200 Subject: [PATCH 08/11] working codebook labelling for variables in use in validation --- src/components/Codebook/Codebook.js | 1 + src/components/Codebook/Tag.js | 7 +- src/components/Codebook/UsageColumn.js | 17 +++-- .../Codebook/__tests__/helpers.test.js | 19 ++++- src/components/Codebook/helpers.js | 72 +++++++++++++++---- src/selectors/indexes.js | 6 +- src/styles/components/_codebook.scss | 17 +++-- 7 files changed, 114 insertions(+), 25 deletions(-) diff --git a/src/components/Codebook/Codebook.js b/src/components/Codebook/Codebook.js index ccb711304..a5b4339d3 100644 --- a/src/components/Codebook/Codebook.js +++ b/src/components/Codebook/Codebook.js @@ -34,6 +34,7 @@ const Codebook = ({ )} + {hasNodes && ( diff --git a/src/components/Codebook/Tag.js b/src/components/Codebook/Tag.js index 597e4a33c..8adbdea1d 100644 --- a/src/components/Codebook/Tag.js +++ b/src/components/Codebook/Tag.js @@ -1,14 +1,19 @@ import React from 'react'; import PropTypes from 'prop-types'; -const Tag = ({ children }) => (
{children}
); +const Tag = ({ children, notUsed = false }) => { + const classes = notUsed ? 'codebook__tag codebook__tag--not-used' : 'codebook__tag'; + return (
{children}
); +}; Tag.propTypes = { children: PropTypes.node, + notUsed: PropTypes.bool, }; Tag.defaultProps = { children: null, + notUsed: false, }; export default Tag; diff --git a/src/components/Codebook/UsageColumn.js b/src/components/Codebook/UsageColumn.js index 0fd856e9f..c183ed217 100644 --- a/src/components/Codebook/UsageColumn.js +++ b/src/components/Codebook/UsageColumn.js @@ -7,12 +7,21 @@ const UsageColumn = ({ inUse, usage, }) => { - if (!inUse) { return (not in use); } + if (!inUse) { return (not in use); } const stages = usage - .map(({ id, label }) => ( - {label} - )); + .map(({ id, label }) => { + // If there is no id, don't create a link. This is the case for + // variables that are only in use as validation options. + if (!id) { + return ( + {label} + ); + } + return ( + {label} + ); + }); return (
diff --git a/src/components/Codebook/__tests__/helpers.test.js b/src/components/Codebook/__tests__/helpers.test.js index 818f33c89..2e7249669 100644 --- a/src/components/Codebook/__tests__/helpers.test.js +++ b/src/components/Codebook/__tests__/helpers.test.js @@ -1,6 +1,6 @@ /* eslint-env jest */ -import { getUsage, getUsageAsStageMeta } from '../helpers'; +import { getUsage, getUsageAsStageMeta, getCodebookVariableIndexFromValidationPath } from '../helpers'; const state = { protocol: { @@ -36,3 +36,20 @@ it('getUsageAsStageMeta()', () => { ]; expect(getUsageAsStageMeta(state, usage)).toEqual(expectedResult); }); + +it('getCodebookVariableIndexFromValidationPath()', () => { + const testStrings = [ + "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs", + "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom", + "codebook.node[nodeType].variables[variableType].validation.sameAs", + "codebook.node[nodeType].variables[variableType].validation.differentFrom", + "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs", + "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom", + ]; + + // Run the function for each string in the array + testStrings.forEach((testString) => { + expect(getCodebookVariableIndexFromValidationPath(testString)).toEqual('4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c'); + } + +}); diff --git a/src/components/Codebook/helpers.js b/src/components/Codebook/helpers.js index 1407a0c85..a62e00ac1 100644 --- a/src/components/Codebook/helpers.js +++ b/src/components/Codebook/helpers.js @@ -29,9 +29,25 @@ const getStageIndexFromPath = (path) => { return get(matches, 1, null); }; +export const getCodebookVariableIndexFromValidationPath = (path) => { + // Regexp that matches all of the following: + // "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs" + // "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom" + // "codebook.node[nodeType].variables[variableType].validation.sameAs" + // "codebook.node[nodeType.variables[variableType].validation.differentFrom" + // "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs" + // "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom" + + // eslint-disable-next-line max-len + const matches = /codebook\.(ego|node\[([^\]]+)\]|edge\[([^\]]+)\])\.variables\[([^\]]+)\]\.validation\.(sameAs|differentFrom)/.exec(path); + return get(matches, 1, null); +}; + /** - * Filters a usage index for items that match value. - * @param {Object.}} index Usage index in (in format `{[path]: value}`) + * Takes an object in the format of `{[path]: variableID}` and a variableID to + * search for. Returns an array of paths that match the variableID. + * + * @param {Object.}} index Usage index in (in format `{[path]: variableID}`) * @param {any} value Value to match in usage index * @returns {string[]} List of paths ("usage array") */ @@ -41,37 +57,70 @@ export const getUsage = (index, value) => reduce(index, (acc, indexValue, path) }, []); /** - * Get stage meta that matches "usage array" (deduped). - * See `getUsage()` for details of usage array, + * Get stage meta (wtf is stage meta, Steve? 🤦) that matches "usage array" + * (with duplicates removed). + * + * See `getUsage()` for how the usage array is generated. + * * Any stages that can't be found in the index are omitted. * @param {Object} state Application state - * @param {string[]} usage "Usage array" + * @param {string[]} usageArray "Usage array" as created by `getUsage()` * @returns {Object[]} List of stage meta `{ label, id }`. */ -export const getUsageAsStageMeta = (state, usage) => { +export const getUsageAsStageMeta = (state, usageArray) => { + console.log('usageArray', usageArray); const stageMetaByIndex = getStageMetaByIndex(state); - const stageIndexes = compact(uniq(usage.map(getStageIndexFromPath))); - return stageIndexes.map((stageIndex) => get(stageMetaByIndex, stageIndex)); + + // Filter codebook variables from usage array + const codebookVariablePaths = usageArray.filter(getCodebookVariableIndexFromValidationPath); + + const codebookVariablesWithMeta = codebookVariablePaths.map((path) => { + console.log('codebookVariables:', path); + return { + label: 'Used as validation for variable', + }; + }); + + const stageIndexes = compact(uniq(usageArray.map(getStageIndexFromPath))); + const stageVariablesWithMeta = stageIndexes.map((stageIndex) => get(stageMetaByIndex, stageIndex)); + + return [ + ...stageVariablesWithMeta, + ...codebookVariablesWithMeta, + ]; }; -// Function to be used with Array.sort. Sorts a collection of variable -// definitions by the label property. +/** + * Helper function to be used with Array.sort. Sorts a collection of variable + * definitions by the label property. + * + * @param {Object} a { label: string } + * @param {Object} b { label: string } + * @returns {number} -1 if a < b, 1 if a > b, 0 if a === b + */ export const sortByLabel = (a, b) => { if (a.label < b.label) { return -1; } if (a.label > b.label) { return 1; } return 0; }; +/** + * Returns entity meta data for use in the codebook. + * @param {*} state + * @param {*} param1 + * @returns + */ export const getEntityProperties = (state, { entity, type }) => { const { name, color, variables, } = getType(state, { entity, type }); + console.log('getEntityProperties', entity, type); const variableIndex = getVariableIndex(state); const isUsedIndex = getIsUsed(state); - + console.log('variableIndex', variableIndex); const variablesWithUsage = map( variables, (variable, id) => { @@ -90,7 +139,6 @@ export const getEntityProperties = (state, { entity, type }) => { const thing = getUsage(variableIndex, id); const usage = getUsageAsStageMeta(state, thing).sort(sortByLabel); const usageString = usage.map(({ label }) => label).join(', ').toUpperCase(); - return ({ ...baseProperties, usage, diff --git a/src/selectors/indexes.js b/src/selectors/indexes.js index d741004fa..f01ac9920 100644 --- a/src/selectors/indexes.js +++ b/src/selectors/indexes.js @@ -60,8 +60,10 @@ export const paths = { // `sameAs` and `differentFrom` are variable references in these locations 'codebook.ego.variables[].validation.sameAs', 'codebook.ego.variables[].validation.differentFrom', - 'codebook.node.[].variables[].validation.sameAs', - 'codebook.edge.[].variables[].validation.differentFrom', + 'codebook.node[].variables[].validation.sameAs', + 'codebook.node[].variables[].validation.differentFrom', + 'codebook.edge[].variables[].validation.sameAs', + 'codebook.edge[].variables[].validation.differentFrom', ], assets: [ 'stages[].panels[].dataSource', diff --git a/src/styles/components/_codebook.scss b/src/styles/components/_codebook.scss index a08f04a70..f03609583 100644 --- a/src/styles/components/_codebook.scss +++ b/src/styles/components/_codebook.scss @@ -15,13 +15,16 @@ &__tag { display: inline-block; - border-radius: unit(.5); - padding: .1em .3em; - margin-top: -.05em; + border-radius: unit(1); + padding: unit(0.5) unit(0.75); font-size: .9em; - color: var(--text-light); - background-color: var(--architect-warning); + color: var(--color-white); + background-color: var(--color-mustard--dark); text-transform: lowercase; + + &--not-used { + background-color: var(--architect-warning); + } } &__category { @@ -164,6 +167,10 @@ display: flex; flex-direction: column; align-items: flex-start; + + > * { + margin: .25rem; + } }; } From 5421b81ebf80aa6793a502bf353d38dbfac2e168 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Mon, 12 Jun 2023 22:31:51 +0200 Subject: [PATCH 09/11] working with variable name fetch added to label --- src/components/Codebook/Codebook.js | 7 +++- src/components/Codebook/helpers.js | 56 ++++++++++++++++------------ src/styles/components/_codebook.scss | 1 - 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/components/Codebook/Codebook.js b/src/components/Codebook/Codebook.js index a5b4339d3..65a534b75 100644 --- a/src/components/Codebook/Codebook.js +++ b/src/components/Codebook/Codebook.js @@ -9,7 +9,7 @@ import EntityType from './EntityType'; import ExternalEntity from './ExternalEntity'; import EgoType from './EgoType'; import CodebookCategory from './CodebookCategory'; -import { getUsage, getUsageAsStageMeta } from './helpers'; +import { getStageMetaByIndex, getUsage, getUsageAsStageMeta, getVariableMetaByIndex } from './helpers'; const Codebook = ({ edges, @@ -99,8 +99,11 @@ const getEntityWithUsage = (state, index, mergeProps) => { return (_, id) => { const inUse = search.has(id); + const variableMeta = getVariableMetaByIndex(state); + const stageMetaByIndex = getStageMetaByIndex(state); + const usage = inUse - ? getUsageAsStageMeta(state, getUsage(index, id)) + ? getUsageAsStageMeta(stageMetaByIndex, variableMeta, getUsage(index, id)) : []; return { diff --git a/src/components/Codebook/helpers.js b/src/components/Codebook/helpers.js index a62e00ac1..1b126e73b 100644 --- a/src/components/Codebook/helpers.js +++ b/src/components/Codebook/helpers.js @@ -4,7 +4,8 @@ import { import { getType } from '@selectors/codebook'; import { makeGetIsUsed } from '@selectors/codebook/isUsed'; import { getVariableIndex } from '@selectors/indexes'; -import { getProtocol } from '@selectors/protocol'; +import { getProtocol, getCodebook } from '@selectors/protocol'; +import { getAllVariablesByUUID } from '../../selectors/codebook'; const getIsUsed = makeGetIsUsed({ formNames: [] }); @@ -13,12 +14,18 @@ const getIsUsed = makeGetIsUsed({ formNames: [] }); * @param {Object} state Application state * @returns {Object[]} Stage meta sorted by index in state */ -const getStageMetaByIndex = (state) => { +export const getStageMetaByIndex = (state) => { const protocol = getProtocol(state); return protocol.stages .map(({ label, id }) => ({ label, id })); }; +export const getVariableMetaByIndex = (state) => { + const codebook = getCodebook(state); + const variables = getAllVariablesByUUID(codebook); + return variables; +}; + /** * Extract the stage name from a path string * @param {string} path {} @@ -29,18 +36,20 @@ const getStageIndexFromPath = (path) => { return get(matches, 1, null); }; +const codebookVariableReferenceRegex = /codebook\.(ego|node\[([^\]]+)\]|edge\[([^\]]+)\])\.variables\[(.*?)\].validation\.(sameAs|differentFrom)/; + export const getCodebookVariableIndexFromValidationPath = (path) => { // Regexp that matches all of the following: - // "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs" - // "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom" - // "codebook.node[nodeType].variables[variableType].validation.sameAs" - // "codebook.node[nodeType.variables[variableType].validation.differentFrom" - // "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs" - // "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom" - - // eslint-disable-next-line max-len - const matches = /codebook\.(ego|node\[([^\]]+)\]|edge\[([^\]]+)\])\.variables\[([^\]]+)\]\.validation\.(sameAs|differentFrom)/.exec(path); - return get(matches, 1, null); + // "codebook.ego.variables[variableId].validation.sameAs" + // "codebook.ego.variables[variableId].validation.differentFrom" + // "codebook.node[nodeType].variables[variableId].validation.sameAs" + // "codebook.node[nodeType.variables[variableId].validation.differentFrom" + // "codebook.edge[edgeType].variables[variableId].validation.sameAs" + // "codebook.edge[edgeType].variables[variableId].validation.differentFrom" + + const match = path.match(codebookVariableReferenceRegex); + + return get(match, 4, null); }; /** @@ -67,17 +76,13 @@ export const getUsage = (index, value) => reduce(index, (acc, indexValue, path) * @param {string[]} usageArray "Usage array" as created by `getUsage()` * @returns {Object[]} List of stage meta `{ label, id }`. */ -export const getUsageAsStageMeta = (state, usageArray) => { - console.log('usageArray', usageArray); - const stageMetaByIndex = getStageMetaByIndex(state); - +export const getUsageAsStageMeta = (stageMetaByIndex, variableMetaByIndex, usageArray) => { // Filter codebook variables from usage array const codebookVariablePaths = usageArray.filter(getCodebookVariableIndexFromValidationPath); - const codebookVariablesWithMeta = codebookVariablePaths.map((path) => { - console.log('codebookVariables:', path); + const variableId = getCodebookVariableIndexFromValidationPath(path); return { - label: 'Used as validation for variable', + label: `Used as validation for "${variableMetaByIndex[variableId].name}"`, }; }); @@ -116,11 +121,12 @@ export const getEntityProperties = (state, { entity, type }) => { color, variables, } = getType(state, { entity, type }); - console.log('getEntityProperties', entity, type); const variableIndex = getVariableIndex(state); + const variableMeta = getVariableMetaByIndex(state); + const stageMetaByIndex = getStageMetaByIndex(state); const isUsedIndex = getIsUsed(state); - console.log('variableIndex', variableIndex); + const variablesWithUsage = map( variables, (variable, id) => { @@ -136,8 +142,12 @@ export const getEntityProperties = (state, { entity, type }) => { return (baseProperties); } - const thing = getUsage(variableIndex, id); - const usage = getUsageAsStageMeta(state, thing).sort(sortByLabel); + const usage = getUsageAsStageMeta( + stageMetaByIndex, + variableMeta, + getUsage(variableIndex, id), + ).sort(sortByLabel); + const usageString = usage.map(({ label }) => label).join(', ').toUpperCase(); return ({ ...baseProperties, diff --git a/src/styles/components/_codebook.scss b/src/styles/components/_codebook.scss index f03609583..875d16539 100644 --- a/src/styles/components/_codebook.scss +++ b/src/styles/components/_codebook.scss @@ -20,7 +20,6 @@ font-size: .9em; color: var(--color-white); background-color: var(--color-mustard--dark); - text-transform: lowercase; &--not-used { background-color: var(--architect-warning); From 5748697a8987ebb7883e3bbbbbbf1ffd13c8f7b1 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Wed, 14 Jun 2023 12:49:27 +0200 Subject: [PATCH 10/11] linting and final CSS fixes --- src/components/Codebook/Codebook.js | 7 ++- src/components/Codebook/EgoType.js | 1 - .../Codebook/__tests__/helpers.test.js | 63 +++++++++++++------ src/components/Codebook/helpers.js | 23 +++---- src/selectors/codebook/isUsed.js | 2 +- src/styles/components/_codebook.scss | 7 ++- 6 files changed, 68 insertions(+), 35 deletions(-) diff --git a/src/components/Codebook/Codebook.js b/src/components/Codebook/Codebook.js index 65a534b75..235480cd6 100644 --- a/src/components/Codebook/Codebook.js +++ b/src/components/Codebook/Codebook.js @@ -9,7 +9,12 @@ import EntityType from './EntityType'; import ExternalEntity from './ExternalEntity'; import EgoType from './EgoType'; import CodebookCategory from './CodebookCategory'; -import { getStageMetaByIndex, getUsage, getUsageAsStageMeta, getVariableMetaByIndex } from './helpers'; +import { + getStageMetaByIndex, + getUsage, + getUsageAsStageMeta, + getVariableMetaByIndex, +} from './helpers'; const Codebook = ({ edges, diff --git a/src/components/Codebook/EgoType.js b/src/components/Codebook/EgoType.js index b237ea55b..54adf79f7 100644 --- a/src/components/Codebook/EgoType.js +++ b/src/components/Codebook/EgoType.js @@ -33,7 +33,6 @@ EgoType.defaultProps = { const mapStateToProps = (state) => { const entityProperties = getEntityProperties(state, { entity: 'ego' }); - console.log('entityProperties', entityProperties); return entityProperties; }; diff --git a/src/components/Codebook/__tests__/helpers.test.js b/src/components/Codebook/__tests__/helpers.test.js index 2e7249669..5e14d42b3 100644 --- a/src/components/Codebook/__tests__/helpers.test.js +++ b/src/components/Codebook/__tests__/helpers.test.js @@ -1,10 +1,41 @@ /* eslint-env jest */ -import { getUsage, getUsageAsStageMeta, getCodebookVariableIndexFromValidationPath } from '../helpers'; +import { getAllVariablesByUUID } from '../../../selectors/codebook'; +import { getUsage, getUsageAsStageMeta } from '../helpers'; const state = { protocol: { present: { + codebook: { + ego: { + variables: { + 1: { + name: 'name', + type: 'text', + }, + }, + }, + node: { + person: { + variables: { + 2: { + name: 'name', + type: 'text', + }, + }, + }, + }, + edge: { + friend: { + variables: { + 3: { + name: 'name', + type: 'text', + }, + }, + }, + }, + }, stages: [ { label: 'foo', id: 'abcd', other: 'ignored' }, { label: 'bar', id: 'efgh', other: 'ignored' }, @@ -30,26 +61,22 @@ it('getUsage() ', () => { it('getUsageAsStageMeta()', () => { const usage = ['stages[0].foo.bar', 'stages[0].foo.bar.bazz', 'stages[1].foo.bar.bazz']; - const expectedResult = [ + + const mockStageMetaByIndex = [ { label: 'foo', id: 'abcd' }, { label: 'bar', id: 'efgh' }, - ]; - expect(getUsageAsStageMeta(state, usage)).toEqual(expectedResult); -}); - -it('getCodebookVariableIndexFromValidationPath()', () => { - const testStrings = [ - "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs", - "codebook.ego.variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom", - "codebook.node[nodeType].variables[variableType].validation.sameAs", - "codebook.node[nodeType].variables[variableType].validation.differentFrom", - "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.sameAs", - "codebook.edge[edgeType].variables[4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c].validation.differentFrom", + { label: 'bazz', id: 'ijkl' }, ]; - // Run the function for each string in the array - testStrings.forEach((testString) => { - expect(getCodebookVariableIndexFromValidationPath(testString)).toEqual('4b27bf9f-7058-4e74-84d8-2cc0bfd7d25c'); - } + const mockVariableMetaByIndex = getAllVariablesByUUID(state.protocol.present.codebook); + const expectedResult = [ + { label: 'foo', id: 'abcd' }, + { label: 'bar', id: 'efgh' }, + ]; + expect(getUsageAsStageMeta( + mockStageMetaByIndex, + mockVariableMetaByIndex, + usage, + )).toEqual(expectedResult); }); diff --git a/src/components/Codebook/helpers.js b/src/components/Codebook/helpers.js index 1b126e73b..e0845a069 100644 --- a/src/components/Codebook/helpers.js +++ b/src/components/Codebook/helpers.js @@ -1,11 +1,10 @@ import { reduce, get, compact, uniq, map, } from 'lodash'; -import { getType } from '@selectors/codebook'; +import { getType, getAllVariablesByUUID } from '@selectors/codebook'; import { makeGetIsUsed } from '@selectors/codebook/isUsed'; import { getVariableIndex } from '@selectors/indexes'; import { getProtocol, getCodebook } from '@selectors/protocol'; -import { getAllVariablesByUUID } from '../../selectors/codebook'; const getIsUsed = makeGetIsUsed({ formNames: [] }); @@ -39,14 +38,6 @@ const getStageIndexFromPath = (path) => { const codebookVariableReferenceRegex = /codebook\.(ego|node\[([^\]]+)\]|edge\[([^\]]+)\])\.variables\[(.*?)\].validation\.(sameAs|differentFrom)/; export const getCodebookVariableIndexFromValidationPath = (path) => { - // Regexp that matches all of the following: - // "codebook.ego.variables[variableId].validation.sameAs" - // "codebook.ego.variables[variableId].validation.differentFrom" - // "codebook.node[nodeType].variables[variableId].validation.sameAs" - // "codebook.node[nodeType.variables[variableId].validation.differentFrom" - // "codebook.edge[edgeType].variables[variableId].validation.sameAs" - // "codebook.edge[edgeType].variables[variableId].validation.differentFrom" - const match = path.match(codebookVariableReferenceRegex); return get(match, 4, null); @@ -72,7 +63,10 @@ export const getUsage = (index, value) => reduce(index, (acc, indexValue, path) * See `getUsage()` for how the usage array is generated. * * Any stages that can't be found in the index are omitted. - * @param {Object} state Application state + * + * @param {Object[]} stageMetaByIndex Stage meta by index (as created by `getStageMetaByIndex()`) + * @param {Object[]} variableMetaByIndex Variable meta by index (as created by + * `getVariableMetaByIndex()`) * @param {string[]} usageArray "Usage array" as created by `getUsage()` * @returns {Object[]} List of stage meta `{ label, id }`. */ @@ -81,13 +75,16 @@ export const getUsageAsStageMeta = (stageMetaByIndex, variableMetaByIndex, usage const codebookVariablePaths = usageArray.filter(getCodebookVariableIndexFromValidationPath); const codebookVariablesWithMeta = codebookVariablePaths.map((path) => { const variableId = getCodebookVariableIndexFromValidationPath(path); + const { name } = variableMetaByIndex[variableId]; return { - label: `Used as validation for "${variableMetaByIndex[variableId].name}"`, + label: `Used as validation for "${name || 'unknown'}"`, }; }); const stageIndexes = compact(uniq(usageArray.map(getStageIndexFromPath))); - const stageVariablesWithMeta = stageIndexes.map((stageIndex) => get(stageMetaByIndex, stageIndex)); + const stageVariablesWithMeta = stageIndexes.map( + (stageIndex) => get(stageMetaByIndex, stageIndex), + ); return [ ...stageVariablesWithMeta, diff --git a/src/selectors/codebook/isUsed.js b/src/selectors/codebook/isUsed.js index 9433e0790..addb36f19 100644 --- a/src/selectors/codebook/isUsed.js +++ b/src/selectors/codebook/isUsed.js @@ -1,4 +1,4 @@ -import { get, omit, cloneDeep, each } from 'lodash'; +import { get, omit, cloneDeep } from 'lodash'; import { getForms } from '../reduxForm'; import { getCodebook, getProtocol } from '../protocol'; import { getIdsFromCodebook } from './helpers'; diff --git a/src/styles/components/_codebook.scss b/src/styles/components/_codebook.scss index 875d16539..a5a1ee3b3 100644 --- a/src/styles/components/_codebook.scss +++ b/src/styles/components/_codebook.scss @@ -16,10 +16,11 @@ &__tag { display: inline-block; border-radius: unit(1); - padding: unit(0.5) unit(0.75); + padding: unit(.5) unit(.75); font-size: .9em; color: var(--color-white); background-color: var(--color-mustard--dark); + word-break: break-word; &--not-used { background-color: var(--architect-warning); @@ -162,6 +163,10 @@ } } + &--usage { + max-width: 300px; + } + &-usage-container { display: flex; flex-direction: column; From 60965713d7b35550f735edd67ab632f6621d49f4 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Wed, 14 Jun 2023 12:50:14 +0200 Subject: [PATCH 11/11] update test snapshot --- src/selectors/__tests__/__snapshots__/indexes.test.js.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/selectors/__tests__/__snapshots__/indexes.test.js.snap b/src/selectors/__tests__/__snapshots__/indexes.test.js.snap index 83bf46f38..aed96b881 100644 --- a/src/selectors/__tests__/__snapshots__/indexes.test.js.snap +++ b/src/selectors/__tests__/__snapshots__/indexes.test.js.snap @@ -67,6 +67,9 @@ Object { exports[`indexes selectors getVariableIndex() extracts variables into index 1`] = ` Object { + "codebook.edge[77199445-9d50-4646-b0bc-6d6b0c0e06bd].variables[e343a91f-628d-4175-870c-957beffa0003].validation.sameAs": "e343a91f-628d-4175-870c-957beffa0002", + "codebook.ego.variables[9c47f32e-b72f-4b1b-93a2-e29766dc3012].validation.differentFrom": "23a10bcb-bd71-46a1-a36d-4ab80cb5a1d1", + "codebook.node[4aebf73e-95e3-4fd1-95e7-237dcc4a4466].variables[0e75ec18-2cb1-4606-9f18-034d28b07c19].validation.differentFrom": "6be95f85-c2d9-4daf-9de1-3939418af888", "stages[0].form.fields[0].variable": "3377af3f-3c79-41da-9b0b-6570fb519b93", "stages[0].form.fields[10].variable": "9c47f32e-b72f-4b1b-93a2-e29766dc3012", "stages[0].form.fields[1].variable": "b4956387-cada-412f-9e97-fe352c3cc44e",