Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
개요이 PR은 그룹 내 권한 관리 기능을 추가합니다. 새로운 REST 엔드포인트를 통해 사용자가 특정 역할에 권한을 추가할 수 있으며, 역할 계층 구조 검증 및 권한 중복 확인을 포함합니다. 도메인 모델에 우선순위 기반 역할 비교 시스템도 도입됩니다. 변경사항
Sequence DiagramsequenceDiagram
participant Client
participant PermissionController
participant AddPermissionService
participant GroupRoleRepositoryPort
participant Database
Client->>PermissionController: POST /v1/groups/{groupId}/permissions<br/>(userId, groupId, changeRole, permission)
PermissionController->>AddPermissionService: addPermission(AddPermissionCommand)
AddPermissionService->>GroupRoleRepositoryPort: findRole(userId, groupId)
GroupRoleRepositoryPort->>Database: SELECT user role
Database-->>GroupRoleRepositoryPort: GroupMemberRole
GroupRoleRepositoryPort-->>AddPermissionService: role
AddPermissionService->>GroupRoleRepositoryPort: checkPermission(userId, groupId, permission)
GroupRoleRepositoryPort->>Database: SELECT user permission
Database-->>GroupRoleRepositoryPort: permission exists
GroupRoleRepositoryPort-->>AddPermissionService: validated
AddPermissionService->>AddPermissionService: role.isHigherThan(changeRole)
AddPermissionService->>GroupRoleRepositoryPort: existPermission(changeRole, groupId, permission)
GroupRoleRepositoryPort->>Database: SELECT target permission
Database-->>GroupRoleRepositoryPort: permission exists?
GroupRoleRepositoryPort-->>AddPermissionService: false (not exists)
AddPermissionService->>GroupRoleRepositoryPort: addPermission(groupId, changeRole, permission)
GroupRoleRepositoryPort->>Database: INSERT permission
Database-->>GroupRoleRepositoryPort: List<GroupPermission>
GroupRoleRepositoryPort-->>AddPermissionService: permissions
AddPermissionService-->>PermissionController: AddPermissionResult
PermissionController->>PermissionController: AddPermissionResponseDto.from(result)
PermissionController-->>Client: 200 OK + AddPermissionResponseDto
코드 리뷰 예상 난이도🎯 3 (중간) | ⏱️ ~20분 관련 PR
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupMemberRepository.java (1)
26-26: 매개변수 이름id가 메서드명과 일치하지 않습니다.메서드명이
existsByUserIdAndRole_Id인데, 두 번째 매개변수가id로 되어 있어 혼란을 줄 수 있습니다.roleId로 변경하면 메서드의 의도가 더 명확해집니다.♻️ 매개변수명 수정 제안
- boolean existsByUserIdAndRole_Id(Long userId, Long id); + boolean existsByUserIdAndRole_Id(Long userId, Long roleId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupMemberRepository.java` at line 26, 메서드 existsByUserIdAndRole_Id의 두 번째 매개변수명이 현재 id로 되어 있어 메서드명과 불일치하므로, 메서드 서명을 boolean existsByUserIdAndRole_Id(Long userId, Long roleId)로 변경해 의도를 명확히 하고 관련 호출부(이 리포지토리 메서드를 사용하는 서비스/테스트 코드)에서 매개변수 이름을 roleId로 맞춰 수정하세요.src/main/java/flipnote/group/application/port/in/command/AddPermissionCommand.java (1)
6-11: 입력 유효성 검증 추가를 고려해보세요.Command 객체에
null또는 잘못된 값이 전달될 경우를 대비하여, Bean Validation 어노테이션(@NotNull,@Positive등)을 추가하거나 compact constructor에서 유효성 검증을 수행하는 것을 권장합니다.♻️ 선택적 유효성 검증 추가 예시
+import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; + public record AddPermissionCommand( - Long userId, - Long groupId, - GroupMemberRole changeRole, - GroupPermission permission) { + `@NotNull` `@Positive` Long userId, + `@NotNull` `@Positive` Long groupId, + `@NotNull` GroupMemberRole changeRole, + `@NotNull` GroupPermission permission) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/in/command/AddPermissionCommand.java` around lines 6 - 11, AddPermissionCommand 레코드의 입력에 대한 유효성 검증이 빠져 있어 null 또는 부적절한 값이 들어올 수 있으니, AddPermissionCommand의 필드(userId, groupId, changeRole, permission)에 대해 `@NotNull` 및 아이디에 대해 `@Positive` 같은 Bean Validation 어노테이션을 추가하거나 AddPermissionCommand의 compact constructor를 구현해 userId와 groupId가 null 또는 <=0 이 아닌지, changeRole와 permission이 null이 아닌지 검사하고 유효하지 않으면 IllegalArgumentException을 던지도록 변경하세요.src/main/java/flipnote/group/api/dto/request/AddPermissionRequestDto.java (1)
6-10: API 요청 DTO에 유효성 검증 어노테이션 추가를 권장합니다.REST API 요청 바인딩에 사용되는 DTO이므로,
@NotNull등의 Bean Validation 어노테이션을 추가하여 잘못된 요청을 조기에 차단하는 것이 좋습니다.♻️ 유효성 검증 추가 제안
+import jakarta.validation.constraints.NotNull; + public record AddPermissionRequestDto( - GroupMemberRole changeRole, - GroupPermission permission + `@NotNull` GroupMemberRole changeRole, + `@NotNull` GroupPermission permission ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/request/AddPermissionRequestDto.java` around lines 6 - 10, Add Bean Validation annotations to the AddPermissionRequestDto record by marking its components (changeRole and permission) with `@NotNull` so invalid requests are rejected early; update the record declaration in AddPermissionRequestDto to annotate the components, add the corresponding import (javax.validation.constraints.NotNull) and ensure controllers that accept this DTO have validation enabled (e.g., `@Valid` on the parameter) so the annotations are enforced.src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRolePermissionRepository.java (1)
13-15: 매개변수 이름을 명확하게 변경하는 것을 권장합니다.
id매개변수는 다소 모호합니다. Spring Data JPA 메서드명에서GroupRoleId를 참조하므로, 일관성을 위해 매개변수 이름을groupRoleId로 변경하면 가독성이 향상됩니다.♻️ 매개변수명 변경 제안
- boolean existsByGroupRoleIdAndPermission(Long id, GroupPermission permission); + boolean existsByGroupRoleIdAndPermission(Long groupRoleId, GroupPermission permission); - List<PermissionEntity> findAllByGroupRoleId(Long id); + List<PermissionEntity> findAllByGroupRoleId(Long groupRoleId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRolePermissionRepository.java` around lines 13 - 15, Rename the ambiguous parameter name `id` to `groupRoleId` in the GroupRolePermissionRepository method signatures — specifically change the parameter in existsByGroupRoleIdAndPermission(Long id, GroupPermission permission) and findAllByGroupRoleId(Long id) to `groupRoleId`, and update any call sites or references accordingly so method signatures and usage are consistent with the Spring Data JPA property name.src/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.java (1)
18-18: 파라미터 이름aLong이 명확하지 않음
aLong은 의미가 불분명합니다. 구현체(GroupRoleRepositoryAdapter)에서는groupId로 사용하고 있으므로 인터페이스에서도 일관성 있게groupId로 명명해주세요.♻️ 파라미터 이름 수정 제안
-boolean existPermission(GroupMemberRole groupMemberRole, Long aLong, GroupPermission permission); +boolean existPermission(GroupMemberRole groupMemberRole, Long groupId, GroupPermission permission);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.java` at line 18, Rename the ambiguous parameter aLong to groupId in the interface method existPermission within GroupRoleRepositoryPort so it matches the meaning used in the implementation; update the method signature boolean existPermission(GroupMemberRole groupMemberRole, Long groupId, GroupPermission permission) and ensure implementing class GroupRoleRepositoryAdapter (and any other implementations) use the same parameter name to keep signatures consistent.src/main/java/flipnote/group/adapter/in/web/PermissionController.java (1)
4-4: 사용되지 않는 import 제거 필요
DeleteMapping이 import되어 있지만 현재 코드에서 사용되지 않습니다. 불필요한 import는 제거해주세요.🧹 사용되지 않는 import 제거
import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.PathVariable;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/in/web/PermissionController.java` at line 4, Remove the unused import of DeleteMapping in PermissionController.java: locate the import line "import org.springframework.web.bind.annotation.DeleteMapping;" and delete it so only used imports remain (ensure compilation still passes and run the IDE/static analyzer to confirm no other unused imports remain).src/main/java/flipnote/group/application/service/AddPermissionService.java (1)
36-49: 예외 타입 세분화 고려현재 모든 비즈니스 검증 실패 시
IllegalArgumentException을 사용하고 있습니다. 클라이언트가 오류 유형을 구분하기 어려울 수 있으므로, 향후 커스텀 예외(예:PermissionDeniedException,DuplicatePermissionException)로 세분화하는 것을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/service/AddPermissionService.java` around lines 36 - 49, Replace the generic IllegalArgumentException usages in AddPermissionService with specific custom exceptions so clients can distinguish error types: create (unchecked) exceptions like HostPermissionNotFoundException and throw it when !existHostPermission, PermissionDeniedException and throw it when !role.isHigherThan(cmd.changeRole()), and DuplicatePermissionException and throw it when groupRoleRepository.existPermission(...) returns true; update any method signatures or exception handling paths that expect IllegalArgumentException and adjust unit/integration tests to assert the new exception types (refer to symbols existHostPermission, role.isHigherThan, groupRoleRepository.existPermission, and AddPermissionService).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java`:
- Around line 140-146: In GroupRoleRepositoryAdapter.existPermission, the call
to groupRoleRepository.findByGroupIdAndRole can return null and is not
null-checked; update existPermission to mirror addPermission by checking the
returned RoleEntity (from findByGroupIdAndRole) and throw the same exception (or
handle identically) when null before calling
groupRolePermissionRepository.existsByGroupRoleIdAndPermission with
roleEntity.getId(); reference existPermission, findByGroupIdAndRole, RoleEntity,
addPermission, and
groupRolePermissionRepository.existsByGroupRoleIdAndPermission to locate and
apply the fix.
- Around line 115-131: In addPermission, guard against a null return from
groupRoleRepository.findByGroupIdAndRole by checking roleEntity for null before
using roleEntity.getId(); if null, throw a clear domain exception (or
IllegalArgumentException/NoSuchElementException) indicating the role for the
given groupId and GroupMemberRole was not found so you avoid the NPE in
PermissionEntity.builder and subsequent groupRolePermissionRepository calls;
update the method to validate roleEntity, throw the chosen exception with
context, and only proceed to build/save PermissionEntity and fetch permissions
when roleEntity is non-null.
---
Nitpick comments:
In `@src/main/java/flipnote/group/adapter/in/web/PermissionController.java`:
- Line 4: Remove the unused import of DeleteMapping in
PermissionController.java: locate the import line "import
org.springframework.web.bind.annotation.DeleteMapping;" and delete it so only
used imports remain (ensure compilation still passes and run the IDE/static
analyzer to confirm no other unused imports remain).
In `@src/main/java/flipnote/group/api/dto/request/AddPermissionRequestDto.java`:
- Around line 6-10: Add Bean Validation annotations to the
AddPermissionRequestDto record by marking its components (changeRole and
permission) with `@NotNull` so invalid requests are rejected early; update the
record declaration in AddPermissionRequestDto to annotate the components, add
the corresponding import (javax.validation.constraints.NotNull) and ensure
controllers that accept this DTO have validation enabled (e.g., `@Valid` on the
parameter) so the annotations are enforced.
In
`@src/main/java/flipnote/group/application/port/in/command/AddPermissionCommand.java`:
- Around line 6-11: AddPermissionCommand 레코드의 입력에 대한 유효성 검증이 빠져 있어 null 또는 부적절한
값이 들어올 수 있으니, AddPermissionCommand의 필드(userId, groupId, changeRole, permission)에
대해 `@NotNull` 및 아이디에 대해 `@Positive` 같은 Bean Validation 어노테이션을 추가하거나
AddPermissionCommand의 compact constructor를 구현해 userId와 groupId가 null 또는 <=0 이
아닌지, changeRole와 permission이 null이 아닌지 검사하고 유효하지 않으면 IllegalArgumentException을
던지도록 변경하세요.
In
`@src/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.java`:
- Line 18: Rename the ambiguous parameter aLong to groupId in the interface
method existPermission within GroupRoleRepositoryPort so it matches the meaning
used in the implementation; update the method signature boolean
existPermission(GroupMemberRole groupMemberRole, Long groupId, GroupPermission
permission) and ensure implementing class GroupRoleRepositoryAdapter (and any
other implementations) use the same parameter name to keep signatures
consistent.
In `@src/main/java/flipnote/group/application/service/AddPermissionService.java`:
- Around line 36-49: Replace the generic IllegalArgumentException usages in
AddPermissionService with specific custom exceptions so clients can distinguish
error types: create (unchecked) exceptions like HostPermissionNotFoundException
and throw it when !existHostPermission, PermissionDeniedException and throw it
when !role.isHigherThan(cmd.changeRole()), and DuplicatePermissionException and
throw it when groupRoleRepository.existPermission(...) returns true; update any
method signatures or exception handling paths that expect
IllegalArgumentException and adjust unit/integration tests to assert the new
exception types (refer to symbols existHostPermission, role.isHigherThan,
groupRoleRepository.existPermission, and AddPermissionService).
In
`@src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupMemberRepository.java`:
- Line 26: 메서드 existsByUserIdAndRole_Id의 두 번째 매개변수명이 현재 id로 되어 있어 메서드명과 불일치하므로,
메서드 서명을 boolean existsByUserIdAndRole_Id(Long userId, Long roleId)로 변경해 의도를 명확히
하고 관련 호출부(이 리포지토리 메서드를 사용하는 서비스/테스트 코드)에서 매개변수 이름을 roleId로 맞춰 수정하세요.
In
`@src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRolePermissionRepository.java`:
- Around line 13-15: Rename the ambiguous parameter name `id` to `groupRoleId`
in the GroupRolePermissionRepository method signatures — specifically change the
parameter in existsByGroupRoleIdAndPermission(Long id, GroupPermission
permission) and findAllByGroupRoleId(Long id) to `groupRoleId`, and update any
call sites or references accordingly so method signatures and usage are
consistent with the Spring Data JPA property name.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/flipnote/group/adapter/in/web/MemberController.javasrc/main/java/flipnote/group/adapter/in/web/PermissionController.javasrc/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.javasrc/main/java/flipnote/group/api/dto/request/AddPermissionRequestDto.javasrc/main/java/flipnote/group/api/dto/response/AddPermissionResponseDto.javasrc/main/java/flipnote/group/application/port/in/AddPermissionUseCase.javasrc/main/java/flipnote/group/application/port/in/command/AddPermissionCommand.javasrc/main/java/flipnote/group/application/port/in/result/AddPermissionResult.javasrc/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.javasrc/main/java/flipnote/group/application/service/AddPermissionService.javasrc/main/java/flipnote/group/domain/model/member/GroupMemberRole.javasrc/main/java/flipnote/group/infrastructure/persistence/jpa/GroupMemberRepository.javasrc/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRolePermissionRepository.java
src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java
Show resolved
Hide resolved
src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java
Show resolved
Hide resolved
This was referenced Mar 1, 2026
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
릴리스 노트