Merge pull request #5197 from metabase/refactor-hungry-catch-all-selectors

Refactor hungry Redux reducers in questions code

frontend/src/metabase/lib/redux.js

@@ -110,6 +110,36 @@ export const updateData = async ({
     }
 }
 
+// helper for working with normalizr
+// merge each entity from newEntities with existing entity, if any
+// this ensures partial entities don't overwrite existing entities with more properties
+export function mergeEntities(entities, newEntities) {
+    entities = { ...entities };
+    for (const id in newEntities) {
+        if (id in entities) {
+            entities[id] = { ...entities[id], ...newEntities[id] };
+        } else {
+            entities[id] = newEntities[id];
+        }
+    }
+    return entities;
+}
+
+// helper for working with normalizr
+// reducer that merges payload.entities
+export function handleEntities(actionPattern, entityType, reducer) {
+    return (state, action) => {
+        if (state === undefined) {
+            state = {};
+        }
+        let entities = getIn(action, ["payload", "entities", entityType]);
+        if (actionPattern.test(action.type) && entities) {
+            state = mergeEntities(state, entities);
+        }
+        return reducer(state, action);
+    }
+}
+
 // for filtering non-DOM props from redux-form field objects
 // https://github.com/erikras/redux-form/issues/1441
 export const formDomOnlyProps = ({

frontend/src/metabase/questions/labels.js

 
-import { createAction, createThunkAction } from "metabase/lib/redux";
+import {createAction, createThunkAction, mergeEntities} from "metabase/lib/redux";
 import { reset } from 'redux-form';
 import { normalize, schema } from "normalizr";
 
@@ -8,7 +8,6 @@ import MetabaseAnalytics from "metabase/lib/analytics";
 const label = new schema.Entity('labels');
 import { LabelApi } from "metabase/services";
 
-import { assoc, merge } from "icepick";
 import _ from "underscore";
 
 const LOAD_LABELS = 'metabase/labels/LOAD_LABELS';
@@ -76,19 +75,17 @@ const initialState = {
 };
 
 export default function(state = initialState, { type, payload, error }) {
-    if (payload && payload.entities) {
-        state = assoc(state, "entities", merge(state.entities, payload.entities));
-    }
-    if (payload && payload.message) {
-        state = assoc(state, "message", payload.message);
-    }
-
     switch (type) {
         case LOAD_LABELS:
             if (error) {
                 return { ...state, error: payload };
             } else {
-                return { ...state, labelIds: payload.result, error: null };
+                return {
+                    ...state,
+                    entities: mergeEntities(state.entities, payload.entities),
+                    labelIds: payload.result,
+                    error: null
+                };
             }
         case SAVE_LABEL:
             if (error || payload == null) {

frontend/src/metabase/questions/questions.js

 
-import { createAction, createThunkAction, momentifyArraysTimestamps } from "metabase/lib/redux";
+import {createAction, createThunkAction, mergeEntities, momentifyArraysTimestamps} from "metabase/lib/redux";
 
 import { normalize, schema } from "normalizr";
-import { getIn, assoc, assocIn, updateIn, merge, chain } from "icepick";
+import { getIn, assocIn, updateIn, chain } from "icepick";
 import _ from "underscore";
 
 import { inflect } from "metabase/lib/formatting";
@@ -225,10 +225,6 @@ const initialState = {
 };
 
 export default function(state = initialState, { type, payload, error }) {
-    if (payload && payload.entities) {
-        state = assoc(state, "entities", merge(state.entities, payload.entities));
-    }
-
     switch (type) {
         case SET_SEARCH_TEXT:
             return { ...state, searchText: payload };
@@ -242,6 +238,7 @@ export default function(state = initialState, { type, payload, error }) {
             } else {
                 return (chain(state)
                     .assoc("loadingInitialEntities", false)
+                    .assoc("entities", mergeEntities(state.entities, payload.entities))
                     .assoc("lastEntityType", payload.entityType)
                     .assoc("lastEntityQuery", payload.entityQuery)
                     .assoc("selectedIds", {})

frontend/src/metabase/redux/metadata.js

@@ -6,6 +6,7 @@ import {
     resourceListToMap,
     fetchData,
     updateData,
+    handleEntities
 } from "metabase/lib/redux";
 
 import { normalize } from "normalizr";
@@ -427,34 +428,6 @@ const revisions = handleActions({
     [FETCH_REVISIONS]: { next: (state, { payload }) => payload }
 }, {});
 
-// merge each entity from newEntities with existing entity, if any
-// this ensures partial entities don't overwrite existing entities with more properties
-function mergeEntities(entities, newEntities) {
-    entities = { ...entities };
-    for (const id in newEntities) {
-        if (id in entities) {
-            entities[id] = { ...entities[id], ...newEntities[id] };
-        } else {
-            entities[id] = newEntities[id];
-        }
-    }
-    return entities;
-}
-
-// reducer that merges payload.entities
-function handleEntities(actionPattern, entityType, reducer) {
-    return (state, action) => {
-        if (state === undefined) {
-            state = {};
-        }
-        let entities = getIn(action, ["payload", "entities", entityType])
-        if (actionPattern.test(action.type) && entities) {
-            state = mergeEntities(state, entities);
-        }
-        return reducer(state, action);
-    }
-}
-
 export default combineReducers({
     metrics:   handleEntities(/^metabase\/metadata\//, "metrics", metrics),
     segments:  handleEntities(/^metabase\/metadata\//, "segments", segments),