From 350f9f16df56b3d500a6a2fbd7ac59eba5d3f97b Mon Sep 17 00:00:00 2001 From: Rebecca Madsen Date: Thu, 12 Nov 2020 19:05:02 -0500 Subject: [PATCH 1/3] non-transposed codebook for categorical and ordinal graphs --- src/main/utils/formatters/network.js | 33 ------------------- src/renderer/containers/SettingsScreen.js | 26 +++++++++++++-- .../workspace/EntityTimeSeriesPanel.js | 33 +++++++++++++++---- .../__tests__/EntityTimeSeriesPanel-test.js | 4 +-- .../withAnswerDistributionCharts-test.js | 2 +- .../workspace/withAnswerDistributionCharts.js | 29 ++++++++-------- .../ducks/modules/__tests__/protocols-test.js | 26 +++++++-------- src/renderer/ducks/modules/protocols.js | 12 +++---- 8 files changed, 84 insertions(+), 81 deletions(-) diff --git a/src/main/utils/formatters/network.js b/src/main/utils/formatters/network.js index ec56a7294..ebe1b394c 100644 --- a/src/main/utils/formatters/network.js +++ b/src/main/utils/formatters/network.js @@ -93,37 +93,6 @@ const insertEgoInNetworks = networks => ( networks.map(network => insertNetworkEgo(network)) ); -const transposedCodebookVariables = (sectionCodebook, definition) => { - if (!definition.variables) { // not required for edges - sectionCodebook[definition.name] = definition; // eslint-disable-line no-param-reassign - return sectionCodebook; - } - - const displayVariable = definition.variables[definition.displayVariable]; - - const variables = Object.values(definition.variables).reduce((acc, variable) => { - acc[variable.name] = variable; - return acc; - }, {}); - sectionCodebook[definition.name] = { // eslint-disable-line no-param-reassign - ...definition, - displayVariable: displayVariable && displayVariable.name, - variables, - }; - return sectionCodebook; -}; - -const transposedCodebookSection = (section = {}) => - Object.values(section).reduce((sectionCodebook, definition) => ( - transposedCodebookVariables(sectionCodebook, definition) - ), {}); - -const transposedCodebook = (codebook = {}) => ({ - edge: transposedCodebookSection(codebook.edge), - node: transposedCodebookSection(codebook.node), - ego: transposedCodebookVariables({}, { ...codebook.ego, name: 'ego' }).ego, -}); - module.exports = { convertUuidToDecimal, filterNetworkEntities, @@ -136,7 +105,5 @@ module.exports = { caseProperty, nodePrimaryKeyProperty, processEntityVariables, - transposedCodebook, - transposedCodebookSection, unionOfNetworks, }; diff --git a/src/renderer/containers/SettingsScreen.js b/src/renderer/containers/SettingsScreen.js index 9c0d34ed4..0276cb40d 100644 --- a/src/renderer/containers/SettingsScreen.js +++ b/src/renderer/containers/SettingsScreen.js @@ -12,6 +12,22 @@ import Types from '../types'; import { actionCreators, selectors as protocolSelectors } from '../ducks/modules/protocols'; import { actionCreators as chartActionCreators, selectors as chartSelectors } from '../ducks/modules/excludedChartVariables'; +const entityName = (entityKey) => { + if (entityKey === 'nodes') return 'node'; + if (entityKey === 'edges') return 'edge'; + if (entityKey === 'ego') return 'ego'; + return null; +}; + +const entityVariableType = (codebook, entity, section) => ( + codebook && codebook[entityName(entity)] && codebook[entityName(entity)][section] && + codebook[entityName(entity)][section].name); + +const entityVariableName = (codebook, entity, section, variable) => ( + codebook && codebook[entityName(entity)] && codebook[entityName(entity)][section] && + codebook[entityName(entity)][section].variables[variable] && + codebook[entityName(entity)][section].variables[variable].name); + class SettingsScreen extends Component { constructor(props) { super(props); @@ -21,7 +37,7 @@ class SettingsScreen extends Component { } get chartConfigSection() { - const { distributionVariables, protocol, setExcludedVariables } = this.props; + const { distributionVariables, protocol, codebook, setExcludedVariables } = this.props; if (!Object.keys(distributionVariables).length) { return null; } @@ -40,7 +56,7 @@ class SettingsScreen extends Component { { @@ -48,7 +64,8 @@ class SettingsScreen extends Component { setExcludedVariables(protocol.id, entity, section, newExcluded); }, }} - options={vars.map(v => ({ value: v, label: v }))} + options={vars.map(v => + ({ value: v, label: entityVariableName(codebook, entity, section, v) }))} /> )))) } @@ -120,6 +137,7 @@ const mapStateToProps = (state, ownProps) => ({ excludedChartVariables: chartSelectors.excludedVariablesForCurrentProtocol(state, ownProps), protocolsHaveLoaded: protocolSelectors.protocolsHaveLoaded(state), protocol: protocolSelectors.currentProtocol(state, ownProps), + codebook: protocolSelectors.currentCodebook(state, ownProps), distributionVariables: protocolSelectors.ordinalAndCategoricalVariables(state, ownProps), }); @@ -133,6 +151,7 @@ SettingsScreen.defaultProps = { apiClient: null, distributionVariables: {}, excludedChartVariables: {}, + codebook: {}, protocol: null, }; @@ -141,6 +160,7 @@ SettingsScreen.propTypes = { distributionVariables: PropTypes.object, match: PropTypes.object.isRequired, excludedChartVariables: PropTypes.object, + codebook: PropTypes.object, protocol: Types.protocol, protocolsHaveLoaded: PropTypes.bool.isRequired, setExcludedVariables: PropTypes.func.isRequired, diff --git a/src/renderer/containers/workspace/EntityTimeSeriesPanel.js b/src/renderer/containers/workspace/EntityTimeSeriesPanel.js index b74b44b61..ca6ed13b8 100644 --- a/src/renderer/containers/workspace/EntityTimeSeriesPanel.js +++ b/src/renderer/containers/workspace/EntityTimeSeriesPanel.js @@ -1,16 +1,24 @@ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; +import { connect } from 'react-redux'; +import { selectors as protocolSelectors } from '../../ducks/modules/protocols'; import { EmptyData, TimeSeriesChart } from '../../components'; import withApiClient from '../../components/withApiClient'; +const { currentCodebook } = protocolSelectors; + // Data series are keyed with node_[subtype] and edge_[subtype]; we can assume subtypes are // meaningfully unique and label with just the subtype -const subtypeLabel = subtype => ({ key: subtype, label: `${subtype.split('_')[1]}` }); +const subtypeLabel = subtype => subtype.split('_')[1]; +const codebookSubtypeLabel = (codebook, entityType, subtype) => ( + (codebook && codebook[entityType][subtypeLabel(subtype)] && + codebook[entityType][subtypeLabel(subtype)].name) || subtypeLabel(subtype) +); // Based on the API response, determine which series to render. // If there's only one node subtype (e.g., 'person'), don't render it. -const dataSeries = (timeSeriesKeys = []) => { +const dataSeries = (timeSeriesKeys = [], codebook) => { const series = []; const nodeSubtypes = timeSeriesKeys.filter(key => (/node_/).test(key)); const edgeSubtypes = timeSeriesKeys.filter(key => (/edge_/).test(key)); @@ -18,13 +26,15 @@ const dataSeries = (timeSeriesKeys = []) => { series.push({ key: 'node', label: 'node' }); } if (nodeSubtypes.length > 1) { - series.push(...nodeSubtypes.map(subtypeLabel)); + series.push(...nodeSubtypes.map(subtype => + ({ key: subtype, label: codebookSubtypeLabel(codebook, 'node', subtype) }))); } if (timeSeriesKeys.includes('edge')) { series.push({ key: 'edge', label: 'edge' }); } if (edgeSubtypes.length > 1) { - series.push(...edgeSubtypes.map(subtypeLabel)); + series.push(...edgeSubtypes.map(subtype => + ({ key: subtype, label: codebookSubtypeLabel(codebook, 'edge', subtype) }))); } return series; }; @@ -67,9 +77,10 @@ class EntityTimeSeriesPanel extends PureComponent { render() { const { timeSeriesData, timeSeriesKeys } = this.state; + const { codebook } = this.props; let content; if (timeSeriesData.length > 0) { - const series = dataSeries(timeSeriesKeys); + const series = dataSeries(timeSeriesKeys, codebook); content = ; } else { content = ; @@ -90,16 +101,24 @@ class EntityTimeSeriesPanel extends PureComponent { EntityTimeSeriesPanel.defaultProps = { apiClient: null, sessionCount: null, + codebook: {}, }; EntityTimeSeriesPanel.propTypes = { apiClient: PropTypes.object, protocolId: PropTypes.string.isRequired, sessionCount: PropTypes.number, + codebook: PropTypes.object, }; -export default withApiClient(EntityTimeSeriesPanel); +const mapStateToProps = (state, ownProps) => ({ + codebook: currentCodebook(state, ownProps), +}); + +const UnconnectedEntityTimeSeriesPanel = (withApiClient(EntityTimeSeriesPanel)); + +export default connect(mapStateToProps)(withApiClient(EntityTimeSeriesPanel)); export { - EntityTimeSeriesPanel as UnconnectedEntityTimeSeriesPanel, + UnconnectedEntityTimeSeriesPanel, }; diff --git a/src/renderer/containers/workspace/__tests__/EntityTimeSeriesPanel-test.js b/src/renderer/containers/workspace/__tests__/EntityTimeSeriesPanel-test.js index da346ae79..b61955a1c 100644 --- a/src/renderer/containers/workspace/__tests__/EntityTimeSeriesPanel-test.js +++ b/src/renderer/containers/workspace/__tests__/EntityTimeSeriesPanel-test.js @@ -2,7 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; -import EntityTimeSeriesPanel from '../EntityTimeSeriesPanel'; +import { UnconnectedEntityTimeSeriesPanel } from '../EntityTimeSeriesPanel'; import AdminApiClient from '../../../utils/adminApiClient'; jest.mock('recharts'); @@ -24,7 +24,7 @@ describe('ProtocolCountsPanel', () => { beforeEach(() => { mockApiClient = new AdminApiClient(); - subject = mount(); + subject = mount(); }); it('renders a dashboard panel', () => { diff --git a/src/renderer/containers/workspace/__tests__/withAnswerDistributionCharts-test.js b/src/renderer/containers/workspace/__tests__/withAnswerDistributionCharts-test.js index dcfd1913f..8fb2a8686 100644 --- a/src/renderer/containers/workspace/__tests__/withAnswerDistributionCharts-test.js +++ b/src/renderer/containers/workspace/__tests__/withAnswerDistributionCharts-test.js @@ -24,7 +24,7 @@ jest.mock('../../../ducks/modules/protocols', () => ({ currentProtocol: jest.fn(), currentProtocolId: jest.fn().mockReturnValue('1'), isDistributionVariable: jest.fn().mockReturnValue(true), - transposedCodebook: jest.fn().mockReturnValue({ + currentCodebook: jest.fn().mockReturnValue({ node: { person: { variables: { diff --git a/src/renderer/containers/workspace/withAnswerDistributionCharts.js b/src/renderer/containers/workspace/withAnswerDistributionCharts.js index 356228fd7..017b07b0f 100644 --- a/src/renderer/containers/workspace/withAnswerDistributionCharts.js +++ b/src/renderer/containers/workspace/withAnswerDistributionCharts.js @@ -8,7 +8,7 @@ import { selectors as protocolSelectors } from '../../ducks/modules/protocols'; import { selectors as variableSelectors } from '../../ducks/modules/excludedChartVariables'; import Types from '../../types'; -const { currentProtocolId, isDistributionVariable, transposedCodebook } = protocolSelectors; +const { currentProtocolId, isDistributionVariable, currentCodebook } = protocolSelectors; const { excludedVariablesForCurrentProtocol } = variableSelectors; const hasData = bucket => bucket && Object.keys(bucket).length > 0; @@ -24,17 +24,18 @@ const hasData = bucket => bucket && Object.keys(bucket).length > 0; * * @private * - * @param {Object} transposedNodeCodebook `transposedCodebook.node`, with transposed names + * @param {Object} nodeCodebook `codebook.node` * @param {Object} buckets The API response from `option_buckets` * @return {Array} chartDefinitions */ const shapeBucketDataByType = ( - transposedNodeCodebook, buckets, excludedChartVariables, entityKey) => - Object.entries(transposedNodeCodebook).reduce((acc, [entityType, { variables }]) => { + nodeCodebook, buckets, excludedChartVariables, entityKey) => + Object.entries(nodeCodebook).reduce((acc, [entityType, { variables }]) => { const excludedSectionVariables = (excludedChartVariables[entityKey] && excludedChartVariables[entityKey][entityType]) || []; - Object.entries(variables || []).forEach(([variableName, def]) => { - if (!isDistributionVariable(def) || excludedSectionVariables.includes(def.name)) { + Object.keys(variables || []).forEach((variableName) => { + const def = variables[variableName]; + if (!isDistributionVariable(def) || excludedSectionVariables.includes(variableName)) { return; } const dataPath = entityKey === 'ego' ? [variableName] : [entityType, variableName]; @@ -53,7 +54,7 @@ const shapeBucketDataByType = ( }); acc.push({ entityKey, - entityType, + entityType: nodeCodebook[entityType].name, variableType: def.type, variableDefinition: def, chartData: values || [], @@ -85,7 +86,7 @@ const withAnswerDistributionCharts = (WrappedComponent) => { excludedChartVariables: PropTypes.object, protocolId: PropTypes.string, totalSessionsCount: PropTypes.number, - transposedCodebook: Types.codebook.isRequired, + codebook: Types.codebook.isRequired, } static defaultProps = { @@ -123,23 +124,23 @@ const withAnswerDistributionCharts = (WrappedComponent) => { const { excludedChartVariables, protocolId, - transposedCodebook: { + codebook: { node: nodeCodebook = {}, edge: edgeCodebook = {}, ego: egoCodebook = {}, }, } = this.props; - const nodeNames = Object.values(nodeCodebook).reduce((acc, nodeTypeDefinition) => ( + const nodeNames = Object.keys(nodeCodebook).reduce((acc, nodeType) => ( { ...acc, - [nodeTypeDefinition.name]: Object.keys(nodeTypeDefinition.variables || {}), + [nodeType]: Object.keys(nodeCodebook[nodeType].variables || {}), } ), {}); - const edgeNames = Object.values(edgeCodebook).reduce((acc, edgeTypeDefinition) => ( + const edgeNames = Object.keys(edgeCodebook).reduce((acc, edgeType) => ( { ...acc, - [edgeTypeDefinition.name]: Object.keys(edgeTypeDefinition.variables || {}), + [edgeType]: Object.keys(edgeCodebook[edgeType].variables || {}), } ), {}); const egoNames = Object.keys(egoCodebook.variables || {}); @@ -168,7 +169,7 @@ const withAnswerDistributionCharts = (WrappedComponent) => { const mapStateToProps = (state, ownProps) => ({ excludedChartVariables: excludedVariablesForCurrentProtocol(state, ownProps), protocolId: currentProtocolId(state, ownProps), - transposedCodebook: transposedCodebook(state, ownProps), + codebook: currentCodebook(state, ownProps), }); return connect(mapStateToProps)(AnswerDistributionPanels); diff --git a/src/renderer/ducks/modules/__tests__/protocols-test.js b/src/renderer/ducks/modules/__tests__/protocols-test.js index b08c6f01c..30bfaa0a2 100644 --- a/src/renderer/ducks/modules/__tests__/protocols-test.js +++ b/src/renderer/ducks/modules/__tests__/protocols-test.js @@ -102,7 +102,7 @@ describe('the protocols module', () => { isDistributionVariable, ordinalAndCategoricalVariables, protocolsHaveLoaded, - transposedCodebook, + currentCodebook, } = selectors; describe('currentProtocol', () => { @@ -136,7 +136,7 @@ describe('the protocols module', () => { }; const state = { protocols: [{ id: '1', codebook }] }; const props = { match: { params: { id: '1' } } }; - expect(ordinalAndCategoricalVariables(state, props)).toEqual({ nodes: { person: ['catVar'] }, edges: { friend: ['ordVar'] }, ego: { ego: ['catVar'] } }); + expect(ordinalAndCategoricalVariables(state, props)).toEqual({ nodes: { 'node-type-id': ['var-id-1'] }, edges: { 'edge-type-id': ['var-id-2'] }, ego: { ego: ['var-id-3'] } }); }); it('ignores sections without these variables', () => { @@ -167,8 +167,8 @@ describe('the protocols module', () => { }); }); - describe('transposedCodebook', () => { - it('returns a modified codebook', () => { + describe('currentCodebook', () => { + it('returns a codebook', () => { const codebook = { node: { 'node-type-id': { name: 'person', variables: {} } }, edge: { 'edge-type-id': { name: 'friend', variables: {} } }, @@ -176,13 +176,13 @@ describe('the protocols module', () => { }; const state = { protocols: [{ id: '1', codebook }] }; const props = { match: { params: { id: '1' } } }; - const transposed = transposedCodebook(state, props); - expect(transposed).toHaveProperty('node'); - expect(transposed.node).toHaveProperty('person'); - expect(transposed).toHaveProperty('edge'); - expect(transposed.edge).toHaveProperty('friend'); - expect(transposed).toHaveProperty('ego'); - expect(transposed.ego.variables).toHaveProperty('ordVar'); + const codebook2 = currentCodebook(state, props); + expect(codebook2).toHaveProperty('node'); + expect(codebook2.node).toHaveProperty('node-type-id'); + expect(codebook2).toHaveProperty('edge'); + expect(codebook2.edge).toHaveProperty('edge-type-id'); + expect(codebook2).toHaveProperty('ego'); + expect(codebook2.ego.variables).toHaveProperty('var-id-1'); }); it('does not require edge variables', () => { @@ -192,8 +192,8 @@ describe('the protocols module', () => { }; const state = { protocols: [{ id: '1', codebook }] }; const props = { match: { params: { id: '1' } } }; - const transposed = transposedCodebook(state, props); - expect(transposed.edge).toEqual({ 'edge-name': { name: 'edge-name' } }); + const codebook2 = currentCodebook(state, props); + expect(codebook2.edge).toEqual({ 'edge-type-id': { name: 'edge-name' } }); }); }); }); diff --git a/src/renderer/ducks/modules/protocols.js b/src/renderer/ducks/modules/protocols.js index 56f86551b..8d81e3cfc 100644 --- a/src/renderer/ducks/modules/protocols.js +++ b/src/renderer/ducks/modules/protocols.js @@ -1,7 +1,6 @@ import AdminApiClient from '../../utils/adminApiClient'; import viewModelMapper from '../../utils/baseViewModelMapper'; import { actionCreators as messageActionCreators } from './appMessages'; -import { transposedCodebook as networkTransposedCodebook } from '../../../main/utils/formatters/network'; // TODO: move const LOAD_PROTOCOLS = 'LOAD_PROTOCOLS'; const PROTOCOLS_LOADED = 'PROTOCOLS_LOADED'; @@ -47,16 +46,13 @@ const currentProtocolId = (state, props) => { return protocol && protocol.id; }; -// Transpose all types & variable IDs to names -// Imported data is transposed; this allows utility components from Architect to work as-is. -const transposedCodebook = (state, props) => { +const currentCodebook = (state, props) => { const protocol = currentProtocol(state, props); if (!protocol) { return {}; } - const codebook = protocol.codebook || {}; - return networkTransposedCodebook(codebook); + return protocol.codebook || {}; }; const distributionVariableTypes = ['ordinal', 'categorical']; @@ -93,7 +89,7 @@ const ordinalAndCategoricalVariablesByEntity = (entities) => { * @return {Object} all node ordinal & categorical variable names, sectioned by node type */ const ordinalAndCategoricalVariables = (state, props) => { - const codebook = transposedCodebook(state, props); + const codebook = currentCodebook(state, props); if (!codebook) { return {}; } @@ -154,9 +150,9 @@ const actionTypes = { const selectors = { currentProtocol, currentProtocolId, + currentCodebook, isDistributionVariable, protocolsHaveLoaded, - transposedCodebook, ordinalAndCategoricalVariables, }; From 8a50c4c650775cb5556dca03ce9bb76800c6616e Mon Sep 17 00:00:00 2001 From: Rebecca Madsen Date: Fri, 13 Nov 2020 10:32:00 -0500 Subject: [PATCH 2/3] fix ego variables --- src/renderer/containers/SettingsScreen.js | 15 +++++++++++---- src/renderer/ducks/modules/protocols.js | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/renderer/containers/SettingsScreen.js b/src/renderer/containers/SettingsScreen.js index 0276cb40d..c6dd75ce1 100644 --- a/src/renderer/containers/SettingsScreen.js +++ b/src/renderer/containers/SettingsScreen.js @@ -23,10 +23,17 @@ const entityVariableType = (codebook, entity, section) => ( codebook && codebook[entityName(entity)] && codebook[entityName(entity)][section] && codebook[entityName(entity)][section].name); -const entityVariableName = (codebook, entity, section, variable) => ( - codebook && codebook[entityName(entity)] && codebook[entityName(entity)][section] && - codebook[entityName(entity)][section].variables[variable] && - codebook[entityName(entity)][section].variables[variable].name); +const entityVariableName = (codebook, entity, section, variable) => { + if (entity === 'ego') { + return (codebook && codebook[entityName(entity)] && codebook[entityName(entity)].variables && + codebook[entityName(entity)].variables[variable] && + codebook[entityName(entity)].variables[variable].name) || variable; + } + + return (codebook && codebook[entityName(entity)] && codebook[entityName(entity)][section] && + codebook[entityName(entity)][section].variables[variable] && + codebook[entityName(entity)][section].variables[variable].name) || variable; +}; class SettingsScreen extends Component { constructor(props) { diff --git a/src/renderer/ducks/modules/protocols.js b/src/renderer/ducks/modules/protocols.js index 8d81e3cfc..b83671890 100644 --- a/src/renderer/ducks/modules/protocols.js +++ b/src/renderer/ducks/modules/protocols.js @@ -96,7 +96,7 @@ const ordinalAndCategoricalVariables = (state, props) => { return { nodes: ordinalAndCategoricalVariablesByEntity(codebook.node), edges: ordinalAndCategoricalVariablesByEntity(codebook.edge), - ego: ordinalAndCategoricalVariablesByEntity(codebook.ego) }; + ego: ordinalAndCategoricalVariablesByEntity({ ...codebook.ego, name: 'ego' }) }; }; const protocolsHaveLoaded = state => state.protocols !== initialState; From a25866cc605b8d851aa2c403bc32e1a918ff87a0 Mon Sep 17 00:00:00 2001 From: Rebecca Madsen Date: Fri, 13 Nov 2020 11:40:46 -0500 Subject: [PATCH 3/3] fix error message --- src/main/data-managers/ProtocolManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/data-managers/ProtocolManager.js b/src/main/data-managers/ProtocolManager.js index 545839195..a22e7aa2a 100644 --- a/src/main/data-managers/ProtocolManager.js +++ b/src/main/data-managers/ProtocolManager.js @@ -455,7 +455,7 @@ class ProtocolManager { if (invalidFileList.length > 0) { processFilePromises.push( Promise.reject(invalidFileList.map(invalidFile => constructErrorObject( - ErrorMessages.InvalidFileExtension, + ErrorMessages.InvalidSessionFileExtension, null, invalidFile, ))),