fix(PM-3270): removing reviewer when new review config is added or review type is updated #1715
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| updateAssignedMembers (challengeResources, challenge, prevChallenge = null) { | ||
| const reviewersWithPhaseName = challenge.reviewers.map(item => { | ||
| const phase = challenge.phases && challenge.phases.find(p => p.phaseId === item.phaseId) | ||
| const phase = challenge.phases && challenge.phases.find(p => (p.id === item.phaseId) || (p.phaseId === item.phaseId)) |
There was a problem hiding this comment.
[💡 maintainability]
The condition (p.id === item.phaseId) || (p.phaseId === item.phaseId) might be redundant if p.id and p.phaseId are always the same. Ensure that both properties are necessary and distinct. If not, consider simplifying the condition.
| if (prevChallenge && prevChallenge.reviewers) { | ||
| const prevReviewers = prevChallenge.reviewers || [] | ||
| challenge.reviewers.forEach((reviewer, index) => { | ||
| const prevReviewer = prevReviewers[index] |
There was a problem hiding this comment.
[correctness]
The logic assumes that prevReviewers and challenge.reviewers have the same order and length. If this assumption is not guaranteed, it could lead to incorrect behavior. Consider using a more robust matching strategy, such as matching by a unique identifier.
| Object.keys(assignedMembers).forEach(indexStr => { | ||
| const index = parseInt(indexStr, 10) | ||
| const reviewer = challenge.reviewers[index] | ||
| if (index >= challenge.reviewers.length || |
There was a problem hiding this comment.
[💡 readability]
The condition index >= challenge.reviewers.length is redundant because index is derived from Object.keys(assignedMembers), which should not exceed the length of challenge.reviewers. Consider removing this check for clarity.
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-3270