-
Notifications
You must be signed in to change notification settings - Fork 13
feat(PM-580): added assign action to applications list #1086
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
...src/pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplicationAction.tsx
Show resolved
Hide resolved
...src/pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplicationAction.tsx
Show resolved
Hide resolved
...src/pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplicationAction.tsx
Show resolved
Hide resolved
...lots/src/pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplications.tsx
Show resolved
Hide resolved
.../copilots/src/pages/copilot-opportunity-details/tabs/copilot-applications/styles.module.scss
Show resolved
Hide resolved
@@ -105,6 +105,21 @@ export const applyCopilotOpportunity = async (opportunityId: number, request?: { | |||
return xhrPostAsync(url, request, {}) | |||
} | |||
|
|||
/** |
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 function name assignCopilotOpportunity
is similar to applyCopilotOpportunity
. Consider renaming for clarity, especially if both functions are used in similar contexts.
...src/pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplicationAction.tsx
Show resolved
Hide resolved
export interface CopilotApplication { | ||
id: number, | ||
notes?: string, | ||
createdAt: Date, | ||
opportunityId: string, | ||
handle?: string, | ||
userId: number, | ||
status: CopilotApplicationStatus, |
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 ensuring that CopilotApplicationStatus
is properly defined and imported in this file to avoid any potential issues with type resolution.
/** | ||
* apply copilot opportunity | ||
* @param opportunityId | ||
* @param applicationId |
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 parameter name in the function signature should be updated to match the JSDoc comment. If the parameter is indeed applicationId
, ensure the function signature reflects this change.
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.
Looks good.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-580
What's in this PR?
Screenshots