-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat: #251 프로젝트 수정 팀원 추가 및 권한 변경 추가 #252
Conversation
Walkthrough이 변경 사항은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (4)
src/services/userService.ts (1)
81-81
: 문서 매개변수 이름 수정이 적절합니다.
매개변수 이름이 실제 구현과 일치하도록 수정되었습니다.
JSDoc 문서의 일관성을 위해 @returns
설명에 반환 타입도 추가하는 것이 좋을 것 같습니다:
- * @returns {Promise<AxiosResponse<TeamListWithApproval[]>>}
+ * @returns {Promise<AxiosResponse<TeamListWithApproval[]>>} 팀 목록과 승인 상태를 포함하는 응답
src/components/modal/project/UpdateModalProject.tsx (1)
100-102
: 역할 변경 시 사용자 피드백 추가 필요
역할 변경 시 사용자에게 피드백을 제공하지 않고 있습니다. 작업의 성공/실패 여부를 알려주는 것이 좋습니다.
다음과 같이 토스트 메시지를 추가하는 것을 제안합니다:
const handleRoleChange = (userId: User['userId'], roleName: ProjectRoleName) => {
- updateProjectCoworkerRoleMutate({ userId, roleName });
+ updateProjectCoworkerRoleMutate({ userId, roleName }, {
+ onSuccess: () => toastInfo('역할이 성공적으로 변경되었습니다'),
+ onError: () => toastInfo('역할 변경에 실패했습니다')
+ });
};
src/mocks/services/projectServiceHandler.ts (2)
277-277
: 에러 메시지 표현 개선 제안
에러 메시지 '팀원 추가 권한이 없습니다.'를 '프로젝트 팀원을 추가할 수 있는 권한이 없습니다.'로 수정하면 의미가 더욱 명확해집니다.
다음과 같이 수정해보세요:
- return new HttpResponse('팀원 추가 권한이 없습니다.', { status: 403 });
+ return new HttpResponse('프로젝트 팀원을 추가할 수 있는 권한이 없습니다.', { status: 403 });
318-318
: 에러 메시지 표현 개선 제안
에러 메시지 '권한 변경 권한이 없습니다.'를 '프로젝트 권한을 변경할 수 있는 권한이 없습니다.'로 수정하면 표현이 더 자연스럽습니다.
다음과 같이 수정해보세요:
- return new HttpResponse('권한 변경 권한이 없습니다.', { status: 403 });
+ return new HttpResponse('프로젝트 권한을 변경할 수 있는 권한이 없습니다.', { status: 403 });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/components/modal/project/UpdateModalProject.tsx (5 hunks)
- src/components/modal/team/ModalTeamForm.tsx (1 hunks)
- src/hooks/query/useProjectQuery.ts (2 hunks)
- src/mocks/services/projectServiceHandler.ts (2 hunks)
- src/services/projectService.ts (2 hunks)
- src/services/userService.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/modal/team/ModalTeamForm.tsx
🔇 Additional comments (8)
src/services/userService.ts (1)
Line range hint 1-85
: 전반적인 구현이 잘 되어있습니다.
- TypeScript 타입이 적절하게 사용되었습니다
- 인증된 요청을 위한
authAxios
사용이 일관적입니다 - 각 함수의 에러 처리가 잘 구현되어 있습니다
- JSDoc 문서화가 상세하게 되어있습니다
src/services/projectService.ts (3)
7-7
: LGTM: 타입 임포트가 적절히 추가되었습니다.
새로운 기능에 필요한 ProjectRoleName
타입이 올바르게 임포트되었습니다.
134-152
: 권한 변경 제한 사항 검증이 필요합니다.
함수 구현은 적절하나, 다음 사항들의 검증이 필요합니다:
- 프로젝트 소유자(OWNER) 권한 변경 제한
- 자기 자신의 권한 변경 제한
- 권한 변경 시 최소 한 명의 OWNER 유지
다음 스크립트로 권한 변경 제한 로직을 확인할 수 있습니다:
#!/bin/bash
# Description: 권한 변경 제한 로직 확인
# 권한 변경 관련 상수나 유틸리티 함수 확인
rg -A 5 "ProjectRoleName.OWNER"
# 권한 변경 시 검증 로직 확인
ast-grep --pattern 'useUpdateProjectRole.*if'
114-132
: 사용자 초대 중복 처리 확인이 필요합니다.
함수 구현은 적절해 보이나, 다음 사항들을 확인해 주시기 바랍니다:
- 이미 초대된 사용자를 다시 초대하는 경우의 처리
- 프로젝트에 이미 존재하는 사용자를 초대하는 경우의 처리
다음 스크립트로 관련 에러 처리 로직을 확인할 수 있습니다:
src/hooks/query/useProjectQuery.ts (2)
4-10
: 새로운 기능을 위한 import 구문이 적절히 추가되었습니다.
프로젝트 팀원 관리 기능 구현에 필요한 서비스 함수와 타입들이 올바르게 import 되었습니다.
Also applies to: 17-18
155-174
:
권한 검증 로직 추가가 필요합니다.
현재 구현에 다음 보안 관련 개선사항을 제안드립니다:
- 역할 변경 전 현재 사용자의 권한 검증
- 유효한 역할 값 검증
다음과 같이 개선해 보시는 것은 어떨까요?
mutationFn: ({ userId, roleName }: { userId: User['userId']; roleName: ProjectRoleName }) => {
+ // 유효한 역할인지 검증
+ const validRoles = ['OWNER', 'MEMBER'] as const;
+ if (!validRoles.includes(roleName)) {
+ throw new Error('Invalid role');
+ }
return updateProjectRole(projectId, userId, roleName);
},
-onError: () => {
- toastError('유저 권한 업데이트에 실패했습니다. 다시 시도해 주세요.');
+onError: (error: any) => {
+ if (error.message === 'Invalid role') {
+ toastError('유효하지 않은 권한입니다.');
+ } else if (error.response?.status === 403) {
+ toastError('권한 변경 권한이 없습니다.');
+ } else {
+ toastError('유저 권한 업데이트에 실패했습니다. 다시 시도해 주세요.');
+ }
},
또한, 권한 변경 전에 현재 사용자의 권한을 확인하는 로직을 추가하는 것이 좋습니다.
src/components/modal/project/UpdateModalProject.tsx (2)
109-109
: 로딩 상태 처리 개선됨
여러 로딩 상태를 통합하여 처리하는 방식이 적절합니다.
16-16
: 새로운 훅 추가에 대한 의존성 검토 필요
새로운 훅들이 추가되었습니다. 이러한 변경은 컴포넌트의 책임을 증가시킬 수 있습니다.
다음 스크립트를 실행하여 새로운 훅들의 사용 범위를 확인하세요:
Also applies to: 23-24
✅ Verification successful
새로운 훅 사용이 적절하게 구현되어 있음을 확인했습니다
프로젝트 협업자 관련 훅들(useAddProjectCoworker
, useUpdateProjectRole
)은 src/hooks/query/useProjectQuery.ts
에 정의되어 있으며, UpdateModalProject.tsx
에서만 사용되고 있습니다. useToast
훅 또한 애플리케이션 전반에 걸쳐 일관되게 사용되고 있어 문제가 없습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 새로운 훅들의 사용 범위 확인
rg "useAddProjectCoworker|useUpdateProjectRole" -t tsx -t ts
Length of output: 90
Script:
#!/bin/bash
# Check for hook usage with proper file extension
rg "useAddProjectCoworker|useUpdateProjectRole"
# Check for the toast hook usage
rg "useToast"
# Look for any project coworker related mutations
ast-grep --pattern 'const $_ = use$_ProjectCoworker($_)'
Length of output: 8402
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.
작업하시느라 고생하셨습니다. 리뷰 확인하시고, 궁금한 점 있으면 말씀해주세요.
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.
유저 역할이나 권한 쪽 작업이 정말 까다로운 작업이었는데 완성이 되었네요🤩 이번 작업도 고생 많으셨습니다!
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.
2차 리뷰 완료했습니다. 승찬님! 담당하신 작업에 집중하시는건 정말 멋지고 좋습니다. 하지만 협업을 하는만큼 다른 사람들이 어떻게 작성하는지 둘러보고 일관되게 작성해주시는 것도 중요할 것 같습니다. 다른 사람 코드를 읽는 연습과 함께 조금씩 개선해 나가면 더 멋진 개발자가 되실거라 생각합니다. 수정 고생하셨어요~
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.
LGTM😄 작업하느라 고생하셨습니다!
…ature/#251-project-user-role-modify
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
Other information
프로젝트 유저 추가
프로젝트 유저 권한 변경