From 62abae43688d3b80432b33f68772bdd16236db87 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Tue, 9 Oct 2018 21:36:41 +0200 Subject: [PATCH 1/2] Bootstrapped reimplementation of Changeset list --- .../src/repos/containers/BranchChangesets.js | 106 ++++++++++++++++++ .../repos/containers/ChangesetContainer.js | 61 ++++++++++ scm-ui/src/repos/containers/RepositoryRoot.js | 52 ++++----- scm-ui/src/repos/modules/branches.js | 14 +-- 4 files changed, 196 insertions(+), 37 deletions(-) create mode 100644 scm-ui/src/repos/containers/BranchChangesets.js create mode 100644 scm-ui/src/repos/containers/ChangesetContainer.js diff --git a/scm-ui/src/repos/containers/BranchChangesets.js b/scm-ui/src/repos/containers/BranchChangesets.js new file mode 100644 index 0000000000..982c500eab --- /dev/null +++ b/scm-ui/src/repos/containers/BranchChangesets.js @@ -0,0 +1,106 @@ +// @flow +import React from "react"; +import type {Branch, Repository} from "@scm-manager/ui-types"; +import {connect} from "react-redux"; +import {fetchBranches, getBranch, getBranchNames, isFetchBranchesPending} from "../modules/branches"; +import DropDown from "../components/DropDown"; +import type {History} from "history"; +import {withRouter} from "react-router-dom"; +import ChangesetContainer from "./ChangesetContainer"; +import {Loading} from "@scm-manager/ui-components"; + +type Props = { + repository: Repository, + branches: Branch[], + branchNames: string[], + fetchBranches: Repository => void, + history: History, + match: any, + selectedBranch: Branch, + label: string, //TODO: Should this be here? + loading: boolean +}; +type State = {}; + +class BranchChangesets extends React.Component { + componentDidMount() { + this.props.fetchBranches(this.props.repository); + } + + render() { + const { + repository, + branchNames, + match, + selectedBranch, + label, + loading + } = this.props; + const selectedBranchName = match.params.branch; + + if (loading) { + return ; + } + + // TODO: Handle errors + + if (!branchNames || branchNames.length === 0) { + return null; + } + + return ( + <> +
+ + this.branchSelected(branch)} + /> +
+ + + ); + } + + //TODO: Maybe extract this and let it be passed from parent component + branchSelected = (branchName: string) => { + const { namespace, name } = this.props.repository; + if (branchName === "") { + this.props.history.push(`/repo/${namespace}/${name}/changesets`); + } else { + this.props.history.push( + `/repo/${namespace}/${name}/branches/${branchName}/changesets` + ); + } + }; +} + +const mapDispatchToProps = dispatch => { + return { + fetchBranches: (repo: Repository) => { + dispatch(fetchBranches(repo)); + } + }; +}; + +const mapStateToProps = (state: any, ownProps: Props) => { + const loading = isFetchBranchesPending(state, ownProps.repository); + const branchNames = getBranchNames(state, ownProps.repository); + const selectedBranch = getBranch( + state, + ownProps.repository, + ownProps.match.params.branch //TODO: Maybe let parent component pass selected branch + ); + return { + loading, + branchNames, + selectedBranch + }; +}; +export default withRouter( + connect( + mapStateToProps, + mapDispatchToProps + )(BranchChangesets) +); diff --git a/scm-ui/src/repos/containers/ChangesetContainer.js b/scm-ui/src/repos/containers/ChangesetContainer.js new file mode 100644 index 0000000000..d6a473431d --- /dev/null +++ b/scm-ui/src/repos/containers/ChangesetContainer.js @@ -0,0 +1,61 @@ +// @flow + +import React from "react"; +import { withRouter } from "react-router-dom"; +import type { Branch, Changeset, Repository } from "@scm-manager/ui-types"; +import { + fetchChangesetsByBranch, + getChangesets, + isFetchChangesetsPending +} from "../modules/changesets"; +import { connect } from "react-redux"; +import ChangesetList from "../components/changesets/ChangesetList"; +import { Loading } from "@scm-manager/ui-components"; + +type Props = { + fetchChangesetsByBranch: (Repository, Branch) => void, + repository: Repository, //TODO: Do we really need/want this here? + branch: Branch, + changesets: Changeset[], + loading: boolean +}; +type State = {}; + +class ChangesetContainer extends React.Component { + componentDidMount() { + const { fetchChangesetsByBranch, repository, branch } = this.props; + fetchChangesetsByBranch(repository, branch); //TODO: fetch by page + } + + render() { + const { repository, changesets, loading } = this.props; + if (loading) { + return ; + } + if (!changesets || changesets.length === 0) { + return null; + } + return ; + } +} + +const mapDispatchToProps = dispatch => { + return { + fetchChangesetsByBranch: (repo: Repository, branch: Branch) => { + dispatch(fetchChangesetsByBranch(repo, branch)); + } + }; +}; + +const mapStateToProps = (state: any, ownProps: Props) => { + const { repository, branch } = ownProps; + const changesets = getChangesets(state, repository, branch); + const loading = isFetchChangesetsPending(state, repository, branch); + return { changesets, loading }; +}; +export default withRouter( + connect( + mapStateToProps, + mapDispatchToProps + )(ChangesetContainer) +); diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js index e9acf80a5d..e3a78d8f34 100644 --- a/scm-ui/src/repos/containers/RepositoryRoot.js +++ b/scm-ui/src/repos/containers/RepositoryRoot.js @@ -1,31 +1,18 @@ //@flow import React from "react"; -import { - deleteRepo, - fetchRepo, - getFetchRepoFailure, - getRepository, - isFetchRepoPending -} from "../modules/repos"; -import { connect } from "react-redux"; -import { Route } from "react-router-dom"; -import type { Repository } from "@scm-manager/ui-types"; -import { - Page, - Loading, - ErrorPage, - Navigation, - NavLink, - Section -} from "@scm-manager/ui-components"; -import { translate } from "react-i18next"; +import {deleteRepo, fetchRepo, getFetchRepoFailure, getRepository, isFetchRepoPending} from "../modules/repos"; +import {connect} from "react-redux"; +import {Route} from "react-router-dom"; +import type {Repository} from "@scm-manager/ui-types"; +import {ErrorPage, Loading, Navigation, NavLink, Page, Section} from "@scm-manager/ui-components"; +import {translate} from "react-i18next"; import RepositoryDetails from "../components/RepositoryDetails"; import DeleteNavAction from "../components/DeleteNavAction"; import Edit from "../containers/Edit"; -import type { History } from "history"; +import type {History} from "history"; import EditNavLink from "../components/EditNavLink"; -import Changesets from "./Changesets"; +import BranchChangesets from "./BranchChangesets"; type Props = { namespace: string, @@ -94,7 +81,7 @@ class RepositoryRoot extends React.Component { } const url = this.matchedUrl(); - + // TODO: Changesets need to be adjusted (i.e. sub-routes need to be handled in sub-components) return (
@@ -108,25 +95,34 @@ class RepositoryRoot extends React.Component { path={`${url}/edit`} component={() => } /> + } + render={() => ( + + )} /> } + render={() => ( + + )} /> } + path={`${url}/branches/:branch/changesets`} + render={() => ( + + )} /> } + path={`${url}/branches/:branch/changesets/:page`} + render={() => ( + + )} />
diff --git a/scm-ui/src/repos/modules/branches.js b/scm-ui/src/repos/modules/branches.js index c771a9734e..6e0ca52d5c 100644 --- a/scm-ui/src/repos/modules/branches.js +++ b/scm-ui/src/repos/modules/branches.js @@ -1,12 +1,8 @@ // @flow -import { - FAILURE_SUFFIX, - PENDING_SUFFIX, - SUCCESS_SUFFIX -} from "../../modules/types"; -import { apiClient } from "@scm-manager/ui-components"; -import type { Repository, Action, Branch } from "@scm-manager/ui-types"; -import { isPending } from "../../modules/pending"; +import {FAILURE_SUFFIX, PENDING_SUFFIX, SUCCESS_SUFFIX} from "../../modules/types"; +import {apiClient} from "@scm-manager/ui-components"; +import type {Action, Branch, Repository} from "@scm-manager/ui-types"; +import {isPending} from "../../modules/pending"; export const FETCH_BRANCHES = "scm/repos/FETCH_BRANCHES"; export const FETCH_BRANCHES_PENDING = `${FETCH_BRANCHES}_${PENDING_SUFFIX}`; @@ -113,7 +109,7 @@ export function getBranchNames( repository: Repository ): ?Array { 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); From 795d0d8015a1f1627fa6471669b0ee6c7fe49a9a Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 10 Oct 2018 19:26:29 +0200 Subject: [PATCH 2/2] Implemented LinkPaginator, refactored code for changeset list --- .../ui-components/src/LinkPaginator.js | 129 ++++++++++ .../ui-components/src/buttons/Button.js | 4 +- .../packages/ui-components/src/index.js | 1 + .../src/repos/containers/BranchChangesets.js | 106 -------- scm-ui/src/repos/containers/BranchChooser.js | 157 +++++++----- .../repos/containers/ChangesetContainer.js | 61 ----- scm-ui/src/repos/containers/Changesets.js | 240 ++++-------------- scm-ui/src/repos/containers/RepositoryRoot.js | 55 ++-- scm-ui/src/repos/modules/branches.js | 19 +- scm-ui/src/repos/modules/branches.test.js | 16 ++ scm-ui/src/repos/modules/changesets.js | 50 ++-- scm-ui/src/repos/modules/changesets.test.js | 43 +--- 12 files changed, 374 insertions(+), 507 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/LinkPaginator.js delete mode 100644 scm-ui/src/repos/containers/BranchChangesets.js delete mode 100644 scm-ui/src/repos/containers/ChangesetContainer.js diff --git a/scm-ui-components/packages/ui-components/src/LinkPaginator.js b/scm-ui-components/packages/ui-components/src/LinkPaginator.js new file mode 100644 index 0000000000..75e53df62a --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/LinkPaginator.js @@ -0,0 +1,129 @@ +//@flow +import React from "react"; +import {translate} from "react-i18next"; +import type {PagedCollection} from "@scm-manager/ui-types"; +import {Button} from "./buttons"; +import {withRouter} from "react-router-dom"; + +type Props = { + collection: PagedCollection, + t: string => string, + match: any +}; + +class LinkPaginator extends React.Component { + renderFirstButton() { + return ( +
@@ -131,7 +138,7 @@ class RepositoryRoot extends React.Component { diff --git a/scm-ui/src/repos/modules/branches.js b/scm-ui/src/repos/modules/branches.js index 6e0ca52d5c..2a4580e194 100644 --- a/scm-ui/src/repos/modules/branches.js +++ b/scm-ui/src/repos/modules/branches.js @@ -1,8 +1,13 @@ // @flow -import {FAILURE_SUFFIX, PENDING_SUFFIX, SUCCESS_SUFFIX} from "../../modules/types"; -import {apiClient} from "@scm-manager/ui-components"; -import type {Action, Branch, Repository} from "@scm-manager/ui-types"; -import {isPending} from "../../modules/pending"; +import { + FAILURE_SUFFIX, + PENDING_SUFFIX, + SUCCESS_SUFFIX +} from "../../modules/types"; +import { apiClient } from "@scm-manager/ui-components"; +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}`; @@ -128,7 +133,7 @@ export function getBranch( state: Object, repository: Repository, name: string -): Branch { +): ?Branch { const key = createKey(repository); if (state.branches[key]) { if (state.branches[key].byNames[name]) { @@ -145,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}`; diff --git a/scm-ui/src/repos/modules/branches.test.js b/scm-ui/src/repos/modules/branches.test.js index d9ade07d40..c958f105ef 100644 --- a/scm-ui/src/repos/modules/branches.test.js +++ b/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(); + }); }); }); diff --git a/scm-ui/src/repos/modules/changesets.js b/scm-ui/src/repos/modules/changesets.js index e7e7601196..191de7b37d 100644 --- a/scm-ui/src/repos/modules/changesets.js +++ b/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}`; @@ -82,7 +72,6 @@ export function fetchChangesetsByPage(repository: Repository, page: number) { return fetchChangesetsWithOptions(repository, undefined, `?page=${page - 1}`); } -// TODO: Rewrite code to fetch changesets by branches, adjust tests and let BranchChooser fetch branches export function fetchChangesetsByBranchAndPage( repository: Repository, branch: Branch, @@ -167,11 +156,11 @@ function byKeyReducer( if (state[key]) { oldChangesets[key] = state[key]; } - const byIds = extractChangesetsByIds(changesets, oldChangesets[key].byId); + const byIds = extractChangesetsByIds(changesets); return { ...state, [key]: { - byId: { ...byIds }, + byId: byIds, list: { entries: changesetIds, entry: { @@ -191,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; } @@ -234,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; } @@ -252,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) { diff --git a/scm-ui/src/repos/modules/changesets.test.js b/scm-ui/src/repos/modules/changesets.test.js index 6a557cf1ac..fd33efbc7b 100644 --- a/scm-ui/src/repos/modules/changesets.test.js +++ b/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", () => {