Reduce duplication in collection and single resource selectors

- Provide selector creator for collections, with ALL_NAMESPACES support - Provide selector creator for single namespaced resource - Add missing tests for selectors
tektoncd · Jun 21, 2019 · 1ba3c93 · 1ba3c93
1 parent 7e7b3d4
commit 1ba3c93
Show file tree

Hide file tree

Showing 14 changed files with 288 additions and 251 deletions.
diff --git a/src/reducers/pipelineResources.js b/src/reducers/pipelineResources.js
@@ -12,25 +12,16 @@ limitations under the License.
 */
 
 import { createNamespacedReducer } from './reducerCreators';
-import { ALL_NAMESPACES } from '../constants';
+import { getCollection, getResource } from './selectorCreators';
 
 export default () => createNamespacedReducer({ type: 'PipelineResource' });
 
 export function getPipelineResources(state, namespace) {
-  if (namespace === ALL_NAMESPACES) {
-    return Object.values(state.byId);
-  }
-
-  const pipelineResources = state.byNamespace[namespace];
-  return pipelineResources
-    ? Object.values(pipelineResources).map(id => state.byId[id])
-    : [];
+  return getCollection(state, namespace);
 }
 
 export function getPipelineResource(state, name, namespace) {
-  const pipelineResources = state.byNamespace[namespace] || {};
-  const pipelineResourceId = pipelineResources[name];
-  return pipelineResourceId ? state.byId[pipelineResourceId] : null;
+  return getResource(state, name, namespace);
 }
 
 export function getPipelineResourcesErrorMessage(state) {

diff --git a/src/reducers/pipelineResources.test.js b/src/reducers/pipelineResources.test.js
@@ -11,53 +11,51 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import * as creators from './reducerCreators';
+import * as reducerCreators from './reducerCreators';
+import * as selectorCreators from './selectorCreators';
 import createReducer, * as selectors from './pipelineResources';
-import { ALL_NAMESPACES } from '../constants';
 
-const createPipelineResource = (name, namespace, uid, params) => {
-  return {
-    metadata: {
-      name,
-      namespace,
-      uid
-    },
-    spec: {
-      params
-    }
-  };
-};
-
-const name = 'pipeline name';
+const name = 'pipeline resource name';
 const namespace = 'default';
-const uid = 'some-uid';
-const pipelineResource = createPipelineResource(name, namespace, uid);
+const state = { fake: 'state' };
 
 it('creates a namespaced reducer for the correct type', () => {
   const type = 'PipelineResource';
-  jest.spyOn(creators, 'createNamespacedReducer');
+  jest.spyOn(reducerCreators, 'createNamespacedReducer');
   createReducer();
-  expect(creators.createNamespacedReducer).toHaveBeenCalledWith({ type });
+  expect(reducerCreators.createNamespacedReducer).toHaveBeenCalledWith({
+    type
+  });
 });
 
 it('getPipelineResources', () => {
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(selectors.getPipelineResources(state, selectedNamespace)).toEqual([]);
+  const collection = { fake: 'collection' };
+  jest
+    .spyOn(selectorCreators, 'getCollection')
+    .mockImplementation(() => collection);
+  expect(selectors.getPipelineResources(state, namespace)).toEqual(collection);
 });
 
-it('getPipelineResources all namespaces', () => {
-  const selectedNamespace = ALL_NAMESPACES;
-  const state = { byId: { id: pipelineResource } };
-  expect(selectors.getPipelineResources(state, selectedNamespace)).toEqual([
-    pipelineResource
-  ]);
+it('getPipelineResource', () => {
+  const resource = { fake: 'resource' };
+  jest
+    .spyOn(selectorCreators, 'getResource')
+    .mockImplementation(() => resource);
+  expect(selectors.getPipelineResource(state, name, namespace)).toEqual(
+    resource
+  );
 });
 
-it('getPipelineResource', () => {
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(
-    selectors.getPipelineResource(state, name, selectedNamespace)
-  ).toBeNull();
+it('getPipelineResourcesErrorMessage', () => {
+  const errorMessage = 'errorMessage';
+  expect(selectors.getPipelineResourcesErrorMessage({ errorMessage })).toEqual(
+    errorMessage
+  );
+});
+
+it('isFetchingPipelineResources', () => {
+  const isFetching = 'isFetching';
+  expect(selectors.isFetchingPipelineResources({ isFetching })).toEqual(
+    isFetching
+  );
 });
diff --git a/src/reducers/pipelineRuns.js b/src/reducers/pipelineRuns.js
@@ -12,25 +12,16 @@ limitations under the License.
 */
 
 import { createNamespacedReducer } from './reducerCreators';
-import { ALL_NAMESPACES } from '../constants';
+import { getCollection, getResource } from './selectorCreators';
 
 export default () => createNamespacedReducer({ type: 'PipelineRun' });
 
 export function getPipelineRuns(state, namespace) {
-  if (namespace === ALL_NAMESPACES) {
-    return Object.values(state.byId);
-  }
-
-  const pipelineRuns = state.byNamespace[namespace];
-  return pipelineRuns
-    ? Object.values(pipelineRuns).map(id => state.byId[id])
-    : [];
+  return getCollection(state, namespace);
 }
 
 export function getPipelineRun(state, name, namespace) {
-  const pipelineRuns = state.byNamespace[namespace] || {};
-  const pipelineRunId = pipelineRuns[name];
-  return pipelineRunId ? state.byId[pipelineRunId] : null;
+  return getResource(state, name, namespace);
 }
 
 export function getPipelineRunsErrorMessage(state) {

diff --git a/src/reducers/pipelineRuns.test.js b/src/reducers/pipelineRuns.test.js
@@ -11,49 +11,47 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import * as creators from './reducerCreators';
+import * as reducerCreators from './reducerCreators';
+import * as selectorCreators from './selectorCreators';
 import createReducer, * as selectors from './pipelineRuns';
-import { ALL_NAMESPACES } from '../constants';
 
-const createPipelineRun = (name, namespace, uid, content = 'other') => {
-  return {
-    metadata: {
-      name,
-      namespace,
-      uid
-    },
-    other: content
-  };
-};
-
-const name = 'pipeline name';
+const name = 'pipeline run name';
 const namespace = 'default';
-const uid = 'some-uid';
-const pipelineRun = createPipelineRun(name, namespace, uid);
+const state = { fake: 'state' };
 
 it('creates a namespaced reducer for the correct type', () => {
   const type = 'PipelineRun';
-  jest.spyOn(creators, 'createNamespacedReducer');
+  jest.spyOn(reducerCreators, 'createNamespacedReducer');
   createReducer();
-  expect(creators.createNamespacedReducer).toHaveBeenCalledWith({ type });
+  expect(reducerCreators.createNamespacedReducer).toHaveBeenCalledWith({
+    type
+  });
 });
 
 it('getPipelineRuns', () => {
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(selectors.getPipelineRuns(state, selectedNamespace)).toEqual([]);
+  const collection = { fake: 'collection' };
+  jest
+    .spyOn(selectorCreators, 'getCollection')
+    .mockImplementation(() => collection);
+  expect(selectors.getPipelineRuns(state, namespace)).toEqual(collection);
 });
 
-it('getPipelineRuns all namespaces', () => {
-  const selectedNamespace = ALL_NAMESPACES;
-  const state = { byId: { id: pipelineRun } };
-  expect(selectors.getPipelineRuns(state, selectedNamespace)).toEqual([
-    pipelineRun
-  ]);
+it('getPipelineRun', () => {
+  const resource = { fake: 'resource' };
+  jest
+    .spyOn(selectorCreators, 'getResource')
+    .mockImplementation(() => resource);
+  expect(selectors.getPipelineRun(state, name, namespace)).toEqual(resource);
 });
 
-it('getPipelineRun', () => {
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(selectors.getPipelineRun(state, name, selectedNamespace)).toBeNull();
+it('getPipelineRunsErrorMessage', () => {
+  const errorMessage = 'errorMessage';
+  expect(selectors.getPipelineRunsErrorMessage({ errorMessage })).toEqual(
+    errorMessage
+  );
+});
+
+it('isFetchingPipelineRuns', () => {
+  const isFetching = 'isFetching';
+  expect(selectors.isFetchingPipelineRuns({ isFetching })).toEqual(isFetching);
 });
diff --git a/src/reducers/pipelines.js b/src/reducers/pipelines.js
@@ -12,23 +12,16 @@ limitations under the License.
 */
 
 import { createNamespacedReducer } from './reducerCreators';
-import { ALL_NAMESPACES } from '../constants';
+import { getCollection, getResource } from './selectorCreators';
 
 export default () => createNamespacedReducer({ type: 'Pipeline' });
 
 export function getPipelines(state, namespace) {
-  if (namespace === ALL_NAMESPACES) {
-    return Object.values(state.byId);
-  }
-
-  const pipelines = state.byNamespace[namespace];
-  return pipelines ? Object.values(pipelines).map(id => state.byId[id]) : [];
+  return getCollection(state, namespace);
 }
 
 export function getPipeline(state, name, namespace) {
-  const pipelines = state.byNamespace[namespace] || {};
-  const pipelineId = pipelines[name];
-  return pipelineId ? state.byId[pipelineId] : null;
+  return getResource(state, name, namespace);
 }
 
 export function getPipelinesErrorMessage(state) {

diff --git a/src/reducers/pipelines.test.js b/src/reducers/pipelines.test.js
@@ -11,37 +11,47 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import * as creators from './reducerCreators';
+import * as reducerCreators from './reducerCreators';
+import * as selectorCreators from './selectorCreators';
 import createReducer, * as selectors from './pipelines';
-import { ALL_NAMESPACES } from '../constants';
+
+const name = 'pipeline name';
+const namespace = 'default';
+const state = { fake: 'state' };
 
 it('creates a namespaced reducer for the correct type', () => {
   const type = 'Pipeline';
-  jest.spyOn(creators, 'createNamespacedReducer');
+  jest.spyOn(reducerCreators, 'createNamespacedReducer');
   createReducer();
-  expect(creators.createNamespacedReducer).toHaveBeenCalledWith({ type });
+  expect(reducerCreators.createNamespacedReducer).toHaveBeenCalledWith({
+    type
+  });
 });
 
 it('getPipelines', () => {
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(selectors.getPipelines(state, selectedNamespace)).toEqual([]);
+  const collection = { fake: 'collection' };
+  jest
+    .spyOn(selectorCreators, 'getCollection')
+    .mockImplementation(() => collection);
+  expect(selectors.getPipelines(state, namespace)).toEqual(collection);
 });
 
-it('getPipelines all namespaces', () => {
-  const selectedNamespace = ALL_NAMESPACES;
-  const selectedPipeline = { fake: 'pipeline' };
-  const state = { byId: { id: selectedPipeline } };
-  expect(selectors.getPipelines(state, selectedNamespace)).toEqual([
-    selectedPipeline
-  ]);
+it('getPipeline', () => {
+  const resource = { fake: 'resource' };
+  jest
+    .spyOn(selectorCreators, 'getResource')
+    .mockImplementation(() => resource);
+  expect(selectors.getPipeline(state, name, namespace)).toEqual(resource);
 });
 
-it('getPipeline', () => {
-  const selectedName = 'name';
-  const selectedNamespace = 'namespace';
-  const state = { byNamespace: {} };
-  expect(
-    selectors.getPipeline(state, selectedName, selectedNamespace)
-  ).toBeNull();
+it('getPipelinesErrorMessage', () => {
+  const errorMessage = 'errorMessage';
+  expect(selectors.getPipelinesErrorMessage({ errorMessage })).toEqual(
+    errorMessage
+  );
+});
+
+it('isFetchingPipelines', () => {
+  const isFetching = 'isFetching';
+  expect(selectors.isFetchingPipelines({ isFetching })).toEqual(isFetching);
 });
diff --git a/src/reducers/selectorCreators.js b/src/reducers/selectorCreators.js
@@ -0,0 +1,29 @@
+/*
+Copyright 2019 The Tekton Authors
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { ALL_NAMESPACES } from '../constants';
+
+export function getCollection(state, namespace) {
+  if (namespace === ALL_NAMESPACES) {
+    return Object.values(state.byId);
+  }
+
+  const resources = state.byNamespace[namespace];
+  return resources ? Object.values(resources).map(id => state.byId[id]) : [];
+}
+
+export function getResource(state, name, namespace) {
+  const resources = state.byNamespace[namespace] || {};
+  const id = resources[name];
+  return id ? state.byId[id] : null;
+}