[JENKINS-38329] REGRESSION Errors on activity and branches tabs (#528)

* [JENKINS-38329] do not fetch pr or branches in case we are not supporting them otherwise we will run into failures the next time. Further in general we should never try to fetch something where we know before hand that it does not exit.

* [JENKINS-38329] remove log statement

* [JENKINS-38329] make sure to remove the branch/pr data when unmounting

* eslint - formating changes and fix offences

* [JENKINS-38329] fix tests
This commit is contained in:
Thorsten Scherler 2016-09-26 20:10:14 +02:00 committed by GitHub
parent 8690946acd
commit 306287176f
4 changed files with 35 additions and 15 deletions

View File

@ -51,7 +51,7 @@ EmptyState.propTypes = {
export class MultiBranch extends Component {
componentWillMount() {
if (this.context.config && this.context.params) {
if (this.context.pipeline && this.context.params && !pipelineBranchesUnsupported(this.context.pipeline)) {
this.props.fetchBranches({
organizationName: this.context.params.organization,
pipelineName: this.context.params.pipeline,
@ -59,14 +59,15 @@ export class MultiBranch extends Component {
}
}
componentWillUnmount() {
this.props.clearBranchData();
}
render() {
const { branches } = this.props;
if (!branches) {
return null;
}
if (!branches.$pending && pipelineBranchesUnsupported(this.context.pipeline)) {
if (!branches || (!branches.$pending && pipelineBranchesUnsupported(this.context.pipeline))) {
return (<NotSupported />);
}
@ -125,6 +126,7 @@ MultiBranch.contextTypes = {
MultiBranch.propTypes = {
branches: array,
fetchBranches: func,
clearBranchData: func,
children: any,
};

View File

@ -48,7 +48,7 @@ EmptyState.propTypes = {
export class PullRequests extends Component {
componentWillMount() {
if (this.context.config && this.context.params) {
if (this.context.pipeline && this.context.params && !pipelineBranchesUnsupported(this.context.pipeline)) {
this.props.fetchPullRequests({
organizationName: this.context.params.organization,
pipelineName: this.context.params.pipeline,
@ -56,20 +56,21 @@ export class PullRequests extends Component {
}
}
componentWillUnmount() {
this.props.clearPRData();
}
render() {
const { pullRequests } = this.props;
if (!pullRequests) {
return null;
if (!pullRequests || (!pullRequests.$pending && pipelineBranchesUnsupported(this.context.pipeline))) {
return (<NotSupported />);
}
if (pullRequests.$pending && !pullRequests.length) {
return <PageLoading />;
}
if (!pullRequests.$pending && pipelineBranchesUnsupported(this.context.pipeline)) {
return (<NotSupported />);
}
if (pullRequests.$failed) {
return <div>Error: {pullRequests.$failed}</div>;
@ -120,6 +121,7 @@ PullRequests.contextTypes = {
PullRequests.propTypes = {
pullRequests: array,
clearPRData: func,
fetchPullRequests: func,
};

View File

@ -70,6 +70,7 @@ export const ACTION_TYPES = keymirror({
SET_CURRENT_PULL_REQUEST_DATA: null,
SET_CURRENT_BRANCHES_DATA: null,
CLEAR_CURRENT_BRANCHES_DATA: null,
CLEAR_CURRENT_PULL_REQUEST_DATA: null,
SET_TEST_RESULTS: null,
SET_STEPS: null,
SET_NODE: null,
@ -126,7 +127,10 @@ export const actionHandlers = {
.set('currentRuns', runs[id]);
},
[ACTION_TYPES.CLEAR_CURRENT_BRANCHES_DATA](state) {
return state.set('currentBranches', null);
return state.delete('currentBranches');
},
[ACTION_TYPES.CLEAR_CURRENT_PULL_REQUEST_DATA](state) {
return state.delete('pullRequests');
},
[ACTION_TYPES.SET_CURRENT_BRANCHES_DATA](state, { payload }): State {
return state.set('currentBranches', payload);
@ -264,6 +268,18 @@ export const actions = {
};
},
clearBranchData() {
return (dispatch) => dispatch({
type: ACTION_TYPES.CLEAR_CURRENT_BRANCHES_DATA,
});
},
clearPRData() {
return (dispatch) => dispatch({
type: ACTION_TYPES.CLEAR_CURRENT_PULL_REQUEST_DATA,
});
},
fetchOrganizationPipelines({ organizationName }) {
return (dispatch) => {
// Note: this is including folders, which we can't deal with, so exclude them with the ?filter=no-folders

View File

@ -20,9 +20,9 @@ describe("PullRequests should render", () => {
});
describe("PullRequests should not render", () => {
it("does not renders the PullRequests without data", () => {
it("does render NotSupported the PullRequests without data", () => {
const wrapper = shallow(<PullRequests />);
assert.isNull(wrapper.node);
assert.equal(wrapper.find('NotSupported').length, 1);
});
});