feat(PM-1506): show already member modal #1165
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| status: CopilotApplicationStatus, | ||
| opportunityStatus: string, | ||
| existingMembership?: ExistingMembership, | ||
| projectName: string, |
There was a problem hiding this comment.
The projectName property is added as a required field. If this is intentional, ensure that all instances of CopilotApplication are updated to include this field. If it should be optional, consider adding a question mark to make it projectName?: string.
|
|
||
| const AlreadyMemberModal: FC<AlreadyMemberModalProps> = props => ( | ||
| <BaseModal | ||
| onClose={props.onClose as () => void} |
There was a problem hiding this comment.
The onClose prop is being cast to () => void, which might lead to runtime errors if the function signature does not match. Consider ensuring that the function signature is compatible with BaseModal's expected type.
| > | ||
| <div className={styles.applyCopilotModal}> | ||
| <div className={styles.info}> | ||
| {`The copilot ${props.handle} is part of ${props.projectName} |
There was a problem hiding this comment.
Consider using template literals for better readability and to avoid unnecessary whitespace in the rendered string. For example: The copilot ${props.handle} is part of ${props.projectName} project with ${props.copilotApplication.existingMembership?.role} role.
| copilotApplication: CopilotApplication, | ||
| allCopilotApplications: CopilotApplication[], | ||
| ): JSX.Element => { | ||
| console.log(copilotApplication, 'copilotApplication') |
There was a problem hiding this comment.
Avoid using console.log statements in production code as it can lead to performance issues and clutter the console output. Consider using a proper logging mechanism if necessary.
| } | ||
|
|
||
| if (copilotApplication.existingMembership) { | ||
| console.log('comes here') |
There was a problem hiding this comment.
Remove console.log statement to avoid unnecessary logging in production code.
| await assignCopilotOpportunity(opportunityId, copilotApplication.id) | ||
| toast.success('Accepted as copilot') | ||
| mutate(`${copilotBaseUrl}/copilots/opportunity/${opportunityId}/applications`) | ||
| setShowAlreadyMemberModal(false) |
There was a problem hiding this comment.
Consider checking if setShowAlreadyMemberModal is necessary here, as it is also called in the catch block. If the modal should always be closed regardless of success or failure, it might be more efficient to call it once after the try-catch block.
| } | ||
| }, [opportunityId, copilotApplication]) | ||
|
|
||
| const onCloseModal = useCallback((e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => { |
There was a problem hiding this comment.
The useCallback dependency array includes showAlreadyMemberModal, but this state is not used within the callback function. Consider removing it from the dependency array to avoid unnecessary re-creations of the callback.
| handle: member?.handle, | ||
| opportunityStatus: props.opportunity.status, | ||
| pastProjects: member?.pastProjects || 0, | ||
| projectName: props.opportunity.projectName, |
There was a problem hiding this comment.
Ensure that props.opportunity.projectName is not undefined or null before assigning it to projectName. Consider using optional chaining or a default value to handle potential undefined values.
| copilotApplication: CopilotApplication, | ||
| allCopilotApplications: CopilotApplication[], | ||
| ): JSX.Element => { | ||
| const { opportunityId }: {opportunityId?: string} = useParams<{ opportunityId?: string }>() |
There was a problem hiding this comment.
Remove the console.log statement if it is not needed for debugging purposes. Leaving console.log statements in production code can lead to performance issues and clutter the console output.
| return | ||
| } | ||
|
|
||
| if (copilotApplication.existingMembership) { |
There was a problem hiding this comment.
Remove the commented-out console.log('comes here') statement to keep the code clean and maintainable.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1506
What's in this PR?