diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index 2c287c1b60..e2fd15aa89 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -470,7 +470,7 @@ export default React.createClass({ GroupStore.registerListener(groupId, this.onGroupStoreUpdated.bind(this, firstInit)); let willDoOnboarding = false; // XXX: This should be more fluxy - let's get the error from GroupStore .getError or something - GroupStore.on('error', (err, errorGroupId) => { + GroupStore.on('error', (err, errorGroupId, stateKey) => { if (this._unmounted || groupId !== errorGroupId) return; if (err.errcode === 'M_GUEST_ACCESS_FORBIDDEN' && !willDoOnboarding) { dis.dispatch({ @@ -483,11 +483,13 @@ export default React.createClass({ dis.dispatch({action: 'require_registration'}); willDoOnboarding = true; } - this.setState({ - summary: null, - error: err, - editing: false, - }); + if (stateKey === GroupStore.STATE_KEY.Summary) { + this.setState({ + summary: null, + error: err, + editing: false, + }); + } }); }, @@ -511,7 +513,6 @@ export default React.createClass({ isUserMember: GroupStore.getGroupMembers(this.props.groupId).some( (m) => m.userId === this._matrixClient.credentials.userId, ), - error: null, }); // XXX: This might not work but this.props.groupIsNew unused anyway if (this.props.groupIsNew && firstInit) { @@ -1157,7 +1158,7 @@ export default React.createClass({ if (this.state.summaryLoading && this.state.error === null || this.state.saving) { return ; - } else if (this.state.summary) { + } else if (this.state.summary && !this.state.error) { const summary = this.state.summary; let avatarNode; diff --git a/src/stores/GroupStore.js b/src/stores/GroupStore.js index bc2be37f51..4ac1e42e2e 100644 --- a/src/stores/GroupStore.js +++ b/src/stores/GroupStore.js @@ -122,10 +122,6 @@ class GroupStore extends EventEmitter { ); }, }; - - this.on('error', (err, groupId) => { - console.error(`GroupStore encountered error whilst fetching data for ${groupId}`, err); - }); } _fetchResource(stateKey, groupId) { @@ -148,7 +144,7 @@ class GroupStore extends EventEmitter { } console.error(`Failed to get resource ${stateKey} for ${groupId}`, err); - this.emit('error', err, groupId); + this.emit('error', err, groupId, stateKey); }).finally(() => { // Indicate finished request, allow for future fetches delete this._fetchResourcePromise[stateKey][groupId]; diff --git a/test/components/structures/GroupView-test.js b/test/components/structures/GroupView-test.js index 3b3510f26e..89632dcc48 100644 --- a/test/components/structures/GroupView-test.js +++ b/test/components/structures/GroupView-test.js @@ -164,7 +164,7 @@ describe('GroupView', function() { it('should indicate failure after failed /summary', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_error'); }); @@ -179,7 +179,7 @@ describe('GroupView', function() { it('should show a group avatar, name, id and short description after successful /summary', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView'); const avatar = ReactTestUtils.findRenderedComponentWithType(root, sdk.getComponent('avatars.GroupAvatar')); @@ -214,7 +214,7 @@ describe('GroupView', function() { it('should show a simple long description after successful /summary', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView'); const longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc'); @@ -235,7 +235,7 @@ describe('GroupView', function() { it('should show a placeholder if a long description is not set', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { const placeholder = ReactTestUtils .findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc_placeholder'); const placeholderElement = ReactDOM.findDOMNode(placeholder); @@ -255,7 +255,7 @@ describe('GroupView', function() { it('should show a complicated long description after successful /summary', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { const longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc'); const longDescElement = ReactDOM.findDOMNode(longDesc); expect(longDescElement).toExist(); @@ -282,7 +282,7 @@ describe('GroupView', function() { it('should disallow images with non-mxc URLs', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { const longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc'); const longDescElement = ReactDOM.findDOMNode(longDesc); expect(longDescElement).toExist(); @@ -305,7 +305,7 @@ describe('GroupView', function() { it('should show a RoomDetailList after a successful /summary & /rooms (no rooms returned)', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { const roomDetailList = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_RoomDetailList'); const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList); expect(roomDetailListElement).toExist(); @@ -322,7 +322,7 @@ describe('GroupView', function() { it('should show a RoomDetailList after a successful /summary & /rooms (with a single room)', function() { const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); - const prom = waitForUpdate(groupView).then(() => { + const prom = waitForUpdate(groupView, 4).then(() => { const roomDetailList = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_RoomDetailList'); const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList); expect(roomDetailListElement).toExist(); @@ -355,4 +355,25 @@ describe('GroupView', function() { httpBackend.flush(undefined, undefined, 0); return prom; }); + + it('should show a summary even if /users fails', function() { + const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); + + // Only wait for 3 updates in this test since we don't change state for + // the /users error case. + const prom = waitForUpdate(groupView, 3).then(() => { + const shortDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_header_shortDesc'); + const shortDescElement = ReactDOM.findDOMNode(shortDesc); + expect(shortDescElement).toExist(); + expect(shortDescElement.innerText).toBe('This is a community'); + }); + + httpBackend.when('GET', '/groups/' + groupIdEncoded + '/summary').respond(200, summaryResponse); + httpBackend.when('GET', '/groups/' + groupIdEncoded + '/users').respond(500, {}); + httpBackend.when('GET', '/groups/' + groupIdEncoded + '/invited_users').respond(200, { chunk: [] }); + httpBackend.when('GET', '/groups/' + groupIdEncoded + '/rooms').respond(200, { chunk: [] }); + + httpBackend.flush(undefined, undefined, 0); + return prom; + }); }); diff --git a/test/test-utils.js b/test/test-utils.js index bc4d29210e..d5bcd9397a 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -310,19 +310,26 @@ export function wrapInMatrixClientContext(WrappedComponent) { /** * Call fn before calling componentDidUpdate on a react component instance, inst. * @param {React.Component} inst an instance of a React component. + * @param {integer} updates Number of updates to wait for. (Defaults to 1.) * @returns {Promise} promise that resolves when componentDidUpdate is called on * given component instance. */ -export function waitForUpdate(inst) { +export function waitForUpdate(inst, updates = 1) { return new Promise((resolve, reject) => { const cdu = inst.componentDidUpdate; + console.log(`Waiting for ${updates} update(s)`); + inst.componentDidUpdate = (prevProps, prevState, snapshot) => { - resolve(); + updates--; + console.log(`Got update, ${updates} remaining`); + + if (updates == 0) { + inst.componentDidUpdate = cdu; + resolve(); + } if (cdu) cdu(prevProps, prevState, snapshot); - - inst.componentDidUpdate = cdu; }; }); }