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
Sc 12514/user details #198
Conversation
This pull request has been linked to Shortcut Story #12514: Beacon. FE: Add User details component. |
@@ -5,8 +5,8 @@ export interface APIKey { | |||
name: string; | |||
owner: string; | |||
permissions: string[]; | |||
created: string; | |||
modified: string; | |||
created?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are optional, so I made the correction here.
<h6 className="font-bold">User Name:</h6> | ||
<span className="ml-3">{name}</span> | ||
</div> | ||
{/* <div className="flex gap-12 pb-8"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Email address isn't in the model for members. It can be discussed if it should be added in sprint 4.
|
||
const handleClose = () => setIsOpen(false); | ||
|
||
const { org, hasOrgFailed, isFetchingOrg, error } = useFetchOrg('orgID'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useFetchOrg('orgID')
may not be the best way to display this. I'm available to discuss this today, if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The orgID should be fetched from another hook right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added a few minor suggestions, feel free to merge if done
expect(requestSpy).toHaveBeenCalledTimes(1); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add more cases but because we are running out of time we could review it in the next sprint by adding failure cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create a spike story in sprint 4 about adding test cases.
|
||
const handleClose = () => setIsOpen(false); | ||
|
||
const { org, hasOrgFailed, isFetchingOrg, error } = useFetchOrg('orgID'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The orgID should be fetched from another hook right ?
@@ -0,0 +1,31 @@ | |||
import { vi } from 'vitest'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be great to add a test case where the id is not set
Scope of changes
Adds the user details component and logic to fetch users. Also adds logic for organizationDetail and updates the organizationDetail component to display fetched data.
Minor style changes were made to the OrganizationDetails component, so a recording of it is included in the acceptance criteria.
Note: Users are listed as Members in the Tenant API, so this is why I labeled the folder Members.
Fixes SC-12514
Type of change
Acceptance criteria
MemberDetails component: https://www.awesomescreenshot.com/video/14715533?key=a79775085fb01d20e59f5d95ffc336fa
OrganizationDetails component: https://www.awesomescreenshot.com/video/14715689?key=a7d3e2b4fff4a5b2120f1bfd1c201508
Author checklist
Reviewer(s) checklist