refactor getMappingsByParameter function in metabase/meta/Dashboard.js (#18340)

* refactor getMappingsByParameter

* handle dashcard.series and dashcard.parameter_mappings being undefined

* pull out getMapping fn from getMappings

* rmv hasDisjointValueSets code + ui warning

frontend/src/metabase-lib/lib/metadata/Field.js

@@ -166,7 +166,7 @@ export default class Field extends Base {
    * @returns {FieldValues}
    */
   fieldValues() {
-    return getFieldValues(this._object);
+    return getFieldValues(this._plainObject);
   }
 
   icon() {

frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.css

@@ -15,11 +15,6 @@
   color: var(--color-white);
 }
 
-:local(.warn) {
-  border-color: var(--color-accent3) !important;
-  color: var(--color-error) !important;
-}
-
 :local(.disabled) {
   composes: disabled from "style";
   border-color: inherit;

frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.jsx

@@ -20,7 +20,6 @@ import {
 import { setParameterMapping } from "../actions";
 
 import cx from "classnames";
-import { getIn } from "icepick";
 
 import type { Card } from "metabase-types/types/Card";
 import type { DashCard } from "metabase-types/types/Dashboard";
@@ -92,25 +91,7 @@ export default class DashCardCardParameterMapper extends Component {
   };
 
   render() {
-    const {
-      mappingOptions,
-      target,
-      mappingsByParameter,
-      parameter,
-      dashcard,
-      card,
-    } = this.props;
-
-    const mapping = getIn(mappingsByParameter, [
-      parameter.id,
-      dashcard.id,
-      card.id,
-    ]);
-    const noOverlap = !!(
-      mapping &&
-      mapping.mappingsWithValues > 1 &&
-      mapping.overlapMax === 1
-    );
+    const { mappingOptions, target, parameter, dashcard, card } = this.props;
 
     let selectedFieldWarning = null;
     if (
@@ -150,8 +131,6 @@ export default class DashCardCardParameterMapper extends Component {
               tooltip={
                 disabled
                   ? "This card doesn't have any fields or parameters that can be mapped to this parameter type."
-                  : noOverlap
-                  ? "The values in this field don't overlap with the values of any other fields you've chosen."
                   : null
               }
               verticalAttachments={["bottom", "top"]}
@@ -163,7 +142,6 @@ export default class DashCardCardParameterMapper extends Component {
               <div
                 className={cx(S.button, {
                   [S.mapped]: !!selected,
-                  [S.warn]: noOverlap,
                   [S.disabled]: disabled,
                 })}
               >

frontend/src/metabase/dashboard/selectors.js

@@ -23,8 +23,6 @@ import type {
 
 export type AugmentedParameterMapping = ParameterMapping & {
   dashcard_id: DashCardId,
-  overlapMax?: number,
-  mappingsWithValues?: number,
   values: Array<string>,
 };
 

frontend/src/metabase/meta/Dashboard.js

@@ -273,95 +273,66 @@ export function isDashboardParameterWithoutMapping(parameter, dashboard) {
   return parameterExistsOnDashboard && !parameterHasMapping;
 }
 
+function getMappingTargetField(card, mapping, metadata) {
+  if (!card) {
+    return null;
+  }
+
+  const fieldId = getParameterTargetFieldId(mapping.target, card.dataset_query);
+  let field = metadata.field(fieldId);
+
+  if (!field) {
+    const rawField = _.findWhere(card.result_metadata, {
+      name: fieldId,
+    });
+    field = rawField && new Field(rawField, metadata);
+  }
+
+  return field;
+}
+
+function getMapping(dashcard, metadata) {
+  const cards = [dashcard.card, ...(dashcard.series || [])];
+  return (dashcard.parameter_mappings || []).map(mapping => {
+    const card = _.findWhere(cards, { id: mapping.card_id });
+    const field = getMappingTargetField(card, mapping, metadata);
+
+    return {
+      ...mapping,
+      parameter_id: mapping.parameter_id,
+      dashcard_id: dashcard.id,
+      card_id: mapping.card_id,
+      field_id: field?.id ?? field?.name,
+      field,
+    };
+  });
+}
+
+function getMappings(dashboard, metadata) {
+  const mappings = dashboard.ordered_cards.flatMap(dashcard =>
+    getMapping(dashcard, metadata),
+  );
+
+  return mappings;
+}
+
 export function getMappingsByParameter(metadata, dashboard) {
   if (!dashboard) {
     return {};
   }
 
-  let mappingsByParameter = {};
-  const mappings = [];
-  const countsByParameter = {};
-  for (const dashcard of dashboard.ordered_cards) {
-    const cards: Card[] = [dashcard.card].concat(dashcard.series);
-    for (const mapping of dashcard.parameter_mappings || []) {
-      const card = _.findWhere(cards, { id: mapping.card_id });
-      const fieldId =
-        card && getParameterTargetFieldId(mapping.target, card.dataset_query);
-      let field = metadata.field(fieldId);
-
-      if (!field) {
-        const rawField = _.findWhere(dashcard.card.result_metadata, {
-          name: fieldId,
-        });
-
-        field = rawField && new Field(rawField, metadata);
-      }
-
-      const values = field?.fieldValues() || [];
-      if (values.length) {
-        countsByParameter[mapping.parameter_id] =
-          countsByParameter[mapping.parameter_id] || {};
-      }
-      for (const value of values) {
-        countsByParameter[mapping.parameter_id][value] =
-          (countsByParameter[mapping.parameter_id][value] || 0) + 1;
-      }
-
-      const augmentedMapping = {
-        ...mapping,
-        parameter_id: mapping.parameter_id,
-        dashcard_id: dashcard.id,
-        card_id: mapping.card_id,
-        field_id: fieldId,
-        field,
-        values,
-      };
-      mappingsByParameter = setIn(
-        mappingsByParameter,
-        [mapping.parameter_id, dashcard.id, mapping.card_id],
-        augmentedMapping,
-      );
-      mappings.push(augmentedMapping);
-    }
-  }
-  const mappingsWithValuesByParameter = {};
-  // update max values overlap for each mapping
-  for (const mapping of mappings) {
-    if (mapping.values && mapping.values.length > 0) {
-      const overlapMax = Math.max(
-        ...mapping.values.map(
-          value => countsByParameter[mapping.parameter_id][value],
-        ),
-      );
-      mappingsByParameter = setIn(
-        mappingsByParameter,
-        [
-          mapping.parameter_id,
-          mapping.dashcard_id,
-          mapping.card_id,
-          "overlapMax",
-        ],
-        overlapMax,
-      );
-      mappingsWithValuesByParameter[mapping.parameter_id] =
-        (mappingsWithValuesByParameter[mapping.parameter_id] || 0) + 1;
-    }
-  }
-  // update count of mappings with values
-  for (const mapping of mappings) {
-    mappingsByParameter = setIn(
-      mappingsByParameter,
-      [
-        mapping.parameter_id,
-        mapping.dashcard_id,
-        mapping.card_id,
-        "mappingsWithValues",
-      ],
-      mappingsWithValuesByParameter[mapping.parameter_id] || 0,
-    );
-  }
+  const mappings = getMappings(dashboard, metadata);
+  const mappingsByParameterIdByDashcardIdByCardId = mappings.reduce(
+    (map, mapping) =>
+      setIn(
+        map,
+        [mapping.parameter_id, mapping.dashcard_id, mapping.card_id],
+        mapping,
+      ),
+    {},
+  );
 
-  return mappingsByParameter;
+  return mappingsByParameterIdByDashcardIdByCardId;
 }
 
 export function getDashboardParametersWithFieldMetadata(

frontend/test/metabase/meta/Dashboard.unit.spec.js

@@ -424,10 +424,8 @@ describe("meta/Dashboard", () => {
                 table_id: 6,
               }),
               field_id: 120,
-              mappingsWithValues: 0,
               parameter_id: "parameter1",
               target: ["dimension", ["field", 120, null]],
-              values: [],
             },
           },
           "86": {
@@ -443,10 +441,8 @@ describe("meta/Dashboard", () => {
                 table_id: 8,
               }),
               field_id: 134,
-              mappingsWithValues: 0,
               parameter_id: "parameter1",
               target: ["dimension", ["template-tag", "bbb"]],
-              values: [],
             },
           },
           "87": {
@@ -481,13 +477,11 @@ describe("meta/Dashboard", () => {
                 semantic_type: null,
               }),
               field_id: "boolean",
-              mappingsWithValues: 0,
               parameter_id: "parameter1",
               target: [
                 "dimension",
                 ["field", "boolean", { "base-type": "type/Boolean" }],
               ],
-              values: [],
             },
           },
         },