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.

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

Users & Roles #35

Merged
merged 11 commits into from
Mar 1, 2021
Merged

Users & Roles #35

merged 11 commits into from
Mar 1, 2021

Conversation

0xHericles
Copy link
Member

@0xHericles 0xHericles commented Feb 24, 2021

Description

Add users overview + list and roles page.
Add API Integration + Object and Response Types support

Issues

#22 #15

Affected Dependencies

"@reach/accordion": "^0.13.2"

Checklist

@0xHericles 0xHericles added the Type: New Feature ➕ Introduction of a completely new addition to the codebase label Feb 24, 2021
Copy link
Member

@ariannmichael ariannmichael left a comment

Choose a reason for hiding this comment

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

Good work, the only additional point is in the future divide the interfaces on other files besides types/index.ts

canManageInfrastructure,
canUploadData
}) => {
const permissions = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract permissions out of the component. Maybe this could be in a JSON somewhere?

Copy link
Collaborator

@tcp tcp left a comment

Choose a reason for hiding this comment

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

Solid! Some minor changes needed. I'll see what it looks like!

const {data: groupsData} = useQuery<Record<string, IGroup>, Error>('groups', fetchGroups)
const {data: rolesData} = useQuery<Record<string, IRole>, Error>('roles', fetchRoles)

useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I failed to see why we need to keep states to track the total. Is it possible to replace this effect with a direct call to each of the deps?

))}
</section>
<section className="space-y-6">
{Object.keys(usersData).map(userId => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

usersData is undefined by default and will remain undefined if fetching fails.isLoading will resolve to false and Object.keys(undefined) throws.

@tcp tcp merged commit c6991e9 into dev Mar 1, 2021
@delete-merged-branch delete-merged-branch bot deleted the users/flow branch March 1, 2021 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: New Feature ➕ Introduction of a completely new addition to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants