New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow group summary to load when /users fails #2326

Merged
merged 2 commits into from Dec 5, 2018

Conversation

Projects
None yet
3 participants
@jryans
Copy link
Contributor

jryans commented Dec 5, 2018

This change allows the main group summary to stay on screen as long as the /summary request succeeds. Other group requests may fail, but the group will remain displayed. This has the side effect of allowing group invites to be displayed even if the additional group requests happen to fail.

If the /users or /invited_users requests fail, an error will be shown in the member list.

Fixes vector-im/riot-web#7727.

jryans added some commits Dec 4, 2018

Only mark group as failed to load for summary
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>
Add error to UI when group member list fails to load
Signed-off-by: J. Ryan Stinnett <jryans@gmail.com>

@jryans jryans requested a review from matrix-org/riot-web Dec 5, 2018

@jryans

This comment has been minimized.

Copy link
Contributor

jryans commented Dec 5, 2018

If there is an error when requesting /users for the group, it will look like this:

group-users-error

@jryans

This comment has been minimized.

Copy link
Contributor

jryans commented Dec 5, 2018

Hmm, unclear if the failure in React E2E tests is real or intermittent. I'll look in more detail tomorrow.

@turt2live

This comment has been minimized.

Copy link
Member

turt2live commented Dec 5, 2018

Looks like it was just a transient error

@dbkr

dbkr approved these changes Dec 5, 2018

Copy link
Member

dbkr left a comment

and bonus points for the test, thank you!

@@ -134,6 +134,8 @@
"Failed to join room": "Failed to join room",
"Failed to kick": "Failed to kick",
"Failed to leave room": "Failed to leave room",
"Failed to load %(groupId)s": "Failed to load %(groupId)s",
"Failed to load group members": "Failed to load group members",

This comment has been minimized.

@dbkr

dbkr Dec 5, 2018

Member

ftr, you don't need to add entries to this file: en_US falls back to en_EN automatically (even if there is a US english translation, it's generally better to add it in weblate otherwise you'll get conflicts in weblate).

This comment has been minimized.

@jryans

jryans Dec 5, 2018

Contributor

Ah okay, thanks!

@dbkr dbkr merged commit dad8e6a into matrix-org:develop Dec 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (rxl881) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment