-
Notifications
You must be signed in to change notification settings - Fork 211
migrate: review opportunity details page to v6 review API #7110
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
migrates review opportunity details page to v6 review api fixes #1761
| * the specified challenge. | ||
| * @param {Number} challengeId The ID of the challenge (not the opportunity id) | ||
| * @param {String} tokenV3=null Optional. Topcoder auth token v3. | ||
| * @default test |
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 JSDoc comment mentions @default test, but there is no default value provided in the function parameters. Consider removing the @default tag or providing a default value for tokenV3.
| * @default test | ||
| * @return {Action} | ||
| */ | ||
| function getDetailsDone(challengeId, opportunityId, tokenV3) { |
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 opportunityId is used in the function but is not documented in the JSDoc comment. Consider adding documentation for opportunityId.
| .then(details => ({ details })) | ||
| .catch((error) => { | ||
| if (error.status !== 401) { | ||
| console.log('Error Getting Review Opportunity Details', error.content || error); |
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.
Using console.log for error handling is not recommended in production code. Consider using a logging library or handling the error in a way that is more suitable for production environments.
src/shared/components/ReviewOpportunityDetailsPage/FailedToLoad/index.jsx
Show resolved
Hide resolved
src/shared/components/ReviewOpportunityDetailsPage/FailedToLoad/styles.scss
Show resolved
Hide resolved
src/shared/components/ReviewOpportunityDetailsPage/Header/PhaseList/index.jsx
Show resolved
Hide resolved
| 'Permission Required', | ||
| <span> | ||
| You must have a reviewer role to apply for this review opportunity.{' '} | ||
| <a href="https://www.topcoder.com/community/member/reviewer-become" target="_blank" rel="noopener noreferrer"> |
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.
Ensure the URL https://www.topcoder.com/community/member/reviewer-become is correct and up-to-date, as hardcoding URLs can lead to broken links if the URL changes.
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.
Need a correct url for this @jmgasper @kkartunov
Added gibberish for now
| * @param {Number} challengeId The ID of the challenge (not the opportunity id) | ||
| * @return {Promise} Resolves to the api response in JSON. | ||
| */ | ||
| export async function getDetails(challengeId, opportunityId) { |
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 getDetails is documented to return a Promise that resolves to the API response in JSON, but it actually returns a normalized object combining opportunity and challenge data. Consider updating the documentation to reflect the actual return structure.
ai review feedbacks on return types and logging Fixes #1760
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.
removes link for how to be a reviewer, we can add it later when process is in place fixes #1761
| fireErrorMessage( | ||
| 'Permission Required', | ||
| <span> | ||
| You must have a reviewer role to apply for this review opportunity. |
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 link to 'Learn how to become a reviewer' may lead to confusion for users who are not aware of how to obtain a reviewer role. Consider providing guidance or a reference to where users can find more information about becoming a reviewer.
Migrates review opportunity details page to v6 review api
Adds code from topcoder-react-lib to get details of the review opportunity (getDetails)
Also fixes the frontend to work with the new API response.
For an anonymous user, we are showing the review opportunity details page. On clicking the Apply for Review button, user is redirected to login, where if they're a reviewer, the normal flow occurs. And, if they're not a reviewer, we show them an error message saying Permission denied.
This message needs a link to a page where the user can go apply to be a reviewer - LMK of the url