feat: adding Learner Detail Page component + route #1436
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1436 +/- ##
==========================================
- Coverage 86.48% 86.41% -0.07%
==========================================
Files 660 661 +1
Lines 14936 14968 +32
Branches 3164 3177 +13
==========================================
+ Hits 12917 12935 +18
- Misses 1952 1969 +17
+ Partials 67 64 -3 ☔ View full report in Codecov by Sentry. |
kiram15
force-pushed
the
kiram15/ENT-10006
branch
from
364f4b6 to
207c39e
Compare
|
|
||
| const LearnerDetailPage = ({ enterpriseUUID }) => { | ||
| const { enterpriseSlug, groupUuid, learnerId } = useParams(); | ||
| const { data: enterpriseGroup } = useEnterpriseGroupUuid(groupUuid); |
There was a problem hiding this comment.
[opportunity] Could we typescript annotate the useEnterpriseGroupUuid hook?
| enterprise_customer: enterpriseUUID, | ||
| user_id: learnerId, | ||
| }; | ||
| const data = await LmsApiService.fetchEnterpriseLearnerData(options); |
There was a problem hiding this comment.
[opportunity] Could we typescript annotate the fetchEnterpriseLearnerData api method?
kiram15
force-pushed
the
kiram15/ENT-10006
branch
from
6701192 to
d57766c
Compare
| const LearnerDetailPage = ({ enterpriseUUID }) => { | ||
| const { enterpriseSlug, groupUuid, learnerId } = useParams(); | ||
| const { data: enterpriseGroup } = useEnterpriseGroupUuid(groupUuid); | ||
| const [learnerData, setLearnerData] = useState(null); |
There was a problem hiding this comment.
[suggestion (non-blocking)]: let's initialize the state with an empty object instead of null to avoid potential issues when accessing properties
| }, [enterpriseUUID, learnerId]); | ||
|
|
||
| const intl = useIntl(); | ||
| const links = [{ |
There was a problem hiding this comment.
[sugggestion (non-blocking)]: can we memoize the links array using useMemo to avoid unnecessary re-renders? something like...
const links = useMemo(() => {
const baseLinks = [{
label: intl.formatMessage({
id: 'adminPortal.peopleManagement.learnerDetailPage.breadcrumb.peopleManagement',
defaultMessage: 'People Management',
description: 'Breadcrumb label to go back to People Management page',
}),
to: `/${enterpriseSlug}/admin/${ROUTE_NAMES.peopleManagement}`,
}];
if (groupUuid) {
baseLinks.push({
label: intl.formatMessage({
id: 'adminPortal.peopleManagement.learnerDetailPage.breadcrumb.groupName',
defaultMessage: `${enterpriseGroup?.name}`,
description: 'Breadcrumb label to go back to group detail page',
}),
to: `/${enterpriseSlug}/admin/${ROUTE_NAMES.peopleManagement}/${groupUuid}`,
});
}
return baseLinks;
}, [intl, enterpriseSlug, groupUuid, enterpriseGroup])
| const [learnerData, setLearnerData] = useState(null); | ||
| const [isLoading, setIsLoading] = useState(true); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
[suggestion (non-blocking)]: let's extract this useEffect logic into a custom hook i.e. useLearnerData to simplify this component since it might become convoluted once we add in other sections like the groups section and enrollments
Jira ticket
For all changes
kiram15/ENT-10006and runnpm run test:stageView morebuttonView morebutton or the user's name hyperlinkTesting plan
Only if submitting a visual change