Fixed issue with slashes in branch names

scm-ui/src/repos/containers/BranchChooser.js

@@ -1,89 +1,125 @@
 // @flow
-
-import React from "react";
-import type { Repository, Branch } from "@scm-manager/ui-types";
+import * as React from "react";
+import type { Branch, Repository } from "@scm-manager/ui-types";
 import { connect } from "react-redux";
 import {
   fetchBranches,
-  getBranches,
+  getBranch,
+  getBranchNames,
+  getFetchBranchesFailure,
   isFetchBranchesPending
 } from "../modules/branches";
-
-import { Loading } from "@scm-manager/ui-components";
 import DropDown from "../components/DropDown";
+import type { History } from "history";
+import { withRouter } from "react-router-dom";
+import { ErrorPage, Loading } from "@scm-manager/ui-components";
 
 type Props = {
   repository: Repository,
-  fetchBranches: Repository => void,
-  callback: (?Branch) => void,
   branches: Branch[],
-  selectedBranchName: string,
+  branchNames: string[],
+  fetchBranches: Repository => void,
+  history: History,
+  match: any,
+  selectedBranch?: Branch,
+  label: string, //TODO: Should this be here?
   loading: boolean,
-  label: string
-};
-
-type State = {
-  selectedBranchName: string
+  branchSelected: string => void,
+  error: Error,
+  children: React.Node
 };
+type State = {};
 
 class BranchChooser extends React.Component<Props, State> {
-  constructor(props: Props) {
-    super(props);
-    this.state = {
-      selectedBranchName: props.selectedBranchName
-    };
-  }
-
   componentDidMount() {
-    const { repository, fetchBranches } = this.props;
-    fetchBranches(repository);
+    this.props.fetchBranches(this.props.repository);
   }
 
   render() {
-    const { branches, loading, label } = this.props;
-    if (loading) {
-      return <Loading />;
-    }
-    if (branches && branches.length > 0) {
+    const { selectedBranch, loading, error } = this.props;
+
+    // TODO: i18n
+    if (error) {
       return (
-        <div className={"box"}>
-          <label className="label">{label}</label>
-          <DropDown
-            options={branches.map(b => b.name)}
-            preselectedOption={this.state.selectedBranchName}
-            optionSelected={branch => this.branchChanged(branch)}
-          />
-        </div>
+        <ErrorPage
+          title={"Failed loading branches"}
+          subtitle={"Somethin went wrong"}
+          error={error}
+        />
       );
     }
 
-    return null;
+    if (loading) {
+      return <Loading />;
+    }
+
+    const childrenWithBranch = React.Children.map(
+      this.props.children,
+      child => {
+        return React.cloneElement(child, {
+          branch: selectedBranch
+        });
+      }
+    );
+
+    return (
+      <>
+        {this.renderBranchChooser()}
+        {childrenWithBranch}
+      </>
+    );
   }
 
-  branchChanged = (branchName: string) => {
-    const { callback } = this.props;
-    this.setState({ ...this.state, selectedBranchName: branchName });
-    const branch = this.props.branches.find(b => b.name === branchName);
-    callback(branch);
-  };
+  renderBranchChooser() {
+    const { branchNames, label, branchSelected, match } = this.props;
+    const selectedBranchName = match.params.branch;
+
+    if (!branchNames || branchNames.length === 0) {
+      return null;
+    }
+
+    return (
+      <div className={"box"}>
+        <label className="label">{label}</label>
+        <DropDown
+          options={branchNames}
+          preselectedOption={selectedBranchName}
+          optionSelected={branch => branchSelected(branch)}
+        />
+      </div>
+    );
+  }
 }
 
-const mapStateToProps = (state: State, ownProps: Props) => {
+const mapDispatchToProps = dispatch => {
   return {
-    branches: getBranches(state, ownProps.repository),
-    loading: isFetchBranchesPending(state, ownProps.repository)
-  };
-};
-
-const mapDispatchToProps = (dispatch: any) => {
-  return {
-    fetchBranches: (repository: Repository) => {
-      dispatch(fetchBranches(repository));
+    fetchBranches: (repo: Repository) => {
+      dispatch(fetchBranches(repo));
     }
   };
 };
 
-export default connect(
-  mapStateToProps,
-  mapDispatchToProps
-)(BranchChooser);
+const mapStateToProps = (state: any, ownProps: Props) => {
+  const { repository, match } = ownProps;
+  const loading = isFetchBranchesPending(state, repository);
+  const error = getFetchBranchesFailure(state, repository);
+  const branchNames = getBranchNames(state, repository);
+  const selectedBranch = getBranch(
+    state,
+    repository,
+    decodeURIComponent(match.params.branch)
+  );
+  return {
+    loading,
+    branchNames,
+    selectedBranch,
+    error
+  };
+};
+
+export default withRouter(
+  connect(
+    mapStateToProps,
+    mapDispatchToProps
+  )(BranchChooser)
+);

scm-ui/src/repos/containers/Changesets.js

@@ -1,247 +1,124 @@
 // @flow
-import React from "react";
-import { connect } from "react-redux";
-import { translate } from "react-i18next";
-import {
-  ErrorNotification,
-  Loading,
-  Paginator
-} from "@scm-manager/ui-components";
 
+import React from "react";
+import { withRouter } from "react-router-dom";
+import type {
+  Branch,
+  Changeset,
+  PagedCollection,
+  Repository
+} from "@scm-manager/ui-types";
 import {
-  fetchChangesets,
+  fetchChangesetsByBranch,
   fetchChangesetsByBranchAndPage,
-  fetchChangesetsByLink,
-  fetchChangesetsByPage,
   getChangesets,
-  getChangesetsFromState,
   getFetchChangesetsFailure,
   isFetchChangesetsPending,
   selectListAsCollection
 } from "../modules/changesets";
-import type { History } from "history";
-import type {
-  Changeset,
-  PagedCollection,
-  Repository,
-  Branch
-} from "@scm-manager/ui-types";
+import { connect } from "react-redux";
 import ChangesetList from "../components/changesets/ChangesetList";
-import { withRouter } from "react-router-dom";
-import { fetchBranches, getBranch, getBranchNames } from "../modules/branches";
-import BranchChooser from "./BranchChooser";
+import { ErrorPage, LinkPaginator, Loading } from "@scm-manager/ui-components";
 
 type Props = {
-  repository: Repository,
-  branchName: string,
-  branchNames: string[],
-  history: History,
-  fetchChangesetsByNamespaceNameAndBranch: (
-    namespace: string,
-    name: string,
-    branch: string
-  ) => void,
-  list: PagedCollection,
-  fetchChangesetsByLink: (Repository, string, Branch) => void,
-  fetchChangesetsByPage: (Repository, number) => void,
+  fetchChangesetsByBranch: (Repository, Branch) => void,
   fetchChangesetsByBranchAndPage: (Repository, Branch, number) => void,
-  fetchBranches: Repository => void,
-  page: number,
-  t: string => string,
-  match: any,
+  repository: Repository, //TODO: Do we really need/want this here?
+  branch: Branch,
   changesets: Changeset[],
   loading: boolean,
-  error: boolean,
-  branch: Branch
+  match: any,
+  list: PagedCollection,
+  error: Error
 };
 
 type State = {};
 
-class Changesets extends React.PureComponent<Props, State> {
-  constructor(props: Props) {
-    super(props);
-  }
-
-  onPageChange = (link: string) => {
-    const { repository, branch } = this.props;
-    this.props.fetchChangesetsByLink(repository, link, branch);
-  };
-
+class ChangesetContainer extends React.Component<Props, State> {
   componentDidMount() {
-    if (!this.props.loading) {
-      this.updateContent();
-    }
-  }
-
-  updateContent() {
     const {
+      fetchChangesetsByBranch,
+      fetchChangesetsByBranchAndPage,
       repository,
       branch,
-      page,
-      fetchChangesetsByPage,
-      fetchChangesetsByBranchAndPage
+      match
     } = this.props;
-    if (branch) {
-      fetchChangesetsByBranchAndPage(repository, branch, page);
+    const { page } = match.params;
+    if (!page) {
+      fetchChangesetsByBranch(repository, branch);
     } else {
-      fetchChangesetsByPage(repository, page);
-    }
-  }
-
-  componentDidUpdate(prevProps: Props) {
-    const { page, list, repository, match } = this.props;
-    const { namespace, name } = repository;
-    const branch = match.params.branch;
-
-    if (!this.props.loading) {
-      if (prevProps.branch !== this.props.branch) {
-        this.updateContent();
-      }
-
-      if (list && (list.page || list.page === 0)) {
-        console.log(list);
-        // backend starts paging at 0
-        const statePage: number = list.page + 1;
-        console.log(`page: ${page} - statePage: ${statePage}`);
-        if (page !== statePage) {
-          if (branch) {
-            this.props.history.push(
-              `/repo/${namespace}/${name}/${branch}/changesets/${statePage}`
-            );
-          } else {
-            this.props.history.push(
-              `/repo/${namespace}/${name}/changesets/${statePage}`
-            );
-          }
-        }
-      }
+      fetchChangesetsByBranchAndPage(repository, branch, page);
     }
   }
 
   render() {
     const { changesets, loading, error } = this.props;
+
+    // TODO: i18n
     if (error) {
-      return <ErrorNotification error={error} />;
+      return (
+        <ErrorPage
+          title={"Failed loading branches"}
+          subtitle={"Somethin went wrong"}
+          error={error}
+        />
+      );
     }
 
-    if (loading || !changesets) {
+    if (loading) {
       return <Loading />;
     }
-
+    if (!changesets || changesets.length === 0) {
+      return null;
+    }
     return (
-      <div>
+      <>
         {this.renderList()}
         {this.renderPaginator()}
-      </div>
+      </>
     );
   }
 
   renderList = () => {
-    const branch = decodeURIComponent(this.props.match.params.branch);
-    const { repository, changesets, t } = this.props;
-
-    return (
-      <>
-        <BranchChooser
-          repository={repository}
-          selectedBranchName={branch}
-          label={t("changesets.branchselector-label")}
-          callback={branch => this.branchChanged(branch)}
-        />
-        <ChangesetList repository={repository} changesets={changesets} />
-      </>
-    );
+    const { repository, changesets } = this.props;
+    return <ChangesetList repository={repository} changesets={changesets} />;
   };
 
-  renderPaginator() {
+  renderPaginator = () => {
     const { list } = this.props;
     if (list) {
-      return <Paginator collection={list} onPageChange={this.onPageChange} />;
+      return <LinkPaginator collection={list} />;
     }
     return null;
-  }
-
-  branchChanged = (branch: Branch): void => {
-    const { history, repository } = this.props;
-    if (branch === undefined) {
-      history.push(
-        `/repo/${repository.namespace}/${repository.name}/changesets`
-      );
-    } else {
-      const branchName = encodeURIComponent(branch.name);
-      this.setState({ branch: branchName });
-      history.push(
-        `/repo/${repository.namespace}/${
-          repository.name
-        }/${branchName}/changesets`
-      );
-    }
   };
 }
 
-const getPageFromProps = props => {
-  let page = props.match.params.page;
-  if (page) {
-    page = parseInt(page, 10);
-  } else {
-    page = 1;
-  }
-  return page;
-};
-
-const mapStateToProps = (state, ownProps: Props) => {
-  const { repository } = ownProps;
-  const branchName = ownProps.match.params.branch;
-  const branch = getBranch(state, repository, decodeURIComponent(branchName));
-  const loading = isFetchChangesetsPending(state, repository, branch);
-  const changesets = getChangesets(state, repository, branch);
-  const branchNames = getBranchNames(state, repository);
-  const error = getFetchChangesetsFailure(state, repository, branch);
-  const list = selectListAsCollection(state, repository);
-  const page = getPageFromProps(ownProps);
-
-  return {
-    loading,
-    changesets,
-    branchNames,
-    error,
-    list,
-    page,
-    branch
-  };
-};
-
 const mapDispatchToProps = dispatch => {
   return {
-    fetchBranches: (repository: Repository) => {
-      dispatch(fetchBranches(repository));
-    },
-    fetchChangesets: (repository: Repository) => {
-      dispatch(fetchChangesets(repository));
-    },
-    fetchChangesetsByPage: (repository, page: number) => {
-      dispatch(fetchChangesetsByPage(repository, page));
+    fetchChangesetsByBranch: (repo: Repository, branch: Branch) => {
+      dispatch(fetchChangesetsByBranch(repo, branch));
     },
     fetchChangesetsByBranchAndPage: (
-      repository,
+      repo: Repository,
       branch: Branch,
       page: number
     ) => {
-      dispatch(fetchChangesetsByBranchAndPage(repository, branch, page));
-    },
-    fetchChangesetsByLink: (
-      repository: Repository,
-      link: string,
-      branch?: Branch
-    ) => {
-      dispatch(fetchChangesetsByLink(repository, link, branch));
+      dispatch(fetchChangesetsByBranchAndPage(repo, branch, page));
     }
   };
 };
 
+const mapStateToProps = (state: any, ownProps: Props) => {
+  const { repository, branch } = ownProps;
+  const changesets = getChangesets(state, repository, branch);
+  const loading = isFetchChangesetsPending(state, repository, branch);
+  const error = getFetchChangesetsFailure(state, repository, branch);
+  const list = selectListAsCollection(state, repository, branch);
+  return { changesets, list, loading, error };
+};
 export default withRouter(
   connect(
     mapStateToProps,
     mapDispatchToProps
-  )(translate("repos")(Changesets))
+  )(ChangesetContainer)
 );

scm-ui/src/repos/containers/RepositoryRoot.js

@@ -11,11 +11,11 @@ import { connect } from "react-redux";
 import { Route } from "react-router-dom";
 import type { Repository } from "@scm-manager/ui-types";
 import {
-  Page,
-  Loading,
   ErrorPage,
+  Loading,
   Navigation,
   NavLink,
+  Page,
   Section
 } from "@scm-manager/ui-components";
 import { translate } from "react-i18next";
@@ -25,6 +25,7 @@ import Edit from "../containers/Edit";
 
 import type { History } from "history";
 import EditNavLink from "../components/EditNavLink";
+import BranchChooser from "./BranchChooser";
 import Changesets from "./Changesets";
 
 type Props = {
@@ -76,6 +77,17 @@ class RepositoryRoot extends React.Component<Props> {
     return route.location.pathname.match(regex);
   };
 
+  branchSelected = (branchName: string) => {
+    const url = this.matchedUrl();
+    if (branchName === "") {
+      this.props.history.push(`${url}/changesets/`);
+    } else {
+      this.props.history.push(
+        `${url}/branches/${encodeURIComponent(branchName)}/changesets/`
+      );
+    }
+  };
+
   render() {
     const { loading, error, repository, t } = this.props;
 
@@ -94,7 +106,7 @@ class RepositoryRoot extends React.Component<Props> {
     }
 
     const url = this.matchedUrl();
-
+    // TODO: Changesets need to be adjusted (i.e. sub-routes need to be handled in sub-components)
     return (
       <Page title={repository.namespace + "/" + repository.name}>
         <div className="columns">
@@ -108,25 +120,31 @@ class RepositoryRoot extends React.Component<Props> {
               path={`${url}/edit`}
               component={() => <Edit repository={repository} />}
             />
+
             <Route
-              exact
-              path={`${url}/changesets`}
-              render={() => <Changesets repository={repository} />}
+              path={`${url}/changesets/:page?`}
+              component={() => (
+                <BranchChooser
+                  repository={repository}
+                  label={"Branches"}
+                  branchSelected={this.branchSelected}
+                >
+                  <Changesets repository={repository} />
+                </BranchChooser>
+              )}
             />
+
             <Route
-              exact
-              path={`${url}/changesets/:page`}
-              render={() => <Changesets repository={repository} />}
-            />
-            <Route
-              exact
-              path={`${url}/:branch/changesets`}
-              render={() => <Changesets repository={repository} />}
-            />
-            <Route
-              exact
-              path={`${url}/:branch/changesets/:page`}
-              render={() => <Changesets repository={repository} />}
+              path={`${url}/branches/:branch/changesets/:page?`}
+              component={() => (
+                <BranchChooser
+                  repository={repository}
+                  label={"Branches"}
+                  branchSelected={this.branchSelected}
+                >
+                  <Changesets repository={repository} />
+                </BranchChooser>
+              )}
             />
           </div>
           <div className="column">
@@ -135,7 +153,7 @@ class RepositoryRoot extends React.Component<Props> {
                 <NavLink to={url} label={t("repository-root.information")} />
                 <NavLink
                   activeOnlyWhenExact={false}
-                  to={`${url}/changesets`}
+                  to={`${url}/changesets/`}
                   label={t("repository-root.history")}
                   activeWhenMatch={this.matches}
                 />

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

@@ -5,8 +5,9 @@ import {
   SUCCESS_SUFFIX
 } from "../../modules/types";
 import { apiClient } from "@scm-manager/ui-components";
-import type { Repository, Action, Branch } from "@scm-manager/ui-types";
+import type { Action, Branch, Repository } from "@scm-manager/ui-types";
 import { isPending } from "../../modules/pending";
+import { getFailure } from "../../modules/failure";
 
 export const FETCH_BRANCHES = "scm/repos/FETCH_BRANCHES";
 export const FETCH_BRANCHES_PENDING = `${FETCH_BRANCHES}_${PENDING_SUFFIX}`;
@@ -113,7 +114,7 @@ export function getBranchNames(
   repository: Repository
 ): ?Array<Branch> {
   const key = createKey(repository);
-  if (!state.branches[key] || !state.branches[key].byNames) {
+  if (!state.branches || !state.branches[key] || !state.branches[key].byNames) {
     return [];
   }
   return Object.keys(state.branches[key].byNames);
@@ -149,6 +150,10 @@ export function isFetchBranchesPending(
   return isPending(state, FETCH_BRANCHES, createKey(repository));
 }
 
+export function getFetchBranchesFailure(state: Object, repository: Repository) {
+  return getFailure(state, FETCH_BRANCHES, createKey(repository));
+}
+
 function createKey(repository: Repository): string {
   const { namespace, name } = repository;
   return `${namespace}/${name}`;

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

@@ -10,6 +10,7 @@ import reducer, {
   getBranch,
   getBranches,
   getBranchNames,
+  getFetchBranchesFailure,
   isFetchBranchesPending
 } from "./branches";
 
@@ -129,6 +130,8 @@ describe("branches", () => {
   });
 
   describe("branch selectors", () => {
+    const error = new Error("Something went wrong");
+
     const state = {
       branches: {
         [key]: {
@@ -173,5 +176,18 @@ describe("branches", () => {
       const branch = getBranch(state, repository, "branch42");
       expect(branch).toBeFalsy();
     });
+    it("should return error if fetching branches failed", () => {
+      const state = {
+        failure: {
+          [FETCH_BRANCHES + "/foo/bar"]: error
+        }
+      };
+
+      expect(getFetchBranchesFailure(state, repository)).toEqual(error);
+    });
+
+    it("should return false if fetching branches did not fail", () => {
+      expect(getFetchBranchesFailure({}, repository)).toBeUndefined();
+    });
   });
 });

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

@@ -1,21 +1,11 @@
 // @flow
 
-import {
-  FAILURE_SUFFIX,
-  PENDING_SUFFIX,
-  SUCCESS_SUFFIX
-} from "../../modules/types";
-import { apiClient } from "@scm-manager/ui-components";
-import { isPending } from "../../modules/pending";
-import { getFailure } from "../../modules/failure";
-import { combineReducers } from "redux";
-import type {
-  Action,
-  Changeset,
-  PagedCollection,
-  Repository,
-  Branch
-} from "@scm-manager/ui-types";
+import {FAILURE_SUFFIX, PENDING_SUFFIX, SUCCESS_SUFFIX} from "../../modules/types";
+import {apiClient} from "@scm-manager/ui-components";
+import {isPending} from "../../modules/pending";
+import {getFailure} from "../../modules/failure";
+import {combineReducers} from "redux";
+import type {Action, Branch, Changeset, PagedCollection, Repository} from "@scm-manager/ui-types";
 
 export const FETCH_CHANGESETS = "scm/repos/FETCH_CHANGESETS";
 export const FETCH_CHANGESETS_PENDING = `${FETCH_CHANGESETS}_${PENDING_SUFFIX}`;
@@ -166,7 +156,7 @@ function byKeyReducer(
       if (state[key]) {
         oldChangesets[key] = state[key];
       }
-      const byIds = extractChangesetsByIds(changesets, oldChangesets[key].byId);
+      const byIds = extractChangesetsByIds(changesets);
       return {
         ...state,
         [key]: {
@@ -190,17 +180,13 @@ export default combineReducers({
   byKey: byKeyReducer
 });
 
-function extractChangesetsByIds(changesets: any, oldChangesetsByIds: any) {
+function extractChangesetsByIds(changesets: any) {
   const changesetsByIds = {};
 
   for (let changeset of changesets) {
     changesetsByIds[changeset.id] = changeset;
   }
 
-  for (let id in oldChangesetsByIds) {
-    changesetsByIds[id] = oldChangesetsByIds[id];
-  }
-
   return changesetsByIds;
 }
 
@@ -233,16 +219,20 @@ export function getFetchChangesetsFailure(
   return getFailure(state, FETCH_CHANGESETS, createItemId(repository, branch));
 }
 
-const selectList = (state: Object, repository: Repository) => {
-  const itemId = createItemId(repository);
+const selectList = (state: Object, repository: Repository, branch?: Branch) => {
+  const itemId = createItemId(repository, branch);
   if (state.changesets.byKey[itemId] && state.changesets.byKey[itemId].list) {
     return state.changesets.byKey[itemId].list;
   }
   return {};
 };
 
-const selectListEntry = (state: Object, repository: Repository): Object => {
-  const list = selectList(state, repository);
+const selectListEntry = (
+  state: Object,
+  repository: Repository,
+  branch?: Branch
+): Object => {
+  const list = selectList(state, repository, branch);
   if (list.entry) {
     return list.entry;
   }
@@ -251,9 +241,10 @@ const selectListEntry = (state: Object, repository: Repository): Object => {
 
 export const selectListAsCollection = (
   state: Object,
-  repository: Repository
+  repository: Repository,
+  branch?: Branch
 ): PagedCollection => {
-  return selectListEntry(state, repository);
+  return selectListEntry(state, repository, branch);
 };
 
 export function getChangesetsFromState(state: Object, repository: Repository) {

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

@@ -3,21 +3,21 @@
 import configureMockStore from "redux-mock-store";
 import thunk from "redux-thunk";
 import fetchMock from "fetch-mock";
-import {
+import reducer, {
   FETCH_CHANGESETS,
   FETCH_CHANGESETS_FAILURE,
   FETCH_CHANGESETS_PENDING,
   FETCH_CHANGESETS_SUCCESS,
   fetchChangesets,
-  fetchChangesetsByBranchAndPage,
   fetchChangesetsByBranch,
+  fetchChangesetsByBranchAndPage,
   fetchChangesetsByPage,
   fetchChangesetsSuccess,
   getChangesets,
   getFetchChangesetsFailure,
   isFetchChangesetsPending
 } from "./changesets";
-import reducer from "./changesets";
+
 const branch = {
   name: "specific",
   revision: "123",
@@ -40,7 +40,10 @@ const repository = {
     changesets: {
       href: "http://scm/api/rest/v2/repositories/foo/bar/changesets"
     },
-    branches: [branch]
+    branches: {
+      href:
+        "http://scm/api/rest/v2/repositories/foo/bar/branches/specific/branches"
+    }
   }
 };
 
@@ -238,38 +241,6 @@ describe("changesets", () => {
         entries: ["changeset1", "changeset2", "changeset3"]
       });
     });
-
-    it("should not delete existing changesets from state", () => {
-      const responseBody = {
-        _embedded: {
-          changesets: [
-            { id: "changeset1", author: { mail: "z@phod.com", name: "zaphod" } }
-          ],
-          _embedded: {
-            tags: [],
-            branches: [],
-            parents: []
-          }
-        }
-      };
-      const newState = reducer(
-        {
-          byKey: {
-            "foo/bar": {
-              byId: {
-                ["changeset2"]: {
-                  id: "changeset2",
-                  author: { mail: "mail@author.com", name: "author" }
-                }
-              }
-            }
-          }
-        },
-        fetchChangesetsSuccess(responseBody, repository)
-      );
-      expect(newState.byKey["foo/bar"].byId["changeset2"]).toBeDefined();
-      expect(newState.byKey["foo/bar"].byId["changeset1"]).toBeDefined();
-    });
   });
 
   describe("changeset selectors", () => {