Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions src/apps/copilots/src/models/CopilotApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ export interface CopilotApplication {
opportunityStatus: string,
existingMembership?: ExistingMembership,
projectName: string,
onApplied: () => void,
}
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ const CopilotOpportunityDetails: FC<{}> = () => {
copilotApplications={copilotApplications}
opportunity={opportunity}
members={members}
onApplied={onApplied}
/>
)}

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

import { BaseModal, Button } from '~/libs/ui'

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

interface ConfirmModalProps {
onClose: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void
onApply: () => void
}

const ConfirmModal: FC<ConfirmModalProps> = 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.

Suggestion

The onClose prop is being cast to () => void, which might suppress type errors. Consider ensuring that the onClose function matches the expected type without casting.

open
size='lg'
title='Confirm to accept as copilot'
buttons={(
<>
<Button primary onClick={props.onApply} label='Confirm' />

Choose a reason for hiding this comment

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

Suggestion

Consider adding type='button' to the Button components to explicitly define their type and avoid potential form submission issues if this component is used within a form.

<Button secondary onClick={props.onClose} label='Cancel' />

Choose a reason for hiding this comment

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

Suggestion

Consider adding type='button' to the Button components to explicitly define their type and avoid potential form submission issues if this component is used within a form.

</>
)}
>
<div className={styles.applyCopilotModal}>
<div className={styles.info}>
Click &apos;Confirm&apos; to accept by updating project role to &apos;Copilot&apos;
and complete this opportunity
</div>
</div>
</BaseModal>
)

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

Choose a reason for hiding this comment

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

The import statement for mutate from 'swr' has been removed. If this function is still used elsewhere in the code, ensure that it is not causing any issues or errors due to its removal.


import { assignCopilotOpportunity, copilotBaseUrl } from '~/apps/copilots/src/services/copilot-opportunities'
import { assignCopilotOpportunity } 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 ConfirmModal from './ConfirmModal'
import styles from './styles.module.scss'

const CopilotApplicationAction = (
Expand All @@ -16,6 +16,7 @@ const CopilotApplicationAction = (
): JSX.Element => {
const { opportunityId }: {opportunityId?: string} = useParams<{ opportunityId?: string }>()
const [showAlreadyMemberModal, setShowAlreadyMemberModal] = useState(false)
const [showConfirmModal, setShowConfirmModal] = useState(false)
const isInvited = useMemo(
() => allCopilotApplications
&& allCopilotApplications.findIndex(item => item.status === CopilotApplicationStatus.INVITED) > -1,
Expand All @@ -32,19 +33,8 @@ const CopilotApplicationAction = (

if (copilotApplication.existingMembership) {
setShowAlreadyMemberModal(true)

Choose a reason for hiding this comment

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

The return statement has been removed, which changes the control flow of the function. Ensure that this change is intentional and that the function should continue execution after setting setShowAlreadyMemberModal(true).

return
}

if (opportunityId) {
try {
await assignCopilotOpportunity(opportunityId, copilotApplication.id)
toast.success('Invited a copilot')
mutate(`${copilotBaseUrl}/copilots/opportunity/${opportunityId}/applications`)
} catch (e) {
const error = e as Error
toast.error(error.message)
}

} else {

Choose a reason for hiding this comment

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

The else block has been added, which changes the logic of when setShowConfirmModal(true) is called. Verify that this logic change aligns with the intended behavior of the application.

setShowConfirmModal(true)
}
}, [opportunityId, copilotApplication])

Expand All @@ -56,8 +46,9 @@ const CopilotApplicationAction = (

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

Choose a reason for hiding this comment

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

Consider checking if copilotApplication.onApplied is defined before calling it to prevent potential runtime errors.

setShowAlreadyMemberModal(false)
setShowConfirmModal(false)
} catch (e) {
const error = e as Error
toast.error(error.message)
Expand All @@ -68,6 +59,7 @@ const CopilotApplicationAction = (
e.preventDefault()
e.stopPropagation()
setShowAlreadyMemberModal(false)
setShowConfirmModal(false)
}, [showAlreadyMemberModal])

return (
Expand All @@ -84,7 +76,7 @@ const CopilotApplicationAction = (
!isInvited
&& copilotApplication.status === CopilotApplicationStatus.PENDING
&& copilotApplication.opportunityStatus === 'active' && (
<Tooltip content='Send Invitation'>
<Tooltip content='Accept'>

Choose a reason for hiding this comment

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

The tooltip content has been changed from 'Send Invitation' to 'Accept'. Ensure that this change aligns with the intended functionality and user experience. If the action is still related to sending an invitation, consider revisiting the tooltip text for clarity.

<IconSolid.UserAddIcon />
</Tooltip>
)
Expand All @@ -107,6 +99,13 @@ const CopilotApplicationAction = (
copilotApplication={copilotApplication}
/>
)}

{showConfirmModal && (
<ConfirmModal
onClose={onCloseModal}
onApply={onApply}
/>
)}
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const CopilotApplications: FC<{
copilotApplications?: CopilotApplication[]
members?: FormattedMembers[]
opportunity: CopilotOpportunity
onApplied: () => void
}> = props => {
const getData = (): CopilotApplication[] => (props.copilotApplications ? props.copilotApplications.map(item => {
const member = props.members && props.members.find(each => each.userId === item.userId)
Expand All @@ -85,6 +86,7 @@ const CopilotApplications: FC<{
activeProjects: member?.activeProjects || 0,
fulfilment: member?.copilotFulfillment || 0,
handle: member?.handle,
onApplied: props.onApplied,

Choose a reason for hiding this comment

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

The onApplied property is being added to the object, but it is not clear from the diff if this property is being used or handled correctly elsewhere in the code. Ensure that onApplied is being utilized appropriately and that it does not introduce any unintended side effects.

opportunityStatus: props.opportunity.status,
pastProjects: member?.pastProjects || 0,
projectName: props.opportunity.projectName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ const OpportunityDetails: FC<{
{props.opportunity?.skills.map(item => (<span className={styles.skillPill}>{item.name}</span>))}
</div>
<h2 className={styles.subHeading}> Description </h2>
<p>
{props.opportunity?.overview}
</p>
{props.opportunity?.overview && (
<div dangerouslySetInnerHTML={{

Choose a reason for hiding this comment

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

Using dangerouslySetInnerHTML can expose the application to XSS attacks if the overview content is not properly sanitized. Consider sanitizing the HTML content before rendering it to ensure security.

__html: props.opportunity.overview.replace(/\n/g, '<br />'),
}}
/>
)}
</div>
<div>
<h2 className={styles.subHeading}> Complexity </h2>
Expand Down
10 changes: 9 additions & 1 deletion src/apps/copilots/src/pages/copilot-request-form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const CopilotRequestForm: FC<{}> = () => {
const [formErrors, setFormErrors] = useState<any>({})
const [paymentType, setPaymentType] = useState<string>('')
const [projectFromQuery, setProjectFromQuery] = useState<Project>()
const activeProjectStatuses = ['active', 'approved', 'draft', 'new']

Choose a reason for hiding this comment

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

Consider defining activeProjectStatuses as a constant outside of the component to avoid re-initializing it on every render. This can improve performance slightly by reducing unnecessary re-creation of the array.


const { data: copilotRequestData }: CopilotRequestResponse = useCopilotRequest(routeParams.requestId)

Expand Down Expand Up @@ -114,7 +115,13 @@ const CopilotRequestForm: FC<{}> = () => {
label: string;
value: string;
}>> {
const response = await getProjects(inputValue)
const response = await getProjects(inputValue, {
filter: {
status: {
$in: [activeProjectStatuses],

Choose a reason for hiding this comment

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

The variable activeProjectStatuses should be an array or a list of statuses. Ensure that activeProjectStatuses is defined and contains the appropriate values for filtering projects by status.

},
},
})
return response.map(project => ({ label: project.name, value: project.id }))
}

Expand Down Expand Up @@ -494,6 +501,7 @@ const CopilotRequestForm: FC<{}> = () => {
.setFullYear(new Date()
.getFullYear() + 2))}
minYear={new Date()}
className={styles.datepicker}
/>
<p className={styles.formRow}>How many weeks will you need the copilot for?</p>
<InputText
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,8 @@ $gradient: linear-gradient(
margin-right: $sp-1;
}
}

.datepicker input{

Choose a reason for hiding this comment

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

Consider adding a space before the opening curly brace for consistency with the rest of the file's formatting.

color: black;
}
}

Choose a reason for hiding this comment

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

It's a good practice to ensure that files end with a newline character to avoid potential issues with some tools and version control systems.

5 changes: 4 additions & 1 deletion src/apps/copilots/src/pages/copilot-requests/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ const CopilotRequestsPage: FC = () => {
className: styles.opportunityTitle,
label: 'Title',
propertyName: 'opportunityTitle',
type: 'text',
renderer: (copilotRequest: CopilotRequest) => (
<div className={styles.title}>{copilotRequest.opportunityTitle}</div>

Choose a reason for hiding this comment

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

Consider adding a fallback or default value for copilotRequest.opportunityTitle to handle cases where it might be undefined or null.

),
type: 'element',
},
{
label: 'Type',
Expand Down
4 changes: 2 additions & 2 deletions src/apps/copilots/src/services/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ export const getProject = (projectId: string): Promise<Project> => {
return xhrGetAsync<Project>(url)
}

export const getProjects = (search?: string, filter?: any): Promise<Project[]> => {
const params = { name: `"${search}"`, ...filter }
export const getProjects = (search?: string, config?: {filter: any}): Promise<Project[]> => {

Choose a reason for hiding this comment

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

The type for config parameter should be more specific if possible, instead of using any for filter. Consider defining a type or interface for filter to improve type safety.

const params = { name: search, ...config?.filter }

Choose a reason for hiding this comment

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

The name parameter in params is now directly assigned search without quotes. Ensure that this change is intentional and that the API expects the search term without quotes.

const url = buildUrl(baseUrl, params)
return xhrGetAsync<Project[]>(url)
}