imterah/element-portable
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Only mark group as failed to load for summary

Browse source 
Currently, any error in the `GroupStore`s several requests can cause the whole
`GroupView` component to hide and be mark the group as failed to load.

Since it is known that group members may fail to load in some cases, let's only
show failed to load for the whole group when the summary fails.

This also strengthens the `GroupView` test by ensuring we wait for multiple
updates for checking results.

Signed-off-by: J. Ryan Stinnett <jryans@gmail.com>
This commit is contained in:
J. Ryan Stinnett 2018-12-03 20:38:29 -06:00
parent e3f2e69087
commit 5fc25fd6ba
4 changed files with 50 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

17
src/components/structures/GroupView.js
View file

@ -470,7 +470,7 @@ export default React.createClass({
GroupStore.registerListener(groupId, this.onGroupStoreUpdated.bind(this, firstInit)); GroupStore.registerListener(groupId, this.onGroupStoreUpdated.bind(this, firstInit));
let willDoOnboarding = false; let willDoOnboarding = false;
// XXX: This should be more fluxy - let's get the error from GroupStore .getError or something // 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 (this._unmounted || groupId !== errorGroupId) return;
if (err.errcode === 'M_GUEST_ACCESS_FORBIDDEN' && !willDoOnboarding) { if (err.errcode === 'M_GUEST_ACCESS_FORBIDDEN' && !willDoOnboarding) {
dis.dispatch({ dis.dispatch({
@ -483,11 +483,13 @@ export default React.createClass({
dis.dispatch({action: 'require_registration'}); dis.dispatch({action: 'require_registration'});
willDoOnboarding = true; willDoOnboarding = true;
} }
this.setState({ if (stateKey === GroupStore.STATE_KEY.Summary) {
summary: null, this.setState({
error: err, summary: null,
editing: false, error: err,
}); editing: false,
});
}
}); });
}, },
@ -511,7 +513,6 @@ export default React.createClass({
isUserMember: GroupStore.getGroupMembers(this.props.groupId).some( isUserMember: GroupStore.getGroupMembers(this.props.groupId).some(
(m) => m.userId === this._matrixClient.credentials.userId, (m) => m.userId === this._matrixClient.credentials.userId,
), ),
error: null,
}); });
// XXX: This might not work but this.props.groupIsNew unused anyway // XXX: This might not work but this.props.groupIsNew unused anyway
if (this.props.groupIsNew && firstInit) { if (this.props.groupIsNew && firstInit) {
@ -1157,7 +1158,7 @@ export default React.createClass({
if (this.state.summaryLoading && this.state.error === null || this.state.saving) { if (this.state.summaryLoading && this.state.error === null || this.state.saving) {
return <Spinner />; return <Spinner />;
} else if (this.state.summary) { } else if (this.state.summary && !this.state.error) {
const summary = this.state.summary; const summary = this.state.summary;
let avatarNode; let avatarNode;

6
src/stores/GroupStore.js
View file

@ -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) { _fetchResource(stateKey, groupId) {
@ -148,7 +144,7 @@ class GroupStore extends EventEmitter {
} }
console.error(`Failed to get resource ${stateKey} for ${groupId}`, err); console.error(`Failed to get resource ${stateKey} for ${groupId}`, err);
this.emit('error', err, groupId); this.emit('error', err, groupId, stateKey);
}).finally(() => { }).finally(() => {
// Indicate finished request, allow for future fetches // Indicate finished request, allow for future fetches
delete this._fetchResourcePromise[stateKey][groupId]; delete this._fetchResourcePromise[stateKey][groupId];

37
test/components/structures/GroupView-test.js
View file

@ -164,7 +164,7 @@ describe('GroupView', function() {
it('should indicate failure after failed /summary', function() { it('should indicate failure after failed /summary', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView);
const prom = waitForUpdate(groupView).then(() => { const prom = waitForUpdate(groupView, 4).then(() => {
ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_error'); 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() { it('should show a group avatar, name, id and short description after successful /summary', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView);
const prom = waitForUpdate(groupView).then(() => { const prom = waitForUpdate(groupView, 4).then(() => {
ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView'); ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView');
const avatar = ReactTestUtils.findRenderedComponentWithType(root, sdk.getComponent('avatars.GroupAvatar')); 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() { it('should show a simple long description after successful /summary', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView);
const prom = waitForUpdate(groupView).then(() => { const prom = waitForUpdate(groupView, 4).then(() => {
ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView'); ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView');
const longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc'); 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() { it('should show a placeholder if a long description is not set', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView);
const prom = waitForUpdate(groupView).then(() => { const prom = waitForUpdate(groupView, 4).then(() => {
const placeholder = ReactTestUtils const placeholder = ReactTestUtils
.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc_placeholder'); .findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc_placeholder');
const placeholderElement = ReactDOM.findDOMNode(placeholder); const placeholderElement = ReactDOM.findDOMNode(placeholder);
@ -255,7 +255,7 @@ describe('GroupView', function() {
it('should show a complicated long description after successful /summary', function() { it('should show a complicated long description after successful /summary', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); 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 longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc');
const longDescElement = ReactDOM.findDOMNode(longDesc); const longDescElement = ReactDOM.findDOMNode(longDesc);
expect(longDescElement).toExist(); expect(longDescElement).toExist();
@ -282,7 +282,7 @@ describe('GroupView', function() {
it('should disallow images with non-mxc URLs', function() { it('should disallow images with non-mxc URLs', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); 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 longDesc = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_GroupView_groupDesc');
const longDescElement = ReactDOM.findDOMNode(longDesc); const longDescElement = ReactDOM.findDOMNode(longDesc);
expect(longDescElement).toExist(); expect(longDescElement).toExist();
@ -305,7 +305,7 @@ describe('GroupView', function() {
it('should show a RoomDetailList after a successful /summary & /rooms (no rooms returned)', function() { it('should show a RoomDetailList after a successful /summary & /rooms (no rooms returned)', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); 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 roomDetailList = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_RoomDetailList');
const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList); const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList);
expect(roomDetailListElement).toExist(); 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() { it('should show a RoomDetailList after a successful /summary & /rooms (with a single room)', function() {
const groupView = ReactTestUtils.findRenderedComponentWithType(root, GroupView); 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 roomDetailList = ReactTestUtils.findRenderedDOMComponentWithClass(root, 'mx_RoomDetailList');
const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList); const roomDetailListElement = ReactDOM.findDOMNode(roomDetailList);
expect(roomDetailListElement).toExist(); expect(roomDetailListElement).toExist();
@ -355,4 +355,25 @@ describe('GroupView', function() {
httpBackend.flush(undefined, undefined, 0); httpBackend.flush(undefined, undefined, 0);
return prom; 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;
});
}); });

15
test/test-utils.js
View file

@ -310,19 +310,26 @@ export function wrapInMatrixClientContext(WrappedComponent) {
/** /**
* Call fn before calling componentDidUpdate on a react component instance, inst. * Call fn before calling componentDidUpdate on a react component instance, inst.
* @param {React.Component} inst an instance of a React component. * @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 * @returns {Promise} promise that resolves when componentDidUpdate is called on
* given component instance. * given component instance.
*/ */
export function waitForUpdate(inst) { export function waitForUpdate(inst, updates = 1) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const cdu = inst.componentDidUpdate; const cdu = inst.componentDidUpdate;
console.log(`Waiting for ${updates} update(s)`);
inst.componentDidUpdate = (prevProps, prevState, snapshot) => { 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); if (cdu) cdu(prevProps, prevState, snapshot);
inst.componentDidUpdate = cdu;
}; };
}); });
} }