PROD: Copilot portal CRs & Bugs V3 #1181
Conversation
fix(PM-1584): title field too narrow
PM-1270 Filter inactive projects in copilot request form
fix(PM-1610): Updated toast message and tooltip copy for directly accepting copilot
fix(PM-1610): Show confirm modal while accepting application
UI bugs: Fix font color, preserve line breaks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| const ConfirmModal: FC<ConfirmModalProps> = props => ( | ||
| <BaseModal | ||
| onClose={props.onClose as () => void} |
There was a problem hiding this comment.
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.
| title='Confirm to accept as copilot' | ||
| buttons={( | ||
| <> | ||
| <Button primary onClick={props.onApply} label='Confirm' /> |
There was a problem hiding this comment.
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.
| buttons={( | ||
| <> | ||
| <Button primary onClick={props.onApply} label='Confirm' /> | ||
| <Button secondary onClick={props.onClose} label='Cancel' /> |
There was a problem hiding this comment.
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.
| @@ -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' | |||
There was a problem hiding this comment.
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.
| @@ -32,19 +33,8 @@ const CopilotApplicationAction = ( | |||
|
|
|||
| if (copilotApplication.existingMembership) { | |||
| setShowAlreadyMemberModal(true) | |||
There was a problem hiding this comment.
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).
| toast.error(error.message) | ||
| } | ||
|
|
||
| } else { |
There was a problem hiding this comment.
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.
| @@ -56,8 +46,9 @@ const CopilotApplicationAction = ( | |||
|
|
|||
| await assignCopilotOpportunity(opportunityId, copilotApplication.id) | |||
| toast.success('Accepted as copilot') | |||
| mutate(`${copilotBaseUrl}/copilots/opportunity/${opportunityId}/applications`) | |||
| copilotApplication.onApplied() | |||
There was a problem hiding this comment.
Consider checking if copilotApplication.onApplied is defined before calling it to prevent potential runtime errors.
| @@ -84,7 +76,7 @@ const CopilotApplicationAction = ( | |||
| !isInvited | |||
| && copilotApplication.status === CopilotApplicationStatus.PENDING | |||
| && copilotApplication.opportunityStatus === 'active' && ( | |||
| <Tooltip content='Send Invitation'> | |||
| <Tooltip content='Accept'> | |||
There was a problem hiding this comment.
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.
| @@ -85,6 +86,7 @@ const CopilotApplications: FC<{ | |||
| activeProjects: member?.activeProjects || 0, | |||
| fulfilment: member?.copilotFulfillment || 0, | |||
| handle: member?.handle, | |||
| onApplied: props.onApplied, | |||
There was a problem hiding this comment.
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.
| {props.opportunity?.overview} | ||
| </p> | ||
| {props.opportunity?.overview && ( | ||
| <div dangerouslySetInnerHTML={{ |
There was a problem hiding this comment.
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.
| @@ -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'] | |||
There was a problem hiding this comment.
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 response = await getProjects(inputValue, { | ||
| filter: { | ||
| status: { | ||
| $in: [activeProjectStatuses], |
There was a problem hiding this comment.
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.
| @@ -97,4 +97,8 @@ $gradient: linear-gradient( | |||
| margin-right: $sp-1; | |||
| } | |||
| } | |||
|
|
|||
| .datepicker input{ | |||
There was a problem hiding this comment.
Consider adding a space before the opening curly brace for consistency with the rest of the file's formatting.
|
|
||
| .datepicker input{ | ||
| color: black; | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
| @@ -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> | |||
There was a problem hiding this comment.
Consider adding a fallback or default value for copilotRequest.opportunityTitle to handle cases where it might be undefined or null.
| @@ -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[]> => { | |||
There was a problem hiding this comment.
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.
| export const getProjects = (search?: string, filter?: any): Promise<Project[]> => { | ||
| const params = { name: `"${search}"`, ...filter } | ||
| export const getProjects = (search?: string, config?: {filter: any}): Promise<Project[]> => { | ||
| const params = { name: search, ...config?.filter } |
There was a problem hiding this comment.
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.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1270
https://topcoder.atlassian.net/browse/PM-1584
https://topcoder.atlassian.net/browse/PM-1610
https://topcoder.atlassian.net/browse/PM-1613
What's in this PR?