Conversation
PM-3840 Show multiple users in popover
PM-3878 Make open to work fields required
PM-3913 Add profile completeness check for private engagements
PM-3916 Fix Industry enum for healthcare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const users = leaveEntry?.usersOnLeave ?? [] | ||
| const sortedUsers = [...users].sort(compareUsersByName) | ||
| const displayedUsers = sortedUsers.slice(0, 10) | ||
| const displayedUsers = sortedUsers.slice(0, 2) |
There was a problem hiding this comment.
[design]
Reducing the number of displayed users from 10 to 2 might significantly change the user experience. Ensure that this change is intentional and aligns with the design requirements.
| </div> | ||
|
|
||
| {isMobile && openDateKey && (() => { | ||
| {openDateKey && (() => { |
There was a problem hiding this comment.
[correctness]
The condition isMobile && openDateKey was removed. Ensure that this change does not unintentionally affect the mobile-specific behavior of the calendar.
| </span> | ||
| <a | ||
| className={styles.signInLink} | ||
| href={`${EnvironmentConfig.URLS.USER_PROFILE}/${props.profileHandle}`} |
There was a problem hiding this comment.
[❗❗ security]
The URL construction for the profile link uses string interpolation with props.profileHandle. Ensure that profileHandle is properly sanitized to prevent potential injection attacks or malformed URLs.
| const showAssignedActions = assignmentStatus === 'assigned' | ||
| const showOfferActions = assignmentStatus === 'selected' | ||
|
|
||
| const renderOfferActions = ( |
There was a problem hiding this comment.
[performance]
The renderOfferActions function is defined within the AssignmentCard component, which means it will be re-created on every render. Consider using useCallback to memoize this function if it doesn't depend on any props or state that change frequently.
| const isLoggedIn = profileContext.isLoggedIn | ||
| const userId = profileContext.profile?.userId | ||
| const profileHandle = profileContext.profile?.handle | ||
| const profileCompleteness = useProfileCompleteness(profileHandle) |
There was a problem hiding this comment.
[❗❗ correctness]
The useProfileCompleteness hook is called with profileHandle, which could be undefined if profileContext.profile is not available. Ensure that useProfileCompleteness can handle an undefined argument gracefully.
|
|
||
| if ( | ||
| profileCompleteness | ||
| && typeof profileCompleteness.percent === 'number' |
There was a problem hiding this comment.
[❗❗ correctness]
The check typeof profileCompleteness.percent === 'number' is necessary, but consider adding a check to ensure profileCompleteness is not undefined before accessing percent to avoid potential runtime errors.
| function handleFormChange(nextValue: OpenToWorkData): void { | ||
| setFormValue(nextValue) | ||
|
|
||
| if (submitAttempted) { |
There was a problem hiding this comment.
[performance]
The validateOpenToWork function is called on every form change when submitAttempted is true. This could lead to performance issues if the validation logic is complex or if the form changes frequently. Consider optimizing the validation logic or debouncing the validation calls.
| const errors = validateOpenToWork(formValue) | ||
| setFormErrors(errors) | ||
|
|
||
| if (Object.keys(errors).length > 0) { |
There was a problem hiding this comment.
[❗❗ correctness]
The check if (Object.keys(errors).length > 0) is used to determine if there are validation errors. Ensure that validateOpenToWork always returns an object, even if there are no errors, to avoid potential runtime errors.
| setFormValue(nextValue) | ||
|
|
||
| if (submitAttempted) { | ||
| setFormErrors(validateOpenToWork(nextValue)) |
There was a problem hiding this comment.
[performance]
The validateOpenToWork function is called twice: once in handleFormChange and again in handleOpenForWorkSave. Consider optimizing this by storing the result of the validation in a variable and reusing it if possible.
| const errors = validateOpenToWork(formValue) | ||
| setFormErrors(errors) | ||
|
|
||
| if (Object.keys(errors).length > 0) { |
There was a problem hiding this comment.
[💡 maintainability]
The check Object.keys(errors).length > 0 is used to determine if there are errors. Consider using a utility function like isEmptyObject for better readability and maintainability.
| [key: string]: string | ||
| } = { | ||
| ConsumerGoods: 'Consumer goods', | ||
| HealthCare: 'Healthcare', |
There was a problem hiding this comment.
[❗❗ correctness]
Inconsistent casing: 'HealthCare' should be 'Healthcare' to match the label 'Healthcare'.
| [key: string]: string | ||
| } = { | ||
| 'Consumer goods': 'ConsumerGoods', | ||
| Healthcare: 'HealthCare', |
There was a problem hiding this comment.
[❗❗ correctness]
Inconsistent casing: 'HealthCare' should be 'Healthcare' to match the label 'Healthcare'.
2 participants
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-3916
https://topcoder.atlassian.net/browse/PM-3913
https://topcoder.atlassian.net/browse/PM-3878
https://topcoder.atlassian.net/browse/PM-3840
What's in this PR?