feat(be): implement polygon collaborator api#3522
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 new collaborator management system for polygon problems, including GraphQL resolvers and service methods for inviting, approving, rejecting, updating roles, and removing collaborators. The review identified potential security and logic issues: the Owner role should not be assignable via the API to prevent privilege escalation, and the getCollaboratorsByStatus method requires access control to prevent unauthorized users from viewing collaborator lists.
| } | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
inviteCollaborator에서 같은 문제의 Owner, Editor가 동시에 같은 사람을 협업자로 초대하거나, requestCollaborator 요청을 보내는 사람을 동시에 inviteCollaborator에서 초대하는 등
상황에서 findfirst (existing 찾기) & create (새로운 협업자 생성) 원자성이 보장이 안돼서 (findfirst로 중복이 없는걸 체크했는데, create 하기 전 다른 요청에서 생겨버린다거나) 중복 생성하다가 예외가 터지는 상황을 서비스 코드에서 꼭 잡아주는게 좋을것 같습니다.
찾아본 결과 create 할때 try-catch로 동시 요청시 하나의 요청만 성공하고 나머지 하나는 Prisma unique violation(P2002)로 실패하게 설정하는 방법이 있다고 합니다!
There was a problem hiding this comment.
requestCollaborator도 마찬가지 예외 상황이 발생가능함
| }) | ||
| if (existing || userId === problem.createdById) { | ||
| throw new DuplicateFoundException('Collaborator') | ||
| } |
There was a problem hiding this comment.
초대한 사람이 이미 존재하는 협업자인 경우 & 문제 owner인 경우가 한번에 예외처리 돼서, 간단하게 분기 후 예외를 각각 명시해주면 더 좋을것 같아요.
| }) | ||
| const canInvite = | ||
| inviterInfo?.status === CollaboratorStatus.Active && | ||
| inviterInfo?.role === CollaboratorRole.Editor |
There was a problem hiding this comment.
혹시 role ENUM에서 viewer -> Reviewer로 용어 변경을 하는게 좋을지..? (번거롭지만 않으면 하는것도 낫배드?) 기획안도 그렇고 reviewer로 명칭 바꾸기 전에 구현한거 같아서요!
| return await this.prisma.polygonCollaborator.update({ | ||
| where: { id: collaborator.id }, | ||
| data: { role } | ||
| }) |
There was a problem hiding this comment.
지금 로직대로라면 협업자가 승인전 Pending 중일때도, 권한 종류를 바꿀수 있는데 (ex. owner가 Viewer 하고싶다는 요청을 승인하기 전 Reviewer로 바꾸고 승인) 이건 상관없는지 한번 생각해봐야 할거 같아요. 근데 상관없을거 같긴해요 Owner가 왕 아닌가..?
There was a problem hiding this comment.
해당 부분에 대해서는 그래도 pending 상태에서 권한을 변경하는건 과한 것 같아
owner가 active 사용자에 대해서만 변경 가능하도록 수정했습니다.
commited 01bf317
| where: { id: collaborator.id } | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
rejectCollaborator에서 직접 요청한 Pending과 Editor가 초대한 협업자의 Pending을 다 거절하니까, remove에서는 Active한 status에 대해서만 제거하도록 수정하면 함수 역할 분리가 좀더 확실해질거 같아요. (기능상 에러는 안날거 같기도..?)
2 participants
Description
문제 생성시 협업자를 관리하는 API를 구현하였습니다.
<구현 기능>
inviteCollaborator
-협업자 초대는 해당 문제의 소유자, active editor 인 경우에만 가능
-소유자가 초대시 active, active editor가 초대시 pending
getActive/PendingCollaborator
-협업자 목록 반환, 요청 대기자 목록 반환
approve/rejectInvite
-요청에 대한 수락 여부 -> 해당 문제 소유자만 가능
updateCollaboratorRole
-협업자의 role 변경 -> 해당 문제 소유자만 가능
removeCollaborator
-협업자 제거 -> 해당 문제 소유자만 가능
requestCollaboration
Additional context
Before submitting the PR, please make sure you do the following
fixes #123).