diff --git a/frontend/src/metabase/components/FieldValuesWidget.jsx b/frontend/src/metabase/components/FieldValuesWidget.jsx index 7f5b1d333790918014b1d3723d889779cf539d8f..21828460f5d42d1c86760d536ad6a1a3334dfb3a 100644 --- a/frontend/src/metabase/components/FieldValuesWidget.jsx +++ b/frontend/src/metabase/components/FieldValuesWidget.jsx @@ -385,6 +385,7 @@ export class FieldValuesWidget extends Component { className, style, parameter, + prefix, } = this.props; const { loadingState } = this.state; @@ -429,6 +430,7 @@ export class FieldValuesWidget extends Component { )} {!hasListData && !isFetchingList && ( <TokenField + prefix={prefix} value={value.filter(v => v != null)} onChange={onChange} placeholder={placeholder} diff --git a/frontend/src/metabase/components/TokenField.jsx b/frontend/src/metabase/components/TokenField.jsx index 04891d52695d70930e0bd562a7f7afd44a04c0e7..84c7de25a4a32caa8746fd8157455756ff01f4f6 100644 --- a/frontend/src/metabase/components/TokenField.jsx +++ b/frontend/src/metabase/components/TokenField.jsx @@ -454,6 +454,7 @@ export default class TokenField extends Component { valueStyle, optionsStyle, optionsClassName, + prefix, } = this.props; let { inputValue, @@ -500,11 +501,16 @@ export default class TokenField extends Component { <ul className={cx( className, - "pl1 pt1 pb0 pr0 flex flex-wrap bg-white scroll-x scroll-y", + "pl1 pt1 pb0 pr0 flex align-center flex-wrap bg-white scroll-x scroll-y", )} style={{ maxHeight: 130, ...style }} onMouseDownCapture={this.onMouseDownCapture} > + {!!prefix && ( + <span className="text-medium mb1 py1 pr1" data-testid="input-prefix"> + {prefix} + </span> + )} {value.map((v, index) => ( <TokenFieldItem key={index} isValid={validateValue(v)}> <span diff --git a/frontend/src/metabase/lib/formatting.js b/frontend/src/metabase/lib/formatting.js index 91a70c20466d6a564fdd78c274b6b1d90e68f4af..7bbd323cde99867347de2f1a9c14e661cc543542 100644 --- a/frontend/src/metabase/lib/formatting.js +++ b/frontend/src/metabase/lib/formatting.js @@ -44,6 +44,7 @@ import { renderLinkURLForClick, } from "metabase/lib/formatting/link"; import { NULL_DISPLAY_VALUE, NULL_NUMERIC_VALUE } from "metabase/lib/constants"; +import { currency } from "cljs/metabase.shared.util.currency"; // a one or two character string specifying the decimal and grouping separator characters @@ -108,6 +109,15 @@ export function numberFormatterForOptions(options) { }); } +let currencyMapCache; +export function getCurrencySymbol(currencyCode) { + if (!currencyMapCache) { + // only turn the array into a map if we call this function + currencyMapCache = Object.fromEntries(currency); + } + return currencyMapCache[currencyCode]?.symbol || currencyCode || "$"; +} + export function formatNumber(number, options = {}) { options = { ...getDefaultNumberOptions(options), ...options }; diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx index 38600c108a3a9856b4504bab49e6b9f8fe2591b4..10ea8a06af0e6504d07b0a427db2de8c33c72c93 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx @@ -13,8 +13,13 @@ import FieldValuesWidget from "metabase/components/FieldValuesWidget"; import { getFilterArgumentFormatOptions, isFuzzyOperator, + isCurrency, } from "metabase/lib/schema_metadata"; +import { getCurrencySymbol } from "metabase/lib/formatting"; + +import { keyForColumn } from "metabase/lib/dataset"; + import { BetweenLayoutContainer, BetweenLayoutFieldSeparator, @@ -61,6 +66,25 @@ export default function DefaultPicker({ const isBetweenLayout = operator.name === "between" && operatorFields.length === 2; + const visualizationSettings = filter + ?.query() + ?.question() + ?.settings(); + + const key = keyForColumn(dimension.column()); + const columnSettings = visualizationSettings?.column_settings?.[key]; + + const fieldMetadata = field?.metadata?.fields[field?.id]; + const fieldSettings = { + ...(fieldMetadata?.settings ?? {}), + ...(columnSettings ?? {}), + }; + + const currencyPrefix = + isCurrency(field) || fieldSettings?.currency + ? getCurrencySymbol(fieldSettings?.currency) + : null; + const fieldWidgets = operatorFields .map((operatorField, index) => { let values, onValuesChange; @@ -102,6 +126,7 @@ export default function DefaultPicker({ multi={operator.multi} placeholder={placeholder} fields={underlyingField ? [underlyingField] : []} + prefix={currencyPrefix} disablePKRemappingForSearch={true} autoFocus={index === 0} alwaysShowOptions={operator.fields.length === 1} @@ -131,6 +156,7 @@ export default function DefaultPicker({ values={values} onValuesChange={onValuesChange} placeholder={placeholder} + prefix={currencyPrefix} multi={operator.multi} onCommit={onCommit} /> diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx index 190e0bc27361234776ea2b607e72396cf46c0ebf..da46f02853b1daa8d0946337cfa18c3e1644af52 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx @@ -49,6 +49,7 @@ export default class NumberPicker extends Component { <TextPicker {...this.props} isSingleLine + prefix={this.props.prefix} values={values} validations={this.state.validations} onValuesChange={values => this.onValuesChange(values)} diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx index 3d428ff27b292161f31a8cfdd6d0d8718b30303f..01bfdf804996a3cab8ae1128dc02888bc811627d 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx @@ -56,6 +56,7 @@ export default class TextPicker extends Component { onCommit, isSingleLine, autoFocus, + prefix, } = this.props; const hasInvalidValues = _.some(validations, v => v === false); @@ -67,7 +68,16 @@ export default class TextPicker extends Component { return ( <div> - <div className="FilterInput px1 pt1 relative"> + <div className="FilterInput px1 pt1 relative flex align-center"> + {!!prefix && ( + <span + data-testid="input-prefix" + className="text-medium px1" + style={{ marginRight: -30, width: 30, zIndex: 2 }} + > + {prefix} + </span> + )} {!isSingleLine && ( <AutosizeTextarea className={cx("input block full border-purple", { @@ -89,6 +99,11 @@ export default class TextPicker extends Component { className={cx("input block full border-purple", { "border-error": hasInvalidValues, })} + style={{ + paddingLeft: this.props.prefix + ? `${this.props.prefix.length}.2rem` + : "", + }} type="text" value={this.state.fieldString} onChange={e => this.setValue(e.target.value)} diff --git a/frontend/src/metabase/visualizations/lib/settings/column.js b/frontend/src/metabase/visualizations/lib/settings/column.js index 596ce9702fa3eaace245420a5352efbf8303bef0..c190073dac52b7bd27bb30b0b93cc8a8600734fd 100644 --- a/frontend/src/metabase/visualizations/lib/settings/column.js +++ b/frontend/src/metabase/visualizations/lib/settings/column.js @@ -24,6 +24,7 @@ function getVisualizationRaw(...args) { import { formatColumn, numberFormatterForOptions, + getCurrencySymbol, } from "metabase/lib/formatting"; import { getDateFormatFromStyle, @@ -302,7 +303,7 @@ export const NUMBER_COLUMN_SETTINGS = { widget: "radio", getProps: (column, settings) => { const c = settings["currency"] || "USD"; - const symbol = getCurrency(c, "symbol"); + const symbol = getCurrencySymbol(c); const code = getCurrency(c, "code"); const name = getCurrency(c, "name"); return { @@ -328,7 +329,7 @@ export const NUMBER_COLUMN_SETTINGS = { }, getDefault: (column, settings) => { const c = settings["currency"] || "USD"; - return getCurrency(c, "symbol") !== getCurrency(c, "code") + return getCurrencySymbol(c) !== getCurrency(c, "code") ? "symbol" : "code"; }, @@ -401,6 +402,9 @@ export const NUMBER_COLUMN_SETTINGS = { settings["number_style"] === "currency" && settings["currency_in_header"] ) { + if (settings["currency_style"] === "symbol") { + return getCurrencySymbol(settings["currency"]); + } return getCurrency(settings["currency"], settings["currency_style"]); } return null; diff --git a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js index 9090eeea00a6362bed1e213ba65fabd46f2a26d2..449c30812c9de7bbd24891c43d7af3dc20624619 100644 --- a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js +++ b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js @@ -33,7 +33,7 @@ describe("FieldValuesWidget", () => { it("should have 'Enter some text' as the placeholder text", () => { renderFieldValuesWidget({ ...props }); - screen.findByLabelText("Enter some text"); + screen.getByPlaceholderText("Enter some text"); }); }); describe("has_field_values = list", () => { @@ -47,16 +47,19 @@ describe("FieldValuesWidget", () => { expect(fetchFieldValues).toHaveBeenCalledWith(PRODUCTS.CATEGORY.id); }); - it("should not have 'Search the list' as the placeholder text for fields with less or equal than 10 values", () => { + // current version of this component always shows the search box + it.skip("should not have 'Search the list' as the placeholder text for fields with less or equal than 10 values", async () => { renderFieldValuesWidget({ ...props }); - expect(screen.queryByLabelText("Search the list")).toBeNull(); + expect( + await screen.queryByPlaceholderText("Search the list"), + ).toBeNull(); }); - it("should have 'Search the list' as the placeholder text for fields with less than 10 values", () => { + it("should have 'Enter some text' as the placeholder text for fields with more than 10 values", () => { renderFieldValuesWidget({ fields: [mock(PRODUCTS.TITLE, { has_field_values: "list" })], }); - screen.findByLabelText("Search the list"); + screen.getByPlaceholderText("Enter some text"); }); }); @@ -73,7 +76,7 @@ describe("FieldValuesWidget", () => { it("should have 'Search by Category' as the placeholder text", () => { renderFieldValuesWidget({ ...props }); - screen.findByLabelText("Search the list"); + screen.getByPlaceholderText("Search by Category"); }); }); }); @@ -84,7 +87,7 @@ describe("FieldValuesWidget", () => { renderFieldValuesWidget({ fields: [mock(ORDERS.PRODUCT_ID, { has_field_values: "none" })], }); - screen.findByLabelText("Enter an ID"); + screen.getByPlaceholderText("Enter an ID"); }); }); @@ -98,7 +101,7 @@ describe("FieldValuesWidget", () => { }), ], }); - screen.findByLabelText("Search the list"); + screen.getByPlaceholderText("Search the list"); }); }); @@ -112,10 +115,10 @@ describe("FieldValuesWidget", () => { }), ], }); - screen.findByLabelText("Search by Category or enter an ID"); + screen.getByPlaceholderText("Search by Category or enter an ID"); }); - it("should not duplicate 'ID' in placeholder when ID itself is searchable", () => { + it("should not duplicate 'ID' in placeholder when ID itself is searchable", async () => { const fields = [ mock(ORDERS.PRODUCT_ID, { base_type: "type/Text", @@ -123,19 +126,19 @@ describe("FieldValuesWidget", () => { }), ]; renderFieldValuesWidget({ fields }); - screen.findByLabelText("Search by Product"); + screen.getByPlaceholderText("Search by Product"); }); }); }); describe("multiple fields", () => { - it("list multiple fields together", () => { + it("list multiple fields together", async () => { const fields = [ mock(PEOPLE.SOURCE, { has_field_values: "list" }), mock(PEOPLE.STATE, { has_field_values: "list" }), ]; renderFieldValuesWidget({ fields }); - screen.findByLabelText("Search the list"); + screen.getByPlaceholderText("Search the list"); screen.getByText("AZ"); screen.getByText("Facebook"); @@ -147,7 +150,7 @@ describe("FieldValuesWidget", () => { mock(PEOPLE.STATE, { has_field_values: "list" }), ]; renderFieldValuesWidget({ fields }); - screen.findByLabelText("Search"); + screen.getByPlaceholderText("Search"); expect(screen.queryByText("AZ")).toBeNull(); expect(screen.queryByText("Facebook")).toBeNull(); @@ -159,10 +162,27 @@ describe("FieldValuesWidget", () => { mock(PEOPLE.STATE, { has_field_values: "list" }), ]; renderFieldValuesWidget({ fields }); - screen.findByLabelText("Enter some text"); + screen.getByPlaceholderText("Enter some text"); expect(screen.queryByText("AZ")).toBeNull(); expect(screen.queryByText("Facebook")).toBeNull(); }); }); + + describe("prefix", () => { + it("should render a passed prefix", () => { + renderFieldValuesWidget({ + fields: [mock(PRODUCTS.PRICE, { has_field_values: "none" })], + prefix: "$$$", + }); + expect(screen.getByTestId("input-prefix")).toHaveTextContent("$$$"); + }); + + it("should not render a prefix if omitted", () => { + renderFieldValuesWidget({ + fields: [mock(PRODUCTS.PRICE, { has_field_values: "none" })], + }); + expect(screen.queryByTestId("input-prefix")).toBeNull(); + }); + }); }); diff --git a/frontend/test/metabase/components/TokenField.unit.spec.js b/frontend/test/metabase/components/TokenField.unit.spec.js index 416d6b395f598a35095837e1fb6bc2a9667f9499..abff6d550b148de14ea19f5dc139a3f3b7bb1db9 100644 --- a/frontend/test/metabase/components/TokenField.unit.spec.js +++ b/frontend/test/metabase/components/TokenField.unit.spec.js @@ -108,6 +108,16 @@ describe("TokenField", () => { expect(screen.queryByText("bar")).toBeNull(); }); + it("should render input prefix with prefix prop", () => { + render(<TokenFieldWithStateAndDefaults prefix="$$$" />); + expect(screen.getByTestId("input-prefix")).toHaveTextContent("$$$"); + }); + + it("should not render input prefix without prefix prop", () => { + render(<TokenFieldWithStateAndDefaults />); + expect(screen.queryByTestId("input-prefix")).toBeNull(); + }); + it("should render with 1 options and 1 values", () => { render( <TokenFieldWithStateAndDefaults value={["foo"]} options={["bar"]} />, diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js index 92cbf5742ece8428e631b661fc3ad6c2df520582..d0dd6e4fe8858141a903a83be3bfc683ea206651 100644 --- a/frontend/test/metabase/lib/formatting.unit.spec.js +++ b/frontend/test/metabase/lib/formatting.unit.spec.js @@ -10,6 +10,7 @@ import { formatTime, formatTimeWithUnit, slugify, + getCurrencySymbol, } from "metabase/lib/formatting"; import ExternalLink from "metabase/core/components/ExternalLink"; import { TYPE } from "metabase/lib/types"; @@ -605,4 +606,26 @@ describe("formatting", () => { expect(slugify("än umlaut")).toEqual("%C3%A4n_umlaut"); }); }); + + describe("getCurrencySymbol", () => { + const currencySymbols = [ + ["USD", "$"], + ["EUR", "€"], + ["GBP", "£"], + ["JPY", "¥"], + ["CNY", "CN¥"], + ["CAD", "CA$"], + ["AUD", "AU$"], + ["NZD", "NZ$"], + ["HKD", "HK$"], + ["BTC", "₿"], + ["OOPS", "OOPS"], + ]; + + currencySymbols.forEach(([currency, symbol]) => { + it(`should get a ${symbol} for ${currency}`, () => { + expect(getCurrencySymbol(currency)).toEqual(symbol); + }); + }); + }); }); diff --git a/frontend/test/metabase/query_builder/components/filters/pickers/NumberPicker.unit.spec.js b/frontend/test/metabase/query_builder/components/filters/pickers/NumberPicker.unit.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..8ba04ade2c16d85a3a1400f03a61070782a84ef7 --- /dev/null +++ b/frontend/test/metabase/query_builder/components/filters/pickers/NumberPicker.unit.spec.js @@ -0,0 +1,47 @@ +import React from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; + +import NumberPicker from "metabase/query_builder/components/filters/pickers/NumberPicker"; + +describe("NumberPicker", () => { + it("should display provided values", () => { + const spy = jest.fn(); + render(<NumberPicker values={[16, 17]} onValuesChange={spy} />); + screen.getByDisplayValue("16, 17"); + }); + + it("should fire onValuesChange function on change", async () => { + const spy = jest.fn(); + render(<NumberPicker values={[16, 17]} onValuesChange={spy} />); + + await fireEvent.change( + screen.getByPlaceholderText("Enter desired number"), + { target: { value: 18 } }, + ); + expect(spy).toHaveBeenCalledWith([18]); + }); + + it("should display error border on non-numeric values", async () => { + const spy = jest.fn(); + const { container } = render( + <NumberPicker values={["foo", "bar"]} onValuesChange={spy} />, + ); + expect(container.querySelectorAll(".border-error")).toHaveLength(1); + }); + + it("should display prefix if prefix prop is populated", async () => { + const spy = jest.fn(); + render( + <NumberPicker values={[16, 17]} prefix="$$$" onValuesChange={spy} />, + ); + + expect(screen.getByTestId("input-prefix")).toHaveTextContent("$$$"); + }); + + it("should not display prefix if prefix prop is not populated", async () => { + const spy = jest.fn(); + render(<NumberPicker values={["foo", "bar"]} onValuesChange={spy} />); + + expect(screen.queryByTestId("input-prefix")).toBeNull(); + }); +}); diff --git a/frontend/test/metabase/query_builder/components/filters/pickers/TextPicker.unit.spec.js b/frontend/test/metabase/query_builder/components/filters/pickers/TextPicker.unit.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..31da763f88e2ddd4661b6c587fe0af74e9330fd8 --- /dev/null +++ b/frontend/test/metabase/query_builder/components/filters/pickers/TextPicker.unit.spec.js @@ -0,0 +1,38 @@ +import React from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; + +import TextPicker from "metabase/query_builder/components/filters/pickers/TextPicker"; + +describe("TextPicker", () => { + it("should display provided values", () => { + const spy = jest.fn(); + render(<TextPicker values={["foo", "bar"]} onValuesChange={spy} />); + screen.getByDisplayValue("foo, bar"); + }); + + it("should fire onValuesChange function on change", async () => { + const spy = jest.fn(); + render(<TextPicker values={["foo", "bar"]} onValuesChange={spy} />); + + await fireEvent.change(screen.getByPlaceholderText("Enter desired text"), { + target: { value: "baz" }, + }); + expect(spy).toHaveBeenCalledWith(["baz"]); + }); + + it("should display prefix if prefix prop is populated", async () => { + const spy = jest.fn(); + render( + <TextPicker values={["foo", "bar"]} prefix="$$$" onValuesChange={spy} />, + ); + + expect(screen.getByTestId("input-prefix")).toHaveTextContent("$$$"); + }); + + it("should not display prefix if prefix prop is not populated", async () => { + const spy = jest.fn(); + render(<TextPicker values={["foo", "bar"]} onValuesChange={spy} />); + + expect(screen.queryByTestId("input-prefix")).toBeNull(); + }); +}); diff --git a/frontend/test/metabase/scenarios/question/filter.cy.spec.js b/frontend/test/metabase/scenarios/question/filter.cy.spec.js index 2bf1402d157a894bb97b2371f00e1a835e0a5c0a..00492f46ebb433675e1a73e35ba2e2d35ac7304b 100644 --- a/frontend/test/metabase/scenarios/question/filter.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/filter.cy.spec.js @@ -799,6 +799,37 @@ describe("scenarios > question > filter", () => { } }); + describe("currency filters", () => { + beforeEach(() => { + // set the currency on the Orders/Discount column to Euro + cy.visit("/admin/datamodel/database/1/table/2"); + // this value isn't actually selected, it's just the default + cy.findByText("US Dollar").click(); + cy.findByText("Euro").click(); + + openOrdersTable(); + }); + + it("should show correct currency symbols in currency single field filter", () => { + cy.findByText("Discount (€)").click(); + cy.findByText("Filter by this column").click(); + cy.findByTestId("input-prefix").should("contain", "€"); + }); + + it("should show correct currency symbols in currency between field filter", () => { + cy.findByText("Discount (€)").click(); + cy.findByText("Filter by this column").click(); + cy.findByText("Equal to").click(); + cy.findByText("Between").click(); + + cy.findAllByTestId("input-prefix").then(els => { + expect(els).to.have.lengthOf(2); + expect(els[0].innerText).to.equal("€"); + expect(els[1].innerText).to.equal("€"); + }); + }); + }); + describe("specific combination of filters can cause frontend reload or blank screen (metabase#16198)", () => { it("shouldn't display chosen category in a breadcrumb (metabase#16198-1)", () => { visitQuestionAdhoc({ diff --git a/shared/src/metabase/shared/util/currency.cljc b/shared/src/metabase/shared/util/currency.cljc index 0ab626fd567d1b6cf6bd752356c886e9b7d223c2..b0dec43416a769ceb12914a941b7e5e35a7970c4 100644 --- a/shared/src/metabase/shared/util/currency.cljc +++ b/shared/src/metabase/shared/util/currency.cljc @@ -151,7 +151,7 @@ :rounding 0, :code "BRL", :name_plural "Brazilian reals"}], - [:BTC {:symbol "BTC", + [:BTC {:symbol "₿", :name "Bitcoin", :symbol_native "BTC", :decimal_digits 8,