diff --git a/client/app/components/visualizations/visualizationComponents.jsx b/client/app/components/visualizations/visualizationComponents.jsx index fcfc2926ed..890d500316 100644 --- a/client/app/components/visualizations/visualizationComponents.jsx +++ b/client/app/components/visualizations/visualizationComponents.jsx @@ -59,6 +59,8 @@ function wrapComponentWithSettings(WrappedComponent) { "dateTimeFormat", "integerFormat", "floatFormat", + "thousandsSeparator", + "decimalSeparator", "nullValue", "booleanValues", "tableCellMaxJSONSize", diff --git a/client/app/pages/settings/components/GeneralSettings/FormatSettings.jsx b/client/app/pages/settings/components/GeneralSettings/FormatSettings.jsx index f1c6cf2646..ad8cbecd35 100644 --- a/client/app/pages/settings/components/GeneralSettings/FormatSettings.jsx +++ b/client/app/pages/settings/components/GeneralSettings/FormatSettings.jsx @@ -1,6 +1,7 @@ import React from "react"; import { SettingsEditorPropTypes, SettingsEditorDefaultProps } from "../prop-types"; import Form from "antd/lib/form"; +import Input from "antd/lib/input"; import Select from "antd/lib/select"; import Skeleton from "antd/lib/skeleton"; import DynamicComponent from "@/components/DynamicComponent"; @@ -41,6 +42,36 @@ export default function FormatSettings(props) { )} + + {loading ? ( + + ) : ( + onChange({ thousands_separator: e.target.value })} + data-test="ThousandsSeparatorInput" + /> + )} + + + {loading ? ( + + ) : ( + onChange({ decimal_separator: e.target.value })} + data-test="DecimalSeparatorInput" + /> + )} + ); } diff --git a/client/cypress/integration/settings/organization_settings_spec.js b/client/cypress/integration/settings/organization_settings_spec.js index 8318d7028b..1c6687c702 100644 --- a/client/cypress/integration/settings/organization_settings_spec.js +++ b/client/cypress/integration/settings/organization_settings_spec.js @@ -42,4 +42,26 @@ describe("Settings", () => { .should("exist"); }); }); + + it("can set a custom thousands separator", () => { + cy.intercept("POST", "/api/settings/organization").as("saveSettings"); + cy.getByTestId("ThousandsSeparatorInput").clear().type(" "); + cy.getByTestId("OrganizationSettingsSaveButton").click(); + cy.wait("@saveSettings"); + + // the setting round-trips through the backend + cy.reload(); + cy.getByTestId("ThousandsSeparatorInput").should("have.value", " "); + + cy.createQuery({ + name: "test thousands separator", + query: "SELECT 1234567 AS n", + }).then(({ id: queryId }) => { + cy.visit(`/queries/${queryId}`); + cy.findByText("Refresh Now").click(); + + // the integer is grouped with the configured space separator + cy.getByTestId("TableVisualization").should("contain", "1 234 567"); + }); + }); }); diff --git a/redash/handlers/authentication.py b/redash/handlers/authentication.py index ca49382a34..33f593212b 100644 --- a/redash/handlers/authentication.py +++ b/redash/handlers/authentication.py @@ -252,6 +252,8 @@ def number_format_config(): return { "integerFormat": current_org.get_setting("integer_format"), "floatFormat": current_org.get_setting("float_format"), + "thousandsSeparator": current_org.get_setting("thousands_separator"), + "decimalSeparator": current_org.get_setting("decimal_separator"), } diff --git a/redash/settings/organization.py b/redash/settings/organization.py index 4f6de8aacb..290b2e62d2 100644 --- a/redash/settings/organization.py +++ b/redash/settings/organization.py @@ -27,6 +27,11 @@ TIME_FORMAT = os.environ.get("REDASH_TIME_FORMAT", "HH:mm") INTEGER_FORMAT = os.environ.get("REDASH_INTEGER_FORMAT", "0,0") FLOAT_FORMAT = os.environ.get("REDASH_FLOAT_FORMAT", "0,0.00") +# Thousands/decimal separators applied to numeral.js at frontend init. The `,` and `.` +# in numeral format strings are locale markers, not literals, so these control the actual +# characters rendered. Defaults preserve the previous en-locale behavior. +THOUSANDS_SEPARATOR = os.environ.get("REDASH_THOUSANDS_SEPARATOR", ",") +DECIMAL_SEPARATOR = os.environ.get("REDASH_DECIMAL_SEPARATOR", ".") NULL_VALUE = os.environ.get("REDASH_NULL_VALUE", "null") MULTI_BYTE_SEARCH_ENABLED = parse_boolean(os.environ.get("MULTI_BYTE_SEARCH_ENABLED", "false")) @@ -60,6 +65,8 @@ "time_format": TIME_FORMAT, "integer_format": INTEGER_FORMAT, "float_format": FLOAT_FORMAT, + "thousands_separator": THOUSANDS_SEPARATOR, + "decimal_separator": DECIMAL_SEPARATOR, "null_value": NULL_VALUE, "multi_byte_search_enabled": MULTI_BYTE_SEARCH_ENABLED, "auth_jwt_login_enabled": JWT_LOGIN_ENABLED, diff --git a/tests/handlers/test_settings.py b/tests/handlers/test_settings.py index 8292d37bbd..4540e5c33e 100644 --- a/tests/handlers/test_settings.py +++ b/tests/handlers/test_settings.py @@ -24,6 +24,27 @@ def test_post(self): self.assertEqual(rv.json["settings"]["auth_password_login_enabled"], True) self.assertEqual(updated_org.settings["settings"]["auth_password_login_enabled"], True) + def test_updates_number_separators(self): + admin = self.factory.create_admin() + rv = self.make_request( + "post", + "/api/settings/organization", + data={"thousands_separator": " ", "decimal_separator": ","}, + user=admin, + ) + self.assertEqual(rv.json["settings"]["thousands_separator"], " ") + self.assertEqual(rv.json["settings"]["decimal_separator"], ",") + + updated_org = Organization.get_by_slug(self.factory.org.slug) + self.assertEqual(updated_org.get_setting("thousands_separator"), " ") + self.assertEqual(updated_org.get_setting("decimal_separator"), ",") + + def test_get_returns_default_number_separators(self): + admin = self.factory.create_admin() + rv = self.make_request("get", "/api/settings/organization", user=admin) + self.assertEqual(rv.json["settings"]["thousands_separator"], ",") + self.assertEqual(rv.json["settings"]["decimal_separator"], ".") + def test_updates_google_apps_domains(self): admin = self.factory.create_admin() domains = ["example.com"] diff --git a/viz-lib/src/lib/value-format.test.ts b/viz-lib/src/lib/value-format.test.ts new file mode 100644 index 0000000000..cdd7f7a05b --- /dev/null +++ b/viz-lib/src/lib/value-format.test.ts @@ -0,0 +1,40 @@ +import { createNumberFormatter } from "./value-format"; +import { updateVisualizationsSettings } from "@/visualizations/visualizationsSettings"; + +// numeral keeps a single global locale, so reset the defaults around every test — before, +// so the suite is order-independent even if an earlier suite mutated the delimiters, and +// after, so it doesn't leak overrides into unrelated specs. +beforeEach(() => { + updateVisualizationsSettings({ thousandsSeparator: ",", decimalSeparator: "." }); +}); + +afterEach(() => { + updateVisualizationsSettings({ thousandsSeparator: ",", decimalSeparator: "." }); +}); + +describe("createNumberFormatter", () => { + test("uses the default en delimiters", () => { + expect(createNumberFormatter("0,0")(1234567)).toBe("1,234,567"); + expect(createNumberFormatter("0,0.00")(1234567.89)).toBe("1,234,567.89"); + }); + + test("applies a custom thousands separator (the reported space case)", () => { + updateVisualizationsSettings({ thousandsSeparator: " " }); + expect(createNumberFormatter("0,0")(1234567)).toBe("1 234 567"); + }); + + test("applies continental-European separators together", () => { + updateVisualizationsSettings({ thousandsSeparator: " ", decimalSeparator: "," }); + expect(createNumberFormatter("0,0.00")(1234567.89)).toBe("1 234 567,89"); + }); + + test("applies one separator independently while the other stays default", () => { + updateVisualizationsSettings({ thousandsSeparator: " " }); + expect(createNumberFormatter("0,0.00")(1234567.5)).toBe("1 234 567.50"); + }); + + test("falls back to default separators for non-string overrides", () => { + updateVisualizationsSettings({ thousandsSeparator: undefined, decimalSeparator: undefined }); + expect(createNumberFormatter("0,0.00")(1234.5)).toBe("1,234.50"); + }); +}); diff --git a/viz-lib/src/visualizations/visualizationsSettings.tsx b/viz-lib/src/visualizations/visualizationsSettings.tsx index 6e25ee32de..bbdd30ed36 100644 --- a/viz-lib/src/visualizations/visualizationsSettings.tsx +++ b/viz-lib/src/visualizations/visualizationsSettings.tsx @@ -1,5 +1,6 @@ import React from "react"; -import { extend } from "lodash"; +import { extend, isString } from "lodash"; +import numeral from "numeral"; import Tooltip from "antd/lib/tooltip"; type HelpTriggerProps = { @@ -35,6 +36,9 @@ function Link(props: any) { return ; } +const DEFAULT_THOUSANDS_SEPARATOR = ","; +const DEFAULT_DECIMAL_SEPARATOR = "."; + export const visualizationsSettings = { HelpTriggerComponent: HelpTrigger, LinkComponent: Link, @@ -42,6 +46,8 @@ export const visualizationsSettings = { dateTimeFormat: "DD/MM/YYYY HH:mm", integerFormat: "0,0", floatFormat: "0,0.00", + thousandsSeparator: DEFAULT_THOUSANDS_SEPARATOR, + decimalSeparator: DEFAULT_DECIMAL_SEPARATOR, nullValue: "null", booleanValues: ["false", "true"], tableCellMaxJSONSize: 50000, @@ -52,4 +58,15 @@ export const visualizationsSettings = { export function updateVisualizationsSettings(options: any) { extend(visualizationsSettings, options); + + // `,` and `.` in numeral format strings are locale markers, not literal characters — + // the rendered separators come from the active locale's delimiters. Override them so + // settings like a space thousands separator (e.g. "1 234 567") are reachable. Each + // separator is applied independently and falls back to its en default when not a string, + // so a non-string value never leaves numeral stuck on a stale override. + const { thousandsSeparator, decimalSeparator } = visualizationsSettings; + extend(numeral.localeData().delimiters, { + thousands: isString(thousandsSeparator) ? thousandsSeparator : DEFAULT_THOUSANDS_SEPARATOR, + decimal: isString(decimalSeparator) ? decimalSeparator : DEFAULT_DECIMAL_SEPARATOR, + }); }