hotfix(PM-3208): add manager comment button showing to submitters and normal reviewer #1380
Conversation
hentrymartin
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ? ownedMemberIds.has(submissionOwnerId) | ||
| : false | ||
| }, | ||
| canRespondToAppeals: isAdmin || hasReviewerRole, |
There was a problem hiding this comment.
[security]
The condition isAdmin || hasReviewerRole is used to determine canRespondToAppeals. Ensure that this logic aligns with the intended access control policy, as it might inadvertently allow more users than intended to respond to appeals.
| isAppealsTab: false, | ||
| }), | ||
| [], | ||
| [canRespondToAppeals], |
There was a problem hiding this comment.
[performance]
The useMemo dependency array now includes canRespondToAppeals, which is correct for ensuring the memoized value updates when canRespondToAppeals changes. However, ensure that canRespondToAppeals is a stable value (e.g., derived from props or state) to prevent unnecessary recalculations.
| <Link | ||
| className={styles.respondButton} | ||
| to={getReviewRoute(submission.id, reviewId)} | ||
| to={getReviewRoute(submission.id, reviewId, canRespondToAppeals)} |
There was a problem hiding this comment.
[❗❗ correctness]
The getReviewRoute function now receives an additional canRespondToAppeals argument. Ensure that this function is designed to handle this new parameter correctly and that it does not introduce any unintended side effects.
| const reviewUrl = getReviewUrl ? getReviewUrl(reviewId) : getReviewRoute(submission.id, reviewId) | ||
| const reviewUrl = getReviewUrl | ||
| ? getReviewUrl(reviewId) | ||
| : getReviewRoute(submission.id, reviewId, canRespondToAppeals) |
There was a problem hiding this comment.
[❗❗ correctness]
The getReviewRoute function now takes an additional canRespondToAppeals parameter. Ensure that this function can handle the new parameter correctly and that it doesn't introduce any unintended side effects or errors.
| const reviewUrl = getReviewUrl ? getReviewUrl(reviewId) : getReviewRoute(submission.id, reviewId) | ||
| const reviewUrl = getReviewUrl | ||
| ? getReviewUrl(reviewId) | ||
| : getReviewRoute(submission.id, reviewId, canRespondToAppeals) |
There was a problem hiding this comment.
[❗❗ correctness]
The getReviewRoute function now takes an additional canRespondToAppeals parameter. Ensure that this function can handle the new parameter correctly and that it doesn't introduce any unintended side effects or errors.
| addRespondToAppeals?: boolean, | ||
| currentPathname?: string, | ||
| ): string { | ||
| console.log('getReviewRoute called with:', addRespondToAppeals) |
There was a problem hiding this comment.
[maintainability]
Remove the console.log statement. Debugging logs should not be present in production code as they can clutter the console and potentially leak sensitive information.
…ewer
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-3208
What's in this PR?