From 9ae7fc945c28015cde07d30dd738893eca7c5214 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 01/25] Add build to be used by CI that has no color or progress --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 213947a..1092dc4 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "preinstall": "npm run npmcheckversion", "prebuild": "npm run build:clean", "build": "cross-env NODE_ENV=production webpack --config webpack.config.js --color -p --progress --hide-modules --display-optimization-bailout", + "build:ci": "cross-env NODE_ENV=production webpack --config webpack.config.js -p --hide-modules --display-optimization-bailout", "build:clean": "rimraf ./build", "start": "cross-env NODE_ENV=development WEBPACK_SERVE=1 webpack-serve --port 3000 --hmr", "start:production": "npm run test && npm run build && npm run start:prod", From abbe2c8afa782b04c998f048e567a8f4df775ceb Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 02/25] Fix cbuild yml file name --- cbuildci.yml => .cbuildci.yml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename cbuildci.yml => .cbuildci.yml (100%) diff --git a/cbuildci.yml b/.cbuildci.yml similarity index 100% rename from cbuildci.yml rename to .cbuildci.yml From 3b15b02e5fa89f616cd6bb69972ebf107d5d4fbd Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 03/25] Change build to use cache --- .cbuildci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cbuildci.yml b/.cbuildci.yml index a905798..22462fb 100644 --- a/.cbuildci.yml +++ b/.cbuildci.yml @@ -4,3 +4,4 @@ builds: build: timeoutInMinutes: 5 image: 'aws/codebuild/nodejs:8.11.0' + useCache: true From 3b8241400b7957b90f973f4d2564e0eaf650c83e Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 04/25] Fix duplicate commit message --- app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js b/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js index c76b259..8a8687c 100644 --- a/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js +++ b/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js @@ -53,10 +53,6 @@ function ExecutionSummaryPanel({ author={author} /> - -
- – {commitMessage.substr(0, 100)}{commitMessage.length > 100 ? '…' : ''} -
From df4f41c7db4fee179cc7d22b701af0d70900e52c Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 05/25] Fix LanguageProvider tests --- app/containers/LanguageProvider/tests/index.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/containers/LanguageProvider/tests/index.test.js b/app/containers/LanguageProvider/tests/index.test.js index 92da969..472018d 100644 --- a/app/containers/LanguageProvider/tests/index.test.js +++ b/app/containers/LanguageProvider/tests/index.test.js @@ -2,7 +2,7 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; import { FormattedMessage, defineMessages } from 'react-intl'; import { Provider } from 'react-redux'; -import { browserHistory } from 'react-router-dom'; +import createHistory from 'history/createBrowserHistory'; import ConnectedLanguageProvider, { LanguageProvider } from '../index'; import configureStore from '../../../configureStore'; @@ -33,7 +33,7 @@ describe('', () => { let store; beforeAll(() => { - store = configureStore({}, browserHistory); + store = configureStore({}, createHistory()); }); it('should render the default language messages', () => { From e79237aeaa7e5a1afe7d84c591a68ac4dd258999 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 06/25] Fix tests for ExecutionStartMessage --- .../tests/__snapshots__/index.test.js.snap | 62 +++++++++++++------ .../ExecutionStartMessage/tests/index.test.js | 24 ++++--- 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/app/components/ExecutionStartMessage/tests/__snapshots__/index.test.js.snap b/app/components/ExecutionStartMessage/tests/__snapshots__/index.test.js.snap index 3be9ca9..379761d 100644 --- a/app/components/ExecutionStartMessage/tests/__snapshots__/index.test.js.snap +++ b/app/components/ExecutionStartMessage/tests/__snapshots__/index.test.js.snap @@ -7,7 +7,7 @@ exports[` should render expected JSX for "opened" pull r synchronize {{actionHtml} to} other {{actionHtml}} } - {pullRequestLinkHtml}" + {pullRequestLinkHtml} by {usernameHtml}" id="app.components.ExecutionStartMessage.startedForPullRequest" values={ Object { @@ -27,7 +27,7 @@ exports[` should render expected JSX for "opened" pull r } /> , - "event": "pull_request", + "eventType": "pull_request", "pullRequestLinkHtml": @@ -51,6 +51,12 @@ exports[` should render expected JSX for "opened" pull r /> , + "username": "bsmith", + "usernameHtml": + bsmith + , } } /> @@ -86,6 +92,12 @@ exports[` should render expected JSX for "opened" pull r pull request #5 + by + + bsmith + `; @@ -98,7 +110,7 @@ exports[` should render expected JSX for "rerun" user ev id="app.components.ExecutionStartMessage.startedByUserAction" values={ Object { - "event": "check_run", + "eventType": "check_run", "identifier": "rerun", "identifierHtml": should render expected JSX for "rerun" user ev /> , - "username": "amekkawi-office", + "username": "bsmith", "usernameHtml": - @ - amekkawi-office + bsmith , } } @@ -159,9 +170,9 @@ exports[` should render expected JSX for "rerun" user ev initiated by - @amekkawi-office + bsmith `; @@ -173,7 +184,7 @@ exports[` should render expected JSX for "synchronize" p synchronize {{actionHtml} to} other {{actionHtml}} } - {pullRequestLinkHtml}" + {pullRequestLinkHtml} by {usernameHtml}" id="app.components.ExecutionStartMessage.startedForPullRequest" values={ Object { @@ -193,7 +204,7 @@ exports[` should render expected JSX for "synchronize" p } /> , - "event": "pull_request", + "eventType": "pull_request", "pullRequestLinkHtml": @@ -217,6 +228,12 @@ exports[` should render expected JSX for "synchronize" p /> , + "username": "bsmith", + "usernameHtml": + bsmith + , } } /> @@ -252,16 +269,22 @@ exports[` should render expected JSX for "synchronize" p pull request #5 + by + + bsmith + `; exports[` should render expected JSX for unknown event 1`] = ` should render expected JSX for unknown user ev id="app.components.ExecutionStartMessage.startedByUserAction" values={ Object { - "event": "check_run", + "eventType": "check_run", "identifier": "foobar", "identifierHtml": should render expected JSX for unknown user ev /> , - "username": "amekkawi-office", + "username": "bsmith", "usernameHtml": - @ - amekkawi-office + bsmith , } } @@ -366,9 +388,9 @@ exports[` should render expected JSX for unknown user ev initiated by - @amekkawi-office + bsmith `; diff --git a/app/components/ExecutionStartMessage/tests/index.test.js b/app/components/ExecutionStartMessage/tests/index.test.js index bd14884..d0ba374 100644 --- a/app/components/ExecutionStartMessage/tests/index.test.js +++ b/app/components/ExecutionStartMessage/tests/index.test.js @@ -22,7 +22,7 @@ describe('', () => { repo="bar" createTime={1500000000000} event={{ - event: 'foobar', + type: 'foobar', }} />, ); @@ -36,11 +36,16 @@ describe('', () => { repo="bar" createTime={1500000000000} event={{ - event: 'pull_request', + type: 'pull_request', action: 'opened', pull_request: { number: 5, }, + sender: { + login: 'bsmith', + type: 'User', + id: 13106037, + }, }} />, ); @@ -54,11 +59,16 @@ describe('', () => { repo="bar" createTime={1500000000000} event={{ - event: 'pull_request', + type: 'pull_request', action: 'synchronize', pull_request: { number: 5, }, + sender: { + login: 'bsmith', + type: 'User', + id: 13106037, + }, }} />, ); @@ -72,13 +82,13 @@ describe('', () => { repo="bar" createTime={1500000000000} event={{ - event: 'check_run', + type: 'check_run', action: 'requested_action', requested_action: { identifier: 'rerun', }, sender: { - login: 'amekkawi-office', + login: 'bsmith', type: 'User', id: 13106037, }, @@ -95,13 +105,13 @@ describe('', () => { repo="bar" createTime={1500000000000} event={{ - event: 'check_run', + type: 'check_run', action: 'requested_action', requested_action: { identifier: 'foobar', }, sender: { - login: 'amekkawi-office', + login: 'bsmith', type: 'User', id: 13106037, }, From 4ab82b2aaef48cb08e71c529e386407218fde7d3 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 07/25] Fix test snapshot for ExecutionStopMessage --- .../tests/__snapshots__/index.test.js.snap | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/components/ExecutionStopMessage/tests/__snapshots__/index.test.js.snap b/app/components/ExecutionStopMessage/tests/__snapshots__/index.test.js.snap index 6bdf77d..deefe1f 100644 --- a/app/components/ExecutionStopMessage/tests/__snapshots__/index.test.js.snap +++ b/app/components/ExecutionStopMessage/tests/__snapshots__/index.test.js.snap @@ -72,7 +72,6 @@ exports[` should render expected JSX when user requests s "stopUserHtml": - @ foo , } @@ -99,7 +98,7 @@ exports[` should render expected JSX when user requests s - @foo + foo @@ -140,7 +139,6 @@ exports[` should render expected JSX when user requests s "stopUserHtml": - @ foo , } @@ -167,7 +165,7 @@ exports[` should render expected JSX when user requests s - @foo + foo and took From bdf65c8756ebaf60e29a7e3569e04469fc5546fe Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 08/25] Fix test snapshot for PageHeader --- .../tests/__snapshots__/index.test.js.snap | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/app/components/PageHeader/tests/__snapshots__/index.test.js.snap b/app/components/PageHeader/tests/__snapshots__/index.test.js.snap index 3513de8..f1ee6e7 100644 --- a/app/components/PageHeader/tests/__snapshots__/index.test.js.snap +++ b/app/components/PageHeader/tests/__snapshots__/index.test.js.snap @@ -1,23 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[` should render expected JSX 1`] = ` -

+ Foobar -

+ `; exports[` should render expected JSX 2`] = `

Foobar

From b5b1047c59ef2ecc89f0e553847657e69637a0ec Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 09/25] Fix test snapshot for PhasesTable component --- .../tests/__snapshots__/index.test.js.snap | 54 +++++++------------ 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/app/components/PhasesTable/tests/__snapshots__/index.test.js.snap b/app/components/PhasesTable/tests/__snapshots__/index.test.js.snap index d6d6812..ee0eb0b 100644 --- a/app/components/PhasesTable/tests/__snapshots__/index.test.js.snap +++ b/app/components/PhasesTable/tests/__snapshots__/index.test.js.snap @@ -10,8 +10,8 @@ exports[` should render expected JSX 1`] = ` @@ -32,9 +32,7 @@ exports[` should render expected JSX 1`] = ` - + SUBMITTED @@ -49,9 +47,7 @@ exports[` should render expected JSX 1`] = ` /> - + PROVISIONING @@ -66,9 +62,7 @@ exports[` should render expected JSX 1`] = ` /> - + DOWNLOAD_SOURCE @@ -83,9 +77,7 @@ exports[` should render expected JSX 1`] = ` /> - + INSTALL @@ -100,9 +92,7 @@ exports[` should render expected JSX 1`] = ` /> - + PRE_BUILD @@ -117,9 +107,7 @@ exports[` should render expected JSX 1`] = ` /> - + BUILD @@ -134,9 +122,7 @@ exports[` should render expected JSX 1`] = ` /> - + POST_BUILD @@ -151,9 +137,7 @@ exports[` should render expected JSX 1`] = ` /> - + UPLOAD_ARTIFACTS @@ -168,9 +152,7 @@ exports[` should render expected JSX 1`] = ` /> - + FINALIZING @@ -185,9 +167,7 @@ exports[` should render expected JSX 1`] = ` /> - + COMPLETED @@ -196,7 +176,9 @@ exports[` should render expected JSX 1`] = ` status="SUCCEEDED" /> - + + - + @@ -212,7 +194,7 @@ exports[` should render expected JSX 2`] = ` - Build + Phase @@ -442,7 +424,9 @@ exports[` should render expected JSX 2`] = ` - + + - + From 72994d29bba420583d915f385685666a8ff13cb7 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 10/25] Fix test snapshot for NotFoundPage --- .../NotFoundPage/tests/__snapshots__/index.test.js.snap | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/containers/NotFoundPage/tests/__snapshots__/index.test.js.snap b/app/containers/NotFoundPage/tests/__snapshots__/index.test.js.snap index 9b38422..734c179 100644 --- a/app/containers/NotFoundPage/tests/__snapshots__/index.test.js.snap +++ b/app/containers/NotFoundPage/tests/__snapshots__/index.test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[` should render the expected JSX 1`] = ` - + should render the expected JSX 1`] = ` values={Object {}} />

- + `; exports[` should render the expected JSX 2`] = ` Array [

Page Not Found From 3b977414fdff6a99661a78f2780a6d04cc8d1dd3 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 11/25] Fix ExecutionsBreadcrumb not setting owner/repo link to "active" --- app/components/ExecutionsBreadcrumb/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/ExecutionsBreadcrumb/index.js b/app/components/ExecutionsBreadcrumb/index.js index c0fc5e4..f29571d 100644 --- a/app/components/ExecutionsBreadcrumb/index.js +++ b/app/components/ExecutionsBreadcrumb/index.js @@ -23,7 +23,7 @@ function ExecutionsBreadcrumb({ if (repo != null) { if (commit == null) { repoHtml = ( -
  • +
  • {owner}/{repo}
    • @@ -46,7 +46,7 @@ function ExecutionsBreadcrumb({ commitHtml = (
  • - {commit.substr(0, 10)} + {commit.substr(0, 10)}
    • ); } @@ -55,7 +55,7 @@ function ExecutionsBreadcrumb({
  • - {commit.substr(0, 10)} + {commit.substr(0, 10)}
    • ); From d2fa49e57dfd929480d3903dcbc35dd62f37267c Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 12/25] Fix tests for App container's selectors --- app/containers/App/selectors.js | 1 + app/containers/App/tests/selectors.test.js | 133 ++++++++++++++------- 2 files changed, 91 insertions(+), 43 deletions(-) diff --git a/app/containers/App/selectors.js b/app/containers/App/selectors.js index 4573949..7ccb467 100644 --- a/app/containers/App/selectors.js +++ b/app/containers/App/selectors.js @@ -39,6 +39,7 @@ const selectLocation = createSelector( export { selectApp, + selectRoute, selectLoggingIn, selectLoginUrl, selectLoginError, diff --git a/app/containers/App/tests/selectors.test.js b/app/containers/App/tests/selectors.test.js index fb0ec77..9b9444e 100644 --- a/app/containers/App/tests/selectors.test.js +++ b/app/containers/App/tests/selectors.test.js @@ -1,13 +1,33 @@ import { fromJS } from 'immutable'; -import { +import * as selectors from '../selectors'; + +describe('exports', () => { + it('should have the expected exports', () => { + expect(Object.keys(selectors).sort()) + .toEqual([ + 'selectApp', + 'selectRoute', + 'selectLoggingIn', + 'selectLoginUrl', + 'selectLoginError', + 'selectIsLoggedIn', + 'selectGithubHost', + 'selectLocation', + ].sort()); + }); +}); + +const { selectApp, - makeSelectCurrentUser, - makeSelectLoading, - makeSelectError, - makeSelectRepos, - makeSelectLocation, -} from '../selectors'; + selectRoute, + selectLoggingIn, + selectLoginUrl, + selectLoginError, + selectIsLoggedIn, + selectGithubHost, + selectLocation, +} = selectors; describe('selectApp', () => { it('should select the app state', () => { @@ -19,71 +39,98 @@ describe('selectApp', () => { }); }); -describe('makeSelectCurrentUser', () => { - const currentUserSelector = makeSelectCurrentUser(); - it('should select the current user', () => { - const username = 'mxstbr'; - const mockedState = fromJS({ +describe('selectLoggingIn', () => { + it('should select if user is logging in', () => { + const mockedStateA = fromJS({ + app: { + loggingIn: true, + }, + }); + expect(selectLoggingIn(mockedStateA)).toEqual(true); + + const mockedStateB = fromJS({ app: { - currentUser: username, + loggingIn: false, }, }); - expect(currentUserSelector(mockedState)).toEqual(username); + expect(selectLoggingIn(mockedStateB)).toEqual(false); }); }); -describe('makeSelectLoading', () => { - const loadingSelector = makeSelectLoading(); - it('should select the loading', () => { - const loading = false; +describe('selectLoginUrl', () => { + it('should select login URL', () => { const mockedState = fromJS({ app: { - loading, + loginUrl: 'http://foobar/', }, }); - expect(loadingSelector(mockedState)).toEqual(loading); + expect(selectLoginUrl(mockedState)).toEqual('http://foobar/'); }); }); -describe('makeSelectError', () => { - const errorSelector = makeSelectError(); - it('should select the error', () => { - const error = 404; +describe('selectLoginError', () => { + it('should select login error', () => { + const err = new Error(); const mockedState = fromJS({ app: { - error, + loginError: err, + }, + }); + expect(selectLoginError(mockedState)).toEqual(err); + }); +}); + +describe('selectIsLoggedIn', () => { + it('should select if user is logged in', () => { + const mockedStateA = fromJS({ + app: { + isLoggedIn: true, }, }); - expect(errorSelector(mockedState)).toEqual(error); + expect(selectIsLoggedIn(mockedStateA)).toEqual(true); + + const mockedStateB = fromJS({ + app: { + isLoggedIn: false, + }, + }); + expect(selectIsLoggedIn(mockedStateB)).toEqual(false); }); }); -describe('makeSelectRepos', () => { - const reposSelector = makeSelectRepos(); - it('should select the repos', () => { - const repositories = fromJS([]); +describe('selectGithubHost', () => { + it('should select Github host', () => { const mockedState = fromJS({ app: { - userData: { - repositories, - }, + githubHost: 'foobar.github.com', }, }); - expect(reposSelector(mockedState)).toEqual(repositories); + expect(selectGithubHost(mockedState)).toEqual('foobar.github.com'); }); }); -describe('makeSelectLocation', () => { - const locationStateSelector = makeSelectLocation(); - it('should select the location', () => { - const route = fromJS({ - location: { pathname: '/foo' }, +describe('selectRoute', () => { + it('should select the route state', () => { + const routeState = fromJS({}); + const mockedState = fromJS({ + route: routeState, }); + expect(selectRoute(mockedState)).toEqual(routeState); + }); +}); + +describe('selectLocation', () => { + it('should select route location', () => { const mockedState = fromJS({ - route, + route: { + location: { + path: '/foo/bar', + }, + }, + }); + + expect(selectLocation(mockedState)).toEqual({ + path: '/foo/bar', }); - expect(locationStateSelector(mockedState)).toEqual( - route.get('location').toJS(), - ); }); }); From e032348c903ebf702e109cbc25bdadd8ff31002d Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 13/25] Rework app state and how user login is handled --- app/app.js | 25 ++- app/containers/App/LoginModal.js | 26 +-- app/containers/App/actions.js | 39 +++-- app/containers/App/constants.js | 14 +- app/containers/App/index.js | 13 +- app/containers/App/reducer.js | 69 ++++++-- app/containers/App/saga.js | 157 ++++++++++++++++++ app/containers/App/selectors.js | 84 +++++++--- app/containers/CommitExecutionsPage/saga.js | 62 ++----- .../CommitExecutionsPage/selectors.js | 15 ++ app/containers/ExecutionDetailPage/saga.js | 86 ++-------- app/containers/RepoExecutionsPage/index.js | 1 - app/containers/RepoExecutionsPage/saga.js | 51 ++---- app/containers/TopNav/index.js | 87 ++++++---- app/reducers.js | 2 - app/utils/request.js | 36 ++++ 16 files changed, 517 insertions(+), 250 deletions(-) create mode 100644 app/containers/App/saga.js diff --git a/app/app.js b/app/app.js index 074df2c..3610f8b 100644 --- a/app/app.js +++ b/app/app.js @@ -16,8 +16,14 @@ import { ConnectedRouter } from 'connected-react-router/immutable'; import createHistory from 'history/createBrowserHistory'; // import 'sanitize.css/sanitize.css'; -// Import root app +// Import root app and constants import App from 'containers/App'; +import { + WINDOW_BLUR, + WINDOW_FOCUS, + WINDOW_VISIBLE, + WINDOW_HIDDEN, +} from 'containers/App/constants'; // Import providers import LanguageProvider from 'containers/LanguageProvider'; @@ -49,6 +55,23 @@ history.listen((location, action) => { }); const store = configureStore(initialState, history); + +// Dispatch events on window focus/visibility events. +window.addEventListener('blur', () => { + store.dispatch({ type: WINDOW_BLUR }); +}); +window.addEventListener('focus', () => { + store.dispatch({ type: WINDOW_FOCUS }); +}); +window.addEventListener('visibilitychange', () => { + if (document.visibilityState === 'visible') { + store.dispatch({ type: WINDOW_VISIBLE }); + } + if (document.visibilityState === 'hidden') { + store.dispatch({ type: WINDOW_HIDDEN }); + } +}); + const MOUNT_NODE = document.getElementById('app'); const render = (messages) => { diff --git a/app/containers/App/LoginModal.js b/app/containers/App/LoginModal.js index a1853cb..0dfc843 100644 --- a/app/containers/App/LoginModal.js +++ b/app/containers/App/LoginModal.js @@ -7,18 +7,21 @@ import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import { createStructuredSelector } from 'reselect'; import { FormattedMessage } from 'react-intl'; +import { buildApiUrl } from '../../utils/request'; + import messages from './messages'; import { - selectLoggingIn, - selectLoginUrl, + selectGithubHost, + selectIsLoginRequired, + selectEndpoints, } from './selectors'; export class LoginModal extends React.Component { static propTypes = { - loginUrl: PropTypes.string.isRequired, githubHost: PropTypes.string.isRequired, + loginUrl: PropTypes.string.isRequired, }; constructor(props) { @@ -81,7 +84,7 @@ export class LoginModal extends React.Component { />
    - + +function LoginModalContaner({ isLoggingIn, ...props }) { + return isLoggingIn + ? : null; } LoginModalContaner.propTypes = { - loggingIn: PropTypes.bool, + isLoggingIn: PropTypes.bool, }; LoginModalContaner.defaultProps = { - loggingIn: false, + isLoggingIn: false, }; const mapStateToProps = createStructuredSelector({ - loggingIn: selectLoggingIn, - loginUrl: selectLoginUrl, + githubHost: selectGithubHost, + isLoggingIn: selectIsLoginRequired, + loginUrl: (state) => selectEndpoints(state).authRedirectUrl, }); export default connect( diff --git a/app/containers/App/actions.js b/app/containers/App/actions.js index 072e62b..a432945 100644 --- a/app/containers/App/actions.js +++ b/app/containers/App/actions.js @@ -3,27 +3,46 @@ */ import { - LOGIN_REQUEST, - LOGIN_SUCCESS, - LOGIN_FAILURE, + STATE_RESTORE, + STATE_REQUEST, + STATE_SUCCESS, + STATE_FAILURE, } from './constants'; -export function loginRequest(loginUrl) { +export function stateRestore(state) { return { - type: LOGIN_REQUEST, - loginUrl, + type: STATE_RESTORE, + state, }; } -export function fetchExecutionSuccess() { +export function stateRequest(resetUser) { return { - type: LOGIN_SUCCESS, + type: STATE_REQUEST, + resetUser, }; } -export function fetchExecutionFailure(error) { +export function stateSuccess( + githubUrl, + githubHost, + userLogin, + userName, + endpoints, +) { return { - type: LOGIN_FAILURE, + type: STATE_SUCCESS, + githubUrl, + githubHost, + userLogin, + userName, + endpoints, + }; +} + +export function stateFailure(error) { + return { + type: STATE_FAILURE, error, }; } diff --git a/app/containers/App/constants.js b/app/containers/App/constants.js index 2418c77..60c401a 100644 --- a/app/containers/App/constants.js +++ b/app/containers/App/constants.js @@ -2,6 +2,14 @@ * App Constants */ -export const LOGIN_REQUEST = 'container/App/LOGIN_REQUEST'; -export const LOGIN_SUCCESS = 'container/App/LOGIN_SUCCESS'; -export const LOGIN_FAILURE = 'container/App/LOGIN_FAILURE'; +export const SESSION_STORAGE_KEY = 'appRestoreState'; + +export const WINDOW_BLUR = 'container/App/WINDOW_BLUR'; +export const WINDOW_FOCUS = 'container/App/WINDOW_FOCUS'; +export const WINDOW_VISIBLE = 'container/App/WINDOW_VISIBLE'; +export const WINDOW_HIDDEN = 'container/App/WINDOW_HIDDEN'; + +export const STATE_RESTORE = 'container/App/STATE_RESTORE'; +export const STATE_REQUEST = 'container/App/STATE_REQUEST'; +export const STATE_SUCCESS = 'container/App/STATE_SUCCESS'; +export const STATE_FAILURE = 'container/App/STATE_FAILURE'; diff --git a/app/containers/App/index.js b/app/containers/App/index.js index 53d731f..7a3554f 100644 --- a/app/containers/App/index.js +++ b/app/containers/App/index.js @@ -8,6 +8,7 @@ import React from 'react'; import { Helmet } from 'react-helmet'; +import { compose } from 'redux'; import { hot } from 'react-hot-loader'; import { Switch, Route } from 'react-router-dom'; @@ -15,6 +16,9 @@ import { ErrorContextProvider } from 'components/ErrorBoundary'; import NotFoundPage from 'containers/NotFoundPage/Loadable'; import TopNav from 'containers/TopNav'; +import reducer, { injectReducer } from './reducer'; +import saga, { injectSaga } from './saga'; + // Pages import RepoExecutionsPage from 'containers/RepoExecutionsPage/Loadable'; import CommitExecutionsPage from 'containers/CommitExecutionsPage/Loadable'; @@ -109,4 +113,11 @@ export function App() { ); } -export default hot(module)(App); +const withReducer = injectReducer('app', reducer); +const withSaga = injectSaga('app', saga); + +export default compose( + withReducer, + withSaga, + hot(module) +)(App); diff --git a/app/containers/App/reducer.js b/app/containers/App/reducer.js index dc42ea6..28599bc 100644 --- a/app/containers/App/reducer.js +++ b/app/containers/App/reducer.js @@ -1,31 +1,74 @@ +/* + * App reducer + */ + import { fromJS } from 'immutable'; +import createInjector from 'utils/injectReducer'; import { - LOGIN_REQUEST, + STATE_RESTORE, + STATE_REQUEST, + STATE_SUCCESS, + STATE_FAILURE, } from './constants'; +const userInitial = { + userName: null, + userLogin: null, +}; + // The initial state of the App const initialState = fromJS({ - loggingIn: false, - loginUrl: null, - loginError: null, - isLoggedIn: false, - userData: null, - githubHost: 'github.com', + hasRestoredState: false, + hasFetchedState: false, + isFetchingState: false, + stateError: null, + githubUrl: null, + githubHost: null, + endpoints: null, + ...userInitial, }); function appReducer(state = initialState, action) { switch (action.type) { - case LOGIN_REQUEST: + case STATE_RESTORE: return state - .set('loggingIn', true) - .set('loginUrl', action.loginUrl) - .set('loginError', null) - .set('isLoggedIn', null) - .set('userData', null); + .set('hasRestoredState', true) + .set('githubUrl', action.state.githubUrl) + .set('githubHost', action.state.githubHost) + .set('endpoints', action.state.endpoints) + .set('userName', action.state.userName) + .set('userLogin', action.state.userLogin); + + case STATE_REQUEST: + state = state + .set('isFetchingState', true) + .set('stateError', null); + + return action.resetUser + ? state.merge(userInitial) + : state; + + case STATE_SUCCESS: + return state + .set('hasFetchedState', true) + .set('isFetchingState', false) + .set('stateError', null) + .set('githubUrl', action.githubUrl) + .set('githubHost', action.githubHost) + .set('endpoints', action.endpoints) + .set('userName', action.userName) + .set('userLogin', action.userLogin); + + case STATE_FAILURE: + return state + .set('isFetchingState', false) + .set('stateError', action.error); + default: return state; } } +export const injectReducer = createInjector(module); export default appReducer; diff --git a/app/containers/App/saga.js b/app/containers/App/saga.js new file mode 100644 index 0000000..6294a66 --- /dev/null +++ b/app/containers/App/saga.js @@ -0,0 +1,157 @@ +import { call, put, select, take, takeLatest, race, throttle } from 'redux-saga/effects'; +import createInjector from 'utils/injectSaga'; +import { delay } from '../../utils/saga-util'; +import { requestJson, buildApiUrl } from 'utils/request'; + +import { + WINDOW_VISIBLE, + WINDOW_FOCUS, + SESSION_STORAGE_KEY, + STATE_REQUEST, + STATE_RESTORE, + STATE_SUCCESS, +} from './constants'; + +import { + stateRestore, + stateRequest, + stateSuccess, + stateFailure, +} from './actions'; + +import { + selectEndpoints, + selectHasRestoredState, + selectRestoreState, + selectIsFetchingState, + selectIsUserLoggedIn, + selectIsLoginRequired, +} from './selectors'; + +function* init() { + // Skip init if already restored state. + if (yield select(selectHasRestoredState)) { + return; + } + + try { + yield put(stateRestore( + JSON.parse(sessionStorage.getItem(SESSION_STORAGE_KEY) || '{}'), + )); + } + catch (err) { + // eslint-disable-next-line no-console + console.warn(`Failed to restore state: ${err.stack}`); + } + + // Request the state on app init. + yield put(stateRequest()); +} + +function* fetchState() { + try { + yield delay(500); // TODO: Remove + + const { app, user, endpoints } = yield call( + requestJson, + '/api/v1', + ); + + yield put(stateSuccess( + app.githubUrl, + app.githubHost, + user && user.login || null, + user && user.name || null, + endpoints, + )); + } + catch (err) { + if (err.isJson && err.body) { + yield put(stateFailure( + err.body, + )); + } + else { + yield put(stateFailure( + { + message: err.message, + }, + )); + } + } +} + +function* saveStateToSession() { + const state = yield select(selectRestoreState); + + // Persist the state on success. + sessionStorage.setItem( + SESSION_STORAGE_KEY, + JSON.stringify(state), + ); +} + +function* checkForLoginSuccess() { + if (!(yield select(selectIsFetchingState)) && (yield select(selectIsLoginRequired))) { + yield put(stateRequest()); + } +} + +export function* waitForLogin() { + while (!(yield select(selectIsUserLoggedIn))) { + + // Request state if not already requesting it. + if (!(yield select(selectIsFetchingState)) && !(yield select(selectIsLoginRequired))) { + yield put(stateRequest()); + } + + yield race({ + restore: take(STATE_RESTORE), + success: take(STATE_SUCCESS), + }); + } +} + +export function* endpointRequest(endpoint, params, ...args) { + while (true) { + yield call(waitForLogin); + + const endpoints = yield select(selectEndpoints); + const url = buildApiUrl(endpoints[endpoint], params); + + try { + return yield call(requestJson, url, ...args); + } + catch (err) { + if (err.isJson && err.status === 403 && err.body && err.body.authRedirectUrl) { + yield put(stateRequest(true)); + } + else { + throw err; + } + } + } +} + +export const injectSaga = createInjector(module); + +export default function* defaultSaga() { + yield takeLatest( + STATE_REQUEST, + fetchState, + ); + + yield takeLatest( + [STATE_REQUEST, STATE_SUCCESS], + saveStateToSession, + ); + + // Refetch state on focus and visibility just in case + // the user logged in using another window. + yield throttle(10000, [ + WINDOW_VISIBLE, + WINDOW_FOCUS, + ], checkForLoginSuccess); + + yield* init(); +} diff --git a/app/containers/App/selectors.js b/app/containers/App/selectors.js index 7ccb467..05e93ce 100644 --- a/app/containers/App/selectors.js +++ b/app/containers/App/selectors.js @@ -2,48 +2,84 @@ * The app state selectors */ -import { createSelector } from 'reselect'; +import { + createSelector, + createStructuredSelector, +} from 'reselect'; -const selectApp = (state) => state.get('app'); -const selectRoute = (state) => state.get('route'); +export const selectApp = (state) => state.get('app'); +export const selectRoute = (state) => state.get('route'); -const selectLoggingIn = createSelector( +export const selectHasRestoredState = createSelector( selectApp, - (appState) => appState.get('loggingIn'), + (appState) => appState.get('hasRestoredState'), ); -const selectLoginUrl = createSelector( +export const selectHasFetchedState = createSelector( selectApp, - (appState) => appState.get('loginUrl'), + (appState) => appState.get('hasFetchedState'), ); -const selectLoginError = createSelector( +export const selectIsFetchingState = createSelector( selectApp, - (appState) => appState.get('loginError'), + (appState) => appState.get('isFetchingState'), ); -const selectIsLoggedIn = createSelector( +export const selectStateError = createSelector( selectApp, - (appState) => appState.get('isLoggedIn'), + (appState) => appState.get('stateError'), ); -const selectGithubHost = createSelector( +export const selectGithubUrl = createSelector( selectApp, - (routeState) => routeState.get('githubHost') + (appState) => appState.get('githubUrl') ); -const selectLocation = createSelector( - selectRoute, - (routeState) => routeState.get('location').toJS() +export const selectGithubHost = createSelector( + selectApp, + (appState) => appState.get('githubHost') +); + +export const selectUserName = createSelector( + selectApp, + (appState) => appState.get('userName'), ); -export { +export const selectUserLogin = createSelector( selectApp, + (appState) => appState.get('userLogin'), +); + +export const selectEndpoints = createSelector( + selectApp, + (appState) => appState.get('endpoints') || {}, +); + +export const selectIsUserLoggedIn = createSelector( + selectUserLogin, + (userLogin) => userLogin != null, +); + +export const selectIsLoginRequired = createSelector( + selectHasFetchedState, + selectUserLogin, + (hasFetchedState, userLogin) => hasFetchedState && userLogin == null, +); + +export const selectHasLoadedState = createSelector( + selectGithubUrl, + (githubUrl) => !!githubUrl, +); + +export const selectRestoreState = createStructuredSelector({ + githubUrl: selectGithubUrl, + githubHost: selectGithubHost, + endpoints: selectEndpoints, + userName: selectUserName, + userLogin: selectUserLogin, +}); + +export const selectLocation = createSelector( selectRoute, - selectLoggingIn, - selectLoginUrl, - selectLoginError, - selectIsLoggedIn, - selectGithubHost, - selectLocation, -}; + (routeState) => routeState.get('location').toJS() +); diff --git a/app/containers/CommitExecutionsPage/saga.js b/app/containers/CommitExecutionsPage/saga.js index 142d874..975fe7e 100644 --- a/app/containers/CommitExecutionsPage/saga.js +++ b/app/containers/CommitExecutionsPage/saga.js @@ -1,7 +1,10 @@ -import { take, call, put, select, takeLatest } from 'redux-saga/effects'; +import { call, put, select, takeLatest } from 'redux-saga/effects'; import createInjector from 'utils/injectSaga'; import { raceCancel } from '../../utils/saga-util'; -import { requestJson } from 'utils/request'; + +import { + endpointRequest, +} from 'containers/App/saga'; import { PAGE_OPENED, @@ -10,18 +13,10 @@ import { } from './constants'; import { - LOGIN_SUCCESS, -} from 'containers/App/constants'; - -import { - loginRequest, -} from 'containers/App/actions'; - -import { - selectLoggingIn, -} from 'containers/App/selectors'; - -import selectDomain from './selectors'; + selectOwner, + selectRepo, + selectCommit, +} from './selectors'; import { fetchExecutions, @@ -30,18 +25,10 @@ import { } from './actions'; export function* fetchingExecution() { - // Check if a login is in process. - const isLoggingIn = yield select(selectLoggingIn); - if (isLoggingIn) { - yield take(LOGIN_SUCCESS); - } - try { - const { - owner, - repo, - commit, - } = yield select(selectDomain); + const owner = yield select(selectOwner); + const repo = yield select(selectRepo); + const commit = yield select(selectCommit); if (!owner) { return; @@ -51,8 +38,9 @@ export function* fetchingExecution() { executions, lastEvaluatedKey, } = yield call( - requestJson, - `/api/v1/repo/${owner}/${repo}/commit/${commit}`, + endpointRequest, + 'searchByCommitUrl', + { owner, repo, commit }, ); yield put(fetchExecutionsSuccess( @@ -62,23 +50,9 @@ export function* fetchingExecution() { } catch (err) { if (err.isJson && err.body) { - if (err.status === 403 && err.body.authRedirectUrl) { - // Start a login request. - yield put(loginRequest(err.body.authRedirectUrl)); - - // Wait for the login to succeed or the - const loginSuccessResult = yield take(LOGIN_SUCCESS); - - if (loginSuccessResult) { - // Try the fetch again... - yield call(fetchExecutions()); - } - } - else { - yield put(fetchExecutionsFailure( - err.body, - )); - } + yield put(fetchExecutionsFailure( + err.body, + )); } else { yield put(fetchExecutionsFailure( diff --git a/app/containers/CommitExecutionsPage/selectors.js b/app/containers/CommitExecutionsPage/selectors.js index 240d9b4..eb78c91 100644 --- a/app/containers/CommitExecutionsPage/selectors.js +++ b/app/containers/CommitExecutionsPage/selectors.js @@ -15,6 +15,21 @@ export default createSelector( * Other specific selectors */ +export const selectOwner = createSelector( + selectDomain, + (domainState) => domainState.get('owner'), +); + +export const selectRepo = createSelector( + selectDomain, + (domainState) => domainState.get('repo'), +); + +export const selectCommit = createSelector( + selectDomain, + (domainState) => domainState.get('commit'), +); + export const selectIsLoading = createSelector( selectDomain, (domainState) => domainState.get('isLoading'), diff --git a/app/containers/ExecutionDetailPage/saga.js b/app/containers/ExecutionDetailPage/saga.js index 2c8b800..f2cbe24 100644 --- a/app/containers/ExecutionDetailPage/saga.js +++ b/app/containers/ExecutionDetailPage/saga.js @@ -1,7 +1,10 @@ -import { take, call, put, select, takeLatest } from 'redux-saga/effects'; +import { call, put, select, takeLatest } from 'redux-saga/effects'; import createInjector from 'utils/injectSaga'; import { delay, raceCancel } from '../../utils/saga-util'; -import { requestJson } from 'utils/request'; + +import { + endpointRequest, +} from 'containers/App/saga'; import { EXECUTION_OPENED, @@ -12,18 +15,6 @@ import { FETCH_BUILD_LOGS_REQUEST, } from './constants'; -import { - LOGIN_SUCCESS, -} from 'containers/App/constants'; - -import { - loginRequest, -} from 'containers/App/actions'; - -import { - selectLoggingIn, -} from 'containers/App/selectors'; - import selectDomain, { selectExecution, selectBuildKey, @@ -41,12 +32,6 @@ import { export function* fetchingExecution({ accessKey = null, }) { - // Check if a login is in process. - const isLoggingIn = yield select(selectLoggingIn); - if (isLoggingIn) { - yield take(LOGIN_SUCCESS); - } - try { const { owner, @@ -66,8 +51,9 @@ export function* fetchingExecution({ } const execution = yield call( - requestJson, - `/api/v1/repo/${owner}/${repo}/commit/${commit}/exec/${executionNum}`, + endpointRequest, + 'getExecutionUrl', + { owner, repo, commit, executionNum }, { headers } ); @@ -92,25 +78,9 @@ export function* fetchingExecution({ } catch (err) { if (err.isJson && err.body) { - if (err.status === 403 && err.body.authRedirectUrl) { - // Start a login request. - yield put(loginRequest(err.body.authRedirectUrl)); - - // Wait for the login to succeed or the - const loginSuccessResult = yield take(LOGIN_SUCCESS); - - if (loginSuccessResult) { - // Try the fetch again... - yield call(fetchExecution({ - accessKey, - })); - } - } - else { - yield put(fetchExecutionFailure( - err.body, - )); - } + yield put(fetchExecutionFailure( + err.body, + )); } else { yield put(fetchExecutionFailure( @@ -125,12 +95,6 @@ export function* fetchingExecution({ export function* fetchingBuildLogs({ accessKey = null, }) { - // Check if a login is in process. - const isLoggingIn = yield select(selectLoggingIn); - if (isLoggingIn) { - yield take(LOGIN_SUCCESS); - } - try { const execution = yield select(selectExecution); if (!execution) { @@ -145,7 +109,6 @@ export function* fetchingBuildLogs({ return; } - let accessKey; if (buildState.codeBuild) { const headers = {}; @@ -154,8 +117,9 @@ export function* fetchingBuildLogs({ } const response = yield call( - requestJson, - `/api/v1/repo/${owner}/${repo}/commit/${commit}/exec/${executionNum}/build/${buildKey}/logs`, + endpointRequest, + 'getExecutionBuildLogsUrl', + { owner, repo, commit, executionNum, buildKey }, // limit, nextToken { headers } ); @@ -185,25 +149,9 @@ export function* fetchingBuildLogs({ } catch (err) { if (err.isJson && err.body) { - if (err.status === 403 && err.body.authRedirectUrl) { - // Start a login request. - yield put(loginRequest(err.body.authRedirectUrl)); - - // Wait for the login to succeed or the - const loginSuccessResult = yield take(LOGIN_SUCCESS); - - if (loginSuccessResult) { - // Try the fetch again... - yield call(fetchBuildLogs({ - accessKey, - })); - } - } - else { - yield put(fetchBuildLogsFailure( - err.body, - )); - } + yield put(fetchBuildLogsFailure( + err.body, + )); } else { yield put(fetchBuildLogsFailure( diff --git a/app/containers/RepoExecutionsPage/index.js b/app/containers/RepoExecutionsPage/index.js index e7529aa..0162298 100644 --- a/app/containers/RepoExecutionsPage/index.js +++ b/app/containers/RepoExecutionsPage/index.js @@ -38,7 +38,6 @@ import { pageClosed, } from './actions'; -/* eslint-disable react/prefer-stateless-function */ export class RepoExecutionsPage extends React.Component { componentDidMount() { diff --git a/app/containers/RepoExecutionsPage/saga.js b/app/containers/RepoExecutionsPage/saga.js index 81fff2f..3549e7c 100644 --- a/app/containers/RepoExecutionsPage/saga.js +++ b/app/containers/RepoExecutionsPage/saga.js @@ -1,7 +1,10 @@ -import { take, call, put, select, takeLatest } from 'redux-saga/effects'; +import { call, put, select, takeLatest } from 'redux-saga/effects'; import createInjector from 'utils/injectSaga'; import { raceCancel } from '../../utils/saga-util'; -import { requestJson } from 'utils/request'; + +import { + endpointRequest, +} from 'containers/App/saga'; import { PAGE_OPENED, @@ -9,18 +12,6 @@ import { FETCH_EXECUTIONS_REQUEST, } from './constants'; -import { - LOGIN_SUCCESS, -} from 'containers/App/constants'; - -import { - loginRequest, -} from 'containers/App/actions'; - -import { - selectLoggingIn, -} from 'containers/App/selectors'; - import selectDomain from './selectors'; import { @@ -30,12 +21,6 @@ import { } from './actions'; export function* fetchingExecution() { - // Check if a login is in process. - const isLoggingIn = yield select(selectLoggingIn); - if (isLoggingIn) { - yield take(LOGIN_SUCCESS); - } - try { const { owner, @@ -50,8 +35,9 @@ export function* fetchingExecution() { executions, lastEvaluatedKey, } = yield call( - requestJson, - `/api/v1/repo/${owner}/${repo}`, + endpointRequest, + 'searchByRepoUrl', + { owner, repo }, ); yield put(fetchExecutionsSuccess( @@ -61,23 +47,9 @@ export function* fetchingExecution() { } catch (err) { if (err.isJson && err.body) { - if (err.status === 403 && err.body.authRedirectUrl) { - // Start a login request. - yield put(loginRequest(err.body.authRedirectUrl)); - - // Wait for the login to succeed or the - const loginSuccessResult = yield take(LOGIN_SUCCESS); - - if (loginSuccessResult) { - // Try the fetch again... - yield call(fetchExecutions()); - } - } - else { - yield put(fetchExecutionsFailure( - err.body, - )); - } + yield put(fetchExecutionsFailure( + err.body, + )); } else { yield put(fetchExecutionsFailure( @@ -104,7 +76,6 @@ export default function* defaultSaga() { FETCH_EXECUTIONS_REQUEST, raceCancel(fetchingExecution, [ PAGE_CLOSED, - FETCH_EXECUTIONS_REQUEST, ]), ); } diff --git a/app/containers/TopNav/index.js b/app/containers/TopNav/index.js index 2dfbc5d..a7ab32f 100644 --- a/app/containers/TopNav/index.js +++ b/app/containers/TopNav/index.js @@ -3,13 +3,19 @@ */ import React from 'react'; -// import PropTypes from 'prop-types'; +import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import { Link } from 'react-router-dom'; import { FormattedMessage } from 'react-intl'; import styled from 'styled-components'; import messages from './messages'; +import { + selectGithubUrl, + selectUserName, + selectUserLogin, +} from 'containers/App/selectors'; + const DropdownBG = styled.div` position: fixed; top: 0; @@ -53,7 +59,16 @@ export class TopNav extends React.PureComponent { } render() { - const { showNavi, showUserDropdown } = this.state; + const { + githubUrl, + userName, + userLogin, + } = this.props; + + const { + showNavi, + showUserDropdown, + } = this.state; return ( @@ -94,27 +109,30 @@ export class TopNav extends React.PureComponent {
    -
    - - - - - {showUserDropdown && ( -
    - -
    - amekkawi -     -
    - Log Out -
    - )} -
    + {userName && ( +
    + + + + + {showUserDropdown && ( + + )}
    @@ -126,16 +144,23 @@ export class TopNav extends React.PureComponent { } } -TopNav.propTypes = {}; -TopNav.defaultProps = {}; +TopNav.propTypes = { + githubUrl: PropTypes.string, + userName: PropTypes.string, + userLogin: PropTypes.string, +}; -function mapStateToProps(/* state, ownProps */) { - return { - // Route params - // myparam: ownProps.match.params.myparam, +TopNav.defaultProps = { + githubUrl: null, + userName: null, + userLogin: null, +}; - // Store values - // topnav: selectTopNav, +function mapStateToProps(state) { + return { + githubUrl: selectGithubUrl(state), + userName: selectUserName(state), + userLogin: selectUserLogin(state), }; } diff --git a/app/reducers.js b/app/reducers.js index 64e2a5a..dfd56ae 100644 --- a/app/reducers.js +++ b/app/reducers.js @@ -6,7 +6,6 @@ import { fromJS } from 'immutable'; import { combineReducers } from 'redux-immutable'; import { LOCATION_CHANGE } from 'connected-react-router'; -import appReducer from 'containers/App/reducer'; import languageProviderReducer from 'containers/LanguageProvider/reducer'; /* @@ -47,7 +46,6 @@ export function routeReducer(state = routeInitialState, action) { export default function createReducer(injectedReducers) { return combineReducers({ route: routeReducer, - app: appReducer, language: languageProviderReducer, ...injectedReducers, }); diff --git a/app/utils/request.js b/app/utils/request.js index 0b6f5ba..22673b8 100644 --- a/app/utils/request.js +++ b/app/utils/request.js @@ -20,6 +20,42 @@ export async function getBody(response) { : response.text(); } +export function buildApiUrl(url, params) { + let [ + path, + query, + ] = url.split('?'); + + path = path.replace(/{(\w+)}/g, (match, key) => { + return params[key] == null + ? String(params[key]) + : encodeURIComponent(params[key]); + }); + + query = (query || '').split('&') + .map((part) => { + const match = part.match(/^([\w_]+)={(\w+)}$/); + + // Keep the part as-is if it doesn't contain a param to replace. + if (!match) { + return part; + } + + const value = params[match[2]]; + + // Skip the part if the param value is undefined. + if (value === undefined) { + return null; + } + + return `${match[1]}=${encodeURIComponent(value)}`; + }) + .filter((v) => v !== null) + .join('&'); + + return `${path}${query ? `?${query}` : ''}`; +} + /** * * @param {string} url From 907a6317a515071a4fde4391aaec63b88290e937 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 14/25] Change user image to reduce resolution --- app/components/CommitMeta/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/CommitMeta/index.js b/app/components/CommitMeta/index.js index a5c0d2e..ba2cf49 100644 --- a/app/components/CommitMeta/index.js +++ b/app/components/CommitMeta/index.js @@ -15,7 +15,7 @@ function CommitMeta({ {author.login && ( Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 15/25] Improve style for commit messages --- app/containers/CommitExecutionsPage/index.js | 2 +- app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js | 2 +- app/styles.scss | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/containers/CommitExecutionsPage/index.js b/app/containers/CommitExecutionsPage/index.js index d55148d..954bf88 100644 --- a/app/containers/CommitExecutionsPage/index.js +++ b/app/containers/CommitExecutionsPage/index.js @@ -76,7 +76,7 @@ export class CommitExecutionsPage extends React.Component { {commit.substr(0, 10)}) }} /> diff --git a/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js b/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js index 8a8687c..fc41d17 100644 --- a/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js +++ b/app/containers/ExecutionDetailPage/ExecutionSummaryPanel.js @@ -67,7 +67,7 @@ function ExecutionSummaryPanel({ - {commit.substr(0, 10)} + {commit.substr(0, 10)}
    diff --git a/app/styles.scss b/app/styles.scss index 06d64c7..b53ed8e 100644 --- a/app/styles.scss +++ b/app/styles.scss @@ -1,5 +1,7 @@ $breadcrumb-bg: transparent; $breadcrumb-divider: quote(">") !default; +$code-color: inherit; +$pre-color: inherit; @import "~bootstrap/scss/functions"; @import "~bootstrap/scss/variables"; From 3181b32a3aaceed97eb0ca73f4ce8dc5aee97ec0 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 16/25] Add state loading error modal --- app/containers/App/StateErrorModal.js | 146 ++++++++++++++++++++++++++ app/containers/App/index.js | 2 + app/containers/App/messages.js | 12 +++ app/containers/App/saga.js | 10 +- 4 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 app/containers/App/StateErrorModal.js diff --git a/app/containers/App/StateErrorModal.js b/app/containers/App/StateErrorModal.js new file mode 100644 index 0000000..1258c7a --- /dev/null +++ b/app/containers/App/StateErrorModal.js @@ -0,0 +1,146 @@ +/** + * StateErrorModal + */ + +import React from 'react'; +import PropTypes from 'prop-types'; +import styled from 'styled-components'; +import { bindActionCreators } from 'redux'; +import { connect } from 'react-redux'; +import { createStructuredSelector } from 'reselect'; +import { FormattedMessage } from 'react-intl'; + +import messages from './messages'; + +import { + selectIsUserLoggedIn, + selectHasFetchedState, + selectStateError, +} from './selectors'; + +import { + stateRequest, +} from './actions'; + +const ErrorDetail = styled.div` + margin-top: 1rem; + background-color: #EEE; + padding: 6px 12px; + border-left: 3px solid #666; + white-space: pre-wrap; +`; + +export class StateErrorModal extends React.Component { + + static propTypes = { + stateError: PropTypes.object.isRequired, + onRetry: PropTypes.func.isRequired, + }; + + constructor(props) { + super(props); + + this.state = { + show: false, + }; + + this.buttonRef = React.createRef(); + } + + state = { + show: false, + }; + + componentDidMount() { + this.buttonRef.current.focus(); + + this._showTimeoutId = setTimeout(() => { + this.setState({ + show: true, + }); + }, 10); + } + + componentWillUnmount() { + clearTimeout(this._showTimeoutId); + } + + render() { + const { stateError, onRetry } = this.props; + const { show } = this.state; + + return ( + + +
    + + ); + } +} + +function StateErrorModalContaner({ + isUserLoggedIn, + hasFetchedState, + stateError, + ...props +}) { + return !isUserLoggedIn && !hasFetchedState && stateError + ? + : null; +} + +StateErrorModalContaner.propTypes = { + isUserLoggedIn: PropTypes.bool.isRequired, + hasFetchedState: PropTypes.bool.isRequired, + stateError: PropTypes.object, +}; + +StateErrorModalContaner.defaultProps = { + stateError: null, +}; + +const mapStateToProps = createStructuredSelector({ + isUserLoggedIn: selectIsUserLoggedIn, + hasFetchedState: selectHasFetchedState, + stateError: selectStateError, +}); + +const mapDispatchToProps = (dispatch) => bindActionCreators({ + onRetry: stateRequest, +}, dispatch); + +export default connect( + mapStateToProps, + mapDispatchToProps, +)(StateErrorModalContaner); diff --git a/app/containers/App/index.js b/app/containers/App/index.js index 7a3554f..03db761 100644 --- a/app/containers/App/index.js +++ b/app/containers/App/index.js @@ -25,6 +25,7 @@ import CommitExecutionsPage from 'containers/CommitExecutionsPage/Loadable'; import ExecutionDetailPage from 'containers/ExecutionDetailPage/Loadable'; import LoginModal from './LoginModal'; +import StateErrorModal from './StateErrorModal'; const onError = (err, info) => { // TODO: How to better handle this? @@ -108,6 +109,7 @@ export function App() {
    +
    ); diff --git a/app/containers/App/messages.js b/app/containers/App/messages.js index 8f1381a..d18ea07 100644 --- a/app/containers/App/messages.js +++ b/app/containers/App/messages.js @@ -5,6 +5,18 @@ import { defineMessages } from 'react-intl'; export default defineMessages({ + stateErrorModalTitle: { + id: 'app.containers.App.stateErrorModalTitle', + defaultMessage: 'Failed to Load Application', + }, + stateErrorModalBody: { + id: 'app.containers.App.stateErrorModalBody', + defaultMessage: 'There was a problem loading the application.', // TODO: Use a better message. + }, + stateErrorModalButton: { + id: 'app.containers.App.stateErrorModalButton', + defaultMessage: 'Retry', + }, loginModalTitle: { id: 'app.containers.App.loginModalTitle', defaultMessage: 'Authorization Required', diff --git a/app/containers/App/saga.js b/app/containers/App/saga.js index 6294a66..6d0a108 100644 --- a/app/containers/App/saga.js +++ b/app/containers/App/saga.js @@ -1,6 +1,5 @@ import { call, put, select, take, takeLatest, race, throttle } from 'redux-saga/effects'; import createInjector from 'utils/injectSaga'; -import { delay } from '../../utils/saga-util'; import { requestJson, buildApiUrl } from 'utils/request'; import { @@ -26,6 +25,7 @@ import { selectIsFetchingState, selectIsUserLoggedIn, selectIsLoginRequired, + selectStateError, } from './selectors'; function* init() { @@ -50,8 +50,6 @@ function* init() { function* fetchState() { try { - yield delay(500); // TODO: Remove - const { app, user, endpoints } = yield call( requestJson, '/api/v1', @@ -100,8 +98,12 @@ function* checkForLoginSuccess() { export function* waitForLogin() { while (!(yield select(selectIsUserLoggedIn))) { + const isFetchingState = yield select(selectIsFetchingState); + const isLoginRequired = yield select(selectIsLoginRequired); + const stateError = yield select(selectStateError); + // Request state if not already requesting it. - if (!(yield select(selectIsFetchingState)) && !(yield select(selectIsLoginRequired))) { + if (!isFetchingState && !isLoginRequired && !stateError) { yield put(stateRequest()); } From b55a97e5843d5026b8ddf184ed72ffc6fd0a5fc6 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 17/25] Improve error message for JSON parsing --- app/utils/request.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/app/utils/request.js b/app/utils/request.js index 22673b8..05b689d 100644 --- a/app/utils/request.js +++ b/app/utils/request.js @@ -84,13 +84,25 @@ export async function requestJson(url, options = {}) { } if (response.ok) { - return response.json(); + try { + return await response.json(); + } + catch (err) { + err.message = `Invalid JSON: ${err.message}`; + throw err; + } } const error = new Error(response.statusText); error.status = response.status; error.isJson = true; - error.body = await response.json(); + + try { + error.body = await response.json(); + } + catch (err) { + error.message = `Invalid JSON: ${err.message}`; + } Object.defineProperty(error, 'getResponse', { enumerable: false, From f7cc64480d4b3c1b59dd0c5d1fdd88be880d076d Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 18/25] Change LanguageProvider selectors to remove "make" functions --- app/containers/LanguageProvider/index.js | 4 ++-- app/containers/LanguageProvider/selectors.js | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/containers/LanguageProvider/index.js b/app/containers/LanguageProvider/index.js index e3dd582..14652d3 100644 --- a/app/containers/LanguageProvider/index.js +++ b/app/containers/LanguageProvider/index.js @@ -11,7 +11,7 @@ import { connect } from 'react-redux'; import { createSelector } from 'reselect'; import { IntlProvider } from 'react-intl'; -import { makeSelectLocale } from './selectors'; +import { selectLocale } from './selectors'; export class LanguageProvider extends React.PureComponent { render() { @@ -34,7 +34,7 @@ LanguageProvider.propTypes = { }; const mapStateToProps = createSelector( - makeSelectLocale(), + selectLocale, (locale) => ({ locale }) ); diff --git a/app/containers/LanguageProvider/selectors.js b/app/containers/LanguageProvider/selectors.js index e0ac41c..2a0d94a 100644 --- a/app/containers/LanguageProvider/selectors.js +++ b/app/containers/LanguageProvider/selectors.js @@ -7,16 +7,14 @@ import { initialState } from './reducer'; * @param {object} state * @returns {object} */ -const selectLanguage = (state) => state.get('language', initialState); +export const selectLanguage = (state) => state.get('language', initialState); /** * Select the language locale. * * @returns {string} */ -const makeSelectLocale = () => createSelector( +export const selectLocale = createSelector( selectLanguage, (languageState) => languageState.get('locale') ); - -export { selectLanguage, makeSelectLocale }; From a4d5e37492537d566485b17d3df98b37f34d0530 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 19/25] Change lint-staged to just do eslint and without fixes --- package.json | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 1092dc4..5aa02c1 100644 --- a/package.json +++ b/package.json @@ -33,8 +33,8 @@ "generate": "plop --plopfile internals/generators/index.js", "lint": "npm run lint:js && npm run lint:css", "lint:css": "stylelint './app/**/*.js'", - "lint:eslint": "eslint --ignore-path .gitignore --ignore-pattern internals/scripts", - "lint:eslint:fix": "eslint --ignore-path .gitignore --ignore-pattern internals/scripts --fix", + "lint:eslint": "eslint --max-warnings 0 --ignore-path .gitignore --ignore-pattern internals/scripts", + "lint:eslint:fix": "eslint --max-warnings 0 --ignore-path .gitignore --ignore-pattern internals/scripts --fix", "lint:js": "npm run lint:eslint -- . ", "lint:staged": "lint-staged", "pretest": "npm run test:clean && npm run lint", @@ -47,12 +47,7 @@ }, "lint-staged": { "*.js": [ - "npm run lint:eslint:fix", - "git add --force" - ], - "*.json": [ - "prettier --write", - "git add --force" + "npm run lint:eslint" ] }, "pre-commit": "lint:staged", From 4225b8f7bfa3f12ba9a2332bcf790dff90846ad5 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 20/25] Change TopNav to use logoutUrl from state --- app/containers/TopNav/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/containers/TopNav/index.js b/app/containers/TopNav/index.js index a7ab32f..842fa08 100644 --- a/app/containers/TopNav/index.js +++ b/app/containers/TopNav/index.js @@ -8,12 +8,14 @@ import { connect } from 'react-redux'; import { Link } from 'react-router-dom'; import { FormattedMessage } from 'react-intl'; import styled from 'styled-components'; +import { buildApiUrl } from '../../utils/request'; import messages from './messages'; import { selectGithubUrl, selectUserName, selectUserLogin, + selectEndpoints, } from 'containers/App/selectors'; const DropdownBG = styled.div` @@ -55,7 +57,8 @@ export class TopNav extends React.PureComponent { handleLogOut(evt) { evt.preventDefault(); - window.location = `/api/v1/auth/logout?redirect=${encodeURIComponent(`${window.location.origin}/app`)}`; + const { logoutUrl } = this.props; + window.location = buildApiUrl(logoutUrl, { url: `${window.location.origin}/app` }); } render() { @@ -109,7 +112,7 @@ export class TopNav extends React.PureComponent {
    - {userName && ( + {userLogin && (
    Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 21/25] Update tests for App actions, selectors, reducer and saga --- app/containers/App/actions.js | 2 +- app/containers/App/saga.js | 92 +++-- app/containers/App/selectors.js | 8 +- app/containers/App/tests/actions.test.js | 92 +++-- app/containers/App/tests/reducer.test.js | 138 ++++++-- app/containers/App/tests/saga.test.js | 381 +++++++++++++++++++++ app/containers/App/tests/selectors.test.js | 224 ++++++++++-- 7 files changed, 803 insertions(+), 134 deletions(-) create mode 100644 app/containers/App/tests/saga.test.js diff --git a/app/containers/App/actions.js b/app/containers/App/actions.js index a432945..9a4fca0 100644 --- a/app/containers/App/actions.js +++ b/app/containers/App/actions.js @@ -16,7 +16,7 @@ export function stateRestore(state) { }; } -export function stateRequest(resetUser) { +export function stateRequest(resetUser = false) { return { type: STATE_REQUEST, resetUser, diff --git a/app/containers/App/saga.js b/app/containers/App/saga.js index 6d0a108..e83cdbf 100644 --- a/app/containers/App/saga.js +++ b/app/containers/App/saga.js @@ -1,6 +1,7 @@ import { call, put, select, take, takeLatest, race, throttle } from 'redux-saga/effects'; import createInjector from 'utils/injectSaga'; import { requestJson, buildApiUrl } from 'utils/request'; +import { delay } from '../../utils/saga-util'; import { WINDOW_VISIBLE, @@ -28,7 +29,18 @@ import { selectStateError, } from './selectors'; -function* init() { +export function getStoredSessionState() { + return JSON.parse(sessionStorage.getItem(SESSION_STORAGE_KEY) || '{}'); +} + +export function setStoredSessionState(state) { + sessionStorage.setItem( + SESSION_STORAGE_KEY, + JSON.stringify(state), + ); +} + +export function* init() { // Skip init if already restored state. if (yield select(selectHasRestoredState)) { return; @@ -36,7 +48,7 @@ function* init() { try { yield put(stateRestore( - JSON.parse(sessionStorage.getItem(SESSION_STORAGE_KEY) || '{}'), + yield call(getStoredSessionState), )); } catch (err) { @@ -48,7 +60,7 @@ function* init() { yield put(stateRequest()); } -function* fetchState() { +export function* fetchState() { try { const { app, user, endpoints } = yield call( requestJson, @@ -79,34 +91,30 @@ function* fetchState() { } } -function* saveStateToSession() { - const state = yield select(selectRestoreState); - +export function* saveStateToSession() { // Persist the state on success. - sessionStorage.setItem( - SESSION_STORAGE_KEY, - JSON.stringify(state), + yield call( + setStoredSessionState, + yield select(selectRestoreState), ); } -function* checkForLoginSuccess() { - if (!(yield select(selectIsFetchingState)) && (yield select(selectIsLoginRequired))) { - yield put(stateRequest()); +export function* checkForLoginSuccess() { + const isFetchingState = yield select(selectIsFetchingState); + if (isFetchingState) { + return; + } + + const isLoginRequired = yield select(selectIsLoginRequired); + if (!isLoginRequired) { + return; } + + yield put(stateRequest()); } export function* waitForLogin() { while (!(yield select(selectIsUserLoggedIn))) { - - const isFetchingState = yield select(selectIsFetchingState); - const isLoginRequired = yield select(selectIsLoginRequired); - const stateError = yield select(selectStateError); - - // Request state if not already requesting it. - if (!isFetchingState && !isLoginRequired && !stateError) { - yield put(stateRequest()); - } - yield race({ restore: take(STATE_RESTORE), success: take(STATE_SUCCESS), @@ -114,8 +122,39 @@ export function* waitForLogin() { } } +/** + * Fetch the state if needed. + */ +export function* requestStateIfNeeded() { + // Skip if already logged in. + if (yield select(selectIsUserLoggedIn)) { + return; + } + + // Skip if already fetching state. + if (yield select(selectIsFetchingState)) { + return; + } + + // Skip if showing the login modal. + if (yield select(selectIsLoginRequired)) { + return; + } + + // Skip if showing a state fetch error. + if (yield select(selectStateError)) { + return; + } + + yield put(stateRequest()); +} + export function* endpointRequest(endpoint, params, ...args) { + let attempt = 0; while (true) { + attempt++; + + yield call(requestStateIfNeeded); yield call(waitForLogin); const endpoints = yield select(selectEndpoints); @@ -125,7 +164,12 @@ export function* endpointRequest(endpoint, params, ...args) { return yield call(requestJson, url, ...args); } catch (err) { - if (err.isJson && err.status === 403 && err.body && err.body.authRedirectUrl) { + // Allow up to 10 attempts if the response indicates the user needs to login. + if (attempt <= 10 && err.isJson && err.status === 403 && err.body && err.body.authRedirectUrl) { + + // Add a small delay + yield delay(100 * attempt); + yield put(stateRequest(true)); } else { @@ -155,5 +199,5 @@ export default function* defaultSaga() { WINDOW_FOCUS, ], checkForLoginSuccess); - yield* init(); + yield call(init); } diff --git a/app/containers/App/selectors.js b/app/containers/App/selectors.js index 05e93ce..ba2a82b 100644 --- a/app/containers/App/selectors.js +++ b/app/containers/App/selectors.js @@ -8,7 +8,6 @@ import { } from 'reselect'; export const selectApp = (state) => state.get('app'); -export const selectRoute = (state) => state.get('route'); export const selectHasRestoredState = createSelector( selectApp, @@ -66,11 +65,6 @@ export const selectIsLoginRequired = createSelector( (hasFetchedState, userLogin) => hasFetchedState && userLogin == null, ); -export const selectHasLoadedState = createSelector( - selectGithubUrl, - (githubUrl) => !!githubUrl, -); - export const selectRestoreState = createStructuredSelector({ githubUrl: selectGithubUrl, githubHost: selectGithubHost, @@ -79,6 +73,8 @@ export const selectRestoreState = createStructuredSelector({ userLogin: selectUserLogin, }); +export const selectRoute = (state) => state.get('route'); + export const selectLocation = createSelector( selectRoute, (routeState) => routeState.get('location').toJS() diff --git a/app/containers/App/tests/actions.test.js b/app/containers/App/tests/actions.test.js index e40da1d..a059953 100644 --- a/app/containers/App/tests/actions.test.js +++ b/app/containers/App/tests/actions.test.js @@ -1,43 +1,75 @@ -import { LOAD_REPOS, LOAD_REPOS_SUCCESS, LOAD_REPOS_ERROR } from '../constants'; +import { + STATE_RESTORE, + STATE_REQUEST, + STATE_SUCCESS, + STATE_FAILURE, +} from '../constants'; -import { loadRepos, reposLoaded, repoLoadingError } from '../actions'; +import { + stateRestore, + stateRequest, + stateSuccess, + stateFailure, +} from '../actions'; -describe('App Actions', () => { - describe('loadRepos', () => { - it('should return the correct type', () => { - const expectedResult = { - type: LOAD_REPOS, - }; - - expect(loadRepos()).toEqual(expectedResult); +describe('App container actions', () => { + describe('stateRestore', () => { + it('should return the correct type and props', () => { + expect(stateRestore({ + foobar: true, + })).toEqual({ + type: STATE_RESTORE, + state: { + foobar: true, + }, + }); }); }); - describe('reposLoaded', () => { - it('should return the correct type and the passed repos', () => { - const fixture = ['Test']; - const username = 'test'; - const expectedResult = { - type: LOAD_REPOS_SUCCESS, - repos: fixture, - username, - }; + describe('stateRequest', () => { + it('should return the correct type and props', () => { + expect(stateRequest()).toEqual({ + type: STATE_REQUEST, + resetUser: false, + }); - expect(reposLoaded(fixture, username)).toEqual(expectedResult); + expect(stateRequest(true)).toEqual({ + type: STATE_REQUEST, + resetUser: true, + }); }); }); - describe('repoLoadingError', () => { - it('should return the correct type and the error', () => { - const fixture = { - msg: 'Something went wrong!', - }; - const expectedResult = { - type: LOAD_REPOS_ERROR, - error: fixture, - }; + describe('stateSuccess', () => { + it('should return the correct type and props', () => { + expect(stateSuccess( + 'gurl', + 'ghost', + 'ulogin', + 'uname', + { + foo: 'bar', + }, + )).toEqual({ + type: STATE_SUCCESS, + githubUrl: 'gurl', + githubHost: 'ghost', + userLogin: 'ulogin', + userName: 'uname', + endpoints: { + foo: 'bar', + }, + }); + }); + }); - expect(repoLoadingError(fixture)).toEqual(expectedResult); + describe('stateFailure', () => { + it('should return the correct type and error', () => { + const err = new Error(); + expect(stateFailure(err)).toEqual({ + type: STATE_FAILURE, + error: err, + }); }); }); }); diff --git a/app/containers/App/tests/reducer.test.js b/app/containers/App/tests/reducer.test.js index 06e0277..8ea33c5 100644 --- a/app/containers/App/tests/reducer.test.js +++ b/app/containers/App/tests/reducer.test.js @@ -1,62 +1,126 @@ import { fromJS } from 'immutable'; import appReducer from '../reducer'; -import { loadRepos, reposLoaded, repoLoadingError } from '../actions'; -describe('appReducer', () => { +import { + stateRestore, + stateRequest, + stateSuccess, + stateFailure, +} from '../actions'; + +describe('App container reducer', () => { let state; + beforeEach(() => { state = fromJS({ - loading: false, - error: false, - currentUser: false, - userData: fromJS({ - repositories: false, - }), + hasRestoredState: false, + hasFetchedState: false, + isFetchingState: false, + stateError: null, + githubUrl: null, + githubHost: null, + endpoints: null, + userName: null, + userLogin: null, }); }); it('should return the initial state', () => { - const expectedResult = state; - expect(appReducer(undefined, {})).toEqual(expectedResult); + expect(appReducer(undefined, {})).toEqual(state); }); - it('should handle the loadRepos action correctly', () => { + it('should handle the stateRestore action correctly', () => { const expectedResult = state - .set('loading', true) - .set('error', false) - .setIn(['userData', 'repositories'], false); + .set('hasRestoredState', true) + .set('githubUrl', 'gurl') + .set('githubHost', 'ghost') + .set('endpoints', { foo: 'bar' }) + .set('userName', 'uname') + .set('userLogin', 'ulogin'); - expect(appReducer(state, loadRepos())).toEqual(expectedResult); + expect(appReducer(state, stateRestore({ + githubUrl: 'gurl', + githubHost: 'ghost', + endpoints: { foo: 'bar' }, + userName: 'uname', + userLogin: 'ulogin', + }))).toEqual(expectedResult); }); - it('should handle the reposLoaded action correctly', () => { - const fixture = [ - { - name: 'My Repo', - }, - ]; - const username = 'test'; + it('should handle the stateRequest action correctly', () => { + const expectedResultA = state + .set('isFetchingState', true); + + expect(appReducer(state, stateRequest())).toEqual(expectedResultA); + + const startingStateB = state + .set('userName', 'foo') + .set('userLogin', 'bar'); + + const expectedResultB = state + .set('isFetchingState', true) + .set('userName', 'foo') + .set('userLogin', 'bar'); + + expect(appReducer(startingStateB, stateRequest())).toEqual(expectedResultB); + }); + + it('should handle the stateRequest action correctly if there was already an error', () => { + const startingState = state + .set('stateError', new Error()); + const expectedResult = state - .setIn(['userData', 'repositories'], fixture) - .set('loading', false) - .set('currentUser', username); + .set('isFetchingState', true); - expect(appReducer(state, reposLoaded(fixture, username))).toEqual( - expectedResult, - ); + expect(appReducer(startingState, stateRequest())).toEqual(expectedResult); }); - it('should handle the repoLoadingError action correctly', () => { - const fixture = { - msg: 'Not found', - }; + it('should handle the stateRequest action correctly if resetUser is true', () => { + const startingState = state + .set('userName', 'foo') + .set('userLogin', 'bar'); + + const expectedResult = state + .set('isFetchingState', true); + + expect(appReducer(startingState, stateRequest(true))).toEqual(expectedResult); + }); + + it('should handle the stateSuccess action correctly', () => { + const startingState = state + .set('isFetchingState', true); + + const expectedResult = state + .set('hasFetchedState', true) + .set('isFetchingState', false) + .set('githubUrl', 'gurl') + .set('githubHost', 'ghost') + .set('endpoints', { foo: 'bar' }) + .set('userName', 'uname') + .set('userLogin', 'ulogin'); + + expect(appReducer(startingState, stateSuccess( + 'gurl', + 'ghost', + 'ulogin', + 'uname', + { foo: 'bar' }, + ))).toEqual(expectedResult); + }); + + it('should handle the stateFailure action correctly', () => { + const err = new Error('foobar'); + + const startingState = state + .set('isFetchingState', true); + const expectedResult = state - .set('error', fixture) - .set('loading', false); + .set('isFetchingState', false) + .set('stateError', err); - expect(appReducer(state, repoLoadingError(fixture))).toEqual( - expectedResult, - ); + expect(appReducer(startingState, stateFailure( + err, + ))).toEqual(expectedResult); }); }); diff --git a/app/containers/App/tests/saga.test.js b/app/containers/App/tests/saga.test.js new file mode 100644 index 0000000..894ddae --- /dev/null +++ b/app/containers/App/tests/saga.test.js @@ -0,0 +1,381 @@ +import { call, put, select, take, race } from 'redux-saga/effects'; +import { cloneableGenerator } from 'redux-saga/utils'; + +import * as saga from '../saga'; + +import { + selectHasRestoredState, selectIsFetchingState, selectIsLoginRequired, selectIsUserLoggedIn, selectRestoreState, selectStateError, +} from '../selectors'; + +import { stateFailure, stateRequest, stateRestore, stateSuccess } from '../actions'; +import { requestJson } from '../../../utils/request'; +import { STATE_RESTORE, STATE_SUCCESS } from '../constants'; + +describe('App container saga', () => { + describe('*init()', () => { + it('should abort if has already restored state', () => { + const gen = saga.init(); + + // Checks if the state has been restored. + expect(gen.next().value) + .toEqual(select(selectHasRestoredState)); + + // Done if already restored. + expect(gen.next(true).done) + .toEqual(true); + }); + + it('should restore state from sessionStorage', () => { + const gen = saga.init(); + + // Checks if the state has been restored. + expect(gen.next().value) + .toEqual(select(selectHasRestoredState)); + + // Attempts to get state stored in sessionStorage. + expect(gen.next(false).value) + .toEqual(call(saga.getStoredSessionState)); + + // Puts stateRestore action with retrieved state. + const storedJSON = { foo: 'bar' }; + expect(gen.next(storedJSON).value) + .toEqual(put(stateRestore(storedJSON))); + + // Puts stateRequest action. + expect(gen.next().value) + .toEqual(put(stateRequest())); + + expect(gen.next(true).done) + .toEqual(true); + }); + }); + + describe('*fetchState()', () => { + it('should request API JSON and put stateSuccess action', () => { + const gen = cloneableGenerator(saga.fetchState)(); + + // Call requestJson + expect(gen.next().value) + .toEqual(call( + requestJson, + '/api/v1', + )); + + { + const genNoUser = gen.clone(); + + // Put STATE_SUCCESS action. + expect(genNoUser.next({ + app: { + githubUrl: 'gurl', + githubHost: 'ghost', + }, + endpoints: { + foo: 'bar', + }, + user: null, + }).value) + .toEqual(put(stateSuccess( + 'gurl', + 'ghost', + null, + null, + { + foo: 'bar', + }, + ))); + + expect(genNoUser.next(true).done) + .toEqual(true); + } + + { + const genHasUser = gen.clone(); + + // Put STATE_SUCCESS action. + expect(genHasUser.next({ + app: { + githubUrl: 'gurl', + githubHost: 'ghost', + }, + endpoints: { + foo: 'bar', + }, + user: { + login: 'ulogin', + name: 'uname', + }, + }).value) + .toEqual(put(stateSuccess( + 'gurl', + 'ghost', + 'ulogin', + 'uname', + { + foo: 'bar', + }, + ))); + + expect(genHasUser.next(true).done) + .toEqual(true); + } + }); + + it('should request API JSON and put stateFailure action', () => { + const gen = cloneableGenerator(saga.fetchState)(); + + // Call requestJson + expect(gen.next().value) + .toEqual(call( + requestJson, + '/api/v1', + )); + + { + const genJsonResponse = gen.clone(); + + const err = new Error('Foo'); + err.isJson = true; + err.body = { + message: 'foobar', + }; + + // Put STATE_FAILURE action. + expect(genJsonResponse.throw(err).value) + .toEqual(put(stateFailure({ + message: 'foobar', + }))); + + expect(genJsonResponse.next(true).done) + .toEqual(true); + } + + { + const genNonJsonResponse = gen.clone(); + + const err = new Error('Foo'); + + // Put STATE_FAILURE action. + expect(genNonJsonResponse.throw(err).value) + .toEqual(put(stateFailure({ + message: 'Foo', + }))); + + expect(genNonJsonResponse.next(true).done) + .toEqual(true); + } + }); + }); + + describe('*saveStateToSession()', () => { + it('should save the state to sessionStorage', () => { + const gen = saga.saveStateToSession(); + + // Select state to store. + expect(gen.next().value) + .toEqual(select(selectRestoreState)); + + const state = { + foo: 'bar', + }; + + // Call helper to save state. + expect(gen.next(state).value) + .toEqual(call( + saga.setStoredSessionState, + state, + )); + + expect(gen.next(true).done) + .toEqual(true); + }); + }); + + describe('*checkForLoginSuccess()', () => { + it('should abort if already fetching state', () => { + const gen = saga.checkForLoginSuccess(); + + // Select if is fetching state already. + expect(gen.next().value) + .toEqual(select(selectIsFetchingState)); + + expect(gen.next(true).done) + .toEqual(true); + }); + + it('should abort if login is not required', () => { + const gen = saga.checkForLoginSuccess(); + + // Select if is fetching state already. + expect(gen.next().value) + .toEqual(select(selectIsFetchingState)); + + // Select if login is required. + expect(gen.next(false).value) + .toEqual(select(selectIsLoginRequired)); + + expect(gen.next(false).done) + .toEqual(true); + }); + + it('should put stateRequest action', () => { + const gen = saga.checkForLoginSuccess(); + + // Select if is fetching state already. + expect(gen.next().value) + .toEqual(select(selectIsFetchingState)); + + // Select if login is required. + expect(gen.next(false).value) + .toEqual(select(selectIsLoginRequired)); + + expect(gen.next(true).value) + .toEqual(put(stateRequest())); + + expect(gen.next().done) + .toEqual(true); + }); + }); + + describe('*requestStateIfNeeded()', () => { + it('should not request if user is logged in', () => { + const gen = saga.requestStateIfNeeded(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(true).done) + .toEqual(true); + }); + + it('should not request if already fetching state', () => { + const gen = saga.requestStateIfNeeded(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(false).value) + .toEqual(select(selectIsFetchingState)); + + expect(gen.next(true).done) + .toEqual(true); + }); + + it('should not request if already showing login modal', () => { + const gen = saga.requestStateIfNeeded(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(false).value) + .toEqual(select(selectIsFetchingState)); + + expect(gen.next(false).value) + .toEqual(select(selectIsLoginRequired)); + + expect(gen.next(true).done) + .toEqual(true); + }); + + it('should not request if there was a state fetch error', () => { + const gen = saga.requestStateIfNeeded(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(false).value) + .toEqual(select(selectIsFetchingState)); + + expect(gen.next(false).value) + .toEqual(select(selectIsLoginRequired)); + + expect(gen.next(false).value) + .toEqual(select(selectStateError)); + + expect(gen.next(new Error('foobar')).done) + .toEqual(true); + }); + + it('should request state if needed', () => { + const gen = saga.requestStateIfNeeded(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(false).value) + .toEqual(select(selectIsFetchingState)); + + expect(gen.next(false).value) + .toEqual(select(selectIsLoginRequired)); + + expect(gen.next(false).value) + .toEqual(select(selectStateError)); + + expect(gen.next(null).value) + .toEqual(put(stateRequest())); + + expect(gen.next().done) + .toEqual(true); + }); + }); + + describe('*waitForLogin()', () => { + it('should loop until user is logged in', () => { + const gen = saga.waitForLogin(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next( + false // Result of selectIsUserLoggedIn + ).value) + .toEqual(race({ + restore: take(STATE_RESTORE), + success: take(STATE_SUCCESS), + })); + + // Loop repeats. + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next( + false // Result of selectIsUserLoggedIn + ).value) + .toEqual(race({ + restore: take(STATE_RESTORE), + success: take(STATE_SUCCESS), + })); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + // Exits if user is now logged in. + expect(gen.next( + true // Result of selectIsUserLoggedIn + ).done) + .toEqual(true); + }); + + it('should bail immediately if user is already logged in', () => { + const gen = saga.waitForLogin(); + + expect(gen.next().value) + .toEqual(select(selectIsUserLoggedIn)); + + expect(gen.next(true).done) + .toEqual(true); + }); + }); + + describe('*endpointRequest()', () => { + it.skip('TODO', () => {}); + }); + + describe('*endpointRequest()', () => { + it.skip('TODO', () => {}); + }); + + describe('*defaultSaga()', () => { + it.skip('TODO', () => {}); + }); +}); diff --git a/app/containers/App/tests/selectors.test.js b/app/containers/App/tests/selectors.test.js index 9b9444e..15fc51b 100644 --- a/app/containers/App/tests/selectors.test.js +++ b/app/containers/App/tests/selectors.test.js @@ -2,17 +2,24 @@ import { fromJS } from 'immutable'; import * as selectors from '../selectors'; -describe('exports', () => { +describe('App container selectors', () => { it('should have the expected exports', () => { expect(Object.keys(selectors).sort()) .toEqual([ 'selectApp', - 'selectRoute', - 'selectLoggingIn', - 'selectLoginUrl', - 'selectLoginError', - 'selectIsLoggedIn', + 'selectHasRestoredState', + 'selectHasFetchedState', + 'selectIsFetchingState', + 'selectStateError', + 'selectGithubUrl', 'selectGithubHost', + 'selectUserName', + 'selectUserLogin', + 'selectEndpoints', + 'selectIsUserLoggedIn', + 'selectIsLoginRequired', + 'selectRestoreState', + 'selectRoute', 'selectLocation', ].sort()); }); @@ -20,12 +27,19 @@ describe('exports', () => { const { selectApp, - selectRoute, - selectLoggingIn, - selectLoginUrl, - selectLoginError, - selectIsLoggedIn, + selectHasRestoredState, + selectHasFetchedState, + selectIsFetchingState, + selectStateError, + selectGithubUrl, selectGithubHost, + selectUserName, + selectUserLogin, + selectEndpoints, + selectIsUserLoggedIn, + selectIsLoginRequired, + selectRestoreState, + selectRoute, selectLocation, } = selectors; @@ -39,62 +53,73 @@ describe('selectApp', () => { }); }); -describe('selectLoggingIn', () => { - it('should select if user is logging in', () => { +describe('selectHasRestoredState', () => { + it('should select if state has been restored from the sessionStore', () => { const mockedStateA = fromJS({ app: { - loggingIn: true, + hasRestoredState: true, }, }); - expect(selectLoggingIn(mockedStateA)).toEqual(true); + expect(selectHasRestoredState(mockedStateA)).toEqual(true); const mockedStateB = fromJS({ app: { - loggingIn: false, + hasRestoredState: false, }, }); - expect(selectLoggingIn(mockedStateB)).toEqual(false); + expect(selectHasRestoredState(mockedStateB)).toEqual(false); }); }); -describe('selectLoginUrl', () => { - it('should select login URL', () => { - const mockedState = fromJS({ +describe('selectHasFetchedState', () => { + it('should select if state has been fetched successfully', () => { + const mockedStateA = fromJS({ app: { - loginUrl: 'http://foobar/', + hasFetchedState: true, }, }); - expect(selectLoginUrl(mockedState)).toEqual('http://foobar/'); + expect(selectHasFetchedState(mockedStateA)).toEqual(true); + + const mockedStateB = fromJS({ + app: { + hasFetchedState: false, + }, + }); + expect(selectHasFetchedState(mockedStateB)).toEqual(false); }); }); -describe('selectLoginError', () => { - it('should select login error', () => { - const err = new Error(); - const mockedState = fromJS({ +describe('selectIsFetchingState', () => { + it('should select if state has been fetched successfully', () => { + const mockedStateA = fromJS({ app: { - loginError: err, + isFetchingState: true, }, }); - expect(selectLoginError(mockedState)).toEqual(err); + expect(selectIsFetchingState(mockedStateA)).toEqual(true); }); }); -describe('selectIsLoggedIn', () => { - it('should select if user is logged in', () => { - const mockedStateA = fromJS({ +describe('selectStateError', () => { + it('should select state fetch error', () => { + const err = new Error(); + const mockedState = fromJS({ app: { - isLoggedIn: true, + stateError: err, }, }); - expect(selectIsLoggedIn(mockedStateA)).toEqual(true); + expect(selectStateError(mockedState)).toEqual(err); + }); +}); - const mockedStateB = fromJS({ +describe('selectGithubUrl', () => { + it('should select Github URL', () => { + const mockedState = fromJS({ app: { - isLoggedIn: false, + githubUrl: 'http://foobar.github.com', }, }); - expect(selectIsLoggedIn(mockedStateB)).toEqual(false); + expect(selectGithubUrl(mockedState)).toEqual('http://foobar.github.com'); }); }); @@ -109,6 +134,133 @@ describe('selectGithubHost', () => { }); }); +describe('selectUserName', () => { + it('should select user name', () => { + const mockedStateA = fromJS({ + app: { + userName: null, + }, + }); + expect(selectUserName(mockedStateA)).toEqual(null); + + const mockedStateB = fromJS({ + app: { + userName: 'foobar', + }, + }); + expect(selectUserName(mockedStateB)).toEqual('foobar'); + }); +}); + +describe('selectUserLogin', () => { + it('should select user login', () => { + const mockedStateA = fromJS({ + app: { + userLogin: null, + }, + }); + expect(selectUserLogin(mockedStateA)).toEqual(null); + + const mockedStateB = fromJS({ + app: { + userLogin: 'foobar', + }, + }); + expect(selectUserLogin(mockedStateB)).toEqual('foobar'); + }); +}); + +describe('selectEndpoints', () => { + it('should select endpoints', () => { + const mockedStateA = fromJS({ + app: {}, + }); + expect(selectEndpoints(mockedStateA)).toEqual({}); + + const mockedStateB = fromJS({ + app: {}, + }) + .setIn(['app', 'endpoints'], { foo: true }); + expect(selectEndpoints(mockedStateB)).toEqual({ foo: true }); + }); +}); + +describe('selectIsUserLoggedIn', () => { + it('should select if user is logged in', () => { + const mockedStateA = fromJS({ + app: { + userLogin: null, + }, + }); + expect(selectIsUserLoggedIn(mockedStateA)).toEqual(false); + + const mockedStateB = fromJS({ + app: { + userLogin: 'foobar', + }, + }); + expect(selectIsUserLoggedIn(mockedStateB)).toEqual(true); + }); +}); + +describe('selectIsLoginRequired', () => { + it('should select if user is logged in', () => { + const mockedStateA = fromJS({ + app: { + hasFetchedState: false, + userLogin: null, + }, + }); + expect(selectIsLoginRequired(mockedStateA)).toEqual(false); + + const mockedStateB = fromJS({ + app: { + hasFetchedState: false, + userLogin: 'foobar', + }, + }); + expect(selectIsLoginRequired(mockedStateB)).toEqual(false); + + const mockedStateC = fromJS({ + app: { + hasFetchedState: true, + userLogin: null, + }, + }); + expect(selectIsLoginRequired(mockedStateC)).toEqual(true); + + const mockedStateD = fromJS({ + app: { + hasFetchedState: true, + userLogin: 'foobar', + }, + }); + expect(selectIsLoginRequired(mockedStateD)).toEqual(false); + }); +}); + +describe('selectRestoreState', () => { + it('should select state to be saved to sessionStorage', () => { + const mockedStateA = fromJS({ + app: { + githubUrl: 'gurl', + githubHost: 'ghost', + userName: 'uname', + userLogin: 'ulogin', + }, + }) + .setIn(['app', 'endpoints'], { foo: 'bar' }); + + expect(selectRestoreState(mockedStateA)).toEqual({ + githubUrl: 'gurl', + githubHost: 'ghost', + endpoints: { foo: 'bar' }, + userName: 'uname', + userLogin: 'ulogin', + }); + }); +}); + describe('selectRoute', () => { it('should select the route state', () => { const routeState = fromJS({}); From 17773086eb25a7f3015f7d35a68bfc88acf2d681 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 22/25] Add RenderErrorPanel component --- app/components/RenderErrorPanel/index.js | 98 +++++++++++++++++++ app/components/RenderErrorPanel/messages.js | 16 +++ .../RenderErrorPanel/tests/index.test.js | 11 +++ 3 files changed, 125 insertions(+) create mode 100644 app/components/RenderErrorPanel/index.js create mode 100644 app/components/RenderErrorPanel/messages.js create mode 100644 app/components/RenderErrorPanel/tests/index.test.js diff --git a/app/components/RenderErrorPanel/index.js b/app/components/RenderErrorPanel/index.js new file mode 100644 index 0000000..7257c01 --- /dev/null +++ b/app/components/RenderErrorPanel/index.js @@ -0,0 +1,98 @@ +/* + * RenderErrorPanel + */ + +import React from 'react'; +import PropTypes from 'prop-types'; +import { Panel } from '../Panel'; +import { FormattedMessage } from 'react-intl'; + +import messages from './messages'; + +class RenderErrorPanel extends React.Component { + + constructor(props) { + super(props); + + this.handleToggleDetail = this.handleToggleDetail.bind(this); + } + + state = { + showDetail: false, + }; + + handleToggleDetail() { + this.setState({ + showDetail: !this.state.showDetail, + }); + } + + render() { + const { debugMessage } = this.props; + const { showDetail } = this.state; + + return ( + +
    + +
    + + {debugMessage != null && ( + + +
    + +
    + + {showDetail && ( + 
    +                                {debugMessage}
+
    + )} + +     + )} +     + ); + } +} + +RenderErrorPanel.propTypes = { + debugMessage: PropTypes.string, +}; + +RenderErrorPanel.defaultProps = { + debugMessage: null, +}; + +export const panelErrorMessage = (error, info) => { + let debugMessages = []; + + try { + if (error) { + debugMessages.push(`Error Stack\n-------------------------\n${error.stack || error.message || error}`); + } + } + catch (err) { + // Ignore + } + + try { + if (info.componentStack) { + debugMessages.push(`Component Stack\n-------------------------${info.componentStack}`); + } + } + catch (err) { + // Ignore + } + + return ( + + ); +}; + +export default RenderErrorPanel; diff --git a/app/components/RenderErrorPanel/messages.js b/app/components/RenderErrorPanel/messages.js new file mode 100644 index 0000000..b4be40e --- /dev/null +++ b/app/components/RenderErrorPanel/messages.js @@ -0,0 +1,16 @@ +/* + * RenderErrorPanel Messages + */ + +import { defineMessages } from 'react-intl'; + +export default defineMessages({ + bodyMessage: { + id: 'app.components.RenderErrorPanel.bodyMessage', + defaultMessage: 'An problem was encountered while rendering this part of the page.', + }, + detailButton: { + id: 'app.components.RenderErrorPanel.detailButton', + defaultMessage: 'Toggle Detail', + }, +}); diff --git a/app/components/RenderErrorPanel/tests/index.test.js b/app/components/RenderErrorPanel/tests/index.test.js new file mode 100644 index 0000000..06e8e5a --- /dev/null +++ b/app/components/RenderErrorPanel/tests/index.test.js @@ -0,0 +1,11 @@ +import React from 'react'; +import { snapshots } from '../../../../internals/testing/snapshot-util'; +import RenderErrorPanel from '../index'; + +describe('', () => { + it.skip('should render expected JSX', () => { + snapshots( + , + ); + }); +}); From ccd55e4b4adde43492808337d5e51da4eaa6f8c0 Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 23/25] Change ExecutionDetailPage to better catch errors --- app/containers/ExecutionDetailPage/index.js | 64 ++++++++++++--------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/app/containers/ExecutionDetailPage/index.js b/app/containers/ExecutionDetailPage/index.js index dbd3616..803a65f 100644 --- a/app/containers/ExecutionDetailPage/index.js +++ b/app/containers/ExecutionDetailPage/index.js @@ -9,6 +9,8 @@ import { Helmet } from 'react-helmet'; import { compose, bindActionCreators } from 'redux'; import { Switch, Route } from 'react-router-dom'; +import ErrorBoundary from 'components/ErrorBoundary'; +import { panelErrorMessage } from 'components/RenderErrorPanel'; import { WindowHeightConsumer } from 'contexts/WindowHeight'; import { Panel, PanelHeader, PanelHeaderMini } from 'components/Panel'; import LoadingIndicator from 'components/LoadingIndicator'; @@ -59,11 +61,13 @@ function BuildDetail({ execution, buildKey }) { {(height) => (
    - + + +
    )}     @@ -145,21 +149,23 @@ export class ExecutionDetailPage extends React.Component { )} {execution && ( - + + + )} {execution && ( @@ -167,19 +173,23 @@ export class ExecutionDetailPage extends React.Component { ( - + + + )} /> ( - + + + )} /> From 07d4907df204492d9cab0990ebe6f74f4649112d Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 24/25] Add "no results" messages --- app/containers/CommitExecutionsPage/index.js | 6 ++++++ app/containers/RepoExecutionsPage/index.js | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/app/containers/CommitExecutionsPage/index.js b/app/containers/CommitExecutionsPage/index.js index 954bf88..2241730 100644 --- a/app/containers/CommitExecutionsPage/index.js +++ b/app/containers/CommitExecutionsPage/index.js @@ -91,6 +91,12 @@ export class CommitExecutionsPage extends React.Component { )} + {executions && !executions.length && ( + +
    No executions found for the repo/commit.
    +     + )} + {executions && executions.map((execution) => (
    diff --git a/app/containers/RepoExecutionsPage/index.js b/app/containers/RepoExecutionsPage/index.js index 0162298..3be00c3 100644 --- a/app/containers/RepoExecutionsPage/index.js +++ b/app/containers/RepoExecutionsPage/index.js @@ -88,6 +88,12 @@ export class RepoExecutionsPage extends React.Component { )} + {executions && !executions.length && ( + +
    No executions found for the repo.
    +     + )} + {executions && executions.map((execution) => (
    From 7c8ae6ba8731954aa7be7525165e153c502c59bb Mon Sep 17 00:00:00 2001 From: Andre Mekkawi Date: Sun, 9 Sep 2018 14:56:55 -0400 Subject: [PATCH 25/25] Add sub-resources integrity support --- package-lock.json | 36 ++++++++++++++++++++++++++++++++++++ package.json | 3 ++- webpack.config.js | 7 +++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 4a24520..4db7ded 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19787,6 +19787,33 @@ } } }, + "webpack-core": { + "version": "0.6.9", + "resolved": "https://registry.npmjs.org/webpack-core/-/webpack-core-0.6.9.tgz", + "integrity": "sha1-/FcViMhVjad76e+23r3Fo7FyvcI=", + "dev": true, + "requires": { + "source-list-map": "0.1.8", + "source-map": "0.4.4" + }, + "dependencies": { + "source-list-map": { + "version": "0.1.8", + "resolved": "https://registry.npmjs.org/source-list-map/-/source-list-map-0.1.8.tgz", + "integrity": "sha1-xVCyq1Qn9rPyH1r+rYjE9Vh7IQY=", + "dev": true + }, + "source-map": { + "version": "0.4.4", + "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.4.4.tgz", + "integrity": "sha1-66T12pwNyZneaAMti092FzZSA2s=", + "dev": true, + "requires": { + "amdefine": "1.0.1" + } + } + } + }, "webpack-dev-middleware": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/webpack-dev-middleware/-/webpack-dev-middleware-3.1.3.tgz", @@ -20065,6 +20092,15 @@ } } }, + "webpack-subresource-integrity": { + "version": "1.1.0-rc.4", + "resolved": "https://registry.npmjs.org/webpack-subresource-integrity/-/webpack-subresource-integrity-1.1.0-rc.4.tgz", + "integrity": "sha1-xcTj1pD50vZKlVDgeodn+Xlqpdg=", + "dev": true, + "requires": { + "webpack-core": "0.6.9" + } + }, "whatwg-encoding": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/whatwg-encoding/-/whatwg-encoding-1.0.4.tgz", diff --git a/package.json b/package.json index 5aa02c1..1506502 100644 --- a/package.json +++ b/package.json @@ -186,6 +186,7 @@ "url-loader": "^1.0.1", "webpack": "^4.16.5", "webpack-bundle-analyzer": "^2.13.1", - "webpack-cli": "^3.1.0" + "webpack-cli": "^3.1.0", + "webpack-subresource-integrity": "^1.1.0-rc.4" } } diff --git a/webpack.config.js b/webpack.config.js index 5e2d21c..4b7a0e6 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -2,6 +2,7 @@ const path = require('path'); const { HashedModuleIdsPlugin } = require('webpack'); const HtmlWebpackPlugin = require('html-webpack-plugin'); const CircularDependencyPlugin = require('circular-dependency-plugin'); +const SriPlugin = require('webpack-subresource-integrity'); const isProduction = process.env.NODE_ENV === 'production'; const isServe = !!process.env.WEBPACK_SERVE; @@ -21,6 +22,7 @@ module.exports = { ], output: { + crossOriginLoading: 'anonymous', path: path.resolve(__dirname, 'build'), publicPath: '/static/', ...(isProduction && !isServe ? { @@ -176,6 +178,11 @@ module.exports = { plugins: [ !!process.env.BUNDLE_ANALYZER && new (require('webpack-bundle-analyzer').BundleAnalyzerPlugin)(), + new SriPlugin({ + hashFuncNames: ['sha256', 'sha384'], + enabled: process.env.NODE_ENV === 'production', + }), + new HtmlWebpackPlugin({ inject: true, // Inject all files that are generated by webpack, e.g. bundle.js template: 'app/index.html',