From 3453eae8f4cc5d5886a5a4dc0941b2d391407546 Mon Sep 17 00:00:00 2001 From: Kateryna Musina Date: Wed, 4 Jan 2017 15:01:26 +0100 Subject: [PATCH] Fixed routing by project full name instead of by project ID (#23) --- doc/codestyle.md | 12 ++++ .../PullRequestListItem/index.js | 14 +++-- .../RepositoryItem/RepositoryItem.stories.js | 1 + .../RepositoryList/RepositoryItem/index.js | 9 ++- .../RepositoryList/RepositoryList.stories.js | 3 + .../containers/PullRequestsPaginated/index.js | 22 +++++-- .../containers/SideBar/SideBarConfig.js | 12 ++-- src/client/ducks/repositories/index.js | 5 ++ .../repositories/selectors/__tests__/index.js | 50 +++++++++++++++ .../ducks/repositories/selectors/index.js | 7 +++ src/client/pages/Project/Files/index.js | 2 +- src/client/pages/Project/Project/index.js | 62 +++++++++++++------ .../pages/Project/PullRequests/index.js | 14 ++++- src/client/pages/index.js | 18 ------ src/client/routes/helpers/__tests__/index.js | 18 ++++-- src/client/routes/helpers/index.js | 11 ++-- src/client/routes/index.js | 29 ++++----- .../ono/queries/pullrequests/index.js | 4 ++ 18 files changed, 210 insertions(+), 83 deletions(-) delete mode 100644 src/client/pages/index.js diff --git a/doc/codestyle.md b/doc/codestyle.md index 5f197ef..f80f400 100644 --- a/doc/codestyle.md +++ b/doc/codestyle.md @@ -5,3 +5,15 @@ https://github.com/airbnb/javascript/tree/master/react#basic-rules On top of it some extra rules: 1. No semicolons in javascript +2. Minimizing bundle size + +Prefer imporing bindings from library libs directory(if available) instead of importing full lib bindings, especially when importing +Bootstrap or Material-UI components: + +Good: +`import Navbar from 'react-bootstrap/lib/Navbar'` +`import Route from 'react-router/lib/Route'` + +Bad: +`import { Navbar } from 'react-bootstrap'` +`import { Route, IndexRoute } from 'react-router'` diff --git a/src/client/components/PullRequestList/PullRequestListItem/index.js b/src/client/components/PullRequestList/PullRequestListItem/index.js index 0010224..26d7d0f 100644 --- a/src/client/components/PullRequestList/PullRequestListItem/index.js +++ b/src/client/components/PullRequestList/PullRequestListItem/index.js @@ -5,7 +5,7 @@ import React, { Component } from 'react' import Col from 'react-bootstrap/lib/Col' import Row from 'react-bootstrap/lib/Row' import ListGroupItem from 'react-bootstrap/lib/ListGroupItem' - +import { buildPullRequestLink, buildProjectLink } from 'routes/helpers' import TestAvatar from 'components/TestAvatar' import { Link } from 'react-router' import { fromNow } from 'utils/datetime' @@ -45,7 +45,11 @@ class PullRequestListItem extends Component {
- {pullRequest.title} + + {pullRequest.title} +
{pullRequest.owner.fullName} updated {fromNow(pullRequest.updated)} @@ -59,7 +63,9 @@ class PullRequestListItem extends Component {
{pullRequest.origin.repository.name} #{pullRequest.origin.branch} @@ -73,7 +79,7 @@ class PullRequestListItem extends Component {
{pullRequest.target.repository.name} diff --git a/src/client/components/RepositoryList/RepositoryItem/RepositoryItem.stories.js b/src/client/components/RepositoryList/RepositoryItem/RepositoryItem.stories.js index 4a5dc18..ec5ea3d 100644 --- a/src/client/components/RepositoryList/RepositoryItem/RepositoryItem.stories.js +++ b/src/client/components/RepositoryList/RepositoryItem/RepositoryItem.stories.js @@ -7,6 +7,7 @@ import RepositoryItem from './index' const repository = { name: 'Some-Repository', + groupPath: 'group/subgroup', description: 'Testing out the list item', id: '5', owner: { fullName: 'William Sprent' }, diff --git a/src/client/components/RepositoryList/RepositoryItem/index.js b/src/client/components/RepositoryList/RepositoryItem/index.js index f42e3c2..859fe03 100644 --- a/src/client/components/RepositoryList/RepositoryItem/index.js +++ b/src/client/components/RepositoryList/RepositoryItem/index.js @@ -6,7 +6,7 @@ import Row from 'react-bootstrap/lib/Row' import ListGroupItem from 'react-bootstrap/lib/ListGroupItem' import { Link } from 'react-router' import { fromNow } from 'utils/datetime' -import { buildProjectLinkNoBranch } from 'routes/helpers' +import { buildProjectLink } from 'routes/helpers' const subHeader = text => (
@@ -17,6 +17,7 @@ const subHeader = text => ( export type RepositoryType = { name: string, description: ?string, + groupPath: string, id: string, owner: { fullName: string }, updated: string, @@ -39,7 +40,11 @@ function RepositoryItem(props: Props) {
- {repository.name} + + {repository.name} +
{repository.description}
diff --git a/src/client/components/RepositoryList/RepositoryList.stories.js b/src/client/components/RepositoryList/RepositoryList.stories.js index ca37c0c..751406c 100644 --- a/src/client/components/RepositoryList/RepositoryList.stories.js +++ b/src/client/components/RepositoryList/RepositoryList.stories.js @@ -10,6 +10,7 @@ const repos = [ name: 'Some-Repository1', description: 'Testing out the list item', id: '1', + groupPath: 'group/subgroup', owner: { fullName: 'William Sprent' }, updated: '2016-12-09 10:00:12.250926', }, @@ -17,6 +18,7 @@ const repos = [ name: 'Some-Repository2', description: 'Testing out the list item2', id: '2', + groupPath: 'group/subgroup', owner: { fullName: 'William Sprent' }, updated: '2016-12-09 10:00:12.250926', }, @@ -24,6 +26,7 @@ const repos = [ name: 'Some-Repository3', description: 'Testing out the list item3', id: '3', + groupPath: 'group/subgroup', owner: { fullName: 'William Sprent' }, updated: '2016-12-09 10:00:12.250926', }, diff --git a/src/client/containers/PullRequestsPaginated/index.js b/src/client/containers/PullRequestsPaginated/index.js index c23fdf8..b5a1e9a 100644 --- a/src/client/containers/PullRequestsPaginated/index.js +++ b/src/client/containers/PullRequestsPaginated/index.js @@ -1,6 +1,5 @@ /* @flow */ - import React, { Component } from 'react' import { connect } from 'react-redux' import { FiltersFields } from 'ducks/pullrequests' @@ -10,6 +9,7 @@ import PullRequestList from 'components/PullRequestList' import BranchSelect from 'containers/BranchSelect' import RepoSelect from 'containers/RepoSelect' import OrderSelect from 'containers/OrderSelect' +import Alert from 'react-bootstrap/lib/Alert' import type { OrderByType, DirectionType } from 'ducks/order' export type Props = { @@ -84,6 +84,7 @@ class PullRequestsPaginated extends Component { } render() { + const pullRequestsExists = !!this.props.total return (
{this.props.isFetching && } {this.props.error && } - + {!pullRequestsExists && !this.props.isFetching && !this.props.error && + + + + There is no pull request + + } + {pullRequestsExists && + + }
) } diff --git a/src/client/containers/SideBar/SideBarConfig.js b/src/client/containers/SideBar/SideBarConfig.js index 226f7c0..87c9cfb 100644 --- a/src/client/containers/SideBar/SideBarConfig.js +++ b/src/client/containers/SideBar/SideBarConfig.js @@ -22,14 +22,14 @@ export const app = (store) => { store.dispatch({ type: SIDE_BAR_SUBITEMS, subitems }) } -export const project = (store, id) => { +export const project = (store, name) => { const items = [ { title: 'Home', route: '/', icon: 'home' }, - { title: 'Project', route: `/project/${id}`, icon: 'folder' }, - { title: 'Files', route: `/project/${id}/files`, icon: 'files-o' }, - { title: 'Changelog', route: `/project/${id}/changelog`, icon: 'calendar' }, - { title: 'Pull Requests', route: `/project/${id}/pullrequests`, icon: 'tasks' }, - { title: 'Statistics', route: `/project/${id}/statistics`, icon: 'bar-chart' }, + { title: 'Project', route: `/project/${name}`, icon: 'folder' }, + { title: 'Files', route: `/project/${name}/files`, icon: 'files-o' }, + { title: 'Changelog', route: `/project/${name}/changelog`, icon: 'calendar' }, + { title: 'Pull Requests', route: `/project/${name}/pullrequests`, icon: 'tasks' }, + { title: 'Statistics', route: `/project/${name}/statistics`, icon: 'bar-chart' }, ] store.dispatch({ type: SIDE_BAR_ITEMS, items }) diff --git a/src/client/ducks/repositories/index.js b/src/client/ducks/repositories/index.js index bc1acab..38fad89 100644 --- a/src/client/ducks/repositories/index.js +++ b/src/client/ducks/repositories/index.js @@ -25,6 +25,7 @@ export const types = { SET_REPOSITORIES_NAMES: 'REPOSITORIES/SET_REPOSITORIES_NAMES', SET_GROUPS: 'REPOSITORIES/SET_GROUPS', FETCH_REPOSITORIES: 'REPOSITORIES/FETCH_REPOSITORIES', + FETCH_REPOSITORY: 'REPOSITORIES/FETCH_REPOSITORY', SEARCH_REPOSITORY: 'REPOSITORIES/SEARCH_REPOSITORY', FETCH_REPOSITORY_BRANCHES: 'REPOSITORIES/FETCH_REPOSITORY_BRANCHES', } @@ -100,6 +101,10 @@ export const searchRepository = (filter: string, first: number): Object => ({ type: types.SEARCH_REPOSITORY, filter, first }) +export const fetchRepository = (name: string, queryStr: string): Object => + fetchActionCreator(types.FETCH_REPOSITORY, { name }, queryStr, + (data: Object, cbArgs: Object): Array => + [{ type: types.SET_REPOSITORY, node: parseRepository(data) }]) export const fetchRepositoryBranches = (id: number): Object => fetchActionCreator(types.FETCH_REPOSITORY_BRANCHES, { id }, REPOSITORY_BRANCHES, diff --git a/src/client/ducks/repositories/selectors/__tests__/index.js b/src/client/ducks/repositories/selectors/__tests__/index.js index e7945e4..881ef39 100644 --- a/src/client/ducks/repositories/selectors/__tests__/index.js +++ b/src/client/ducks/repositories/selectors/__tests__/index.js @@ -4,6 +4,7 @@ import { repositories, groupsEntitiesSelector, groups, + getRepositoryId, } from '../index' const expect = chai.expect @@ -320,4 +321,53 @@ describe('projects selectors', () => { expect(groups(state, props)).to.eql([node1]) }) + + it('getRepositoryId selector', () => { + const node1 = { + id: 1, + name: 'name1', + description: 'description1', + fullName: 'group/name1', + + } + const node2 = { + id: 2, + name: 'name2', + description: 'description2', + fullName: 'group/name2', + + } + const node3 = { + id: 3, + name: 'name3', + description: 'description3', + fullName: 'group/name3', + + } + + const state = { + repositories: { + entities: { + 1: node1, + 2: node2, + 3: node3, + }, + }, + } + + const props = { + params: { + splat: 'group/name3', + }, + } + + expect(getRepositoryId(state, props)).to.equal('3') + + const props2 = { + params: { + }, + } + + expect(getRepositoryId(state, props2)).to.equal(undefined) + }) }) diff --git a/src/client/ducks/repositories/selectors/index.js b/src/client/ducks/repositories/selectors/index.js index 7af31ac..6da1a31 100644 --- a/src/client/ducks/repositories/selectors/index.js +++ b/src/client/ducks/repositories/selectors/index.js @@ -61,3 +61,10 @@ export const repositoriesNames = createSelector( repoNames => repoNames.map(x => ({ label: x.fullName, value: x.id })) ) +export const repoNameSelector = (state: Object, props: Object): any => + _.findKey(state.repositories.entities, { fullName: props.params.splat }) + +export const getRepositoryId = createSelector( + repoNameSelector, + repoName => repoName +) diff --git a/src/client/pages/Project/Files/index.js b/src/client/pages/Project/Files/index.js index 915176d..f38c09e 100644 --- a/src/client/pages/Project/Files/index.js +++ b/src/client/pages/Project/Files/index.js @@ -50,7 +50,7 @@ class Files extends Component { updateCurrentNode = (nextProps) => { const { params: { splat } } = nextProps if (splat) { - const nestedItem = this.findChild(nextProps.data, splat.split('/')) + const nestedItem = this.findChild(nextProps.data, splat[0].split('/')) if (nestedItem) { this.setState({ currentNode: nestedItem.children }) } diff --git a/src/client/pages/Project/Project/index.js b/src/client/pages/Project/Project/index.js index 37f6aac..38959b1 100644 --- a/src/client/pages/Project/Project/index.js +++ b/src/client/pages/Project/Project/index.js @@ -1,7 +1,9 @@ /* @flow */ -import React from 'react' +import React, { Component } from 'react' import Helmet from 'react-helmet' +import { connect } from 'react-redux' +import { fetchRepository } from 'ducks/repositories' export type Props = { params: { @@ -9,22 +11,46 @@ export type Props = { }, children?: Object, theme?: Object, -}; - -function Project(props: Props) { - const { theme } = props - const childrenWithProps = React.Children.map(props.children, - child => React.cloneElement(child, { - theme, - }) - ) - - return ( -
- - {childrenWithProps} -
- ) + name: string, } -export default Project +export const REPOSITORY_QUERY = ` +query($name: String!) { + repository(name: $name) { + id + name, + fullName, + branches { + name + revision + } + } +}` + +class Project extends Component { + componentDidMount() { + this.props.dispatch(fetchRepository(this.props.name, REPOSITORY_QUERY)) + } + + render() { + const childrenWithProps = React.Children.map(this.props.children, + child => React.cloneElement(child, { + theme: this.props.theme, + }) + ) + + return ( +
+ + {childrenWithProps} +
+ ) + } +} + +export default connect( + (state, props) => ({ + name: props.params.splat, + }) +)(Project) + diff --git a/src/client/pages/Project/PullRequests/index.js b/src/client/pages/Project/PullRequests/index.js index c76d1b6..98b81b0 100644 --- a/src/client/pages/Project/PullRequests/index.js +++ b/src/client/pages/Project/PullRequests/index.js @@ -2,7 +2,7 @@ import React from 'react' import Helmet from 'react-helmet' - +import { connect } from 'react-redux' import PullRequestsPaginated from 'containers/PullRequestsPaginated' import { fetchPullRequests } from 'ducks/pullrequests' import { @@ -10,11 +10,14 @@ import { getPageFetchError, getPullRequests } from 'ducks/pullrequests/selectors' +import { getRepositoryId } from 'ducks/repositories/selectors' + export type Props = { project_pullrequests: Array, params: { id: string, }, + repo: string, items: Array, } @@ -37,10 +40,15 @@ function PullRequests(props: Props) { hideRepoSelect mapStateToProps={mapStateToProps} fetchData={fetchPullRequests} - repo={props.params.id} + repo={props.repo} /> ) } -export default PullRequests +export default connect( + (state, props) => ({ + repo: getRepositoryId(state, props), + }) +)(PullRequests) + diff --git a/src/client/pages/index.js b/src/client/pages/index.js deleted file mode 100644 index 7281a6c..0000000 --- a/src/client/pages/index.js +++ /dev/null @@ -1,18 +0,0 @@ -/* @flow */ - -import App from './App' -import Home from './Home' -import NotFound from './NotFound' -import Projects from './Projects' // eslint-disable-line import/no-named-as-default -import PullRequests from './PullRequests' -import Html from './Html' - - -export default { - App, - Home, - Html, - Projects, - PullRequests, - NotFound, -} diff --git a/src/client/routes/helpers/__tests__/index.js b/src/client/routes/helpers/__tests__/index.js index c9a5201..606a406 100644 --- a/src/client/routes/helpers/__tests__/index.js +++ b/src/client/routes/helpers/__tests__/index.js @@ -25,16 +25,22 @@ describe('routes helpers', () => { it('buildPullRequestLink', () => { expect(helpers.buildPullRequestLink('projectname', '1234')) - .equals('/project/projectname/pullrequests/1234') + .equals('/project/projectname/pullrequest/1234') }) - it('buildProjectLink', () => { - expect(helpers.buildProjectLink('projectname', 'testbranch')) - .equals('/project/projectname?branch=testbranch') + it('buildProjectLink with group', () => { + expect(helpers.buildProjectLink('projectname', 'group1/subgroup')) + .equals('/project/group1/subgroup/projectname') }) - it('buildProjectsLink', () => { - expect(helpers.buildProjectsLink('projectname')) + it('buildProjectLink with group and branch', () => { + expect(helpers.buildProjectLink('projectname', 'group1/subgroup', 'testbranch')) + .equals('/project/group1/subgroup/projectname?branch=testbranch') + }) + + + it('buildProjectsLink with empty group', () => { + expect(helpers.buildProjectsLink('projectname', null)) .equals('/projects/projectname') }) diff --git a/src/client/routes/helpers/index.js b/src/client/routes/helpers/index.js index f6c5de0..859a02a 100644 --- a/src/client/routes/helpers/index.js +++ b/src/client/routes/helpers/index.js @@ -16,18 +16,19 @@ export const breadcrumbItems = (pathname: string): Array => { export const groupPathFromPath = (path: string): string => path.replace(/^\/projects(\/)?/, '') export const buildPullRequestLink = - (projectName: string, id: string): string => (`/project/${projectName}/pullrequests/${id}`) + (projectName: string, id: string): string => (`/project/${projectName}/pullrequest/${id}`) export const buildProjectLink = - (projectName: string, branch: string): string => (`/project/${projectName}?branch=${branch}`) -export const buildProjectLinkNoBranch = - (projectName: string): string => (`/project/${projectName}`) + (name: string, groupPath: string, branch: string = ''): string => { + const branchQuery = branch ? `?branch=${branch}` : '' + const projectName = groupPath ? `${groupPath}/${name}` : name + return `/project/${projectName}${branchQuery}` + } export const buildProjectsLink = (suffix: string): string => (`/projects/${suffix}`) export const helpers = { buildPullRequestLink, buildProjectLink, - buildProjectLinkNoBranch, buildProjectsLink, groupPathFromPath, breadcrumbItems, diff --git a/src/client/routes/index.js b/src/client/routes/index.js index 97005ab..a49e1e8 100644 --- a/src/client/routes/index.js +++ b/src/client/routes/index.js @@ -1,5 +1,6 @@ import React from 'react' -import { Route, IndexRoute } from 'react-router' +import Route from 'react-router/lib/Route' +import IndexRoute from 'react-router/lib/IndexRoute' import { types } from 'ducks/session' @@ -15,7 +16,6 @@ import Overview from 'pages/Project/Overview' import Changeset from 'pages/Project/Changeset' import ProjectPullRequests from 'pages/Project/PullRequests' import Issues from 'pages/Project/Issues' -import Statistics from 'pages/Project/Statistics' import PullRequest from 'pages/Project/PullRequest' import NewPullRequest from 'pages/Project/NewPullRequest' @@ -27,11 +27,11 @@ export default (store) => { store.dispatch({ type: types.FETCH_USER_PROFILE }) } const onProjectEnter = (nextState) => { - project(store, nextState.params.id) + project(store, nextState.params.splat) } const onPullRequestEnter = (nextState) => { - pullrequest(store, nextState.params.id, nextState.params.prid) + pullrequest(store, nextState.params.splat, nextState.params.prid) } const onChangesetEnter = (nextState) => { @@ -48,21 +48,22 @@ export default (store) => { - + - - - - + + + + + - + - + - - - + + + diff --git a/src/universal/services/ono/queries/pullrequests/index.js b/src/universal/services/ono/queries/pullrequests/index.js index e8a769e..b2f040a 100644 --- a/src/universal/services/ono/queries/pullrequests/index.js +++ b/src/universal/services/ono/queries/pullrequests/index.js @@ -16,6 +16,7 @@ const subQuery = ` repository { id name + fullName } } target { @@ -23,6 +24,7 @@ const subQuery = ` repository { id name + fullName } } owner { @@ -58,6 +60,7 @@ export const projectPullRequestsQuery = ` repository { id name + fullName } } target { @@ -65,6 +68,7 @@ export const projectPullRequestsQuery = ` repository { id name + fullName } } owner {