-
Notifications
You must be signed in to change notification settings - Fork 211
[PROD] - Copilot Portal fixes #7108
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
fix(PM-1346): Removed innovation challenges tab
Disable cache for v6 testing / debugging
fix(PM-1316): dont show more filters button for copilots bucket
fix(PM-1358): Only show provisional or final score in the score list
fixes the title and adds new colour for cancelled status fixes #1517
fix: copilot opportunity title and color for canceled opportunities
fix: sorting on copilots opportunity PM-1314
Previously, the sorting was not right with the grouped data from projects service. Fixes #PM-1314
fix: sorting on copilots opportunity
| branches: | ||
| only: | ||
| - develop | ||
| - pm-1346 |
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 branch name pm-1346 should be checked to ensure it follows the naming conventions used in your project. Consider using a consistent naming pattern if not already in place.
| only: | ||
| - develop | ||
| - pm-1346 | ||
| - pm-1358_1 |
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 branch name pm-1358_1 should be checked to ensure it follows the naming conventions used in your project. Consider using a consistent naming pattern if not already in place.
| }, [location, currentSelected, filterState]); | ||
| const currentTabName = useMemo( | ||
| () => ( | ||
| currentSelected ? TAB_NAME.PAST_CHALLENGES : TAB_NAME.ACTIVE_CHALLENGES |
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 logic for determining currentTabName has been simplified by removing the check for filterState.isInnovationChallenge. Ensure that this change aligns with the intended functionality and that the removal of this condition does not affect the behavior of the application in scenarios where isInnovationChallenge might be true.
| ]); | ||
| const start = moment(opportunity.startDate); | ||
|
|
||
| let statusClass = ''; |
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 a switch statement or a mapping object for statusClass assignment to improve readability and scalability if more statuses are added in the future.
| <span>{PROJECT_TYPE_LABELS[opportunity.type]}</span> | ||
| </div> | ||
| <div styleName={`status ${(['completed', 'canceled'].includes(opportunity.status)) ? 'completed' : ''}`}> | ||
| <div styleName={`status ${statusClass}`}> |
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 variable statusClass is used here, but it is not clear from the diff whether it is defined or how it is computed. Ensure that statusClass is properly defined and initialized before use to avoid potential runtime errors.
| } | ||
|
|
||
| &.canceled { | ||
| color: red; |
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 a variable for the color red to maintain consistency and ease of future updates. For example, $status-canceled-color: red; and then use color: $status-canceled-color;.
| setSort, | ||
| selectBucket, | ||
| }) { | ||
| if (isCopilotOpportunitiesBucket) { |
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.
Returning null here might cause issues if the parent component expects a valid React element. Consider checking if the parent component can handle null gracefully or if a fallback UI should be provided.
| url.searchParams.append('page', page); | ||
| url.searchParams.append('pageSize', pageSize); | ||
| url.searchParams.append('sort', sort); | ||
| if (noGrouping) url.searchParams.append('noGrouping', 'true'); |
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 validation for the noGrouping parameter to ensure it is a boolean before appending it to the URL. This will prevent potential issues if a non-boolean value is passed.
| return fetch(url, { | ||
| return fetch(url.toString(), { | ||
| method: 'GET', | ||
| }).then(res => res.json()); |
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.
It might be beneficial to handle potential errors from the fetch call, such as network issues or non-JSON responses, to improve robustness.
| }, | ||
| [SORTS.COPILOT_OPPORTUNITIES_TITLE_A_TO_Z]: { | ||
| func: (a, b) => a.project.name.localeCompare(b.project.name), | ||
| func(a, b) { |
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 String.prototype.trim() on a.opportunityTitle and b.opportunityTitle to avoid sorting issues caused by leading or trailing whitespace.
| } | ||
|
|
||
| // Fallback: sort by createdAt descending | ||
| return moment(b.createdAt).valueOf() - moment(a.createdAt).valueOf(); |
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 moment(b.createdAt).unix() instead of moment(b.createdAt).valueOf() for clarity, as it explicitly converts the date to a Unix timestamp. This can improve readability and ensure consistency if the codebase primarily uses Unix timestamps.
https://topcoder.atlassian.net/browse/PM-555