From efb857150b338ce26ec2543987e1504230d3b5e1 Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Wed, 17 Oct 2018 11:55:47 +0200
Subject: [PATCH] fixed paging with NaN page numbers

---
 scm-ui/src/repos/containers/BranchRoot.js     | 13 +++++--
 scm-ui/src/repos/containers/Changesets.js     | 38 +++++++++++--------
 .../src/repos/containers/Changesets.test.js   | 26 +++++++++++++
 scm-ui/src/repos/containers/RepositoryRoot.js |  1 -
 4 files changed, 58 insertions(+), 20 deletions(-)
 create mode 100644 scm-ui/src/repos/containers/Changesets.test.js

diff --git a/scm-ui/src/repos/containers/BranchRoot.js b/scm-ui/src/repos/containers/BranchRoot.js
index a37dfb1302..8d0783bb52 100644
--- a/scm-ui/src/repos/containers/BranchRoot.js
+++ b/scm-ui/src/repos/containers/BranchRoot.js
@@ -6,7 +6,7 @@ import { Route, withRouter } from "react-router-dom";
 import Changesets from "./Changesets";
 import BranchSelector from "./BranchSelector";
 import { connect } from "react-redux";
-import { Loading } from "@scm-manager/ui-components";
+import { ErrorNotification, Loading } from "@scm-manager/ui-components";
 import {
   fetchBranches,
   getBranches,
@@ -25,6 +25,7 @@ type Props = {
   // State props
   branches: Branch[],
   loading: boolean,
+  error: Error,
 
   // Dispatch props
   fetchBranches: Repository => void,
@@ -69,17 +70,21 @@ class BranchRoot extends React.Component<Props> {
   };
 
   render() {
-    // TODO error???
-    const { repository, loading, match, branches } = this.props;
-    const url = this.stripEndingSlash(match.url);
+    const { repository, error, loading, match, branches } = this.props;
+
+    if (error) {
+      return <ErrorNotification error={error} />;
+    }
 
     if (loading) {
       return <Loading />;
     }
+
     if (!repository || !branches) {
       return null;
     }
 
+    const url = this.stripEndingSlash(match.url);
     const branch = this.findSelectedBranch();
     const changesets = <Changesets repository={repository} branch={branch} />;
 
diff --git a/scm-ui/src/repos/containers/Changesets.js b/scm-ui/src/repos/containers/Changesets.js
index 5c6934107e..c078e74375 100644
--- a/scm-ui/src/repos/containers/Changesets.js
+++ b/scm-ui/src/repos/containers/Changesets.js
@@ -18,11 +18,16 @@ import {
 
 import { connect } from "react-redux";
 import ChangesetList from "../components/changesets/ChangesetList";
-import { ErrorPage, LinkPaginator, Loading } from "@scm-manager/ui-components";
+import {
+  ErrorNotification,
+  LinkPaginator,
+  Loading
+} from "@scm-manager/ui-components";
 import { translate } from "react-i18next";
+import { compose } from "redux";
 
 type Props = {
-  repository: Repository, //TODO: Do we really need/want this here?
+  repository: Repository,
   branch: Branch,
   page: number,
 
@@ -50,18 +55,13 @@ class Changesets extends React.Component<Props> {
     const { changesets, loading, error, t } = this.props;
 
     if (error) {
-      return (
-        <ErrorPage
-          title={t("changesets.error-title")}
-          subtitle={t("changesets.error-title")}
-          error={error}
-        />
-      );
+      return <ErrorNotification error={error} />;
     }
 
     if (loading) {
       return <Loading />;
     }
+
     if (!changesets || changesets.length === 0) {
       return null;
     }
@@ -95,22 +95,30 @@ const mapDispatchToProps = dispatch => {
   };
 };
 
+export function getPageFromMatch(match: any) {
+  let page = parseInt(match.params.page);
+  if (isNaN(page) || !page) {
+    page = 1;
+  }
+  return page;
+}
+
 const mapStateToProps = (state: any, ownProps: Props) => {
   const { repository, branch, match } = 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);
-
-  // TODO
-  const page = parseInt(match.params.page || "1");
+  const page = getPageFromMatch(match);
 
   return { changesets, list, page, loading, error };
 };
 
-export default withRouter(
+export default compose(
+  withRouter,
   connect(
     mapStateToProps,
     mapDispatchToProps
-  )(translate("repos")(Changesets))
-);
+  ),
+  translate("repos")
+)(Changesets);
diff --git a/scm-ui/src/repos/containers/Changesets.test.js b/scm-ui/src/repos/containers/Changesets.test.js
new file mode 100644
index 0000000000..b2c8683625
--- /dev/null
+++ b/scm-ui/src/repos/containers/Changesets.test.js
@@ -0,0 +1,26 @@
+import { getPageFromMatch } from "./Changesets";
+
+describe("tests for getPageFromMatch", () => {
+  function createMatch(page: string) {
+    return {
+      params: {
+        page
+      }
+    };
+  }
+
+  it("should return 1 for NaN", () => {
+    const match = createMatch("any");
+    expect(getPageFromMatch(match)).toBe(1);
+  });
+
+  it("should return 1 for 0", () => {
+    const match = createMatch("0");
+    expect(getPageFromMatch(match)).toBe(1);
+  });
+
+  it("should return the given number", () => {
+    const match = createMatch("42");
+    expect(getPageFromMatch(match)).toBe(42);
+  });
+});
diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js
index 4e922d1d66..f7410cecc4 100644
--- a/scm-ui/src/repos/containers/RepositoryRoot.js
+++ b/scm-ui/src/repos/containers/RepositoryRoot.js
@@ -94,7 +94,6 @@ class RepositoryRoot extends React.Component<Props> {
     }
 
     const url = this.matchedUrl();
-    // todo: default branch
     return (
       <Page title={repository.namespace + "/" + repository.name}>
         <div className="columns">