Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ workflows:
- LVT-256
- CORE-635
- feat/system-admin
- pm-1448_1
- pm-1506_1

- deployQa:
context: org-global
Expand Down
7 changes: 7 additions & 0 deletions src/apps/copilots/src/models/CopilotApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ export enum CopilotApplicationStatus {
PENDING = 'pending',
}

export interface ExistingMembership {
role: string,
id: number,
}

export interface CopilotApplication {
id: number,
notes?: string,
Expand All @@ -13,4 +18,6 @@ export interface CopilotApplication {
userId: number,
status: CopilotApplicationStatus,
opportunityStatus: string,
existingMembership?: ExistingMembership,
projectName: string,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/* eslint-disable react/jsx-no-bind */
import { FC } from 'react'

import { BaseModal, Button } from '~/libs/ui'
import { CopilotApplication } from '~/apps/copilots/src/models/CopilotApplication'

import styles from './styles.module.scss'

interface AlreadyMemberModalProps {
onClose: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void
copilotApplication: CopilotApplication
handle?: string
onApply: () => void
projectName: string
}

const AlreadyMemberModal: FC<AlreadyMemberModalProps> = props => (
<BaseModal
onClose={props.onClose as () => void}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

open
size='lg'
title='User already member of the project'
buttons={(
<>
<Button primary onClick={props.onApply} label='Confirm' />
<Button secondary onClick={props.onClose} label='Cancel' />
</>
)}
>
<div className={styles.applyCopilotModal}>
<div className={styles.info}>
{`The copilot ${props.handle} is part of ${props.projectName}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

project with ${props.copilotApplication.existingMembership?.role} role.`}
<div>Click &apos;Confirm&apos; to accept and complete this opportunity.</div>
</div>
</div>
</BaseModal>
)

export default AlreadyMemberModal
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import { useParams } from 'react-router-dom'
import { toast } from 'react-toastify'
import { mutate } from 'swr'
import { useCallback, useMemo } from 'react'
import { useCallback, useMemo, useState } from 'react'

import { assignCopilotOpportunity, copilotBaseUrl } from '~/apps/copilots/src/services/copilot-opportunities'
import { CopilotApplication, CopilotApplicationStatus } from '~/apps/copilots/src/models/CopilotApplication'
import { IconSolid, Tooltip } from '~/libs/ui'

import AlreadyMemberModal from './AlreadyMemberModal'
import styles from './styles.module.scss'

const CopilotApplicationAction = (
copilotApplication: CopilotApplication,
allCopilotApplications: CopilotApplication[],
): JSX.Element => {
console.log(copilotApplication, 'copilotApplication')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const { opportunityId }: {opportunityId?: string} = useParams<{ opportunityId?: string }>()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const [showAlreadyMemberModal, setShowAlreadyMemberModal] = useState(false)
const isInvited = useMemo(
() => allCopilotApplications
&& allCopilotApplications.findIndex(item => item.status === CopilotApplicationStatus.INVITED) > -1,
Expand All @@ -28,6 +31,12 @@ const CopilotApplicationAction = (
return
}

if (copilotApplication.existingMembership) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the commented-out console.log('comes here') statement to keep the code clean and maintainable.

console.log('comes here')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove console.log statement to avoid unnecessary logging in production code.

setShowAlreadyMemberModal(true)
return
}

if (opportunityId) {
try {
await assignCopilotOpportunity(opportunityId, copilotApplication.id)
Expand All @@ -40,6 +49,29 @@ const CopilotApplicationAction = (

}
}, [opportunityId, copilotApplication])

const onApply = useCallback(async () => {
try {
if (!opportunityId) {
return
}

await assignCopilotOpportunity(opportunityId, copilotApplication.id)
toast.success('Accepted as copilot')
mutate(`${copilotBaseUrl}/copilots/opportunity/${opportunityId}/applications`)
setShowAlreadyMemberModal(false)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} catch (e) {
const error = e as Error
toast.error(error.message)
}
}, [opportunityId, copilotApplication])

const onCloseModal = useCallback((e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

e.preventDefault()
e.stopPropagation()
setShowAlreadyMemberModal(false)
}, [showAlreadyMemberModal])

return (
<div onClick={onClick} className={styles.actionWrapper}>
{
Expand Down Expand Up @@ -67,6 +99,16 @@ const CopilotApplicationAction = (
</Tooltip>
)
}

{showAlreadyMemberModal && (
<AlreadyMemberModal
projectName={copilotApplication.projectName}
handle={copilotApplication.handle}
onClose={onCloseModal}
onApply={onApply}
copilotApplication={copilotApplication}
/>
)}
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const CopilotApplications: FC<{
handle: member?.handle,
opportunityStatus: props.opportunity.status,
pastProjects: member?.pastProjects || 0,
projectName: props.opportunity.projectName,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
})
.sort((a, b) => (b.fulfilment || 0) - (a.fulfilment || 0)) : [])
Expand Down