[WTH-83] 출석 관리 페이지에서 이번주차 반환#233
Conversation
Walkthrough미팅 조회 흐름에 새로운 도메인 서비스 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 (1)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java (1)
28-32: 사용되지 않는 import 문이 있습니다.
DayOfWeek,LocalDate,TemporalAdjusters,ArrayList,Comparatorimport들은 주차 계산 로직이MeetingDomainService로 이동되면서 이 파일에서 더 이상 사용되지 않습니다. 제거해 주세요.-import java.time.DayOfWeek; -import java.time.LocalDate; -import java.time.temporal.TemporalAdjusters; -import java.util.ArrayList; -import java.util.Comparator; import java.util.List;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java(5 hunks)src/main/java/leets/weeth/domain/schedule/domain/service/MeetingDomainService.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java (1)
84-88: 도메인 서비스로의 리팩토링이 잘 되었습니다.주차 계산 및 정렬 로직을
MeetingDomainService로 분리하여 UseCase의 책임을 적절히 줄였습니다.src/main/java/leets/weeth/domain/schedule/domain/service/MeetingDomainService.java (2)
23-29: 동일 주차에 여러 미팅이 있을 경우 처리 방식 확인이 필요합니다.
findFirst()를 사용하여 이번 주 미팅을 찾고 있는데, 동일 주차에 여러 미팅이 존재할 경우 원본 리스트의 순서에 따라 첫 번째 미팅만 선택됩니다. 의도된 동작인지 확인이 필요합니다.만약 이번 주에 가장 최근(또는 가장 이른) 미팅을 선택해야 한다면, 정렬 후
findFirst()를 사용하는 것이 더 명확합니다.
14-15: 도메인 서비스 구조가 적절합니다.주차 계산 및 정렬 로직을 별도의 도메인 서비스로 분리하여 SRP 원칙을 잘 따르고 있습니다.
| if (thisWeek != null) { | ||
| result.add(thisWeek); | ||
| } | ||
|
|
||
| result.addAll( | ||
| meetings.stream() | ||
| .sorted(Comparator.comparing(Meeting::getStart).reversed()) | ||
| .toList() | ||
| ); |
There was a problem hiding this comment.
이번 주 미팅이 결과 리스트에 중복으로 추가됩니다.
thisWeek가 존재할 경우, 먼저 결과 리스트에 추가된 후 (line 34), 전체 미팅 리스트를 정렬하여 다시 추가됩니다 (lines 37-41). 이로 인해 동일한 미팅이 두 번 반환됩니다.
PR 설명의 예시 JSON에서도 "duplicate 2nd"가 언급되어 있어 이 버그가 확인됩니다.
thisWeek를 제외하고 나머지 미팅들만 정렬하여 추가하도록 수정해야 합니다:
if (thisWeek != null) {
result.add(thisWeek);
}
result.addAll(
meetings.stream()
+ .filter(m -> !m.equals(thisWeek))
.sorted(Comparator.comparing(Meeting::getStart).reversed())
.toList()
);만약 Meeting 클래스에 equals 메서드가 적절히 구현되어 있지 않다면, ID로 비교하는 방식을 사용하세요:
.filter(m -> thisWeek == null || !m.getId().equals(thisWeek.getId()))🤖 Prompt for AI Agents
In
src/main/java/leets/weeth/domain/schedule/domain/service/MeetingDomainService.java
around lines 33 to 41, the code adds thisWeek to result and then adds all
meetings including thisWeek again causing duplicates; modify the second addition
to exclude thisWeek (i.e., filter out meetings whose id equals thisWeek.getId()
when thisWeek is non-null, or use equals if Meeting implements it) and then sort
and add the remaining meetings so thisWeek is not duplicated.
There was a problem hiding this comment.
일부로 이번주차를 맨 앞에 반환해주려고 복사형식으로 맨 앞에 띄워주고 기존 주차 배열도 또 넣은건데
이 의도라도 코드가 이상한가?
There was a problem hiding this comment.
@seokjun01, 의도를 이해했습니다. 하지만 의도적이더라도 동일한 미팅이 결과에 두 번 나타나는 것은 여러 문제를 야기할 수 있습니다:
문제점:
- API 응답 크기 증가 - 불필요한 데이터 중복으로 네트워크 부하 증가
- 데이터 일관성 혼란 - API 사용자(프론트엔드)가 동일한 미팅을 별개의 항목으로 처리할 수 있음
- 집계/필터링 어려움 - 총 미팅 수 계산, 중복 제거 등 추가 처리 필요
- 유지보수성 저하 - 코드 의도가 명확하지 않아 향후 다른 개발자가 버그로 오인할 가능성
권장 방식:
이번 주 미팅을 첫 번째로 보여주고 싶다면, 중복 없이 다음과 같이 구현하는 것이 일반적입니다:
List<Meeting> result = new ArrayList<>();
if (thisWeek != null) {
result.add(thisWeek);
}
result.addAll(
meetings.stream()
.filter(m -> !m.equals(thisWeek)) // thisWeek 제외
.sorted(Comparator.comparing(Meeting::getStart).reversed())
.toList()
);이렇게 하면:
- 이번 주 미팅이 맨 앞에 표시됨 ✓
- 나머지 미팅들이 시작일 기준 내림차순으로 정렬됨 ✓
- 중복 없이 깔끔한 응답 ✓
프론트엔드에서 이번 주 미팅을 특별히 강조 표시하고 싶다면, 중복된 데이터보다는 별도의 필드(예: isCurrentWeek)를 추가하는 방식이 더 명확합니다.
현재 의도대로 중복을 유지하시겠다면, 최소한 API 문서에 이 동작을 명확히 기재하여 혼란을 방지하는 것을 권장드립니다.
There was a problem hiding this comment.
해당 응답대로 응답 dto에 구분은 필요할 것 같아염
별도 필드 추가보다는 란이 구분되게
AS-IS: List
TO-BE: (변수명은 조절 필요)
public record Infos(
Info thisWeek,
List<Info> meetings
)
{}
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java (1)
28-32: 사용되지 않는 import 문을 제거하세요.
DayOfWeek,LocalDate,TemporalAdjusters,ArrayList,Comparator는 이 클래스에서 사용되지 않습니다. 로직이MeetingDomainService로 이동하면서 남겨진 것으로 보이며, 코드 정리를 위해 제거하는 것이 좋습니다.-import java.time.DayOfWeek; -import java.time.LocalDate; -import java.time.temporal.TemporalAdjusters; -import java.util.ArrayList; -import java.util.Comparator; import java.util.List;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java(4 hunks)
🔇 Additional comments (1)
src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java (1)
49-49: 도메인 서비스 활용이 잘 적용되었습니다!주차 계산 및 정렬 로직을
MeetingDomainService로 분리한 것은 좋은 아키텍처 결정입니다. UseCase는 이제 비즈니스 흐름 조율에 집중하고, 도메인 로직은 적절한 계층에 위치하게 되었습니다.Also applies to: 83-87
| return meetings.stream() | ||
| .map(mapper::toInfo) | ||
| .toList(); | ||
| List<Meeting> ordered = meetingDomainService.reorderMeetingsWithThisWeek(meetings); |
There was a problem hiding this comment.
응답 구조를 변경해서 이번주 : null 또는 이번주 미팅, 리스트 : 기존 미팅으로 추가하는게 중복이 없어져서 좋을거같은데 어떠신가요?
There was a problem hiding this comment.
위 부분은 프론트 분들과 더 이야기 나눠봐야 할 것 같습니다!
원하는 응답 방식이 배열 상태에서 이번주차 꺼를 복사형식으로 맨 앞에 두고 기존 배열을 둔 상태로 달라고 하셨어가지고
혹여나 제가 잘 못 이해한 부분이 있다면 코멘트 남겨주세요!
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.time.DayOfWeek; |
There was a problem hiding this comment.
처음에 기능을 useCaseImpl에 작성을 하고 난 뒤 파일 분리해서 구현하느라 삭제를 깜빡했습니다!
지우겠습니다 👍🏻
| import leets.weeth.domain.schedule.domain.entity.Meeting; | ||
|
|
||
| @Service | ||
| public class MeetingDomainService { |
There was a problem hiding this comment.
해당 서비스는 usecase에서 가져온 meeting들을 필터링하는 역할만 수행하나요?
그렇다면 별도 서비스 분리보다는 usecase 내부에 private 메서드로 관리하는 것도 나을 수 있을 것 같아요!
There was a problem hiding this comment.
좋은 의견 감사합니다. 피드백 주신대로 스스로 생각해보니 , 도메인 로직이라 생각해서 useCase에 정의해서 사용하면 안된다 생각했는데, 단순히 조회를 가공하는 로직으로 볼 수도 있겠군요! 반영해두겠습니다
3 participants
📌 PR 내용
UT 피드백 중 일부분인 , 위드 어드민 출석 페이지에서 이번 주차 모임을 찾기 불편하다는 피드백을 반영하기 위해
기존의 meetingUseCaseImpl 에서 사용중인 public List find(Integer cardinal) 메소드를 리팩토링 했습니다.
반환타입은 이번 주차 모임이 제일 위로 뜨고 , 그다음 내림 차순으로 모임이 반환됩니다.
아키텍처 특성에 맞게 리팩토링하기 위해, MeetingDomainService 파일에 주차 계산, 주차 찾기, 반환하는 주차 모임 배치 등의 로직을 포함하고 UseCase에서는 호출을 담당했습니다.
기존 PR에서의 수정 (12/11)
-> 단순 조회 로직 ( 필터링이나 정렬)이므로 도메인 서비스 분리에서 다시 UseCase내에서 private 메소드로 관리하기로 했습니다 .
-> MeetingDTO에 Infos라는 record를 추가하여 반환타입을 재정의 하였습니다.
🔍 PR 세부사항
테스트는 로컬 디비 기준으로 진행하였습니다.
📸 관련 스크린샷
{ "code": 200, "message": "기수별 정기모임 리스트를 성공적으로 조회했습니다.", "data": { "thisWeek": { "id": 4, "cardinal": 1, "title": "12월 2주차 알고리즘 스터디", "start": "2025-12-08T19:00:00" }, "meetings": [ { "id": 7, "cardinal": 1, "title": "12월 5주차 정기모임", "start": "2025-12-29T19:00:00" }, { "id": 6, "cardinal": 1, "title": "12월 4주차 정기모임", "start": "2025-12-22T19:00:00" }, { "id": 5, "cardinal": 1, "title": "12월 3주차 정기모임", "start": "2025-12-15T19:00:00" }, { "id": 4, "cardinal": 1, "title": "12월 2주차 알고리즘 스터디", "start": "2025-12-08T19:00:00" }, { "id": 8, "cardinal": 1, "title": "12월 0주차 정기모임", "start": "2025-11-29T19:00:00" } ] } }📝 주의사항
✅ 체크리스트
Summary by CodeRabbit
✏️ Tip: 리뷰 설정에서 이 요약을 커스터마이즈할 수 있습니다.