Skip to content
Snippets Groups Projects
Unverified Commit 333fbad5 authored by Nick Fitzpatrick's avatar Nick Fitzpatrick Committed by GitHub
Browse files

Adding is-true and is-false operators (#33062)


* Adding is-true and is-false operators

* Adding unit tests

* All Operators adjustment

* PR Feedback

* add "is equal to" in spec

* adding e2e test

---------

Co-authored-by: default avatarJesse Devaney <22608765+JesseSDevaney@users.noreply.github.com>
parent 02a0143d
No related branches found
No related tags found
No related merge requests found
......@@ -190,6 +190,15 @@ export function getTableId({ databaseId = WRITABLE_DB_ID, name }) {
});
}
export function getTable({ databaseId = WRITABLE_DB_ID, name }) {
return cy
.request("GET", `/api/database/${databaseId}/metadata`)
.then(({ body }) => {
const table = body?.tables?.find(table => table.name === name);
return table || null;
});
}
export const createModelFromTableName = ({
tableName,
modelName = "Test Action Model",
......
......@@ -8,8 +8,15 @@ import {
restore,
summarize,
visualize,
resetTestTable,
resyncDatabase,
visitQuestionAdhoc,
getTable,
leftSidebar,
} from "e2e/support/helpers";
import { WRITABLE_DB_ID } from "e2e/support/cypress_data";
describe("scenarios > visualizations > table", () => {
beforeEach(() => {
restore();
......@@ -307,6 +314,71 @@ describe("scenarios > visualizations > table", () => {
});
});
describe("scenarios > visualizations > table > conditional formatting", () => {
beforeEach(() => {
resetTestTable({ type: "postgres", table: "many_data_types" });
restore(`postgres-writable`);
cy.signInAsAdmin();
resyncDatabase({
dbId: WRITABLE_DB_ID,
tableName: "many_data_types",
});
getTable({ name: "many_data_types" }).then(({ id: tableId, fields }) => {
const booleanField = fields.find(field => field.name === "boolean");
const stringField = fields.find(field => field.name === "string");
const idField = fields.find(field => field.name === "id");
visitQuestionAdhoc({
dataset_query: {
database: WRITABLE_DB_ID,
query: {
"source-table": tableId,
fields: [
["field", idField.id, { "base-type": idField["base_type"] }],
[
"field",
stringField.id,
{ "base-type": stringField["base_type"] },
],
[
"field",
booleanField.id,
{ "base-type": booleanField["base_type"] },
],
],
},
type: "query",
},
display: "table",
});
});
});
it("should work with boolean columns", () => {
cy.findByTestId("viz-settings-button").click();
leftSidebar().findByText("Conditional Formatting").click();
cy.findByRole("button", { name: /add a rule/i }).click();
popover().findByRole("option", { name: "Boolean" }).click();
//Dismiss popover
leftSidebar().findByText("Which columns should be affected?").click();
//Check that is-true was applied by default to boolean field rule
cy.findByTestId("conditional-formatting-value-operator-button").should(
"contain.text",
"is true",
);
cy.findByRole("gridcell", { name: "true" }).should(
"have.css",
"background-color",
"rgba(80, 158, 227, 0.65)",
);
});
});
function headerCells() {
return cy.findAllByTestId("header-cell");
}
......@@ -127,6 +127,7 @@ export const AccordionListCell = ({
aria-label={name}
role="option"
aria-selected={isSelected}
aria-disabled={!isClickable}
isClickable={isClickable}
className={cx(
"List-item flex mx1",
......
......@@ -72,7 +72,7 @@
}
.List-item--disabled .List-item-title {
color: var(--color-text-medium);
color: var(--color-text-light);
}
.List-item--cursor:not(.List-item--disabled) .List-item-title,
......
......@@ -540,6 +540,7 @@ class TableInteractive extends Component {
return (
<div
key={key}
role="gridcell"
style={{
...style,
// use computed left if dragging
......
/* eslint-disable react/prop-types */
import { Component } from "react";
import { t, jt } from "ttag";
import { t, jt, msgid, ngettext } from "ttag";
import _ from "underscore";
import cx from "classnames";
......@@ -28,33 +28,41 @@ import {
} from "metabase/components/sortable";
import * as MetabaseAnalytics from "metabase/lib/analytics";
import { isNumeric, isString } from "metabase-lib/types/utils/isa";
import { isNumeric, isString, isBoolean } from "metabase-lib/types/utils/isa";
const COMMON_OPERATOR_NAMES = {
"is-null": t`is null`,
"not-null": t`is not null`,
};
const NUMBER_OPERATOR_NAMES = {
"=": t`is equal to`,
"!=": t`is not equal to`,
"<": t`is less than`,
">": t`is greater than`,
"<=": t`is less than or equal to`,
">=": t`is greater than or equal to`,
"=": t`is equal to`,
"!=": t`is not equal to`,
"is-null": t`is null`,
"not-null": t`is not null`,
};
const STRING_OPERATOR_NAMES = {
"=": t`is equal to`,
"!=": t`is not equal to`,
"is-null": t`is null`,
"not-null": t`is not null`,
contains: t`contains`,
"does-not-contain": t`does not contain`,
"starts-with": t`starts with`,
"ends-with": t`ends with`,
};
const BOOLEAN_OPERATIOR_NAMES = {
"is-true": t`is true`,
"is-false": t`is false`,
};
export const ALL_OPERATOR_NAMES = {
...NUMBER_OPERATOR_NAMES,
...STRING_OPERATOR_NAMES,
...BOOLEAN_OPERATIOR_NAMES,
...COMMON_OPERATOR_NAMES,
};
// TODO
......@@ -94,6 +102,7 @@ export default class ChartSettingsTableFormatting extends Component {
editingRule: null,
editingRuleIsNew: null,
};
render() {
const { value, onChange, cols, canHighlightRow } = this.props;
const { editingRule, editingRuleIsNew } = this.state;
......@@ -104,13 +113,13 @@ export default class ChartSettingsTableFormatting extends Component {
rule={value[editingRule]}
cols={cols}
isNew={editingRuleIsNew}
onChange={rule =>
onChange={rule => {
onChange([
...value.slice(0, editingRule),
rule,
...value.slice(editingRule + 1),
])
}
]);
}}
onRemove={() => {
onChange([
...value.slice(0, editingRule),
......@@ -309,20 +318,42 @@ const RuleEditor = ({
canHighlightRow = true,
}) => {
const selectedColumns = rule.columns.map(name => _.findWhere(cols, { name }));
const hasBooleanRule =
selectedColumns.length > 0 && selectedColumns.some(isBoolean);
const isBooleanRule =
selectedColumns.length > 0 && selectedColumns.every(isBoolean);
const isStringRule =
selectedColumns.length > 0 && _.all(selectedColumns, isString);
!hasBooleanRule &&
selectedColumns.length > 0 &&
selectedColumns.every(isString);
const isNumericRule =
selectedColumns.length > 0 && _.all(selectedColumns, isNumeric);
!hasBooleanRule &&
selectedColumns.length > 0 &&
selectedColumns.every(isNumeric);
const hasOperand =
rule.operator !== "is-null" && rule.operator !== "not-null";
rule.operator !== "is-null" &&
rule.operator !== "not-null" &&
rule.operator !== "is-true" &&
rule.operator !== "is-false";
const handleColumnChange = columns => {
const _cols = columns.map(name => _.findWhere(cols, { name }));
const operatorUpdate =
columns.length === 1 && columns[0] === columns.changedItem
? {
operator: _cols.every(isBoolean) ? "is-true" : "=",
}
: {};
onChange({ ...rule, columns, ...operatorUpdate });
};
return (
<div>
<h3 className="mb1">{t`Which columns should be affected?`}</h3>
<Select
value={rule.columns}
onChange={e => onChange({ ...rule, columns: e.target.value })}
onChange={e => handleColumnChange(e.target.value)}
isInitiallyOpen={rule.columns.length === 0}
placeholder={t`Choose a column`}
multiple
......@@ -332,8 +363,9 @@ const RuleEditor = ({
key={col.name}
value={col.name}
disabled={
(isStringRule && !isString(col)) ||
(isNumericRule && !isNumeric(col))
(isStringRule && (!isString(col) || isBoolean(col))) ||
(isNumericRule && !isNumeric(col)) ||
(isBooleanRule && !isBoolean(col))
}
>
{col.display_name}
......@@ -358,14 +390,30 @@ const RuleEditor = ({
)}
{rule.type === "single" ? (
<div>
<h3 className="mt3 mb1">{t`When a cell in this column…`}</h3>
<h3 className="mt3 mb1">
{ngettext(
msgid`When a cell in this column…`,
`When any cell in these columns…`,
selectedColumns.length,
)}
</h3>
<Select
value={rule.operator}
onChange={e => onChange({ ...rule, operator: e.target.value })}
buttonProps={{
"data-testid": "conditional-formatting-value-operator-button",
}}
>
{Object.entries(
isNumericRule ? NUMBER_OPERATOR_NAMES : STRING_OPERATOR_NAMES,
).map(([operator, operatorName]) => (
{Object.entries({
...COMMON_OPERATOR_NAMES,
...(isBooleanRule
? BOOLEAN_OPERATIOR_NAMES
: isNumericRule
? NUMBER_OPERATOR_NAMES
: isStringRule
? STRING_OPERATOR_NAMES
: {}),
}).map(([operator, operatorName]) => (
<Option key={operatorName} value={operator}>
{operatorName}
</Option>
......
import { useState } from "react";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { createMockColumn } from "metabase-types/api/mocks";
import ChartSettingsTableFormatting from "./ChartSettingsTableFormatting";
const STRING_COLUMN = createMockColumn({
base_type: "type/Text",
display_name: "String Column",
name: "STRING_COLUMN",
});
const STRING_COLUMN_TWO = createMockColumn({
base_type: "type/Text",
display_name: "String Column 2",
name: "STRING_COLUMN_2",
});
const BOOLEAN_COLUMN = createMockColumn({
base_type: "type/Boolean",
display_name: "Boolean Column",
name: "BOOLEAN_COLUMN",
});
const NUMBER_COLUMN = createMockColumn({
base_type: "type/Integer",
display_name: "Number Column",
name: "NUMBER_COLUMN",
});
const COLUMNS = [
STRING_COLUMN,
STRING_COLUMN_TWO,
BOOLEAN_COLUMN,
NUMBER_COLUMN,
];
const STRING_OPERATORS = [
"is null",
"is not null",
"is equal to",
"is not equal to",
"contains",
"does not contain",
"starts with",
"ends with",
];
const NUMBER_OPERATORS = [
"is null",
"is not null",
"is equal to",
"is not equal to",
"is less than",
"is greater than",
"is less than or equal to",
"is greater than or equal to",
];
const BOOLEAN_OPERATORS = ["is null", "is not null", "is true", "is false"];
const Wrapper = props => {
const [value, setValue] = useState([]);
return (
<ChartSettingsTableFormatting
cols={COLUMNS}
onChange={setValue}
value={value}
{...props}
/>
);
};
const setup = props => {
render(<Wrapper {...props} />);
};
describe("ChartSettingsTableFormatting", () => {
it("should allow you to add a rule", () => {
setup();
expect(screen.getByText("Conditional formatting")).toBeInTheDocument();
userEvent.click(screen.getByText("Add a rule"));
userEvent.click(screen.getByText("String Column"));
//Dismiss Popup
userEvent.click(screen.getByText("Which columns should be affected?"));
expect(screen.getByText("is equal to")).toBeInTheDocument();
userEvent.type(
screen.getByTestId("conditional-formatting-value-input"),
"toucan",
);
userEvent.click(screen.getByText("Add rule"));
expect(screen.getByText("String Column")).toBeInTheDocument();
expect(screen.getByText(/is equal to toucan/g)).toBeInTheDocument();
});
it("should only let you choose columns of the same type for a rule", () => {
setup();
expect(screen.getByText("Conditional formatting")).toBeInTheDocument();
userEvent.click(screen.getByText("Add a rule"));
userEvent.click(screen.getByText("String Column"));
expect(
screen.getByRole("option", { name: "Number Column" }),
).toHaveAttribute("aria-disabled", "true");
expect(
screen.getByRole("option", { name: "Boolean Column" }),
).toHaveAttribute("aria-disabled", "true");
expect(
screen.getByRole("option", { name: "String Column 2" }),
).toHaveAttribute("aria-disabled", "false");
});
describe("should show appropriate operators based on column selection", () => {
beforeEach(() => {
setup();
userEvent.click(screen.getByText("Add a rule"));
});
it("string", () => {
userEvent.click(screen.getByText("String Column"));
//Dismiss Popup
userEvent.click(screen.getByText("Which columns should be affected?"));
userEvent.click(
screen.getByTestId("conditional-formatting-value-operator-button"),
);
STRING_OPERATORS.forEach(operator => {
expect(
screen.getByRole("option", { name: operator }),
).toBeInTheDocument();
});
});
it("number", () => {
userEvent.click(screen.getByText("Number Column"));
//Dismiss Popup
userEvent.click(screen.getByText("Which columns should be affected?"));
userEvent.click(
screen.getByTestId("conditional-formatting-value-operator-button"),
);
NUMBER_OPERATORS.forEach(operator => {
expect(
screen.getByRole("option", { name: operator }),
).toBeInTheDocument();
});
// Quick check for color range option on numeric rules
expect(screen.getByText("Formatting style")).toBeInTheDocument();
expect(
screen.getByRole("radio", { name: /single color/i }),
).toBeChecked();
expect(
screen.getByRole("radio", { name: /color range/i }),
).not.toBeChecked();
});
it("boolean", () => {
userEvent.click(screen.getByText("Boolean Column"));
//Dismiss Popup
userEvent.click(screen.getByText("Which columns should be affected?"));
userEvent.click(
screen.getByTestId("conditional-formatting-value-operator-button"),
);
BOOLEAN_OPERATORS.forEach(operator => {
expect(
screen.getByRole("option", { name: operator }),
).toBeInTheDocument();
});
});
});
});
......@@ -94,6 +94,8 @@ export const OPERATOR_FORMATTER_FACTORIES = {
canCompareSubstrings(value, v) && v.startsWith(value) ? color : null,
"ends-with": (value, color) => v =>
canCompareSubstrings(value, v) && v.endsWith(value) ? color : null,
"is-true": (_value, color) => v => v ? color : null,
"is-false": (_value, color) => v => v ? null : color,
};
export function compileFormatter(
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment