From 2953c805f983a1401e74e17102f6e7e0353720f7 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 1 Aug 2018 14:56:24 +0200 Subject: [PATCH 1/3] implemented paging for repository overview --- scm-ui/src/containers/Main.js | 6 ++ scm-ui/src/repos/containers/Overview.js | 55 +++++++++++++++++-- scm-ui/src/repos/modules/repos.js | 41 ++++++++++---- scm-ui/src/repos/modules/repos.test.js | 73 +++++++++++++++++++++++-- 4 files changed, 155 insertions(+), 20 deletions(-) diff --git a/scm-ui/src/containers/Main.js b/scm-ui/src/containers/Main.js index c1dcf54908..17504ef1f2 100644 --- a/scm-ui/src/containers/Main.js +++ b/scm-ui/src/containers/Main.js @@ -32,6 +32,12 @@ class Main extends React.Component { component={Overview} authenticated={authenticated} /> + void, + fetchReposByPage: number => void, + fetchReposByLink: string => void, // context props - t: string => string + t: string => string, + history: History }; class Overview extends React.Component { componentDidMount() { - this.props.fetchRepos(); + this.props.fetchReposByPage(this.props.page); } + + /** + * reflect page transitions in the uri + */ + componentDidUpdate() { + const { page, collection } = this.props; + if (collection) { + // backend starts paging by 0 + const statePage: number = collection.page + 1; + if (page !== statePage) { + this.props.history.push(`/repos/${statePage}`); + } + } + }; + render() { const { error, loading, t } = this.props; return ( @@ -34,21 +56,36 @@ class Overview extends React.Component { } renderList() { - const { collection } = this.props; + const { collection, fetchReposByLink } = this.props; if (collection) { return ( - +
+ + +
); } return null; } } +const getPageFromProps = props => { + let page = props.match.params.page; + if (page) { + page = parseInt(page, 10); + } else { + page = 1; + } + return page; +}; + const mapStateToProps = (state, ownProps) => { + const page = getPageFromProps(ownProps); const collection = getRepositoryCollection(state); const loading = isFetchReposPending(state); const error = getFetchReposFailure(state); return { + page, collection, loading, error @@ -59,10 +96,16 @@ const mapDispatchToProps = dispatch => { return { fetchRepos: () => { dispatch(fetchRepos()); + }, + fetchReposByPage: (page: number) => { + dispatch(fetchReposByPage(page)) + }, + fetchReposByLink: (link: string) => { + dispatch(fetchReposByLink(link)) } }; }; export default connect( mapStateToProps, mapDispatchToProps -)(translate("repos")(Overview)); +)(translate("repos")(withRouter(Overview))); diff --git a/scm-ui/src/repos/modules/repos.js b/scm-ui/src/repos/modules/repos.js index a70bdd7054..92ef8e3922 100644 --- a/scm-ui/src/repos/modules/repos.js +++ b/scm-ui/src/repos/modules/repos.js @@ -15,10 +15,32 @@ const REPOS_URL = "repositories"; const SORT_BY = "sortBy=namespaceAndName"; export function fetchRepos() { + return fetchReposByLink(REPOS_URL); +} + +export function fetchReposByPage(page: number) { + return fetchReposByLink(`${REPOS_URL}?page=${page - 1}`); +} + +function appendSortByLink(url: string) { + if (url.includes(SORT_BY)) { + return url; + } + let urlWithSortBy = url; + if (url.includes("?")) { + urlWithSortBy += "&"; + } else { + urlWithSortBy += "?"; + } + return urlWithSortBy + SORT_BY; +} + +export function fetchReposByLink(link: string) { + const url = appendSortByLink(link); return function(dispatch: any) { dispatch(fetchReposPending()); return apiClient - .get(`${REPOS_URL}?${SORT_BY}`) + .get(url) .then(response => response.json()) .then(repositories => { dispatch(fetchReposSuccess(repositories)); @@ -76,16 +98,15 @@ export default function reducer( state: Object = {}, action: Action = { type: "UNKNOWN" } ): Object { - switch (action.type) { - case FETCH_REPOS_SUCCESS: - if (action.payload) { - return normalizeByNamespaceAndName(action.payload); - } else { - // TODO ??? - return state; - } - default: + if (action.type === FETCH_REPOS_SUCCESS) { + if (action.payload) { + return normalizeByNamespaceAndName(action.payload); + } else { + // TODO ??? return state; + } + } else { + return state; } } diff --git a/scm-ui/src/repos/modules/repos.test.js b/scm-ui/src/repos/modules/repos.test.js index 1031e9efe0..6535da187a 100644 --- a/scm-ui/src/repos/modules/repos.test.js +++ b/scm-ui/src/repos/modules/repos.test.js @@ -8,7 +8,12 @@ import reducer, { fetchRepos, FETCH_REPOS_FAILURE, fetchReposSuccess, - getRepositoryCollection, FETCH_REPOS, isFetchReposPending, getFetchReposFailure + getRepositoryCollection, + FETCH_REPOS, + isFetchReposPending, + getFetchReposFailure, + fetchReposByLink, + fetchReposByPage } from "./repos"; import type { Repository, RepositoryCollection } from "../types/Repositories"; @@ -203,7 +208,26 @@ describe("repos fetch", () => { }); it("should successfully fetch repos", () => { - fetchMock.getOnce(REPOS_URL, repositoryCollection); + const url = REPOS_URL + "&page=42"; + fetchMock.getOnce(url, repositoryCollection); + + const expectedActions = [ + { type: FETCH_REPOS_PENDING }, + { + type: FETCH_REPOS_SUCCESS, + payload: repositoryCollection + } + ]; + + const store = mockStore({}); + return store.dispatch(fetchRepos()).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it("should successfully fetch page 42", () => { + const url = REPOS_URL + "&page=42"; + fetchMock.getOnce(url, repositoryCollection); const expectedActions = [ { type: FETCH_REPOS_PENDING }, @@ -215,7 +239,49 @@ describe("repos fetch", () => { const store = mockStore({}); - return store.dispatch(fetchRepos()).then(() => { + return store.dispatch(fetchReposByPage(43)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it("should successfully fetch repos from link", () => { + fetchMock.getOnce(REPOS_URL, repositoryCollection); + + const expectedActions = [ + { type: FETCH_REPOS_PENDING }, + { + type: FETCH_REPOS_SUCCESS, + payload: repositoryCollection + } + ]; + + const store = mockStore({}); + return store + .dispatch( + fetchReposByLink("/repositories?sortBy=namespaceAndName&page=42") + ) + .then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it("should append sortby parameter and successfully fetch repos from link", () => { + fetchMock.getOnce( + "/scm/api/rest/v2/repositories?one=1&sortBy=namespaceAndName", + repositoryCollection + ); + + const expectedActions = [ + { type: FETCH_REPOS_PENDING }, + { + type: FETCH_REPOS_SUCCESS, + payload: repositoryCollection + } + ]; + + const store = mockStore({}); + + return store.dispatch(fetchReposByLink("/repositories?one=1")).then(() => { expect(store.getActions()).toEqual(expectedActions); }); }); @@ -267,7 +333,6 @@ describe("repos reducer", () => { }); describe("repos selectors", () => { - const error = new Error("something goes wrong"); it("should return the repositories collection", () => { From 6dd7397d14e223ee9081d0ccd7f679388f475134 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 1 Aug 2018 14:58:52 +0200 Subject: [PATCH 2/3] fixed bug in users paging --- scm-ui/src/users/containers/Users.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scm-ui/src/users/containers/Users.js b/scm-ui/src/users/containers/Users.js index c9120ba8ec..d57f2be60f 100644 --- a/scm-ui/src/users/containers/Users.js +++ b/scm-ui/src/users/containers/Users.js @@ -50,9 +50,9 @@ class Users extends React.Component { /** * reflect page transitions in the uri */ - componentDidUpdate = (prevProps: Props) => { + componentDidUpdate() { const { page, list } = this.props; - if (list.page) { + if (list && list.page || list.page === 0) { // backend starts paging by 0 const statePage: number = list.page + 1; if (page !== statePage) { From ac8da1886760d5a9d10700836a31c16b059b5257 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 1 Aug 2018 18:23:16 +0200 Subject: [PATCH 3/3] start implementation repository details --- scm-ui/public/locales/en/repos.json | 4 + scm-ui/src/containers/Main.js | 7 ++ .../src/repos/components/RepositoryDetails.js | 16 +++ scm-ui/src/repos/containers/RepositoryRoot.js | 108 +++++++++++++++++ scm-ui/src/repos/modules/repos.js | 106 +++++++++++++++-- scm-ui/src/repos/modules/repos.test.js | 109 ++++++++++++++++-- scm-ui/src/users/containers/Users.js | 2 +- 7 files changed, 335 insertions(+), 17 deletions(-) create mode 100644 scm-ui/src/repos/components/RepositoryDetails.js create mode 100644 scm-ui/src/repos/containers/RepositoryRoot.js diff --git a/scm-ui/public/locales/en/repos.json b/scm-ui/public/locales/en/repos.json index 5117f64928..428f723f37 100644 --- a/scm-ui/public/locales/en/repos.json +++ b/scm-ui/public/locales/en/repos.json @@ -2,5 +2,9 @@ "overview": { "title": "Repositories", "subtitle": "Overview of available repositories" + }, + "repository-root": { + "error-title": "Error", + "error-subtitle": "Unknown repository error" } } diff --git a/scm-ui/src/containers/Main.js b/scm-ui/src/containers/Main.js index 17504ef1f2..1c5cface29 100644 --- a/scm-ui/src/containers/Main.js +++ b/scm-ui/src/containers/Main.js @@ -12,6 +12,7 @@ import { Switch } from "react-router-dom"; import ProtectedRoute from "../components/ProtectedRoute"; import AddUser from "../users/containers/AddUser"; import SingleUser from "../users/containers/SingleUser"; +import RepositoryRoot from '../repos/containers/RepositoryRoot'; type Props = { authenticated?: boolean @@ -38,6 +39,12 @@ class Main extends React.Component { component={Overview} authenticated={authenticated} /> + { + render() { + const { repository } = this.props; + return
{repository.description}
; + } +} + +export default RepositoryDetails; diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js new file mode 100644 index 0000000000..65de6b2fa5 --- /dev/null +++ b/scm-ui/src/repos/containers/RepositoryRoot.js @@ -0,0 +1,108 @@ +//@flow +import React from "react"; +import {fetchRepo, getFetchRepoFailure, getRepository, isFetchRepoPending} from '../modules/repos'; +import { connect } from "react-redux"; +import {Route} from "react-router-dom" +import type {Repository} from '../types/Repositories'; +import {Page} from '../../components/layout'; +import Loading from '../../components/Loading'; +import ErrorPage from '../../components/ErrorPage'; +import { translate } from "react-i18next"; +import {Navigation} from '../../components/navigation'; +import RepositoryDetails from '../components/RepositoryDetails'; + +type Props = { + namespace: string, + name: string, + repository: Repository, + loading: boolean, + error: Error, + + // dispatch functions + fetchRepo: (namespace: string, name: string) => void, + + // context props + t: string => string, + match: any +}; + +class RepositoryRoot extends React.Component { + componentDidMount() { + const { fetchRepo, namespace, name } = this.props; + + fetchRepo(namespace, name); + } + + stripEndingSlash = (url: string) => { + if (url.endsWith("/")) { + return url.substring(0, url.length - 2); + } + return url; + }; + + matchedUrl = () => { + return this.stripEndingSlash(this.props.match.url); + }; + + + render() { + const { loading, error, repository, t } = this.props; + + if (error) { + return ( + + ); + } + + if (!repository || loading) { + return + } + + const url = this.matchedUrl(); + + return +
+
+ } /> +
+
+ + + +
+
+ + } +} + + +const mapStateToProps = (state, ownProps) => { + const { namespace, name } = ownProps.match.params; + const repository = getRepository(state, namespace, name); + const loading = isFetchRepoPending(state, namespace, name); + const error = getFetchRepoFailure(state, namespace, name); + return { + namespace, + name, + repository, + loading, + error + }; +}; + +const mapDispatchToProps = dispatch => { + return { + fetchRepo : (namespace: string, name: string) => { + dispatch(fetchRepo(namespace, name)) + } + }; +}; + +export default connect( + mapStateToProps, + mapDispatchToProps +)(translate("repos")(RepositoryRoot)); diff --git a/scm-ui/src/repos/modules/repos.js b/scm-ui/src/repos/modules/repos.js index 92ef8e3922..b23cb07a15 100644 --- a/scm-ui/src/repos/modules/repos.js +++ b/scm-ui/src/repos/modules/repos.js @@ -2,7 +2,7 @@ import { apiClient } from "../../apiclient"; import * as types from "../../modules/types"; import type { Action } from "../../types/Action"; -import type { RepositoryCollection } from "../types/Repositories"; +import type {Repository, RepositoryCollection} from "../types/Repositories"; import {isPending} from "../../modules/pending"; import {getFailure} from "../../modules/failure"; @@ -11,7 +11,15 @@ export const FETCH_REPOS_PENDING = `${FETCH_REPOS}_${types.PENDING_SUFFIX}`; export const FETCH_REPOS_SUCCESS = `${FETCH_REPOS}_${types.SUCCESS_SUFFIX}`; export const FETCH_REPOS_FAILURE = `${FETCH_REPOS}_${types.FAILURE_SUFFIX}`; +export const FETCH_REPO = "scm/repos/FETCH_REPO"; +export const FETCH_REPO_PENDING = `${FETCH_REPO}_${types.PENDING_SUFFIX}`; +export const FETCH_REPO_SUCCESS = `${FETCH_REPO}_${types.SUCCESS_SUFFIX}`; +export const FETCH_REPO_FAILURE = `${FETCH_REPO}_${types.FAILURE_SUFFIX}`; + const REPOS_URL = "repositories"; + +// fetch repos + const SORT_BY = "sortBy=namespaceAndName"; export function fetchRepos() { @@ -71,15 +79,66 @@ export function fetchReposFailure(err: Error): Action { }; } +// fetch repo + +export function fetchRepo(namespace: string, name: string) { + return function(dispatch: any) { + dispatch(fetchRepoPending(namespace, name)); + return apiClient.get(`${REPOS_URL}/${namespace}/${name}`) + .then(response => response.json()) + .then( repository => { + dispatch(fetchRepoSuccess(repository)) + } ) + .catch(err => { + dispatch(fetchRepoFailure(namespace, name, err)) + }); + } +} + +export function fetchRepoPending(namespace: string, name: string): Action { + return { + type: FETCH_REPO_PENDING, + payload: { + namespace, + name + }, + itemId: namespace + "/" + name + }; +} + +export function fetchRepoSuccess(repository: Repository): Action { + return { + type: FETCH_REPO_SUCCESS, + payload: repository, + itemId: createIdentifier(repository) + }; +} + +export function fetchRepoFailure(namespace: string, name: string, error: Error): Action { + return { + type: FETCH_REPO_FAILURE, + payload: { + namespace, + name, + error + }, + itemId: namespace + "/" + name + }; +} + // reducer +function createIdentifier(repository: Repository) { + return repository.namespace + "/" + repository.name; +} + function normalizeByNamespaceAndName( repositoryCollection: RepositoryCollection ) { const names = []; const byNames = {}; for (const repository of repositoryCollection._embedded.repositories) { - const identifier = repository.namespace + "/" + repository.name; + const identifier = createIdentifier(repository); names.push(identifier); byNames[identifier] = repository; } @@ -94,18 +153,33 @@ function normalizeByNamespaceAndName( }; } +const reducerByNames = (state: Object, repository: Repository) => { + const identifier = createIdentifier(repository); + const newState = { + ...state, + byNames: { + ...state.byNames, + [identifier]: repository + } + }; + + return newState; +}; + export default function reducer( state: Object = {}, action: Action = { type: "UNKNOWN" } ): Object { - if (action.type === FETCH_REPOS_SUCCESS) { - if (action.payload) { + if (!action.payload) { + return state; + } + + switch (action.type) { + case FETCH_REPOS_SUCCESS: return normalizeByNamespaceAndName(action.payload); - } else { - // TODO ??? - return state; - } - } else { + case FETCH_REPO_SUCCESS: + return reducerByNames(state, action.payload); + default: return state; } } @@ -134,3 +208,17 @@ export function isFetchReposPending(state: Object) { export function getFetchReposFailure(state: Object) { return getFailure(state, FETCH_REPOS); } + +export function getRepository(state: Object, namespace: string, name: string) { + if (state.repos && state.repos.byNames) { + return state.repos.byNames[ namespace + "/" + name]; + } +} + +export function isFetchRepoPending(state: Object, namespace: string, name: string) { + return isPending(state, FETCH_REPO, namespace + "/" + name); +} + +export function getFetchRepoFailure(state: Object, namespace: string, name: string) { + return getFailure(state, FETCH_REPO, namespace + "/" + name); +} diff --git a/scm-ui/src/repos/modules/repos.test.js b/scm-ui/src/repos/modules/repos.test.js index 6535da187a..e214e89520 100644 --- a/scm-ui/src/repos/modules/repos.test.js +++ b/scm-ui/src/repos/modules/repos.test.js @@ -13,7 +13,16 @@ import reducer, { isFetchReposPending, getFetchReposFailure, fetchReposByLink, - fetchReposByPage + fetchReposByPage, + FETCH_REPO, + fetchRepo, + FETCH_REPO_PENDING, + FETCH_REPO_SUCCESS, + FETCH_REPO_FAILURE, + fetchRepoSuccess, + getRepository, + isFetchRepoPending, + getFetchRepoFailure } from "./repos"; import type { Repository, RepositoryCollection } from "../types/Repositories"; @@ -199,7 +208,9 @@ const repositoryCollectionWithNames: RepositoryCollection = { }; describe("repos fetch", () => { - const REPOS_URL = "/scm/api/rest/v2/repositories?sortBy=namespaceAndName"; + const REPOS_URL = "/scm/api/rest/v2/repositories"; + const SORT = "sortBy=namespaceAndName"; + const REPOS_URL_WITH_SORT = REPOS_URL + "?" + SORT; const mockStore = configureMockStore([thunk]); afterEach(() => { @@ -208,8 +219,7 @@ describe("repos fetch", () => { }); it("should successfully fetch repos", () => { - const url = REPOS_URL + "&page=42"; - fetchMock.getOnce(url, repositoryCollection); + fetchMock.getOnce(REPOS_URL_WITH_SORT, repositoryCollection); const expectedActions = [ { type: FETCH_REPOS_PENDING }, @@ -226,7 +236,7 @@ describe("repos fetch", () => { }); it("should successfully fetch page 42", () => { - const url = REPOS_URL + "&page=42"; + const url = REPOS_URL + "?page=42&" + SORT; fetchMock.getOnce(url, repositoryCollection); const expectedActions = [ @@ -245,7 +255,10 @@ describe("repos fetch", () => { }); it("should successfully fetch repos from link", () => { - fetchMock.getOnce(REPOS_URL, repositoryCollection); + fetchMock.getOnce( + REPOS_URL + "?" + SORT + "&page=42", + repositoryCollection + ); const expectedActions = [ { type: FETCH_REPOS_PENDING }, @@ -287,7 +300,7 @@ describe("repos fetch", () => { }); it("should dispatch FETCH_REPOS_FAILURE, it the request fails", () => { - fetchMock.getOnce(REPOS_URL, { + fetchMock.getOnce(REPOS_URL_WITH_SORT, { status: 500 }); @@ -299,6 +312,48 @@ describe("repos fetch", () => { expect(actions[1].payload).toBeDefined(); }); }); + + it("should successfully fetch repo slarti/fjords", () => { + fetchMock.getOnce(REPOS_URL + "/slarti/fjords", slartiFjords); + + const expectedActions = [ + { + type: FETCH_REPO_PENDING, + payload: { + namespace: "slarti", + name: "fjords" + }, + itemId: "slarti/fjords" + }, + { + type: FETCH_REPO_SUCCESS, + payload: slartiFjords, + itemId: "slarti/fjords" + } + ]; + + const store = mockStore({}); + return store.dispatch(fetchRepo("slarti", "fjords")).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it("should dispatch FETCH_REPO_FAILURE, it the request for slarti/fjords fails", () => { + fetchMock.getOnce(REPOS_URL + "/slarti/fjords", { + status: 500 + }); + + const store = mockStore({}); + return store.dispatch(fetchRepo("slarti", "fjords")).then(() => { + const actions = store.getActions(); + expect(actions[0].type).toEqual(FETCH_REPO_PENDING); + expect(actions[1].type).toEqual(FETCH_REPO_FAILURE); + expect(actions[1].payload.namespace).toBe("slarti"); + expect(actions[1].payload.name).toBe("fjords"); + expect(actions[1].payload.error).toBeDefined(); + expect(actions[1].itemId).toBe("slarti/fjords"); + }); + }); }); describe("repos reducer", () => { @@ -330,6 +385,11 @@ describe("repos reducer", () => { expect(newState.byNames["hitchhiker/restatend"]).toBe(hitchhikerRestatend); expect(newState.byNames["slarti/fjords"]).toBe(slartiFjords); }); + + it("should store the repo at byNames", () => { + const newState = reducer({}, fetchRepoSuccess(slartiFjords)); + expect(newState.byNames["slarti/fjords"]).toBe(slartiFjords); + }); }); describe("repos selectors", () => { @@ -376,4 +436,39 @@ describe("repos selectors", () => { it("should return undefined when fetch repos did not fail", () => { expect(getFetchReposFailure({})).toBe(undefined); }); + + it("should return the repository collection", () => { + const state = { + repos: { + byNames: { + "slarti/fjords": slartiFjords + } + } + }; + + const repository = getRepository(state, "slarti", "fjords"); + expect(repository).toEqual(slartiFjords); + }); + + it("should return true, when fetch repo is pending", () => { + const state = { + pending: { + [FETCH_REPO + "/slarti/fjords"]: true + } + }; + expect(isFetchRepoPending(state, "slarti", "fjords")).toEqual(true); + }); + + it("should return false, when fetch repo is not pending", () => { + expect(isFetchRepoPending({}, "slarti", "fjords")).toEqual(false); + }); + + it("should return error when fetch repo did fail", () => { + const state = { + failure: { + [FETCH_REPO + "/slarti/fjords"]: error + } + }; + expect(getFetchRepoFailure(state, "slarti", "fjords")).toEqual(error); + }); }); diff --git a/scm-ui/src/users/containers/Users.js b/scm-ui/src/users/containers/Users.js index d57f2be60f..60f104a42d 100644 --- a/scm-ui/src/users/containers/Users.js +++ b/scm-ui/src/users/containers/Users.js @@ -52,7 +52,7 @@ class Users extends React.Component { */ componentDidUpdate() { const { page, list } = this.props; - if (list && list.page || list.page === 0) { + if (list && (list.page || list.page === 0)) { // backend starts paging by 0 const statePage: number = list.page + 1; if (page !== statePage) {