refactoring branches state

scm-ui/src/repos/branches/modules/branches.js

@@ -3,7 +3,6 @@ import {
   FAILURE_SUFFIX,
   PENDING_SUFFIX,
   SUCCESS_SUFFIX,
-  default as types
 } from "../../../modules/types";
 import { apiClient } from "@scm-manager/ui-components";
 import type { Action, Branch, Repository } from "@scm-manager/ui-types";
@@ -103,15 +102,24 @@ export function createBranch(
 // Selectors
 
 export function getBranches(state: Object, repository: Repository) {
-  const key = createKey(repository);
-  if (state.branches[key]) {
-    return state.branches[key];
+  const repoState = getRepoState(state, repository);
+  if (repoState && repoState.list) {
+    return repoState.list._embedded.branches.map(name => repoState.byName[name]);
   }
-  return null;
 }
 
-export const isPermittedToCreateBranches = (state: Object): boolean => {
-  return false; // TODO
+function getRepoState(state: Object, repository: Repository) {
+  const key = createKey(repository);
+  const repoState = state.branches[key];
+  if (repoState && repoState.byName) {
+    return repoState;
+  }
+}
+
+export const isPermittedToCreateBranches = (state: Object, repository: Repository): boolean => {
+  const repoState = getRepoState(state, repository);
+  return !!(repoState && repoState.list && repoState.list._links && repoState.list._links.create);
+
 };
 
 export function getBranch(
@@ -119,11 +127,10 @@ export function getBranch(
   repository: Repository,
   name: string
 ): ?Branch {
-  const key = createKey(repository);
-  if (state.branches[key]) {
-    return state.branches[key].find((b: Branch) => b.name === name);
+  const repoState = getRepoState(state, repository);
+  if (repoState) {
+    return repoState.byName[name];
   }
-  return null;
 }
 
 // Action creators
@@ -252,58 +259,61 @@ export function fetchBranchFailure(
 
 // Reducers
 
-function extractBranchesFromPayload(payload: any) {
-  if (payload._embedded && payload._embedded.branches) {
-    return payload._embedded.branches;
-  }
-  return [];
-}
+const reduceByBranchesSuccess = (state, payload) => {
+  const repository = payload.repository;
+  const response = payload.data;
 
-function reduceBranchSuccess(state, repositoryName, newBranch) {
-  const newBranches = [];
-  // we do not use filter, because we try to keep the current order
-  let found = false;
-  for (const branch of state[repositoryName] || []) {
-    if (branch.name === newBranch.name) {
-      newBranches.push(newBranch);
-      found = true;
-    } else {
-      newBranches.push(branch);
+  const key = createKey(repository);
+  const repoState = state[key] || {};
+  const byName = repoState.byName || {};
+  repoState.byName = byName;
+
+  const branches = response._embedded.branches;
+  const names = branches.map(b => b.name);
+  response._embedded.branches = names;
+  for (let branch of branches) {
+    byName[branch.name] = branch;
+  }
+  return {
+    [key]: {
+      list: response,
+      byName
     }
-  }
-  if (!found) {
-    newBranches.push(newBranch);
-  }
-  return newBranches;
-}
+  };
+};
 
-type State = { [string]: Branch[] };
+const reduceByBranchSuccess = (state, payload) => {
+  const repository = payload.repository;
+  const branch = payload.branch;
+
+  const key = createKey(repository);
+
+  const repoState = state[key] || {};
+
+  const byName = repoState.byName || {};
+  byName[branch.name] = branch;
+
+  repoState.byName = byName;
+
+  return {
+    ...state,
+    [key]: repoState
+  };
+};
 
 export default function reducer(
-  state: State = {},
+  state: {},
   action: Action = { type: "UNKNOWN" }
-): State {
+): Object {
   if (!action.payload) {
     return state;
   }
   const payload = action.payload;
   switch (action.type) {
     case FETCH_BRANCHES_SUCCESS:
-      const key = createKey(payload.repository);
-      return {
-        ...state,
-        [key]: extractBranchesFromPayload(payload.data)
-      };
+      return reduceByBranchesSuccess(state, payload);
     case FETCH_BRANCH_SUCCESS:
-      if (!action.payload.repository || !action.payload.branch) {
-        return state;
-      }
-      const newBranch = action.payload.branch;
-      const repositoryName = createKey(action.payload.repository);
-      return {
-        ...state,
-        [repositoryName]: reduceBranchSuccess(state, repositoryName, newBranch)
-      };
+      return reduceByBranchSuccess(state, payload);
     default:
       return state;
   }

scm-ui/src/repos/branches/modules/branches.test.js

@@ -22,7 +22,7 @@ import reducer, {
   isFetchBranchesPending,
   createBranch,
   isCreateBranchPending,
-  getCreateBranchFailure
+  getCreateBranchFailure, isPermittedToCreateBranches
 } from "./branches";
 
 const namespace = "foo";
@@ -192,6 +192,14 @@ describe("branches", () => {
     const branches = {
       _embedded: {
         branches: [branch1, branch2]
+      },
+      _links: {
+        self: {
+          href: "/self"
+        },
+        create: {
+          href: "/create"
+        }
       }
     };
     const action = {
@@ -202,82 +210,96 @@ describe("branches", () => {
       }
     };
 
-    it("should update state according to successful fetch", () => {
+    it("should store the branches", () => {
       const newState = reducer({}, action);
-      expect(newState).toBeDefined();
-      expect(newState[key]).toBeDefined();
-      expect(newState[key]).toContain(branch1);
-      expect(newState[key]).toContain(branch2);
+      const repoState = newState["foo/bar"];
+
+      expect(repoState.list._links.create.href).toEqual("/create");
+      expect(repoState.list._embedded.branches).toEqual([ "branch1", "branch2" ]);
+
+      expect(repoState.byName.branch1).toEqual(branch1);
+      expect(repoState.byName.branch2).toEqual(branch2);
     });
 
-    it("should not delete existing branches from state", () => {
-      const oldState = {
-        "hitchhiker/heartOfGold": [branch3]
+    it("should store a single branch", () => {
+      const newState = reducer({}, fetchBranchSuccess(repository, branch1));
+      const repoState = newState["foo/bar"];
+
+      expect(repoState.list).toBeUndefined();
+      expect(repoState.byName.branch1).toEqual(branch1);
+    });
+
+    it("should add a single branch", () => {
+      const state = {
+        "foo/bar": {
+          list: {
+            _links: {
+
+            },
+            _embedded: {
+              branches: ["branch1"]
+            }
+          },
+          byName: {
+            "branch1": branch1
+          }
+        }
       };
-      const newState = reducer(oldState, action);
-      expect(newState[key]).toContain(branch1);
-      expect(newState[key]).toContain(branch2);
-      expect(newState["hitchhiker/heartOfGold"]).toContain(branch3);
+      const newState = reducer(state, fetchBranchSuccess(repository, branch2));
+      const repoState = newState["foo/bar"];
+      const byName = repoState.byName;
+
+      expect(repoState.list._embedded.branches).toEqual(["branch1"]);
+      expect(byName.branch1).toEqual(branch1);
+      expect(byName.branch2).toEqual(branch2);
     });
 
-    it("should update state according to FETCH_BRANCH_SUCCESS action", () => {
-      const newState = reducer({}, fetchBranchSuccess(repository, branch3));
-      expect(newState["foo/bar"]).toEqual([branch3]);
-    });
-
-    it("should not delete existing branch from state", () => {
-      const oldState = {
-        "foo/bar": [branch1]
+    it("should not overwrite non related repositories", () => {
+      const state = {
+        "scm/core": {
+          byName: {
+            "branch1": branch1
+          }
+        }
       };
-      const newState = reducer(
-        oldState,
-        fetchBranchSuccess(repository, branch2)
-      );
-      expect(newState["foo/bar"]).toEqual([branch1, branch2]);
+      const newState = reducer(state, fetchBranchSuccess(repository, branch1));
+      const byName = newState["scm/core"].byName;
+
+      expect(byName.branch1).toEqual(branch1);
     });
 
-    it("should update required branch from state", () => {
-      const oldState = {
-        "foo/bar": [branch1]
+    it("should overwrite existing branch", () => {
+      const state = {
+        "foo/bar": {
+          byName: {
+            "branch1": {
+              "name": "branch1",
+              "revision": "xyz"
+            }
+          }
+        }
       };
-      const newBranch1 = { name: "branch1", revision: "revision2" };
-      const newState = reducer(
-        oldState,
-        fetchBranchSuccess(repository, newBranch1)
-      );
-      expect(newState["foo/bar"]).toEqual([newBranch1]);
+      const newState = reducer(state, fetchBranchSuccess(repository, branch1));
+      const byName = newState["foo/bar"].byName;
+
+      expect(byName.branch1.revision).toEqual("revision1");
     });
 
-    it("should update required branch from state and keeps old repo", () => {
-      const oldState = {
-        "ns/one": [branch1]
+    it("should not overwrite existing branches", () => {
+      const state = {
+        "foo/bar": {
+          byName: {
+            branch3
+          }
+        }
       };
-      const newState = reducer(
-        oldState,
-        fetchBranchSuccess(repository, branch3)
-      );
-      expect(newState["ns/one"]).toEqual([branch1]);
-      expect(newState["foo/bar"]).toEqual([branch3]);
-    });
 
-    it("should return the oldState, if action has no payload", () => {
-      const state = {};
-      const newState = reducer(state, { type: FETCH_BRANCH_SUCCESS });
-      expect(newState).toBe(state);
-    });
-
-    it("should return the oldState, if payload has no branch", () => {
-      const action = {
-        type: FETCH_BRANCH_SUCCESS,
-        payload: {
-          repository
-        },
-        itemId: "foo/bar/"
-      };
-      const state = {};
       const newState = reducer(state, action);
-      expect(newState).toBe(state);
+      expect(newState["foo/bar"].byName.branch1).toEqual(branch1);
+      expect(newState["foo/bar"].byName.branch2).toEqual(branch2);
+      expect(newState["foo/bar"].byName.branch3).toEqual(branch3);
     });
+
   });
 
   describe("branch selectors", () => {
@@ -285,7 +307,20 @@ describe("branches", () => {
 
     const state = {
       branches: {
-        [key]: [branch1, branch2]
+        "foo/bar": {
+          list: {
+            _links: {
+
+            },
+            _embedded: {
+              branches: ["branch1", "branch2"]
+            }
+          },
+          byName: {
+            "branch1": branch1,
+            "branch2": branch2
+          }
+        }
       }
     };
 
@@ -312,9 +347,9 @@ describe("branches", () => {
       expect(one).toBe(two);
     });
 
-    it("should return null, if no branches for the repository available", () => {
+    it("should return undefined, if no branches for the repository available", () => {
       const branches = getBranches({ branches: {} }, repository);
-      expect(branches).toBeNull();
+      expect(branches).toBeUndefined();
     });
 
     it("should return single branch by name", () => {
@@ -333,6 +368,32 @@ describe("branches", () => {
       expect(branch).toBeUndefined();
     });
 
+    it("should return true if the branches list contains the create link", () => {
+      const stateWithLink = {
+        branches: {
+          "foo/bar": {
+            ...state.branches["foo/bar"],
+            list: {
+              ...state.branches["foo/bar"].list,
+              _links: {
+                create: {
+                  href: "http://create-it"
+                }
+              }
+            },
+          }
+        }
+      };
+
+      const permitted = isPermittedToCreateBranches(stateWithLink, repository);
+      expect(permitted).toBe(true);
+    });
+
+    it("should return false if the create link is missing", () => {
+      const permitted = isPermittedToCreateBranches(state, repository);
+      expect(permitted).toBe(false);
+    });
+
     it("should return error if fetching branches failed", () => {
       const state = {
         failure: {
@@ -374,7 +435,7 @@ describe("branches", () => {
       expect(getCreateBranchFailure(state)).toEqual(error);
     });
 
-    it("shouls return undefined when create branch did not fail", () => {
+    it("should return undefined when create branch did not fail", () => {
       expect(getCreateBranchFailure({})).toBe(undefined);
     });
   });