fix(PM-1273): show apply copilot only for projects where user is not a member #1112
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -32,6 +32,7 @@ export const childRoutes = [ | |||
| authRequired: true, | |||
| element: <CopilotsRequestForm />, | |||
| id: 'CopilotRequestForm', | |||
| rolesRequired: [UserRole.administrator, UserRole.projectManager] as UserRole[], | |||
There was a problem hiding this comment.
Consider checking if the rolesRequired array should include any other roles that might be relevant for accessing the Copilot Request Form. Ensure that this change aligns with the intended access control requirements.
| @@ -125,6 +125,7 @@ const CopilotOpportunityDetails: FC<{}> = () => { | |||
| } | |||
|
|
|||
| const application = copilotApplications && copilotApplications[0] | |||
| const isAlreadyMemberOfTheProject = profile && opportunity?.members?.includes(profile.userId); | |||
There was a problem hiding this comment.
Consider using optional chaining for opportunity?.members to ensure it is safe to call .includes on it, in case members is not defined. This will prevent potential runtime errors.
| @@ -133,7 +134,8 @@ const CopilotOpportunityDetails: FC<{}> = () => { | |||
| isCopilot | |||
| && copilotApplications | |||
| && copilotApplications.length === 0 | |||
| && opportunity?.status === 'active' ? applyCopilotOpportunityButton : undefined | |||
| && opportunity?.status === 'active' | |||
| && !isAlreadyMemberOfTheProject ? applyCopilotOpportunityButton : undefined | |||
There was a problem hiding this comment.
Consider renaming isAlreadyMemberOfTheProject to isUserAlreadyMemberOfProject for better clarity and consistency with naming conventions.
| @@ -1,14 +1,14 @@ | |||
| import { FC, useContext, useMemo, useState } from 'react' | |||
| import { bind, isEmpty } from 'lodash' | |||
| import { bind, debounce, isEmpty } from 'lodash' | |||
There was a problem hiding this comment.
The debounce function is imported from lodash but is not used in the code. Consider removing it if it is not needed.
| import { toast } from 'react-toastify' | ||
| import classNames from 'classnames' | ||
|
|
||
| import { profileContext, ProfileContextData } from '~/libs/core' | ||
| import { Button, IconSolid, InputDatePicker, InputMultiselectOption, | ||
| InputRadio, InputSelect, InputSelectOption, InputSelectReact, InputText, InputTextarea } from '~/libs/ui' | ||
| InputRadio, InputSelect, InputSelectReact, InputText, InputTextarea } from '~/libs/ui' |
There was a problem hiding this comment.
The InputSelectOption import has been removed, but it is unclear if this was intentional. Verify if this component is still needed elsewhere in the code.
| import { InputSkillSelector } from '~/libs/shared' | ||
|
|
||
| import { ProjectsResponse, useProjects } from '../../services/projects' | ||
| import { getProjects } from '../../services/projects' |
There was a problem hiding this comment.
The useProjects hook has been replaced with getProjects. Ensure that the new implementation correctly handles any asynchronous data fetching and state management that useProjects might have provided.
| label: string; | ||
| value: string; | ||
| }>> { | ||
| const response = await getProjects(inputValue) |
There was a problem hiding this comment.
Consider adding error handling for the getProjects function call to manage potential API failures or network issues.
| @@ -268,6 +251,11 @@ const CopilotRequestForm: FC<{}> = () => { | |||
| setFormErrors(updatedFormErrors) | |||
| } | |||
|
|
|||
| const debouncedProjectSearch = useMemo(() => debounce((inputValue: string, callback: (options: any[]) => void) => { | |||
There was a problem hiding this comment.
The debounce function is used here, but it is not imported in this file. Ensure that debounce is imported from the appropriate library, such as lodash, to avoid runtime errors.
| @@ -268,6 +251,11 @@ const CopilotRequestForm: FC<{}> = () => { | |||
| setFormErrors(updatedFormErrors) | |||
| } | |||
|
|
|||
| const debouncedProjectSearch = useMemo(() => debounce((inputValue: string, callback: (options: any[]) => void) => { | |||
There was a problem hiding this comment.
Consider specifying the type for the options parameter in the callback function to improve type safety and readability.
| value={formValues.projectId || ''} | ||
| onChange={handleProjectSelect} | ||
| onInputChange={handleProjectSearch} | ||
| loadOptions={debouncedProjectSearch} |
There was a problem hiding this comment.
The async prop should be placed before loadOptions for consistency with other props.
| @@ -42,3 +42,9 @@ export const useProjects = (search?: string, config?: {isPaused?: () => boolean, | |||
| revalidateOnFocus: false, | |||
| }) | |||
| } | |||
|
|
|||
| export const getProjects = (search?: string, filter?: any): Promise<Project[]> => { | |||
There was a problem hiding this comment.
Consider specifying a more precise type than any for the filter parameter to improve type safety and code readability.
|
|
||
| export const getProjects = (search?: string, filter?: any): Promise<Project[]> => { | ||
| const params = { name: `"${search}"`, ...filter } | ||
| const url = buildUrl(baseUrl, params) |
There was a problem hiding this comment.
Ensure that buildUrl correctly handles cases where search might be undefined or an empty string, as this could lead to malformed URLs.
| @@ -8,6 +8,8 @@ import { | |||
| useRef, | |||
| } from 'react' | |||
| import { find } from 'lodash' | |||
| import AsyncCreatable from 'react-select/async-creatable' | |||
There was a problem hiding this comment.
Consider checking if the newly added imports AsyncCreatable and AsyncSelect from react-select are being used in the component. If they are not used, it would be better to remove them to keep the code clean and avoid unnecessary imports.
| @@ -33,7 +35,7 @@ interface InputSelectReactProps { | |||
| readonly name: string | |||
| readonly onChange: (event: ChangeEvent<HTMLInputElement>) => void | |||
| readonly onInputChange?: (newValue: string) => void | |||
| readonly options: OptionsOrGroups<unknown, GroupBase<unknown>> | |||
| readonly options?: OptionsOrGroups<unknown, GroupBase<unknown>> | |||
There was a problem hiding this comment.
Suggestion
Consider providing a default value or handling the case where options is undefined to prevent potential runtime errors when the component tries to access options without checking its existence.
| @@ -43,6 +45,8 @@ interface InputSelectReactProps { | |||
| readonly onBlur?: (event: FocusEvent<HTMLInputElement>) => void | |||
| readonly openMenuOnClick?: boolean | |||
| readonly openMenuOnFocus?: boolean | |||
| readonly async?: boolean | |||
| readonly loadOptions?: (inputValue: string, callback: (option: any) => void) => void | |||
There was a problem hiding this comment.
Consider specifying the type for option in the loadOptions callback function to improve type safety and clarity. Instead of using any, define a specific type or interface that represents the expected structure of option.
| props.creatable ? CreatableSelect : ReactSelect | ||
| ), [props.creatable]) | ||
| const Input = useMemo(() => { | ||
| if (props.async) { |
There was a problem hiding this comment.
Consider adding a type annotation for the Input variable to ensure type safety and improve code readability.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1273
What's in this PR?