Skip to content

[WTH-83] 출석 관리 페이지에서 이번주차 반환 #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seokjun01 merged 8 commits into from
Dec 12, 2025
Merged

[WTH-83] 출석 관리 페이지에서 이번주차 반환 #233

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ed18147
refactor : UX 개선 위한 정기 모임 로직 수정
seokjun01 Dec 8, 2025
6ced669
feat : 아키텍처 통일을 위한 도메인 서비스 파일 생성
seokjun01 Dec 8, 2025
8202115
refactor : 로직 -> 흐름 호출 방식으로 변경
seokjun01 Dec 8, 2025
f61a4bd
del : 안쓰는 변수 삭제
seokjun01 Dec 9, 2025
ce00ce9
del : 사용하지 않는 import 제거
seokjun01 Dec 10, 2025
eff47b5
add : 반환 DTO 추가
seokjun01 Dec 11, 2025
63c46ed
fix : 반환 타입 변경
seokjun01 Dec 11, 2025
3136b43
refactor : 도메인 서비스 분리 -> useCase 내부 private 메소드로 관리
seokjun01 Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/main/java/leets/weeth/domain/schedule/application/usecase/MeetingUseCaseImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import leets.weeth.domain.schedule.application.mapper.MeetingMapper;
import leets.weeth.domain.schedule.domain.entity.Meeting;
import leets.weeth.domain.schedule.domain.service.MeetingDeleteService;
import leets.weeth.domain.schedule.domain.service.MeetingDomainService;
import leets.weeth.domain.schedule.domain.service.MeetingGetService;
import leets.weeth.domain.schedule.domain.service.MeetingSaveService;
import leets.weeth.domain.schedule.domain.service.MeetingUpdateService;
Expand All @@ -24,6 +25,11 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.time.DayOfWeek;
Copy link
Collaborator

@hyxklee hyxklee Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기 import문은 왜 들어갔을까요??

Copy link
Collaborator Author

@seokjun01 seokjun01 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음에 기능을 useCaseImpl에 작성을 하고 난 뒤 파일 분리해서 구현하느라 삭제를 깜빡했습니다!
지우겠습니다 👍🏻

import java.time.LocalDate;
import java.time.temporal.TemporalAdjusters;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;

import static leets.weeth.domain.schedule.application.dto.MeetingDTO.Info;
Expand All @@ -40,6 +46,7 @@ public class MeetingUseCaseImpl implements MeetingUseCase {
private final UserGetService userGetService;
private final MeetingUpdateService meetingUpdateService;
private final MeetingDeleteService meetingDeleteService;
private final MeetingDomainService meetingDomainService;
private final AttendanceGetService attendanceGetService;
private final AttendanceSaveService attendanceSaveService;
private final AttendanceDeleteService attendanceDeleteService;
Expand All @@ -64,6 +71,7 @@ public Response find(Long userId, Long meetingId) {
@Override
public List<Info> find(Integer cardinal) {
List<Meeting> meetings;
List<Info> result = new ArrayList<>();

if (cardinal == null) {
meetings = meetingGetService.findAll();
Expand All @@ -73,9 +81,11 @@ public List<Info> find(Integer cardinal) {

}

return meetings.stream()
.map(mapper::toInfo)
.toList();
List<Meeting> ordered = meetingDomainService.reorderMeetingsWithThisWeek(meetings);
Copy link
Collaborator

@jj0526 jj0526 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

응답 구조를 변경해서 이번주 : null 또는 이번주 미팅, 리스트 : 기존 미팅으로 추가하는게 중복이 없어져서 좋을거같은데 어떠신가요?

Copy link
Collaborator Author

@seokjun01 seokjun01 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 부분은 프론트 분들과 더 이야기 나눠봐야 할 것 같습니다!
원하는 응답 방식이 배열 상태에서 이번주차 꺼를 복사형식으로 맨 앞에 두고 기존 배열을 둔 상태로 달라고 하셨어가지고
혹여나 제가 잘 못 이해한 부분이 있다면 코멘트 남겨주세요!


return ordered.stream()
.map(mapper::toInfo)
.toList();
}

@Override
Expand Down
46 changes: 46 additions & 0 deletions src/main/java/leets/weeth/domain/schedule/domain/service/MeetingDomainService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package leets.weeth.domain.schedule.domain.service;

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;

import org.springframework.stereotype.Service;

import leets.weeth.domain.schedule.domain.entity.Meeting;

@Service
public class MeetingDomainService {
Copy link
Collaborator

@hyxklee hyxklee Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 서비스는 usecase에서 가져온 meeting들을 필터링하는 역할만 수행하나요?
그렇다면 별도 서비스 분리보다는 usecase 내부에 private 메서드로 관리하는 것도 나을 수 있을 것 같아요!

Copy link
Collaborator Author

@seokjun01 seokjun01 Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 의견 감사합니다. 피드백 주신대로 스스로 생각해보니 , 도메인 로직이라 생각해서 useCase에 정의해서 사용하면 안된다 생각했는데, 단순히 조회를 가공하는 로직으로 볼 수도 있겠군요! 반영해두겠습니다


public List<Meeting> reorderMeetingsWithThisWeek(List<Meeting> meetings) {

LocalDate today = LocalDate.now();
LocalDate startOfWeek = today.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY));
LocalDate endOfWeek = today.with(TemporalAdjusters.nextOrSame(DayOfWeek.SUNDAY));

Meeting thisWeek = meetings.stream()
.filter(m -> {
LocalDate date = m.getStart().toLocalDate();
return !date.isBefore(startOfWeek) && !date.isAfter(endOfWeek);
})
.findFirst()
.orElse(null);

List<Meeting> result = new ArrayList<>();

if (thisWeek != null) {
result.add(thisWeek);
}

result.addAll(
meetings.stream()
.sorted(Comparator.comparing(Meeting::getStart).reversed())
.toList()
);
Copy link

@coderabbitai coderabbitai bot Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

이번 주 미팅이 결과 리스트에 중복으로 추가됩니다.

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.

Copy link
Collaborator Author

@seokjun01 seokjun01 Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일부로 이번주차를 맨 앞에 반환해주려고 복사형식으로 맨 앞에 띄워주고 기존 주차 배열도 또 넣은건데
이 의도라도 코드가 이상한가?

Copy link

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seokjun01, 의도를 이해했습니다. 하지만 의도적이더라도 동일한 미팅이 결과에 두 번 나타나는 것은 여러 문제를 야기할 수 있습니다:

문제점:

  1. API 응답 크기 증가 - 불필요한 데이터 중복으로 네트워크 부하 증가
  2. 데이터 일관성 혼란 - API 사용자(프론트엔드)가 동일한 미팅을 별개의 항목으로 처리할 수 있음
  3. 집계/필터링 어려움 - 총 미팅 수 계산, 중복 제거 등 추가 처리 필요
  4. 유지보수성 저하 - 코드 의도가 명확하지 않아 향후 다른 개발자가 버그로 오인할 가능성

권장 방식:

이번 주 미팅을 첫 번째로 보여주고 싶다면, 중복 없이 다음과 같이 구현하는 것이 일반적입니다:

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 문서에 이 동작을 명확히 기재하여 혼란을 방지하는 것을 권장드립니다.

Copy link
Collaborator

@hyxklee hyxklee Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 응답대로 응답 dto에 구분은 필요할 것 같아염
별도 필드 추가보다는 란이 구분되게
AS-IS: List
TO-BE: (변수명은 조절 필요)

public record Infos(
            Info thisWeek,
            List<Info> meetings
    )
    {}

Copy link

@coderabbitai coderabbitai bot Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


return result;
}

}