From aa9b70d8d15202e4862a40db7d2022aaf1ea5712 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Atte=20Kein=C3=A4nen?= <atte.keinanen@gmail.com>
Date: Thu, 14 Sep 2017 18:00:33 +0300
Subject: [PATCH] Add tests, fix issue with public/embedded dashboard filters
---
.../dashboard/dashboard.integ.spec.js | 73 ------------------
frontend/src/metabase/selectors/metadata.js | 13 +++-
.../test/dashboard/dashboard.integ.spec.js | 6 +-
.../test/selectors/metadata.integ.spec.js | 76 +++++++++++++++++++
4 files changed, 88 insertions(+), 80 deletions(-)
delete mode 100644 frontend/src/metabase/dashboard/dashboard.integ.spec.js
create mode 100644 frontend/test/selectors/metadata.integ.spec.js
diff --git a/frontend/src/metabase/dashboard/dashboard.integ.spec.js b/frontend/src/metabase/dashboard/dashboard.integ.spec.js
deleted file mode 100644
index 390b2380ab0..00000000000
--- a/frontend/src/metabase/dashboard/dashboard.integ.spec.js
+++ /dev/null
@@ -1,73 +0,0 @@
-import { PublicApi } from "metabase/services";
-import { fetchDashboard } from "./dashboard"
-
-import {
- createTestStore,
- login
-} from "metabase/__support__/integrated_tests";
-
-import { makeGetMergedParameterFieldValues } from "metabase/selectors/metadata";
-import { ADD_PARAM_VALUES } from "metabase/redux/metadata";
-
-// TODO Atte Keinänen 7/17/17: When we have a nice way to create dashboards in tests, this could use a real saved dashboard
-// instead of mocking the API endpoint
-
-// Mock the dashboard endpoint using a real response of `public/dashboard/:dashId`
-const mockPublicDashboardResponse = {
- "name": "Dashboard",
- "description": "For testing parameter values",
- "id": 40,
- "parameters": [{"name": "Category", "slug": "category", "id": "598ab323", "type": "category"}],
- "ordered_cards": [{
- "sizeX": 6,
- "series": [],
- "card": {
- "id": 25,
- "name": "Orders over time",
- "description": null,
- "display": "line",
- "dataset_query": {"type": "query"}
- },
- "col": 0,
- "id": 105,
- "parameter_mappings": [{
- "parameter_id": "598ab323",
- "card_id": 25,
- "target": ["dimension", ["fk->", 3, 21]]
- }],
- "card_id": 25,
- "visualization_settings": {},
- "dashboard_id": 40,
- "sizeY": 6,
- "row": 0
- }],
- // Parameter values are self-contained in the public dashboard response
- "param_values": {
- "21": {
- "values": ["Doohickey", "Gadget", "Gizmo", "Widget"],
- "human_readable_values": {},
- "field_id": 21
- }
- }
-}
-PublicApi.dashboard = async () => {
- return mockPublicDashboardResponse;
-}
-
-describe("Dashboard redux actions", () => {
- beforeAll(async () => {
- await login();
- })
-
- describe("fetchDashboard(...)", () => {
- it("should add the parameter values to state tree for public dashboards", async () => {
- const store = await createTestStore();
- // using hash as dashboard id should invoke the public API
- await store.dispatch(fetchDashboard('6e59cc97-3b6a-4bb6-9e7a-5efeee27e40f'));
- await store.waitForActions(ADD_PARAM_VALUES)
- const getMergedParameterFieldValues = makeGetMergedParameterFieldValues();
- const fieldValues = await getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [ 21 ] }});
- expect(fieldValues).toEqual(["Doohickey", "Gadget", "Gizmo", "Widget"]);
- })
- })
-})
\ No newline at end of file
diff --git a/frontend/src/metabase/selectors/metadata.js b/frontend/src/metabase/selectors/metadata.js
index 9b715d3c92a..37e808da0fb 100644
--- a/frontend/src/metabase/selectors/metadata.js
+++ b/frontend/src/metabase/selectors/metadata.js
@@ -18,6 +18,7 @@ import {
getBreakouts,
getAggregatorsWithFields
} from "metabase/lib/schema_metadata";
+import { getIn } from "icepick";
export const getNormalizedMetadata = state => state.metadata;
@@ -129,10 +130,14 @@ export const getSegments = createSelector(
// Returns a dictionary of field id:s mapped to matching field values
// Currently this assumes that you are passing the props of <ParameterValueWidget> which contain the
// `field_ids` array inside `parameter` prop.
-const getParameterFieldValuesByFieldId = (state, props) => _.chain(getFields(state))
- .pick(getFields(state), ...props.parameter.field_ids)
- .mapObject(getFieldValues)
- .value()
+const getParameterFieldValuesByFieldId = (state, props) => {
+ // NOTE Atte Keinänen 9/14/17: Reading the state directly instead of using `getFields` selector
+ // because `getMetadata` doesn't currently work with fields of public dashboards
+ return _.chain(getIn(state, ["metadata", "fields"]))
+ .pick(getFields(state), ...props.parameter.field_ids)
+ .mapObject(getFieldValues)
+ .value()
+}
// Custom equality selector for checking if two field value dictionaries contain same fields and field values
// Currently we simply check if fields match and the lengths of field value arrays are equal which makes the comparison fast
diff --git a/frontend/test/dashboard/dashboard.integ.spec.js b/frontend/test/dashboard/dashboard.integ.spec.js
index 186582917e0..df515a0f6eb 100644
--- a/frontend/test/dashboard/dashboard.integ.spec.js
+++ b/frontend/test/dashboard/dashboard.integ.spec.js
@@ -9,7 +9,7 @@ import {
import { DashboardApi, PublicApi } from "metabase/services";
import * as Urls from "metabase/lib/urls";
-import { getParameterFieldValues } from "metabase/selectors/metadata";
+import { getFields, makeGetMergedParameterFieldValues } from "metabase/selectors/metadata";
import { ADD_PARAM_VALUES } from "metabase/redux/metadata";
import { mount } from "enzyme";
import {
@@ -87,14 +87,14 @@ describe("Dashboard", () => {
await store.dispatch(fetchDashboard('6e59cc97-3b6a-4bb6-9e7a-5efeee27e40f'));
await store.waitForActions(ADD_PARAM_VALUES)
- const fieldValues = await getParameterFieldValues(store.getState(), { parameter: { field_id: 21 }});
+ const getMergedParameterFieldValues = makeGetMergedParameterFieldValues();
+ const fieldValues = await getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [ 21 ] }});
expect(fieldValues).toEqual([["Doohickey"], ["Gadget"], ["Gizmo"], ["Widget"]]);
})
})
})
// Converted from Selenium E2E test
-
describe("dashboard page", () => {
let dashboardId = null;
diff --git a/frontend/test/selectors/metadata.integ.spec.js b/frontend/test/selectors/metadata.integ.spec.js
new file mode 100644
index 00000000000..d2b8c43480e
--- /dev/null
+++ b/frontend/test/selectors/metadata.integ.spec.js
@@ -0,0 +1,76 @@
+import { createTestStore, login } from "__support__/integrated_tests";
+import {
+ deleteFieldDimension, fetchTableMetadata,
+ updateFieldDimension,
+ updateFieldValues,
+} from "metabase/redux/metadata";
+import { makeGetMergedParameterFieldValues } from "metabase/selectors/metadata";
+
+const REVIEW_RATING_ID = 33;
+const PRODUCT_CATEGORY_ID = 21;
+
+// NOTE Atte Keinänen 9/14/17: A hybrid of an integration test and a method unit test
+// I wanted to use a real state tree and have a realistic field remapping scenario
+
+describe('makeGetMergedParameterFieldValues', () => {
+ beforeAll(async () => {
+ await login();
+
+ // add remapping
+ const store = await createTestStore()
+
+ await store.dispatch(updateFieldDimension(REVIEW_RATING_ID, {
+ type: "internal",
+ name: "Rating Description",
+ human_readable_field_id: null
+ }));
+ await store.dispatch(updateFieldValues(REVIEW_RATING_ID, [
+ [1, 'Awful'], [2, 'Unpleasant'], [3, 'Meh'], [4, 'Enjoyable'], [5, 'Perfecto']
+ ]));
+ })
+
+ afterAll(async () => {
+ const store = await createTestStore()
+
+ await store.dispatch(deleteFieldDimension(REVIEW_RATING_ID));
+ await store.dispatch(updateFieldValues(REVIEW_RATING_ID, [
+ [1, '1'], [2, '2'], [3, '3'], [4, '4'], [5, '5']
+ ]));
+ })
+
+ it("should return empty array if no field ids", async () => {
+ const store = await createTestStore()
+ await store.dispatch(fetchTableMetadata(3))
+
+ const getMergedParameterFieldValues = makeGetMergedParameterFieldValues()
+ expect(
+ getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [] } })
+ ).toEqual([])
+
+ })
+
+ it("should return the original field values if a single field id", async () => {
+ const store = await createTestStore()
+ await store.dispatch(fetchTableMetadata(3))
+
+ const getMergedParameterFieldValues = makeGetMergedParameterFieldValues()
+ expect(
+ getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [PRODUCT_CATEGORY_ID] } })
+ ).toEqual([ [ 'Doohickey' ], [ 'Gadget' ], [ 'Gizmo' ], [ 'Widget' ] ])
+ })
+
+ it("should merge and sort field values if multiple field ids", async () => {
+ const store = await createTestStore()
+ await store.dispatch(fetchTableMetadata(3))
+ await store.dispatch(fetchTableMetadata(4))
+
+ const getMergedParameterFieldValues = makeGetMergedParameterFieldValues()
+ expect(
+ getMergedParameterFieldValues(store.getState(), { parameter: { field_ids: [PRODUCT_CATEGORY_ID, REVIEW_RATING_ID] } })
+ ).toEqual([
+ [1, 'Awful'], ['Doohickey'], [4, 'Enjoyable'], ['Gadget'],
+ ['Gizmo'], [3, 'Meh'], [5, 'Perfecto'], [2, 'Unpleasant'], ['Widget']
+ ])
+ })
+})
+
--
GitLab