Skip to content
This repository has been archived by the owner on Aug 27, 2024. It is now read-only.

Commit

Permalink
GH-501 adding refresh token functionality to handle edge case of (#571)
Browse files Browse the repository at this point in the history
* GH-501 adding refresh token functionality to handle edge case of outdated access token

* GH-501 adding user name update

* GH-501 providing the correct where query and update username

* GH-501 adding some better UX to the Refresh Token success flow

* GH-501 remove unused code

* GH-501 fixing a crazy weird case when refreshing tokens and before hooks are not triggered

* GH-501 adding migration scripts for PR check files
  • Loading branch information
fokusferit authored Apr 3, 2019
1 parent d5e6a5a commit eeada26
Show file tree
Hide file tree
Showing 18 changed files with 3,050 additions and 8,390 deletions.
34 changes: 34 additions & 0 deletions client/actions/checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { PENDING, SUCCESS, ERROR } from '../actions/status'

export const PUT_CHECK = Symbol('put check')
export const DELETE_CHECK = Symbol('delete check')
export const REFRESH_TOKEN = Symbol('refresh token')


function putCheck(status, payload = null) {
return {
Expand Down Expand Up @@ -50,3 +52,35 @@ export function toggleCheck(check) {
return disableCheck(check)
}
}

function refreshToken(status, payload = null) {
return {
type: REFRESH_TOKEN,
status,
payload
}
}

function updateTokenForChecks(repo, reloadReposFn){
return (dispatch) => {
dispatch(refreshToken(PENDING, {}))
CheckService.refreshTokens(repo.id)
.then((successMsg) => {
dispatch(refreshToken(SUCCESS, successMsg))
dispatch(reloadReposFn(true))
})
.catch(err => dispatch(refreshToken(ERROR, err)))
}
}

/**
* Refresh access token with new user logged in (1% case)
* @param {*} repo - repository data
*/
export function requestRefreshToken(repo, reloadReposFn) {
if(repo) {
return updateTokenForChecks(repo, reloadReposFn)
} else {
return {}
}
}
9 changes: 9 additions & 0 deletions client/components/RefreshTokenButton.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import React from 'react';
import { Button } from 'react-bootstrap';

export default function RefreshTokenButton() {
return <Button>
<span className={classes('fa', 'fa-fw', 'fa-refresh')}></span>
Refresh OAuth Token
</Button>
};
1 change: 0 additions & 1 deletion client/components/RepositoryBrowser.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export default class RepositoryBrowser extends Component {
<Button
style={{marginLeft: 15}}
disabled={isFetching}
lg
onClick={this.onFetchAll.bind(this)}>
<i className={classes('fa', 'fa-refresh', {'fa-spin': isFetching})}/>&nbsp;Sync with Github
</Button>
Expand Down
3 changes: 2 additions & 1 deletion client/components/RepositoryCheck.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ export default class RepositoryCheck extends Component {
marginLeft: 15
}
const {check, onToggle} = this.props
// ToDo: Change this to show "updated_by" value
const checkMeta = check.isEnabled ?
(!!check.created_by ?
<span>was enabled by <a href={`${this.props.githubUrl}/${check.created_by}`}>@{check.created_by}</a> <Time
value={check.createdAt} relative/></span> :
value={check.updatedAt} relative/></span> :
<span>is enabled</span>) :
<span>is disabled</span>
const header = (
Expand Down
18 changes: 14 additions & 4 deletions client/components/RepositoryConfigValidation.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class Result extends React.Component {
}
}

export default function RepositoryConfigValidation({validation, onValidate}) {
export default function RepositoryConfigValidation({validation, onValidate, onRefreshToken, isRefreshing}) {
const placeholder = <div style={{marginTop: '1em'}} />
let result = placeholder
if (validationFinished(validation.status)) {
Expand All @@ -90,7 +90,7 @@ export default function RepositoryConfigValidation({validation, onValidate}) {
} else if (validation.error) {
result = <Alert style={{marginTop: '1em'}} bsStyle='danger'>{validation.error.status} {validation.error.title}</Alert>
}
return <div>
return (<div>
<Button onClick={onValidate}>
<span className={classes('fa', 'fa-fw', {
'fa-spin': validation.status === Status.PENDING,
Expand All @@ -99,8 +99,16 @@ export default function RepositoryConfigValidation({validation, onValidate}) {
})}/>
Validate Zappr configuration
</Button>
<Button onClick={onRefreshToken} className="button-refreshToken">
<span className={classes('fa', 'fa-fw', {
'fa-spin ': isRefreshing,
'fa-refresh': isRefreshing,
'fa-info-circle': !isRefreshing
})} />
Refresh OAuth Token
</Button>
{result}
</div>
</div>);
}

RepositoryConfigValidation.propTypes = {
Expand All @@ -109,5 +117,7 @@ RepositoryConfigValidation.propTypes = {
message: PropTypes.string,
config: PropTypes.object
}).isRequired,
onValidate: PropTypes.func.isRequired
onValidate: PropTypes.func.isRequired,
onRefreshToken: PropTypes.func.isRequired,
isRefreshing: PropTypes.bool.isRequired,
}
24 changes: 19 additions & 5 deletions client/containers/RepositoryDetail.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,31 @@ import DocumentTitle from 'react-document-title'

import RepositoryCheck from './../components/RepositoryCheck.jsx'
import ConfigValidation from './../components/RepositoryConfigValidation.jsx'
import { toggleCheck } from '../actions/checks'
import { toggleCheck, requestRefreshToken } from '../actions/checks'
import { requestReposIfNeeded } from '../actions/repos';
import { requestConfigValidation } from '../actions/validate'

import { checkId } from '../model/schema'
import { CHECK_TYPES, CHECK_NAMES } from '../../server/checks'

function mapStateToProps(state) {
return {
githubUrl: state.env.GITHUB_UI_URL
githubUrl: state.env.GITHUB_UI_URL,
isRefreshing: state.isRefreshingToken
}
}

class RepositoryDetail extends Component {

static propTypes = {
repository: PropTypes.object.isRequired,
checks: PropTypes.object.isRequired,
validations: PropTypes.object.isRequired,
githubUrl: PropTypes.string,
toggleCheck: PropTypes.func.isRequired,
requestConfigValidation: PropTypes.func.isRequired
requestConfigValidation: PropTypes.func.isRequired,
requestRefreshToken: PropTypes.func.isRequired,
requestReposIfNeeded: PropTypes.func.isRequired,
};

onToggleCheck(check, isChecked) {
Expand All @@ -35,10 +40,15 @@ class RepositoryDetail extends Component {
this.props.requestConfigValidation(repo)
}

shouldRefreshToken(repo) {
this.props.requestRefreshToken(repo, this.props.requestReposIfNeeded)
}

render() {
if (!this.props.repository.full_name) return null

const {repository, checks, validations} = this.props
const { isRefreshingToken } = checks;
const header = (<h2>{repository.full_name}</h2>)

return (
Expand All @@ -60,7 +70,11 @@ class RepositoryDetail extends Component {
<Col md={12}>
<ConfigValidation
validation={validations[repository.full_name]}
onValidate={this.onValidateConfig.bind(this, repository)}/>
onValidate={this.onValidateConfig.bind(this, repository)}
isRefreshing={isRefreshingToken}
onRefreshToken={this.shouldRefreshToken.bind(this, repository)}
/>
<span>Test</span>
</Col>
<Col md={12}>
{CHECK_TYPES
Expand All @@ -85,4 +99,4 @@ class RepositoryDetail extends Component {
}
}

export default connect(mapStateToProps, {toggleCheck, requestConfigValidation})(RepositoryDetail)
export default connect(mapStateToProps, {toggleCheck, requestRefreshToken, requestConfigValidation, requestReposIfNeeded})(RepositoryDetail)
4 changes: 4 additions & 0 deletions client/css/main.css
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,7 @@ label.btn-primary.toggle-on:active {
}
}
}

.button-refreshToken {
float: right;
}
23 changes: 21 additions & 2 deletions client/reducers/repos/checks.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { PENDING, SUCCESS, ERROR } from '../../actions/status'
import { PUT_CHECK, DELETE_CHECK } from '../../actions/checks'
import { PUT_CHECK, DELETE_CHECK, REFRESH_TOKEN } from '../../actions/checks'
import { GET_REPOS } from '../../actions/repos'
import { mapValues } from '../../../common/util'

Expand Down Expand Up @@ -68,7 +68,7 @@ function check(state = {
}
}

export default function checks(state = {}, action) {
export default function checks(state = { isRefreshingToken: false, error: false}, action) {
switch (action.type) {
case GET_REPOS:
switch (action.status) {
Expand All @@ -79,6 +79,25 @@ export default function checks(state = {}, action) {
default:
return state
}
case REFRESH_TOKEN:
switch(action.status) {
case PENDING:
return {...state, isRefreshingToken: true}
case SUCCESS:
return {
...state,
isRefreshingToken: false,
error: false
}
case ERROR:
return {
...state,
isRefreshingToken: false,
error: action.payload
}
default:
return state
}
case PUT_CHECK:
case DELETE_CHECK:
return {
Expand Down
21 changes: 21 additions & 0 deletions client/service/CheckService.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,25 @@ export default class CheckService extends Service {
}
})
}

static refreshTokens(repoId) {
return fetch(Service.url(`/api/repos/${repoId}/refreshTokens`), {
method: 'POST',
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json'
},
credentials: 'same-origin'
}).then(response => {
return new Promise((resolve, reject) => {
return response.json().then(json => {
if(response.ok) {
resolve(json)
} else {
reject(json)
}
})
});
})
}
}
7 changes: 7 additions & 0 deletions migrations/007-add-prmergecommit-enum-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports.up = function up(queryInterface) {
return queryInterface.sequelize.query(`ALTER TYPE zappr_data.enum_checks_type ADD VALUE IF NOT EXISTS 'pullrequestmergecommit';`)
}
module.exports.down = function down() {
// there is no good downgrade
return Promise.resolve()
}
7 changes: 7 additions & 0 deletions migrations/008-add-prmilestone-enum-value.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports.up = function up(queryInterface) {
return queryInterface.sequelize.query(`ALTER TYPE zappr_data.enum_checks_type ADD VALUE IF NOT EXISTS 'pullrequestmilestone';`)
}
module.exports.down = function down() {
// there is no good downgrade
return Promise.resolve()
}
Loading

0 comments on commit eeada26

Please sign in to comment.