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 390b2380ab0f7645ad09ccf97052ad1130516bf3..0000000000000000000000000000000000000000 --- 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 9b715d3c92abd7ee9bf6df080c197027b7f73c0e..37e808da0fbbc3d161eaf23bcb8621c5e963414b 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 186582917e07455089dbd6deaebd460c8847100e..df515a0f6ebe0d4c916d4f102736b49bf329aee8 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 0000000000000000000000000000000000000000..d2b8c43480ed863615d41c2e8342030b0ae0bdfb --- /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'] + ]) + }) +}) +