diff --git a/frontend/src/metabase/dashboard/dashboard.integ.spec.js b/frontend/src/metabase/dashboard/dashboard.integ.spec.js index ae151a00afdede63d3545a56c7f7a45932ad31de..bd28987d1c9cd77a6a1be4206847dff7780692f6 100644 --- a/frontend/src/metabase/dashboard/dashboard.integ.spec.js +++ b/frontend/src/metabase/dashboard/dashboard.integ.spec.js @@ -6,7 +6,7 @@ import { login } from "metabase/__support__/integrated_tests"; -import { getParameterFieldValues } from "metabase/selectors/metadata"; +import { getMergedParameterFieldValues } from "metabase/selectors/metadata"; import { ADD_PARAM_VALUES } from "metabase/redux/metadata"; // TODO Atte Keinänen 7/17/17: When we have a nice way to create dashboards in tests, this could use a real saved dashboard @@ -66,7 +66,7 @@ describe("Dashboard redux actions", () => { await store.dispatch(fetchDashboard('6e59cc97-3b6a-4bb6-9e7a-5efeee27e40f')); await store.waitForActions(ADD_PARAM_VALUES) - const fieldValues = await getParameterFieldValues(store.getState(), { parameter: { field_id: 21 }}); + const fieldValues = await getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [ 21 ] }}); expect(fieldValues).toEqual(["Doohickey", "Gadget", "Gizmo", "Widget"]); }) }) diff --git a/frontend/src/metabase/dashboard/selectors.js b/frontend/src/metabase/dashboard/selectors.js index 680cd0bea51db64e0b328cc26289f0c3f16d3075..e74666a5f889ddcee7bf93a85c1626857caf8db8 100644 --- a/frontend/src/metabase/dashboard/selectors.js +++ b/frontend/src/metabase/dashboard/selectors.js @@ -161,7 +161,7 @@ export const getParameters = createSelector( .value(); return { ...parameter, - field_id: fieldIds.length === 1 ? fieldIds[0] : null + field_ids: fieldIds } }) ) diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx index 8ab0733679ce63b76cc13566999d748c895947b6..19a9d0efac7a601c15216e4220ec53dcfa9b7415 100644 --- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx +++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx @@ -31,17 +31,21 @@ const DATE_WIDGETS = { } import { fetchFieldValues } from "metabase/redux/metadata"; -import { getParameterFieldValues } from "metabase/selectors/metadata"; - -const mapStateToProps = (state, props) => ({ - values: getParameterFieldValues(state, props), -}) +import { makeGetMergedParameterFieldValues } from "metabase/selectors/metadata"; + +const makeMapStateToProps = () => { + const getMergedParameterFieldValues = makeGetMergedParameterFieldValues(); + const mapStateToProps = (state, props) => ({ + values: getMergedParameterFieldValues(state, props), + }) + return mapStateToProps; +} const mapDispatchToProps = { fetchFieldValues } -@connect(mapStateToProps, mapDispatchToProps) +@connect(makeMapStateToProps, mapDispatchToProps) export default class ParameterValueWidget extends Component { static propTypes = { diff --git a/frontend/src/metabase/selectors/metadata.js b/frontend/src/metabase/selectors/metadata.js index 517b7b9473212590a6fac00dedb22f44186be3fb..cc56cd3ef4c595437f5672a00dbcfcc966515dde 100644 --- a/frontend/src/metabase/selectors/metadata.js +++ b/frontend/src/metabase/selectors/metadata.js @@ -1,6 +1,6 @@ /* @flow weak */ -import { createSelector } from "reselect"; +import { createSelector, createSelectorCreator, defaultMemoize } from "reselect"; import Metadata from "metabase-lib/lib/metadata/Metadata"; import Database from "metabase-lib/lib/metadata/Database"; @@ -9,7 +9,8 @@ import Field from "metabase-lib/lib/metadata/Field"; import Metric from "metabase-lib/lib/metadata/Metric"; import Segment from "metabase-lib/lib/metadata/Segment"; -import { getIn } from "icepick"; +import _ from "underscore"; +import { shallowEqual } from "recompose"; import { getFieldValues } from "metabase/lib/query/field"; import { @@ -123,8 +124,36 @@ export const getSegments = createSelector( // MISC -export const getParameterFieldValues = (state, props) => { - return getFieldValues(getIn(state, ["metadata", "fields", props.parameter.field_id])); +// Returns a dictionary of field id:s mapped to matching field values +const getParameterFieldValuesByFieldId = (state, props) => _.chain(getFields(state)) + .pick(getFields(state), ...props.parameter.field_ids) + .mapObject(getFieldValues) + .value() + +// Check if the lengths of field value arrays equal for each field +// TODO: Why can't we use plain shallowEqual, i.e. why the field value arrays change very often? +const createFieldValuesEqualSelector = createSelectorCreator(defaultMemoize, (a, b) => { + return shallowEqual(_.mapObject(a, (values) => values.length), _.mapObject(b, (values) => values.length)); +}) + +// Merges the field values of fields linked to a parameter and removes duplicates +// TODO Atte Keinänen 7/20/17: How should this work for remapped values? +// TODO Atte Keinänen 7/20/17: Should we have any thresholds if the count of field values is high or we have many (>2?) fields? +export const makeGetMergedParameterFieldValues = () => { + return createFieldValuesEqualSelector(getParameterFieldValuesByFieldId, (fieldValues) => { + const fieldIds = Object.keys(fieldValues) + + if (fieldIds.length === 0) { + return []; + } else if (fieldIds.length === 1) { + return fieldValues[fieldIds[0]]; + } else { + const sortedMergedValues = _.flatten(Object.values(fieldValues), true).sort() + // run the uniqueness comparision always against a non-remapped value + // we can use `isSorted = true` flag to speed up _.uniq as we just sorted the values + return _.uniq(sortedMergedValues, true, (fieldValue) => fieldValue[0]); + } + }); } // UTILS: