Issue feat PS-3574:Add firstname , lastname gender changes of learner and facilitator #627
Conversation
WalkthroughThe pull request introduces consistent changes across multiple components and pages, primarily focusing on user name formatting and data handling. The modifications involve constructing full names by concatenating Changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/pages/attendance-history.tsx (1)
Line range hint
392-396: Improve error handling with proper error messages and recovery.The current error handling only logs to console and shows a generic message. Consider:
- Adding specific error types and messages
- Implementing retry logic for transient failures
- Providing user-friendly recovery options
} catch (error) { - console.error('Error fetching cohort list:', error); - showToastMessage(t('COMMON.SOMETHING_WENT_WRONG'), 'error'); + const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + console.error('Error fetching cohort list:', errorMessage); + showToastMessage(t('ERRORS.FETCH_COHORT_LIST', { error: errorMessage }), 'error'); + // Implement retry logic here setLoading(false); }src/components/CohortFacilitatorList.tsx (1)
Line range hint
18-26: Update UserDataProps interface to match implementation.The interface is missing the following fields that are being used in the implementation:
firstNamelastNamestatusReasonUpdate the interface to match the implementation:
interface UserDataProps { - name: string; + firstName?: string; + lastName?: string; userId: string; memberStatus: string; + statusReason?: string; cohortMembershipId: string; enrollmentNumber: string; }
🧹 Nitpick comments (7)
src/components/CohortLearnerList.tsx (1)
70-70: Improve name concatenation robustness.The name concatenation could be moved to a utility function to ensure consistent formatting across the application.
Consider creating a utility function:
export const formatFullName = (firstName?: string, lastName?: string): string => { const first = firstName ? toPascalCase(firstName) : ''; const last = lastName ? toPascalCase(lastName) : ''; return [first, last].filter(Boolean).join(' '); };src/components/AddFacilitator.tsx (1)
225-225: Consider refactoring field handling logic.The condition
fieldId === null || fieldId === 'null' || fieldKey===fieldKeys.GENDERsuggests special handling for gender. Consider standardizing the field handling to avoid special cases.src/pages/attendance-history.tsx (4)
244-244: Improve name construction logic to handle edge cases.The current implementation may result in trailing spaces when lastName is empty. Consider using array join for cleaner concatenation.
-name: toPascalCase(entry?.firstName || '') + ' ' + (entry?.lastName ? toPascalCase(entry.lastName) : ""), +name: [toPascalCase(entry?.firstName || ''), entry?.lastName ? toPascalCase(entry?.lastName) : ''].filter(Boolean).join(' '),
Line range hint
571-586: Enhance search functionality for better user experience.Consider the following improvements:
- The minimum search length of 3 characters might be too restrictive for short names
- Search could include additional fields like userName for better results
- Ensure all debounced calls are cancelled when resetting
const handleSearch = (event: React.ChangeEvent<HTMLInputElement>) => { const trimmedValue = event.target.value.replace(/\s{2,}/g, " ").trimStart(); setSearchWord(trimmedValue); ReactGA.event('search-by-keyword-attendance-history-age', { keyword: trimmedValue, }); - if (trimmedValue.length >= 3) { + if (trimmedValue.length >= 1) { debouncedSearch(trimmedValue); } else if (trimmedValue === '') { debouncedSearch.cancel(); setDisplayStudentList(cohortMemberList); } else { setDisplayStudentList(cohortMemberList); } };
Line range hint
1-1000: Consider breaking down this large component for better maintainability.The component is handling too many responsibilities (attendance management, search, sorting, etc.). Consider splitting it into smaller, focused components:
- AttendanceManagement
- SearchAndSort
- AttendanceList
This will improve code organization, testing, and maintenance.
Line range hint
319-320: Remove commented-out code to improve code clarity.There are several commented-out functions and blocks that should be removed if they're no longer needed. This includes:
getAllDatesInRangehandleUpdategetStateByCohortIdIf these functions might be needed later, track them in version control instead of keeping them as comments.
Also applies to: 571-572
src/utils/app.constant.ts (1)
258-260: Consider grouping field-related enums together.While the implementation is correct, consider moving this enum near other field-related enums like
FormContextorFormContextTypefor better code organization.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/AddFacilitator.tsx(3 hunks)src/components/CohortFacilitatorList.tsx(1 hunks)src/components/CohortLearnerList.tsx(1 hunks)src/components/LearnersListItem.tsx(1 hunks)src/pages/attendance-history.tsx(1 hunks)src/pages/attendance-overview.tsx(1 hunks)src/pages/board-enrollment/index.tsx(1 hunks)src/pages/observation/[observationId]/index.tsx(1 hunks)src/utils/app.constant.ts(1 hunks)
🔇 Additional comments (6)
src/components/AddFacilitator.tsx (1)
359-359: Verify email template compatibility.The change from
apiBody['name']toapiBody['firstName']might affect the email template. Ensure the template is updated to handle first name instead of full name.✅ Verification successful
Email template compatibility verified - no issues found
The change from
apiBody['name']toapiBody['firstName']is compatible with the email template system. ThesendEmailOnFacilitatorCreationfunction accepts a name parameter as a string, and the change only affects which field is used as the source of this name, not the template structure itself.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for email templates that might be using the full name rg -l "name.*template" --type ts --type tsxLength of output: 73
Script:
#!/bin/bash # Search for email templates and name usage patterns rg -l "template.*email|email.*template" rg -l "name.*template|template.*name" # Search specifically for firstName or name usage in potential template files rg "(\"|')name(\"|')|(\"|')firstName(\"|')" -A 2 -B 2Length of output: 8153
Script:
#!/bin/bash # Find sendEmail function implementation ast-grep --pattern 'function sendEmail($_) { $$$ }' # Search for email-related files and configurations rg -l "sendEmail|email.*template|template.*email" --type ts --type js --type tsx # Look for the actual implementation with context rg "sendEmail" -A 5 -B 5Length of output: 6700
src/pages/board-enrollment/index.tsx (1)
240-242: LGTM! The name formatting logic looks good.The code properly handles the concatenation of firstName and lastName with appropriate null checks and consistent casing.
src/pages/observation/[observationId]/index.tsx (1)
341-341: LGTM! The name formatting logic looks good.The code properly handles the concatenation of firstName and lastName with appropriate null checks and consistent casing.
src/components/LearnersListItem.tsx (1)
411-419: LGTM! The avatar initials logic looks good.The code properly handles edge cases where name parts might be missing using optional chaining, making it more robust.
src/pages/attendance-overview.tsx (1)
216-217: LGTM! The name formatting logic looks good.The code properly handles the concatenation of firstName and lastName with appropriate null checks and consistent casing.
src/components/CohortFacilitatorList.tsx (1)
80-80: LGTM! Robust name construction implementation.The name construction properly handles:
- Null/undefined cases for both firstName and lastName
- Consistent casing using toPascalCase
- Proper spacing between names
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/assessments/index.tsx (1)
120-120: LGTM! Consider adding fallback for lastName.The name formatting implementation correctly handles null/undefined cases and maintains consistent casing. However, consider adding a fallback empty string for lastName similar to firstName for consistency.
- name: toPascalCase(user?.firstName || '') + ' ' + (user?.lastName ? toPascalCase(user.lastName) : ""), + name: toPascalCase(user?.firstName || '') + ' ' + toPascalCase(user?.lastName || ''),src/utils/Interfaces.ts (1)
99-99: Fix spacing for consistency.The type definition is correct, but the spacing around the colon is inconsistent with other properties in the interface.
- firstName?:string + firstName?: string
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/assessments/index.tsx(1 hunks)src/pages/observation/[observationId]/index.tsx(2 hunks)src/utils/Interfaces.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/observation/[observationId]/index.tsx
| @@ -117,8 +117,7 @@ const Assessments = () => { | |||
| if (resp) { | |||
| const userDetails = resp.map((user: any) => ({ | |||
| ...user, | |||
| name: toPascalCase(user.name), | |||
| userId: user.userId, | |||
| name: toPascalCase(user?.firstName || '') + ' ' + (user?.lastName ? toPascalCase(user.lastName) : ""), userId: user.userId, | |||
There was a problem hiding this comment.
@AkshataKatwal16 add proper formatting
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/AddLeanerModal.tsx (1)
Line range hint
102-117: Improve error handling in email sending logicThe email sending logic should handle cases where name fields are missing or malformed.
const sendEmail = async ( name: string, username: string, password: string, email: string, learnerName: string ) => { try { + if (!name || !learnerName) { + throw new Error('Missing required name fields'); + } const response = await sendEmailOnLearnerCreation( name, username, password, email, learnerName ); if (response?.email?.data?.[0]?.status !== 200) { showToastMessage(t('COMMON.USER_CREDENTIAL_SEND_FAILED'), 'error'); } setOpenModal(true); } catch (error) { - console.error('error in sending email', error); + console.error('Error in sending email:', error); + showToastMessage(t('COMMON.EMAIL_SEND_FAILED'), 'error'); } };
🧹 Nitpick comments (3)
src/components/AddLeanerModal.tsx (1)
Line range hint
73-76: Add type safety for form data handlingThe component uses
anytype for form data states. Consider adding proper TypeScript interfaces to improve type safety and maintainability.interface LearnerFormData { name: string; firstName?: string; lastName?: string; mobile?: string; father_name?: string; // ... other fields } const [learnerFormData, setLearnerFormData] = React.useState<LearnerFormData | null>(null);src/components/GeneratedSchemas.ts (1)
118-122: Consider enhancing date field validation.While the basic date field implementation is correct, consider adding:
- Date range validation using
minimumandmaximum.- Format validation to ensure dates match the expected format.
case 'date': fieldSchema.type = 'string'; fieldSchema.format = 'date'; + if (field?.validation?.includes('future')) { + fieldSchema.minimum = new Date().toISOString().split('T')[0]; + } + if (field?.validation?.includes('past')) { + fieldSchema.maximum = new Date().toISOString().split('T')[0]; + } fieldUiSchema['ui:widget'] = 'date'; break;src/utils/Interfaces.ts (1)
99-99: Consider adding lastName filter for consistency.Since
firstNamehas been added as a filter property, consider addinglastNameas well to maintain consistency with the name-related changes mentioned in the AI summary.firstName?:string +lastName?:string
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/AddLeanerModal.tsx(1 hunks)src/components/GeneratedSchemas.ts(1 hunks)src/utils/Interfaces.ts(2 hunks)
🔇 Additional comments (1)
src/utils/Interfaces.ts (1)
436-436: LGTM!The addition of the 'date' type to the union type is correct and aligns with the schema generation changes.
| @@ -258,7 +258,7 @@ const AddLearnerModal: React.FC<AddLearnerModalProps> = ({ | |||
| username, | |||
| password, | |||
| userEmail, | |||
| apiBody['name'] | |||
| apiBody['firstName'] | |||
There was a problem hiding this comment.
Critical: Accessing undefined field firstName
The code attempts to use apiBody['firstName'] but this field is not set anywhere in the code. The form processing only sets apiBody['name'] without splitting it into first/last name components.
Apply this fix to properly handle the name fields:
- apiBody['firstName']
+ apiBody['name']?.split(' ')?.[0] || apiBody['name']Additionally, consider refactoring the name handling logic:
- Add proper name splitting when processing form data:
if (fieldKey === 'name') {
const nameParts = fieldValue.split(' ');
apiBody['firstName'] = nameParts[0];
apiBody['lastName'] = nameParts.slice(1).join(' ');
setFullname(fieldValue);
}- Add validation to ensure name fields are properly set:
if (!apiBody['firstName']) {
showToastMessage(t('COMMON.INVALID_NAME_FORMAT'), 'error');
return;
}
|
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores