From b7672d114e02b5f8850ba379053017cc476bbe18 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Fri, 24 Jul 2020 09:47:48 -0400
Subject: [PATCH] don't parse unix date query params, fix field value loading
(#12983)
---
frontend/src/metabase/entities/fields.js | 44 +++++++++----------
.../parameters/components/Parameters.jsx | 3 +-
.../components/Parameters.unit.spec.js | 10 +++++
3 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/frontend/src/metabase/entities/fields.js b/frontend/src/metabase/entities/fields.js
index 0d2adf1b10c..794cecbc77a 100644
--- a/frontend/src/metabase/entities/fields.js
+++ b/frontend/src/metabase/entities/fields.js
@@ -1,9 +1,12 @@
import { createEntity } from "metabase/lib/entities";
import {
+ compose,
+ withAction,
+ withCachedDataAndRequestState,
+ withNormalize,
handleActions,
createAction,
createThunkAction,
- fetchData,
updateData,
} from "metabase/lib/redux";
import { normalize } from "normalizr";
@@ -38,7 +41,7 @@ export const ADD_REMAPPINGS = "metabase/entities/fields/ADD_REMAPPINGS";
export const ADD_PARAM_VALUES = "metabase/entities/fields/ADD_PARAM_VALUES";
export const ADD_FIELDS = "metabase/entities/fields/ADD_FIELDS";
-export default createEntity({
+const Fields = createEntity({
name: "fields",
path: "/api/field",
schema: FieldSchema,
@@ -64,18 +67,19 @@ export default createEntity({
// ACTION CREATORS
objectActions: {
- fetchFieldValues: createThunkAction(
- FETCH_FIELD_VALUES,
- ({ id }, reload) => (dispatch, getState) =>
- fetchData({
- dispatch,
- getState,
- requestStatePath: ["entities", "fields", id, "values"],
- existingStatePath: ["entities", "fields", id, "values"],
- getData: () => MetabaseApi.field_values({ fieldId: id }),
- reload,
- }),
- ),
+ fetchFieldValues: compose(
+ withAction(FETCH_FIELD_VALUES),
+ withCachedDataAndRequestState(
+ ({ id }) => [...Fields.getObjectStatePath(id)],
+ ({ id }) => [...Fields.getObjectStatePath(id), "values"],
+ ),
+ withNormalize(FieldSchema),
+ )(({ id: fieldId }) => async (dispatch, getState) => {
+ const { field_id: id, values } = await MetabaseApi.field_values({
+ fieldId,
+ });
+ return { id, values };
+ }),
// Docstring from m.api.field:
// Update the human-readable values for a `Field` whose special type is
@@ -139,16 +143,6 @@ export default createEntity({
reducer: handleActions(
{
- [FETCH_FIELD_VALUES]: {
- next: (state, { payload: fieldValues }) =>
- fieldValues
- ? assocIn(
- state,
- [fieldValues.field_id, "values"],
- fieldValues.values,
- )
- : state,
- },
[ADD_PARAM_VALUES]: {
next: (state, { payload: paramValues }) => {
for (const fieldValues of Object.values(paramValues)) {
@@ -201,3 +195,5 @@ export default createEntity({
].filter(f => f),
},
});
+
+export default Fields;
diff --git a/frontend/src/metabase/parameters/components/Parameters.jsx b/frontend/src/metabase/parameters/components/Parameters.jsx
index ade7ec814f1..c31b35893d5 100644
--- a/frontend/src/metabase/parameters/components/Parameters.jsx
+++ b/frontend/src/metabase/parameters/components/Parameters.jsx
@@ -268,7 +268,8 @@ export function parseQueryParam(
}
// [].every is always true, so only check if there are some fields
if (fields.length > 0) {
- if (fields.every(f => f.isNumeric())) {
+ // unix dates fields are numeric but query params shouldn't be parsed as numbers
+ if (fields.every(f => f.isNumeric() && !f.isDate())) {
return parseFloat(value);
}
if (fields.every(f => f.isBoolean())) {
diff --git a/frontend/test/metabase/parameters/components/Parameters.unit.spec.js b/frontend/test/metabase/parameters/components/Parameters.unit.spec.js
index f3fd1f41ebb..97ea01d93f8 100644
--- a/frontend/test/metabase/parameters/components/Parameters.unit.spec.js
+++ b/frontend/test/metabase/parameters/components/Parameters.unit.spec.js
@@ -1,3 +1,5 @@
+import Field from "metabase-lib/lib/metadata/Field";
+
import { ORDERS, PRODUCTS } from "__support__/sample_dataset_fixture";
import { parseQueryParam } from "metabase/parameters/components/Parameters";
@@ -24,5 +26,13 @@ describe("Parameters", () => {
const result = parseQueryParam("123", []);
expect(result).toBe("123");
});
+ it("should not parse date/numeric fields", () => {
+ const dateField = new Field({
+ ...ORDERS.QUANTITY, // some numeric field
+ special_type: "type/UNIXTimestampSeconds", // make it a date
+ });
+ const result = parseQueryParam("past30days", [dateField]);
+ expect(result).toBe("past30days");
+ });
});
});
--
GitLab