From 5259268ef2e49f22a835cc70aa7a11508de7199d Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Mon, 8 Oct 2018 12:18:40 -0700
Subject: [PATCH] Conditional formatting: add support for strings + null/not
 null

---
 .../src/metabase/components/NumericInput.jsx  |   2 +-
 .../settings/ChartSettingsTableFormatting.jsx | 323 ++++++++++--------
 .../visualizations/lib/table_format.js        |  16 +-
 3 files changed, 198 insertions(+), 143 deletions(-)

diff --git a/frontend/src/metabase/components/NumericInput.jsx b/frontend/src/metabase/components/NumericInput.jsx
index b5e52ba5510..d46ce4df601 100644
--- a/frontend/src/metabase/components/NumericInput.jsx
+++ b/frontend/src/metabase/components/NumericInput.jsx
@@ -13,7 +13,7 @@ const NumericInput = ({ value, onChange, ...props }: Props) => (
   <InputBlurChange
     value={value == null ? "" : String(value)}
     onBlurChange={({ target: { value } }) => {
-      value = value ? parseInt(value, 10) : null;
+      value = value ? parseFloat(value) : null;
       if (!isNaN(value)) {
         onChange(value);
       }
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingsTableFormatting.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingsTableFormatting.jsx
index b4107be8bbd..3a808290638 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingsTableFormatting.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingsTableFormatting.jsx
@@ -9,24 +9,39 @@ import Radio from "metabase/components/Radio";
 import Toggle from "metabase/components/Toggle";
 import ColorPicker from "metabase/components/ColorPicker";
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
+import NumericInput from "metabase/components/NumericInput";
 
 import { SortableContainer, SortableElement } from "react-sortable-hoc";
 
 import MetabaseAnalytics from "metabase/lib/analytics";
-import { formatNumber, capitalize } from "metabase/lib/formatting";
-import { isNumeric } from "metabase/lib/schema_metadata";
+import { capitalize } from "metabase/lib/formatting";
+import { isNumeric, isString } from "metabase/lib/schema_metadata";
 
 import _ from "underscore";
 import d3 from "d3";
 import cx from "classnames";
 
-const OPERATOR_NAMES = {
+const NUMBER_OPERATOR_NAMES = {
   "<": t`less than`,
   ">": t`greater than`,
   "<=": t`less than or equal to`,
   ">=": t`greater than or equal to`,
   "=": t`equal to`,
   "!=": t`not equal to`,
+  "is-null": t`null`,
+  "not-null": t`not null`,
+};
+
+const STRING_OPERATOR_NAMES = {
+  "=": t`equal to`,
+  "!=": t`not equal to`,
+  "is-null": t`null`,
+  "not-null": t`not null`,
+};
+
+const ALL_OPERATOR_NAMES = {
+  ...NUMBER_OPERATOR_NAMES,
+  ...STRING_OPERATOR_NAMES,
 };
 
 import colors, { desaturated, getColorScale } from "metabase/lib/colors";
@@ -46,8 +61,8 @@ const DEFAULTS_BY_TYPE = {
   single: {
     columns: [],
     type: "single",
-    operator: ">",
-    value: 0,
+    operator: "=",
+    value: "",
     color: COLORS[0],
     highlight_row: false,
   },
@@ -63,7 +78,9 @@ const DEFAULTS_BY_TYPE = {
 };
 
 // predicate for columns that can be formatted
-export const isFormattable = isNumeric;
+export const isFormattable = field => isNumeric(field) || isString(field);
+
+const INPUT_CLASSNAME = "AdminSelect input mt1 full";
 
 export default class ChartSettingsTableFormatting extends React.Component {
   state = {
@@ -278,138 +295,181 @@ const RuleDescription = ({ rule }) => (
       : rule.type === "single"
         ? jt`When a cell in these columns is ${(
             <span className="text-bold">
-              {OPERATOR_NAMES[rule.operator]} {formatNumber(rule.value)}
+              {ALL_OPERATOR_NAMES[rule.operator]} {rule.value}
             </span>
           )} it will be tinted this color.`
         : null}
   </span>
 );
 
-const RuleEditor = ({ rule, cols, isNew, onChange, onDone, onRemove }) => (
-  <div>
-    <h3 className="mb1">{t`Which columns should be affected?`}</h3>
-    <Select
-      value={rule.columns}
-      onChange={e => onChange({ ...rule, columns: e.target.value })}
-      isInitiallyOpen={rule.columns.length === 0}
-      placeholder="Choose a column"
-      multiple
-    >
-      {cols.map(col => <Option value={col.name}>{col.display_name}</Option>)}
-    </Select>
-    <h3 className="mt3 mb1">{t`Formatting style`}</h3>
-    <Radio
-      value={rule.type}
-      options={[
-        { name: t`Single color`, value: "single" },
-        { name: t`Color range`, value: "range" },
-      ]}
-      onChange={type => onChange({ ...DEFAULTS_BY_TYPE[type], ...rule, type })}
-      vertical
-    />
-    {rule.type === "single" ? (
-      <div>
-        <h3 className="mt3 mb1">{t`When a cell in this column is…`}</h3>
-        <Select
-          value={rule.operator}
-          onChange={e => onChange({ ...rule, operator: e.target.value })}
-        >
-          {Object.entries(OPERATOR_NAMES).map(([operator, operatorName]) => (
-            <Option value={operator}>{capitalize(operatorName)}</Option>
-          ))}
-        </Select>
-        <NumericInput
-          value={rule.value}
-          onChange={value => onChange({ ...rule, value })}
-        />
-        <h3 className="mt3 mb1">{t`…turn its background this color:`}</h3>
-        <ColorPicker
-          value={rule.color}
-          colors={COLORS}
-          onChange={color => onChange({ ...rule, color })}
-        />
-        <h3 className="mt3 mb1">{t`Highlight the whole row`}</h3>
-        <Toggle
-          value={rule.highlight_row}
-          onChange={highlight_row => onChange({ ...rule, highlight_row })}
-        />
-      </div>
-    ) : rule.type === "range" ? (
-      <div>
-        <h3 className="mt3 mb1">{t`Colors`}</h3>
-        <ColorRangePicker
-          colors={rule.colors}
-          onChange={colors => onChange({ ...rule, colors })}
-        />
-        <h3 className="mt3 mb1">{t`Start the range at`}</h3>
-        <Radio
-          value={rule.min_type}
-          onChange={min_type => onChange({ ...rule, min_type })}
-          options={(rule.columns.length <= 1
-            ? [{ name: t`Smallest value in this column`, value: null }]
-            : [
-                { name: t`Smallest value in each column`, value: null },
-                {
-                  name: t`Smallest value in all of these columns`,
-                  value: "all",
-                },
-              ]
-          ).concat([{ name: t`Custom value`, value: "custom" }])}
-          vertical
-        />
-        {rule.min_type === "custom" && (
-          <NumericInput
-            value={rule.min_value}
-            onChange={min_value => onChange({ ...rule, min_value })}
+const RuleEditor = ({ rule, cols, isNew, onChange, onDone, onRemove }) => {
+  const selectedColumns = rule.columns.map(name => _.findWhere(cols, { name }));
+  const isStringRule =
+    selectedColumns.length > 0 && _.all(selectedColumns, isString);
+  const isNumericRule =
+    selectedColumns.length > 0 && _.all(selectedColumns, isNumeric);
+
+  const hasOperand =
+    rule.operator !== "is-null" && rule.operator !== "not-null";
+
+  return (
+    <div>
+      <h3 className="mb1">{t`Which columns should be affected?`}</h3>
+      <Select
+        value={rule.columns}
+        onChange={e => onChange({ ...rule, columns: e.target.value })}
+        isInitiallyOpen={rule.columns.length === 0}
+        placeholder="Choose a column"
+        multiple
+      >
+        {cols.map(col => (
+          <Option
+            value={col.name}
+            disabled={
+              (isStringRule && !isString(col)) ||
+              (isNumericRule && !isNumeric(col))
+            }
+          >
+            {col.display_name}
+          </Option>
+        ))}
+      </Select>
+      {isNumericRule && (
+        <div>
+          <h3 className="mt3 mb1">{t`Formatting style`}</h3>
+          <Radio
+            value={rule.type}
+            options={[
+              { name: t`Single color`, value: "single" },
+              { name: t`Color range`, value: "range" },
+            ]}
+            onChange={type =>
+              onChange({ ...DEFAULTS_BY_TYPE[type], ...rule, type })
+            }
+            vertical
           />
-        )}
-        <h3 className="mt3 mb1">{t`End the range at`}</h3>
-        <Radio
-          value={rule.max_type}
-          onChange={max_type => onChange({ ...rule, max_type })}
-          options={(rule.columns.length <= 1
-            ? [{ name: t`Largest value in this column`, value: null }]
-            : [
-                { name: t`Largest value in each column`, value: null },
-                {
-                  name: t`Largest value in all of these columns`,
-                  value: "all",
-                },
-              ]
-          ).concat([{ name: t`Custom value`, value: "custom" }])}
-          vertical
-        />
-        {rule.max_type === "custom" && (
-          <NumericInput
-            value={rule.max_value}
-            onChange={max_value => onChange({ ...rule, max_value })}
+        </div>
+      )}
+      {rule.type === "single" ? (
+        <div>
+          <h3 className="mt3 mb1">{t`When a cell in this column is…`}</h3>
+          <Select
+            value={rule.operator}
+            onChange={e => onChange({ ...rule, operator: e.target.value })}
+          >
+            {Object.entries(
+              isNumericRule ? NUMBER_OPERATOR_NAMES : STRING_OPERATOR_NAMES,
+            ).map(([operator, operatorName]) => (
+              <Option value={operator}>{capitalize(operatorName)}</Option>
+            ))}
+          </Select>
+          {hasOperand && isNumericRule ? (
+            <NumericInput
+              className={INPUT_CLASSNAME}
+              type="number"
+              value={rule.value}
+              onChange={value => onChange({ ...rule, value })}
+            />
+          ) : hasOperand ? (
+            <input
+              className={INPUT_CLASSNAME}
+              value={rule.value}
+              onChange={e => onChange({ ...rule, value: e.target.value })}
+            />
+          ) : null}
+          <h3 className="mt3 mb1">{t`…turn its background this color:`}</h3>
+          <ColorPicker
+            value={rule.color}
+            colors={COLORS}
+            onChange={color => onChange({ ...rule, color })}
+          />
+          <h3 className="mt3 mb1">{t`Highlight the whole row`}</h3>
+          <Toggle
+            value={rule.highlight_row}
+            onChange={highlight_row => onChange({ ...rule, highlight_row })}
+          />
+        </div>
+      ) : rule.type === "range" ? (
+        <div>
+          <h3 className="mt3 mb1">{t`Colors`}</h3>
+          <ColorRangePicker
+            colors={rule.colors}
+            onChange={colors => onChange({ ...rule, colors })}
           />
+          <h3 className="mt3 mb1">{t`Start the range at`}</h3>
+          <Radio
+            value={rule.min_type}
+            onChange={min_type => onChange({ ...rule, min_type })}
+            options={(rule.columns.length <= 1
+              ? [{ name: t`Smallest value in this column`, value: null }]
+              : [
+                  { name: t`Smallest value in each column`, value: null },
+                  {
+                    name: t`Smallest value in all of these columns`,
+                    value: "all",
+                  },
+                ]
+            ).concat([{ name: t`Custom value`, value: "custom" }])}
+            vertical
+          />
+          {rule.min_type === "custom" && (
+            <NumericInput
+              className={INPUT_CLASSNAME}
+              type="number"
+              value={rule.min_value}
+              onChange={min_value => onChange({ ...rule, min_value })}
+            />
+          )}
+          <h3 className="mt3 mb1">{t`End the range at`}</h3>
+          <Radio
+            value={rule.max_type}
+            onChange={max_type => onChange({ ...rule, max_type })}
+            options={(rule.columns.length <= 1
+              ? [{ name: t`Largest value in this column`, value: null }]
+              : [
+                  { name: t`Largest value in each column`, value: null },
+                  {
+                    name: t`Largest value in all of these columns`,
+                    value: "all",
+                  },
+                ]
+            ).concat([{ name: t`Custom value`, value: "custom" }])}
+            vertical
+          />
+          {rule.max_type === "custom" && (
+            <NumericInput
+              className={INPUT_CLASSNAME}
+              type="number"
+              value={rule.max_value}
+              onChange={max_value => onChange({ ...rule, max_value })}
+            />
+          )}
+        </div>
+      ) : null}
+      <div className="mt4">
+        {rule.columns.length === 0 ? (
+          <Button
+            primary
+            onClick={onRemove}
+            data-metabase-event={`Chart Settings;Table Formatting;`}
+          >
+            {isNew ? t`Cancel` : t`Delete`}
+          </Button>
+        ) : (
+          <Button
+            primary
+            onClick={onDone}
+            data-metabase-event={`Chart Setttings;Table Formatting;${
+              isNew ? "Add Rule" : "Update Rule"
+            };Rule Type ${rule.type} Color`}
+          >
+            {isNew ? t`Add rule` : t`Update rule`}
+          </Button>
         )}
       </div>
-    ) : null}
-    <div className="mt4">
-      {rule.columns.length === 0 ? (
-        <Button
-          primary
-          onClick={onRemove}
-          data-metabase-event={`Chart Settings;Table Formatting;`}
-        >
-          {isNew ? t`Cancel` : t`Delete`}
-        </Button>
-      ) : (
-        <Button
-          primary
-          onClick={onDone}
-          data-metabase-event={`Chart Setttings;Table Formatting;${
-            isNew ? "Add Rule" : "Update Rule"
-          };Rule Type ${rule.type} Color`}
-        >
-          {isNew ? t`Add rule` : t`Update rule`}
-        </Button>
-      )}
     </div>
-  </div>
-);
+  );
+};
 
 const ColorRangePicker = ({ colors, onChange, className, style }) => (
   <PopoverWithTrigger
@@ -446,12 +506,3 @@ const ColorRangePicker = ({ colors, onChange, className, style }) => (
     )}
   </PopoverWithTrigger>
 );
-
-const NumericInput = ({ value, onChange }) => (
-  <input
-    className="AdminSelect input mt1 full"
-    type="number"
-    value={value}
-    onChange={e => onChange(e.target.value)}
-  />
-);
diff --git a/frontend/src/metabase/visualizations/lib/table_format.js b/frontend/src/metabase/visualizations/lib/table_format.js
index 4bd0004ffa4..6ded6e5b80c 100644
--- a/frontend/src/metabase/visualizations/lib/table_format.js
+++ b/frontend/src/metabase/visualizations/lib/table_format.js
@@ -21,8 +21,8 @@ type SingleFormat = {
   type: "single",
   columns: ColumnName[],
   color: Color,
-  operator: "<" | ">" | "<=" | ">=" | "=" | "!=",
-  value: number,
+  operator: "<" | ">" | "<=" | ">=" | "=" | "!=" | "is-null" | "not-null",
+  value: number | string,
   highlight_row: boolean,
 };
 
@@ -127,17 +127,21 @@ function compileFormatter(
     }
     switch (operator) {
       case "<":
-        return v => (v < value ? color : null);
+        return v => (typeof value === "number" && v < value ? color : null);
       case "<=":
-        return v => (v <= value ? color : null);
+        return v => (typeof value === "number" && v <= value ? color : null);
       case ">=":
-        return v => (v >= value ? color : null);
+        return v => (typeof value === "number" && v >= value ? color : null);
       case ">":
-        return v => (v > value ? color : null);
+        return v => (typeof value === "number" && v > value ? color : null);
       case "=":
         return v => (v === value ? color : null);
       case "!=":
         return v => (v !== value ? color : null);
+      case "is-null":
+        return v => (v === null ? color : null);
+      case "not-null":
+        return v => (v !== null ? color : null);
     }
   } else if (format.type === "range") {
     const columnMin = name =>
-- 
GitLab