Conversation
|
Caution Review failedThe pull request is closed. Walkthrough그룹 삭제 기능이 추가되었습니다: API 엔드포인트( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as GroupController
participant Service as DeleteGroupService
participant RepoPort as GroupRepositoryPort
participant DB as JPARepository
Client->>Controller: DELETE /groups/{groupId} (X-USER-ID)
Controller->>Service: deleteGroup(DeleteGroupCommand(userId, groupId))
Service->>RepoPort: delete(groupId)
RepoPort->>DB: existsById(groupId) / deleteById(groupId)
DB-->>RepoPort: ack
RepoPort-->>Service: ack
Service-->>Controller: completed
Controller-->>Client: 204 No Content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
개요그룹 삭제 기능이 구현되었습니다. 클린 아키텍처의 각 계층을 따라 DeleteGroupUseCase 인터페이스, DeleteGroupCommand 레코드, DeleteGroupService 구현, 그리고 GroupController의 새로운 엔드포인트가 추가되었으며, 저장소 포트와 어댑터도 확장되었습니다. 변경 사항
예상 코드 검토 시간🎯 3 (Moderate) | ⏱️ ~20 minutes 관련 가능성 있는 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: 3
🤖 Fix all issues with AI agents
In `@src/main/java/flipnote/group/adapter/in/web/GroupController.java`:
- Around line 118-136: Fix the typo in the TODO comment inside GroupController
(in the deleteGroup method of class GroupController): change both occurrences of
"권환" to "권한" so the TODO reads correctly (e.g., "추후 권한 체크 후 권한 확인 후 삭제"),
preserving the comment structure and spacing.
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.java`:
- Around line 39-42: The delete method in GroupRepositoryAdapter currently calls
groupRepository.deleteById(groupId) without checking existence, which is
inconsistent with findById's behavior; update GroupRepositoryAdapter.delete(Long
groupId) to first verify the group exists (e.g., use
groupRepository.existsById(groupId) or reuse findById logic) and if not present
throw the same IllegalArgumentException used elsewhere, otherwise call
groupRepository.deleteById(groupId) to perform the deletion so behavior and
client responses remain consistent.
In `@src/main/java/flipnote/group/application/service/DeleteGroupService.java`:
- Around line 10-19: Add transactional and ownership checks to
DeleteGroupService: annotate the class or deleteGroup method with
`@Transactional`, load the group via GroupRepositoryPort (e.g., findById or
getById) inside deleteGroup(DeleteGroupCommand cmd), verify that the fetched
group's ownerId equals cmd.userId, and if not throw an appropriate
access/authorization exception; only call groupRepository.delete(cmd.groupId())
after the existence and ownership checks succeed (and handle group-not-found by
throwing a not-found exception).
🧹 Nitpick comments (2)
src/main/java/flipnote/group/adapter/in/web/MemberController.java (1)
1-18: 스캐폴딩 전용 컨트롤러가 메인 브랜치에 병합됩니다.이 파일은 구현이 전혀 없이 TODO 주석만 포함된 빈 클래스입니다. 그룹 삭제 API PR의 범위에 포함될 필요가 없어 보이며, 해당 기능을 실제 구현할 때 추가하는 것이 더 적절합니다.
만약 의도적으로 포함시킨 것이라면, 최소한
@RestController와@RequestMapping어노테이션을 미리 추가하는 것을 권장합니다.src/main/java/flipnote/group/application/service/DeleteGroupService.java (1)
17-18: 소프트 삭제(Soft Delete) 고려 필요현재 물리적 삭제(
deleteById)를 수행하고 있습니다. 그룹에 속한 멤버, 노트 등 연관 데이터가 있을 경우 데이터 정합성 문제가 발생할 수 있습니다. 소프트 삭제(deleted플래그 또는deletedAt타임스탬프) 도입을 검토해 보세요. 당장이 아니더라도 아키텍처 방향 결정이 필요한 사항입니다.
src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/service/DeleteGroupService.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
릴리스 노트