Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix infinite user list's loading indicator when connected as user with no roles #2390

Merged
merged 4 commits into from
Jun 20, 2018
Merged

Fix infinite user list's loading indicator when connected as user with no roles #2390

merged 4 commits into from
Jun 20, 2018

Conversation

richard-cox
Copy link
Contributor

Issue one

  • Normally for non-admin the list of users is fetched per org
  • Users without any orgs would show indefinite loading bar on users tables
  • Now we mock a response that contains the signed in user, so they can at least see themselves
  • This avoids some odd messages and user counts and generally improves the experience of possibly new visitors

Issue two

  • We would only display org user roles if the user was an org user
  • We now always show all org roles regardless if they are an org user

Note

  • users.effects.ts normal path remains unchanged, it's just been shunted into a function

@richard-cox
Ensure that we show all a users roles regardless of if they're an org…
e24779b 
… user or not
@richard-cox
Handle the case when the connected user is not in any org
7fa5559 
- Normally for non-admin the list of users is fetched per org
- Users without any orgs would show indefinite loading bar on users tables
- Now we mock a response that contains the signed in user, so they can at least see themselves
- This avoids some odd messages and user counts and generally improves the experience of possibly new visitors
@richard-cox
Updated comment
32a1581
@richard-cox richard-cox self-assigned this Jun 14, 2018
@cfdreddbot
Copy link

Hey richard-cox!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Jun 14, 2018 •
edited
Loading

Codecov Report

Merging #2390 into v2-master will decrease coverage by 0.02%.
The diff coverage is 30.9%.

@@             Coverage Diff              @@
##           v2-master   #2390      +/-   ##
============================================
- Coverage      71.03%     71%   -0.03%     
============================================
  Files            585     585              
  Lines          24537   24562      +25     
  Branches        5511    5515       +4     
============================================
+ Hits           17429   17441      +12     
- Misses          7108    7121      +13

@richard-cox richard-cox mentioned this pull request Jun 15, 2018
Merged
@irfanhabib irfanhabib self-requested a review June 20, 2018 08:27
irfanhabib
irfanhabib suggested changes Jun 20, 2018
View reviewed changes
Copy link
Contributor

@irfanhabib irfanhabib left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged in v2-master, I'm still seeing the issue of infinite loading when connected to an endpoint with user that has no roles.

@irfanhabib irfanhabib added the needs attention This PR needs attention label Jun 20, 2018
@irfanhabib
Copy link
Contributor

Sorry, I had multiple stratos running and I was accessing an older instance when reviewing this. Its all good!

@irfanhabib irfanhabib merged commit fa612d3 into v2-master Jun 20, 2018
@irfanhabib irfanhabib deleted the fix-no-users branch June 20, 2018 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irfanhabib irfanhabib irfanhabib approved these changes

Assignees

@richard-cox richard-cox

Labels
needs attention This PR needs attention ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@richard-cox @cfdreddbot @irfanhabib