user component now integrated with /api/users GET endpoint #583
Conversation
ForrestLonganecker
requested review from
marisawallace,
KentShikama and
wittejm
There was a problem hiding this comment.
Given the following set of events occur
1. Admin A opens the users page
2. Admin A switches to the search page
3. Admin B creates a new account User C
4. Admin A opens the users page
Should it show User C or not?
Edit: Ignore the above; I thought I saw a potential issue.
| return ( | ||
| <p className="bg-white mv4 pa4 br3 fw6 tc shadow br3"> | ||
| You are not authorized to view this content, please contact system | ||
| administraitor. |
| render() { | ||
| return ( | ||
| <p className="bg-white mv4 pa4 br3 fw6 tc shadow br3"> | ||
| Something went wrong, please contact system administraitor to report |
| public render() { | ||
| return ( | ||
| render() { | ||
| return this.props.users.userList.length > 0 ? ( |
There was a problem hiding this comment.
Nitpick: I think the True case can be wrapped in its own function just like displayNoUsers.
There was a problem hiding this comment.
Simple fix! I will make that change.
| <NotAuthorized /> | ||
| ) : ( | ||
| <TechnicalDifficulties /> | ||
| ); |
There was a problem hiding this comment.
I would prefer not chaining ternary operators
…) and displayNoUsers(), fixed typo
|
@KentShikama Thank you for the review! I realized that a non-admin user could see the userlist if:
I added a |
|
Are you referring to if the non-admin happens to use the same browser as the admin (aka they share a computer)? |
|
Exactly
|
There was a problem hiding this comment.
I would prefer if clearing was done on unmount given how each time the user goes to the users page they get a fresh set of users (between the time they swapped tabs a new user could have been created). See https://github.com/codeforpdx/recordexpungPDX/blob/master/src/frontend/src/containers/AllRecords.tsx#L20-L22 for a similar pattern.
…ungPDX into users-component/get-integration pulled upstream changes
* user component now integrated with /api/users GET endpoint * changed /api/users GET test to expect updated response object * added CLEAR_USERS action, refactored displayUsers(), displayUserList() and displayNoUsers(), fixed typo * moved clearUsers() call to componentWillUnmount()
This PR now is pulling users from our database: #91
Notes:
/api/usersGET to match up with expected variable names in the front-end. changed:user_idtoidandgroup_nametogroup.Previously I had a function that was converting the snake_case to the proper format but this solution allows us to pass the response object in directly.
<Spinner />component to a generic<LoadingSpinner inputString={} />we are now able to use this loading spinner in anywhere we need to have it and pass in a string such as "Users" or "your search results".To-Do: