-
Notifications
You must be signed in to change notification settings - Fork 21
fix(PM-1610): Updated toast message and tooltip copy for directly accepting copilot #1178
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
| opportunityStatus: string, | ||
| existingMembership?: ExistingMembership, | ||
| projectName: string, | ||
| onApplied: () => void, |
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.
The addition of the onApplied function property to the CopilotApplication interface should include a description of its purpose and usage in the context of the application. Ensure that this function is implemented and utilized correctly wherever the CopilotApplication interface is used.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the mutate import suggests that it is no longer used in this file. Ensure that any related functionality relying on mutate is either updated or removed to prevent potential issues.
| toast.success('Invited a copilot') | ||
| mutate(`${copilotBaseUrl}/copilots/opportunity/${opportunityId}/applications`) | ||
| toast.success('Accepted as copilot') | ||
| copilotApplication.onApplied() |
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.
The mutate function call has been removed. Ensure that the removal of this line does not affect the application's state management or data fetching logic. If this line was responsible for revalidating or updating the cache, consider implementing an alternative approach to maintain the application's data consistency.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line copilotApplication.onApplied() replaces the previous mutate function call. Ensure that onApplied() correctly handles the cache update or any necessary side effects that mutate was performing. If onApplied() does not handle these, consider maintaining the mutate call or implementing the necessary logic within onApplied().
| activeProjects: member?.activeProjects || 0, | ||
| fulfilment: member?.copilotFulfillment || 0, | ||
| handle: member?.handle, | ||
| onApplied: props.onApplied, |
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.
The addition of onApplied: props.onApplied seems to be a new property being passed to the component. Ensure that onApplied is defined and used appropriately within the component to avoid potential runtime errors.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1610
What's in this PR?