Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a duplicate player check during the registration process, replacing the previous separate duplicate check stage with inline error reporting in the parse result view. Key changes include the addition of the useCheckDuplicateNL mutation, UI updates to use the Typography component, and enhanced validation logic in ParseResultCard to handle 9-digit student numbers and allow partial confirmation of valid players. Feedback focuses on removing unnecessary query invalidation in the duplicate check mutation and refactoring the success handler in the bottom sheet component to reduce code duplication and improve readability.
| onSuccess: async () => { | ||
| await qc.invalidateQueries({ queryKey: queryKeys.teams._def }); | ||
| }, |
There was a problem hiding this comment.
onSuccess 콜백에서 teams 쿼리를 무효화하고 있습니다. check-duplicates API는 데이터를 조회하고 중복 여부만 확인하는 기능으로, 서버의 데이터를 변경하지 않습니다. 따라서 쿼리를 무효화할 필요가 없으며, 불필요한 네트워크 요청을 유발할 수 있으므로 onSuccess 콜백을 제거하는 것이 좋습니다.
| onSuccess: (data) => { | ||
| const existPlayers = data.players.filter((p) => p.status === 'EXISTS'); | ||
|
|
||
| if (existPlayers.length > 0) { | ||
| // 중복 선수가 있으면 parse-result로 돌아가서 에러 표시 | ||
| const playersWithErrors: PlayerData[] = data.players.map((p) => ({ | ||
| name: p.name, | ||
| studentNumber: p.studentNumber, | ||
| jerseyNumber: p.jerseyNumber, | ||
| error: | ||
| p.status === 'EXISTS' | ||
| ? '이미 등록된 학번입니다. 수정 후 다시 확인해주세요.' | ||
| : undefined, | ||
| })); | ||
|
|
||
| setLatestPreview({ | ||
| players: data.players.map((p) => ({ | ||
| name: p.name, | ||
| studentNumber: p.studentNumber, | ||
| jerseyNumber: p.jerseyNumber, | ||
| })), | ||
| }); | ||
|
|
||
| addMessages('확인', { | ||
| stage: 'parse-result', | ||
| // [어시스턴트 멘트 3] 중복 학번 있을 때 수정 요청 | ||
| message: '중복된 학번이 있어요.\n해당 학번을 수정 후 다시 확인해주세요.', | ||
| players: playersWithErrors, | ||
| }); | ||
| setStage('parse-result'); | ||
| } else { | ||
| const newPlayers = data.players.map((p) => ({ | ||
| name: p.name, | ||
| studentNumber: p.studentNumber, | ||
| jerseyNumber: p.jerseyNumber, | ||
| })); | ||
| setFinalPlayers(newPlayers); | ||
| addMessages('확인', { | ||
| stage: 'final-confirm', | ||
| // [어시스턴트 멘트 3] 중복 없을 때 최종 명단 안내 | ||
| message: | ||
| '이제 거의 다 왔어요! \n마지막으로 매니저님의 확인이 필요한 내용이 있어요.', | ||
| players: newPlayers, | ||
| }); | ||
| setStage('final-confirm'); | ||
| } | ||
| }, |
There was a problem hiding this comment.
이 onSuccess 핸들러 내에 코드 중복이 있습니다. data.players를 선수 정보 객체 배열로 변환하는 로직이 if와 else 블록 모두에 나타납니다. 이 변환 로직을 if문 앞으로 추출하여 코드를 더 간결하고 유지보수하기 쉽게 만들 수 있습니다. 또한, filter().length > 0 대신 some()을 사용하면 가독성을 높이고 약간의 성능 향상을 기대할 수 있습니다.
onSuccess: (data) => {
const playersForPreview = data.players.map((p) => ({
name: p.name,
studentNumber: p.studentNumber,
jerseyNumber: p.jerseyNumber,
}));
if (data.players.some((p) => p.status === 'EXISTS')) {
// 중복 선수가 있으면 parse-result로 돌아가서 에러 표시
const playersWithErrors: PlayerData[] = data.players.map((p) => ({
name: p.name,
studentNumber: p.studentNumber,
jerseyNumber: p.jerseyNumber,
error:
p.status === 'EXISTS'
? '이미 등록된 학번입니다. 수정 후 다시 확인해주세요.'
: undefined,
}));
setLatestPreview({ players: playersForPreview });
addMessages('확인', {
stage: 'parse-result',
// [어시스턴트 멘트 3] 중복 학번 있을 때 수정 요청
message: '중복된 학번이 있어요.\n해당 학번을 수정 후 다시 확인해주세요.',
players: playersWithErrors,
});
setStage('parse-result');
} else {
setFinalPlayers(playersForPreview);
addMessages('확인', {
stage: 'final-confirm',
// [어시스턴트 멘트 3] 중복 없을 때 최종 명단 안내
message:
'이제 거의 다 왔어요! \n마지막으로 매니저님의 확인이 필요한 내용이 있어요.',
players: playersForPreview,
});
setStage('final-confirm');
}
}
seongminn
left a comment
seongminn
left a comment
There was a problem hiding this comment.
이건 일단 돌아가도록만 하고 나중에 같이 리뷰하는 게 좋겠네요!! 고생하셨습니다!
2 participants
🌍 이슈 번호
✅ 작업 내용
기존 로직: parse -> register
바뀐 로직: parse -> check-duplicate -> register
Typography로 변경하여 ui일관성 유지했어요📝 참고 자료
♾️ 기타