-
Notifications
You must be signed in to change notification settings - Fork 13
PM-1064 Request Copilot button on opportunity feed #1051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -107,9 +112,15 @@ const CopilotOpportunityList: FC<{}> = () => { | |||
|
|||
const opportunitiesLoading = isValidating | |||
|
|||
const addNewRequestButton: ButtonProps = { | |||
label: 'New Copilot Request', | |||
onClick: () => navigate(copilotRoutesMap.CopilotRequestForm), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling for the navigate
function to handle cases where the navigation might fail.
@@ -88,6 +90,9 @@ const tableColumns: TableColumn<CopilotOpportunity>[] = [ | |||
const CopilotOpportunityList: FC<{}> = () => { | |||
const navigate = useNavigate() | |||
|
|||
const { profile }: ProfileContextData = useContext(profileContext) | |||
const isAdmin: boolean = useMemo(() => !!profile?.roles?.includes(UserRole.tcaAdmin), [profile]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@himaniraghav3 can we please extend this to PM role to ensure PMs can use the functionality as well?
@@ -88,6 +90,12 @@ const tableColumns: TableColumn<CopilotOpportunity>[] = [ | |||
const CopilotOpportunityList: FC<{}> = () => { | |||
const navigate = useNavigate() | |||
|
|||
const { profile }: ProfileContextData = useContext(profileContext) | |||
const isAdminOrPM: boolean = useMemo( | |||
() => !!profile?.roles?.some(role => role === UserRole.tcaAdmin || role === UserRole.projectManager), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using includes
instead of some
for checking roles if the list of roles is small and fixed, as it can be more readable and straightforward.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1064
What's in this PR?
Admin can see a "New Copilot Request" button on the opportunity listing page which takes them to the form.