Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Walkthrough그룹 가입 요청에 대한 응답 기능을 추가합니다. PATCH 엔드포인트로 가입 신청을 수락/거절하며, DTO·커맨드·결과 타입과 use case/서비스, 레포지토리·어댑터 및 엔티티 멤버 수 증감 로직이 추가/수정됩니다. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Controller as JoinController
participant Service as JoinRespondService
participant JoinRepo as JoinRepository
participant GroupRepo as GroupRepository
participant GroupMemberRepo as GroupMemberRepository
participant RoleRepo as GroupRoleRepository
Client->>Controller: PATCH /v1/groups/{groupId}/joins/{joinId} (status)
Controller->>Service: joinRespond(command)
Service->>RoleRepo: checkPermission(JOIN_REQUEST_MANAGE)
alt 권한 없음
RoleRepo-->>Service: 권한 없음 에러
Service-->>Controller: 에러
Controller-->>Client: 에러 응답
else 권한 있음
Service->>JoinRepo: findJoin(joinId)
alt Join 없음 또는 이미 ACCEPT
JoinRepo-->>Service: 에러
Service-->>Controller: 에러
Controller-->>Client: 에러 응답
else Join 존재
alt status == REJECT
Service->>JoinRepo: updateJoin(status=REJECT)
JoinRepo-->>Service: updated JoinEntity
else status == ACCEPT
Service->>GroupRepo: checkJoinable(groupId)
alt 그룹 만원
GroupRepo-->>Service: false
Service-->>Controller: 에러
Controller-->>Client: 에러 응답
else 여유 있음
Service->>RoleRepo: findByIdAndRole(MEMBER)
Service->>GroupMemberRepo: save(new GroupMember)
GroupMemberRepo->>GroupRepo: findByIdForUpdate(groupId)
GroupRepo-->>GroupMemberRepo: GroupEntity
GroupMemberRepo->>GroupRepo: plusCount() 및 save
Service->>JoinRepo: updateJoin(status=ACCEPT)
JoinRepo-->>Service: updated JoinEntity
end
end
Service-->>Controller: JoinRespondResult
Controller-->>Client: 200 OK (JoinRespondResponseDto)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.java (1)
56-59:⚠️ Potential issue | 🔴 Critical무한 재귀 호출로 인한 StackOverflowError 발생
findAllByCursorAndCreatedUserId메서드가groupRepository에 위임하지 않고 자기 자신을 호출하고 있습니다. 이는 런타임에StackOverflowError를 발생시킵니다.🐛 수정 제안
`@Override` public List<GroupInfo> findAllByCursorAndCreatedUserId(Long cursorId, Category category, int size, Long userId) { - return findAllByCursorAndCreatedUserId(cursorId, category, size, userId); + return groupRepository.findAllByCursorAndCreatedUserId(cursorId, category, size, 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/GroupRepositoryAdapter.java` around lines 56 - 59, The method findAllByCursorAndCreatedUserId currently calls itself causing infinite recursion; change the implementation to delegate to the injected groupRepository (e.g., call groupRepository.findAllByCursorAndCreatedUserId(cursorId, category, size, userId)) and return its result, ensuring you reference the existing groupRepository field and keep the method signature intact.
🧹 Nitpick comments (1)
src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java (1)
61-63: 상태 전이 검증 누락
updateStatus메서드에서 유효한 상태 전이인지 검증하지 않습니다. 예를 들어 이미ACCEPTED된 요청을 다시PENDING이나REJECTED로 변경하는 것이 허용될 수 있습니다. 비즈니스 로직에 따라 유효하지 않은 상태 전이를 방지하는 검증이 필요할 수 있습니다.♻️ 상태 전이 검증 추가 예시
public void updateStatus(JoinStatus status) { + if (this.status != JoinStatus.PENDING) { + throw new IllegalStateException("이미 처리된 가입 신청입니다."); + } this.status = status; }🤖 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/entity/JoinEntity.java` around lines 61 - 63, Add validation to JoinEntity.updateStatus to enforce allowed state transitions: determine the allowed transitions in the JoinStatus enum (or a transition matrix) and before assigning this.status = status check current this.status against the target status; if the transition is invalid, throw an IllegalStateException (or a domain-specific exception) with a clear message. Locate JoinEntity.updateStatus and refer to JoinStatus values to implement the transition rules and tests that reject transitions like ACCEPTED -> PENDING/REJECTED if those are disallowed.
🤖 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/in/web/JoinController.java`:
- Line 78: The method findGroupJoinList is returning HttpStatus.CREATED for a
GET/listing endpoint; change the response to use HttpStatus.OK (or
ResponseEntity.ok(...)) instead of HttpStatus.CREATED so the controller returns
200 for successful GETs; locate the return in JoinController.findGroupJoinList
and update the ResponseEntity.status(HttpStatus.CREATED).body(res) to use
HttpStatus.OK or ResponseEntity.ok(res).
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java`:
- Around line 31-37: The code calls groupRepository.findById(...) inside
GroupMemberRepositoryAdapter and then groupEntity.plusCount(), which obscures
whether the entity was locked earlier in JoinRespondService.joinRespond() via
checkJoinable(); update the method to either accept the already-locked
GroupEntity from checkJoinable() (pass the entity through instead of
re-querying) or replace findById(...) with the explicit locked lookup method
(e.g., groupRepository.findByIdForUpdate(...)) so the locking intent is clear;
keep using groupEntity.plusCount() and rely on JPA dirty checking (no explicit
save needed) but ensure the code consistently uses the locked entity or locked
finder to avoid ambiguity.
In `@src/main/java/flipnote/group/api/dto/request/JoinRespondRequestDto.java`:
- Around line 6-9: JoinRespondRequestDto currently accepts any JoinStatus
(PENDING, CANCEL, etc.) and these invalid states are being treated as ACCEPT;
restrict allowed values by adding a validation annotation on the status
component (e.g., create and apply a custom constraint like `@AllowedJoinStatus`
that only permits JoinStatus.ACCEPT and JoinStatus.REJECT) and implement a
ConstraintValidator that checks the JoinStatus enum value; update the record
declaration JoinRespondRequestDto( `@NotNull` `@AllowedJoinStatus` JoinStatus status
) so only ACCEPT/REJECT are accepted.
In
`@src/main/java/flipnote/group/application/port/in/result/JoinRespondResult.java`:
- Around line 3-6: JoinRespondResult currently depends on
adapter.out.entity.JoinEntity which violates hexagonal layering; replace that
dependency by using a domain model or an application-level DTO: change the
record JoinRespondResult to carry a domain type (e.g., domain.model.Join or your
Domain Join aggregate) or a new DTO (e.g., JoinResponse/JoinDto placed under
application.port.in.result) and update the places that construct
JoinRespondResult (factories/mappers) to map from JoinEntity to the domain model
or DTO (create a mapper method that converts JoinEntity -> domain Join or
JoinResponse). Ensure all imports reference the new domain/DTO type instead of
flipnote.group.adapter.out.entity.JoinEntity and update callers that construct
or consume JoinRespondResult accordingly.
In `@src/main/java/flipnote/group/application/service/JoinRespondService.java`:
- Around line 50-52: The status validation in JoinRespondService currently only
blocks ACCEPT; update the logic in the method performing the check (where
join.getStatus() is inspected) to allow transitions only when the current status
is PENDING: if status != JoinStatus.PENDING, throw an appropriate exception
(e.g., IllegalStateException) with a clear message indicating the current status
so callers know why the operation is invalid; ensure both ACCEPT and REJECT (and
any other non-PENDING states) are handled by this validation before proceeding
with accept/reject flows.
---
Outside diff comments:
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.java`:
- Around line 56-59: The method findAllByCursorAndCreatedUserId currently calls
itself causing infinite recursion; change the implementation to delegate to the
injected groupRepository (e.g., call
groupRepository.findAllByCursorAndCreatedUserId(cursorId, category, size,
userId)) and return its result, ensuring you reference the existing
groupRepository field and keep the method signature intact.
---
Nitpick comments:
In `@src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java`:
- Around line 61-63: Add validation to JoinEntity.updateStatus to enforce
allowed state transitions: determine the allowed transitions in the JoinStatus
enum (or a transition matrix) and before assigning this.status = status check
current this.status against the target status; if the transition is invalid,
throw an IllegalStateException (or a domain-specific exception) with a clear
message. Locate JoinEntity.updateStatus and refer to JoinStatus values to
implement the transition rules and tests that reject transitions like ACCEPTED
-> PENDING/REJECTED if those are disallowed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/main/java/flipnote/group/adapter/in/web/JoinController.javasrc/main/java/flipnote/group/adapter/out/entity/GroupEntity.javasrc/main/java/flipnote/group/adapter/out/entity/JoinEntity.javasrc/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.javasrc/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.javasrc/main/java/flipnote/group/adapter/out/persistence/JoinRepositoryAdapter.javasrc/main/java/flipnote/group/api/dto/request/JoinRespondRequestDto.javasrc/main/java/flipnote/group/api/dto/response/JoinRespondResponseDto.javasrc/main/java/flipnote/group/application/port/in/JoinRespondUseCase.javasrc/main/java/flipnote/group/application/port/in/command/JoinRespondCommand.javasrc/main/java/flipnote/group/application/port/in/result/JoinRespondResult.javasrc/main/java/flipnote/group/application/port/out/GroupRepositoryPort.javasrc/main/java/flipnote/group/application/port/out/JoinRepositoryPort.javasrc/main/java/flipnote/group/application/service/ChangeGroupService.javasrc/main/java/flipnote/group/application/service/JoinRespondService.javasrc/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRepository.javasrc/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepository.java
💤 Files with no reviewable changes (1)
- src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepository.java
src/main/java/flipnote/group/adapter/in/web/JoinController.java
Outdated
Show resolved
Hide resolved
src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/port/in/result/JoinRespondResult.java
Show resolved
Hide resolved
src/main/java/flipnote/group/application/service/JoinRespondService.java
Show resolved
Hide resolved
This was referenced Mar 1, 2026
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
릴리스 노트
새로운 기능
개선사항