Merged
Conversation
Walkthrough사용자의 그룹 내 역할과 권한을 조회하는 새로운 MyPermission 엔드포인트를 추가합니다. PermissionController의 기본 경로를 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as PermissionController
participant Service as MyPermissionService
participant MemberRepo as GroupMemberRepository
participant RoleRepo as GroupRoleRepository
Client->>Controller: GET /v1/groups/{groupId}/permissions
Note over Controller: X-USER-ID, groupId
Controller->>Service: findMyPermission(command)
Note over Service: groupId, userId
Service->>MemberRepo: findMyRole(groupId, userId)
activate MemberRepo
MemberRepo-->>Service: GroupMemberEntity
deactivate MemberRepo
Service->>RoleRepo: findMyRolePermission(groupId, role)
activate RoleRepo
RoleRepo-->>Service: List<GroupPermission>
deactivate RoleRepo
Service-->>Controller: MyPermissionResult
Note over Controller: role, permissions
Controller-->>Client: ResponseEntity<MyPermissionResponseDto>
Note over Client: HTTP 200 with role & permissions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 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: 3
🧹 Nitpick comments (6)
src/main/java/flipnote/group/application/port/in/result/MyPermissionResult.java (1)
8-14: 리스트 불변성 보강을 권장합니다.Line 10/Line 13에서
permissions를 그대로 보관하면 가변 리스트 유입 시 결과 객체가 외부 변경에 노출될 수 있습니다. record 생성 시List.copyOf(...)로 고정해 두는 편이 안전합니다.보강 예시
public record MyPermissionResult( GroupMemberRole role, List<GroupPermission> permissions ) { + public MyPermissionResult { + permissions = List.copyOf(permissions); + } + public static MyPermissionResult of(GroupMemberRole role, List<GroupPermission> permissions) { return new MyPermissionResult(role, permissions); } }🤖 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/result/MyPermissionResult.java` around lines 8 - 14, The MyPermissionResult record currently stores the incoming mutable permissions list as-is which can lead to external mutation; update the record to defensively copy the list (e.g., use List.copyOf) when constructing instances—either add a canonical/compact constructor in MyPermissionResult that assigns permissions = List.copyOf(permissions) or modify the static factory MyPermissionResult.of(GroupMemberRole role, List<GroupPermission> permissions) to pass List.copyOf(permissions) into the record so the internal permissions are immutable.src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java (1)
15-15: 메서드명과 반환 타입의 의미를 맞춰 주세요.Line 15의
findMyRole은 “역할 조회”로 읽히는데 반환은GroupMemberEntity라 계약 의도가 모호합니다.findMyMembership처럼 이름을 맞추거나, 실제 필요한 값이 역할이면 반환 타입을 더 좁히는 편이 유지보수에 유리합니다.🤖 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/GroupMemberRepositoryPort.java` at line 15, The interface method GroupMemberRepositoryPort.findMyRole has a misleading name/return type mismatch: rename the method to findMyMembership (or similar) if it should return the full GroupMemberEntity, or change its return type to the narrower role type (e.g., GroupRole or Role) if only the role is required; update the interface declaration, all implementations of GroupMemberRepositoryPort, and any callers/tests to use the new method name or return type (ensure method signatures like findMyMembership(Long groupId, Long userId) or findMyRole(Long groupId, Long userId): GroupRole are consistently applied).src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java (1)
83-85: 조회 실패 예외를 도메인 의미에 맞게 분리해 주세요.Line 84의
IllegalArgumentException("entity not exist")는 “요청 인자 오류”로 오해되기 쉽습니다. 그룹 멤버 미존재는 not-found/권한 예외로 분리하면 API 계층에서 404/403 매핑을 더 명확히 가져갈 수 있습니다.예시 수정안
- () -> new IllegalArgumentException("entity not exist") + () -> new GroupMemberNotFoundException(groupId, userId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java` around lines 83 - 85, 현재 GroupMemberRepositoryAdapter에서 groupMemberRepository.findByGroupIdAndUserId(...) 실패 시 IllegalArgumentException("entity not exist")를 던지는데, 이건 요청 인자 오류로 오해되므로 도메인 의미에 맞는 예외로 분리하세요: 실패 시 GroupMemberNotFoundException(또는 GroupMemberNotFoundInGroup) 같은 커스텀 예외를 생성하여 GroupMemberRepositoryAdapter에서 던지도록 바꾸고, 호출부(API/서비스 레이어)가 이 예외를 404/권한 예외로 매핑하도록 처리하세요; 관련 식별자는 GroupMemberRepositoryAdapter, groupMemberRepository.findByGroupIdAndUserId, GroupMemberEntity입니다.src/main/java/flipnote/group/adapter/in/web/PermissionController.java (2)
64-64: 메서드명과 실제 동작 의미를 맞추는 것이 좋겠습니다.Line 64의
changeDownPermission은 삭제 동작(removePermission)을 수행하므로 이름을 맞추면 읽기성이 좋아집니다.✏️ 제안 수정안
- public ResponseEntity<RemovePermissionResponseDto> changeDownPermission( + public ResponseEntity<RemovePermissionResponseDto> removeDownPermission(🤖 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 64, The method name changeDownPermission in PermissionController does not reflect its delete behavior; rename the method to removePermission to match the action (and keep the return type RemovePermissionResponseDto), update the controller method signature and any `@RequestMapping/`@DeleteMapping annotations if present, and refactor all internal references (calls from services, unit/integration tests, and any places using PermissionController.changeDownPermission) to the new name to preserve compile-time correctness and readability.
100-100: TODO 항목은 이슈로 추적 전환을 권장합니다.Line 100 TODO가 남아 있어 누락될 수 있습니다. 원하시면 제가 작업 단위를 이슈 형태로 정리해드릴게요.
🤖 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 100, The inline TODO comment "//todo 가입 신청 허가" in PermissionController should be converted into tracked work: create an issue/ticket describing "가입 신청 허가" (permission approval flow) and any acceptance criteria, then update the code in PermissionController to either remove the bare TODO or replace it with a brief comment that references the new issue ID (e.g., "TODO: see ISSUE-123 for 구현 details") so the task isn't lost; ensure the comment references PermissionController and the relevant method name where the TODO was (or add a // TODO with the issue ID and a one-line summary) before committing.src/main/java/flipnote/group/application/service/MyPermissionService.java (1)
8-13: 애플리케이션 계층이 Adapter 엔티티에 직접 의존하고 있습니다.Line 8에서
MyPermissionService가flipnote.group.adapter.out.entity.GroupMemberEntity를 직접 참조해 포트 경계가 약해집니다. 포트가 도메인 모델(또는 전용 조회 DTO)을 반환하도록 맞추는 쪽이 유지보수에 유리합니다.🤖 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/MyPermissionService.java` around lines 8 - 13, MyPermissionService currently imports and uses the adapter entity GroupMemberEntity, breaking the port boundary; change GroupMemberRepositoryPort (and any calls in GroupRoleRepositoryPort if relevant) to return a domain model or a dedicated DTO instead of the adapter entity, remove the direct import of flipnote.group.adapter.out.entity.GroupMemberEntity from MyPermissionService, and update MyPermissionService to consume the domain model/DTO (e.g., MyPermissionCommand -> MyPermissionResult logic) so mapping from GroupMemberEntity happens inside the adapter implementation (adapter maps entity -> domain/DTO before returning). Ensure method signatures on GroupMemberRepositoryPort and any usages in MyPermissionService are updated accordingly and that the adapter layer performs the entity-to-domain/DTO conversion.
🤖 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 196-198: The exception message thrown by
groupRoleRepository.findByGroupIdAndRole in GroupRoleRepositoryAdapter
(RoleEntity roleEntity = ... .orElseThrow(...)) incorrectly says "not exists
member"; update the IllegalArgumentException message to reflect the actual
failure (role not found) — e.g., use a message like "role does not exist for
group" or "role not found" when throwing from the orElseThrow in the RoleEntity
lookup so logs correctly indicate a missing role rather than a missing member.
In
`@src/main/java/flipnote/group/application/port/in/command/MyPermissionCommand.java`:
- Around line 3-6: MyPermissionCommand currently accepts a client-supplied
userId which allows header spoofing; remove userId from the public API and
populate the authenticated user ID server-side from the security context.
Specifically: stop reading X-USER-ID into MyPermissionCommand (remove the userId
parameter or make it internal), and instead obtain the user id via
`@AuthenticationPrincipal` (or
SecurityContextHolder.getContext().getAuthentication()) in the controller or a
factory method that constructs MyPermissionCommand using the authenticated
principal’s id; update any usages that construct MyPermissionCommand to use that
server-derived id rather than a request header.
In `@src/main/java/flipnote/group/application/service/MyPermissionService.java`:
- Around line 28-32: The code calls
groupMemberRepository.findMyRole(cmd.groupId(), cmd.userId()) and immediately
dereferences groupMember which can cause NPE for non-members or missing roles;
update the service (around GroupMemberEntity groupMember, findMyRolePermission
and MyPermissionResult.of usage) to null-check the returned GroupMemberEntity
(and its role) and throw a clear domain exception (e.g., NotMemberException or
IllegalArgumentException) when absent; ensure you guard calls to
groupRoleRepository.findMyRolePermission(groupMember.getGroupId(),
groupMember.getRole().getRole()) and MyPermissionResult.of(...) behind that
validation so the method returns a controlled error instead of allowing a
NullPointerException.
---
Nitpick comments:
In `@src/main/java/flipnote/group/adapter/in/web/PermissionController.java`:
- Line 64: The method name changeDownPermission in PermissionController does not
reflect its delete behavior; rename the method to removePermission to match the
action (and keep the return type RemovePermissionResponseDto), update the
controller method signature and any `@RequestMapping/`@DeleteMapping annotations
if present, and refactor all internal references (calls from services,
unit/integration tests, and any places using
PermissionController.changeDownPermission) to the new name to preserve
compile-time correctness and readability.
- Line 100: The inline TODO comment "//todo 가입 신청 허가" in PermissionController
should be converted into tracked work: create an issue/ticket describing "가입 신청
허가" (permission approval flow) and any acceptance criteria, then update the code
in PermissionController to either remove the bare TODO or replace it with a
brief comment that references the new issue ID (e.g., "TODO: see ISSUE-123 for
구현 details") so the task isn't lost; ensure the comment references
PermissionController and the relevant method name where the TODO was (or add a
// TODO with the issue ID and a one-line summary) before committing.
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java`:
- Around line 83-85: 현재 GroupMemberRepositoryAdapter에서
groupMemberRepository.findByGroupIdAndUserId(...) 실패 시
IllegalArgumentException("entity not exist")를 던지는데, 이건 요청 인자 오류로 오해되므로 도메인 의미에
맞는 예외로 분리하세요: 실패 시 GroupMemberNotFoundException(또는 GroupMemberNotFoundInGroup)
같은 커스텀 예외를 생성하여 GroupMemberRepositoryAdapter에서 던지도록 바꾸고, 호출부(API/서비스 레이어)가 이 예외를
404/권한 예외로 매핑하도록 처리하세요; 관련 식별자는 GroupMemberRepositoryAdapter,
groupMemberRepository.findByGroupIdAndUserId, GroupMemberEntity입니다.
In
`@src/main/java/flipnote/group/application/port/in/result/MyPermissionResult.java`:
- Around line 8-14: The MyPermissionResult record currently stores the incoming
mutable permissions list as-is which can lead to external mutation; update the
record to defensively copy the list (e.g., use List.copyOf) when constructing
instances—either add a canonical/compact constructor in MyPermissionResult that
assigns permissions = List.copyOf(permissions) or modify the static factory
MyPermissionResult.of(GroupMemberRole role, List<GroupPermission> permissions)
to pass List.copyOf(permissions) into the record so the internal permissions are
immutable.
In
`@src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java`:
- Line 15: The interface method GroupMemberRepositoryPort.findMyRole has a
misleading name/return type mismatch: rename the method to findMyMembership (or
similar) if it should return the full GroupMemberEntity, or change its return
type to the narrower role type (e.g., GroupRole or Role) if only the role is
required; update the interface declaration, all implementations of
GroupMemberRepositoryPort, and any callers/tests to use the new method name or
return type (ensure method signatures like findMyMembership(Long groupId, Long
userId) or findMyRole(Long groupId, Long userId): GroupRole are consistently
applied).
In `@src/main/java/flipnote/group/application/service/MyPermissionService.java`:
- Around line 8-13: MyPermissionService currently imports and uses the adapter
entity GroupMemberEntity, breaking the port boundary; change
GroupMemberRepositoryPort (and any calls in GroupRoleRepositoryPort if relevant)
to return a domain model or a dedicated DTO instead of the adapter entity,
remove the direct import of flipnote.group.adapter.out.entity.GroupMemberEntity
from MyPermissionService, and update MyPermissionService to consume the domain
model/DTO (e.g., MyPermissionCommand -> MyPermissionResult logic) so mapping
from GroupMemberEntity happens inside the adapter implementation (adapter maps
entity -> domain/DTO before returning). Ensure method signatures on
GroupMemberRepositoryPort and any usages in MyPermissionService are updated
accordingly and that the adapter layer performs the entity-to-domain/DTO
conversion.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/flipnote/group/adapter/in/web/PermissionController.javasrc/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.javasrc/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.javasrc/main/java/flipnote/group/api/dto/response/MyPermissionResponseDto.javasrc/main/java/flipnote/group/application/port/in/MyPermissionUseCase.javasrc/main/java/flipnote/group/application/port/in/command/MyPermissionCommand.javasrc/main/java/flipnote/group/application/port/in/result/MyPermissionResult.javasrc/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.javasrc/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.javasrc/main/java/flipnote/group/application/service/MyPermissionService.javasrc/main/java/flipnote/group/application/service/RemovePermissionService.java
💤 Files with no reviewable changes (1)
- src/main/java/flipnote/group/application/service/RemovePermissionService.java
src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/port/in/command/MyPermissionCommand.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/service/MyPermissionService.java
Show resolved
Hide resolved
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
릴리스 노트
New Features
Bug Fixes
Chores