refactor: 오늘의 질문 response 수정#130
Merged
Hidden character warning
The head ref may contain hidden characters: "129-\uc624\ub298\uc758-\uc9c8\ubb38-\uc218\uc815"
Merged
Conversation
Closed
|
Warning Rate limit exceeded@xoruddl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough오늘의 질문 API가 인증 사용자 컨텍스트를 사용하도록 변경되고, 응답 DTO에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as DailyQuestionController
participant DQService as DailyQuestionService
participant DQAService as DailyQuestionAnswerService
Client->>Controller: GET /daily-question (with Auth)
Controller->>DQService: getQuestionInfoForToday()
DQService-->>Controller: QuestionInfo { id, text }
Controller->>DQAService: hasUserAnsweredToday(User)
DQAService-->>Controller: boolean isAnswered
Controller-->>Client: DailyQuestionResponse { id, question, isAnswered }
rect rgba(230,245,255,0.45)
note right of Controller: 변경점: 응답에 id/isAnswered 추가 및 인증 기반 조회
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
|
🚨 PR 본문이 비어있습니다! 아래 템플릿을 복사하여 PR 내용을 작성해주세요. 📝 개요
💻 작업 내용
✅ PR 체크리스트
🔗 관련 이슈
스크린샷 (선택)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java (1)
49-51: 경계 체크 누락: 잘못된 questionId에 대한 방어 로직 추가 권장외부에서 넘어온
id가 범위를 벗어나면IndexOutOfBoundsException이 발생합니다. 명시적으로 검증하여 400 계열로 매핑하기 쉬운 예외를 던지는 편이 안전합니다.다음과 같이 수정 제안합니다.
public String getQuestionById(Long id) { - return QUESTIONS.get(id.intValue()); + int idx = Math.toIntExact(id); + if (idx < 0 || idx >= QUESTIONS.size()) { + throw new IllegalArgumentException("Invalid questionId: " + id); + } + return QUESTIONS.get(idx); }src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java (1)
20-43: 중복 응답 레이스 조건 및 인증/질문 검증 보강 필요
- 존재 여부 체크 후 저장은 동시 요청에서 중복 저장 위험이 있습니다. 2)
user가 null이면 NPE 가능. 3) 요청questionId가 “오늘의 질문”과 불일치해도 저장 가능합니다.다음 보강을 권장합니다.
- 인증 강제 또는 null 가드
- “오늘의 질문” ID 일치 검증
- DB 유니크 제약(사용자+날짜)과 예외 매핑
코드 수정 예시:
public void saveAnswer(User user, DailyQuestionAnswerRequest requestDto) { - LocalDate today = LocalDate.now(); + if (user == null) { + throw new org.springframework.security.authentication.AuthenticationCredentialsNotFoundException("로그인이 필요합니다."); + } + LocalDate today = LocalDate.now(); + + // 오늘의 질문 ID 검증 + DailyQuestionService.QuestionInfo todayInfo = dailyQuestionService.getQuestionInfoForToday(); + if (!todayInfo.getId().equals(requestDto.getQuestionId())) { + throw new IllegalArgumentException("오늘의 질문 ID와 요청 ID가 일치하지 않습니다."); + } // 사용자가 오늘 이미 답변했는지 확인 if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { throw new IllegalStateException("User has already answered today's question."); } DailyQuestionAnswer answer = DailyQuestionAnswer.builder() .user(user) // 매개변수로 받은 user 객체를 바로 사용 .question(dailyQuestionService.getQuestionById(requestDto.getQuestionId())) .answeredDate(today) .build(); answer.setAnswer(requestDto.getAnswer()); - dailyQuestionAnswerRepository.save(answer); + try { + dailyQuestionAnswerRepository.save(answer); + } catch (org.springframework.dao.DataIntegrityViolationException e) { + // DB 유니크 제약 위반 시 중복 응답으로 매핑 + throw new IllegalStateException("User has already answered today's question.", e); + } }엔티티에 유니크 제약 추가(참고, 별도 파일):
// DailyQuestionAnswer.java @Table( name = "daily_question_answer", uniqueConstraints = @UniqueConstraint(name = "uk_user_date", columnNames = {"user_id", "answered_date"}) )src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java (1)
45-49: POST는 인증 강제가 바람직하며 DTO 검증 애노테이션 점검 필요
- 메서드/클래스에 인증 강제를 명시해 주세요(예:
@PreAuthorize("isAuthenticated()")).- 사용 중인
DailyQuestionAnswerRequest의questionId가@NotBlank(문자열용)로 선언되어 있으면 검증이 동작하지 않아 NPE 위험이 있습니다.@NotNull(+ 범위 검사)로 교체하세요.컨트롤러 보완 예시:
@PostMapping("/answer") - public ResponseEntity<ApiResponse<Void>> saveDailyQuestionAnswer( + @org.springframework.security.access.prepost.PreAuthorize("isAuthenticated()") + public ResponseEntity<ApiResponse<Void>> saveDailyQuestionAnswer( @AuthenticationPrincipal User user, @Valid @RequestBody DailyQuestionAnswerRequest request) {DTO 수정 예시(별도 파일:
DailyQuestionAnswerRequest.java):- @NotBlank(message = "Question cannot be blank") - private Long questionId; + @NotNull(message = "questionId cannot be null") + @jakarta.validation.constraints.Min(value = 0, message = "questionId must be >= 0") + private Long questionId;
🧹 Nitpick comments (3)
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java (1)
42-47: ID를 리스트 인덱스에 결합: 재정렬/추가 시 기존 데이터 호환성 이슈
id = questionIndex(0-base)는 질문 목록 변경 시 과거questionId의 의미가 바뀝니다. 운영 중 질문 추가/재정렬 가능성이 있다면, 불변 식별자(예: 미리 고정한 ID/코드, 해시)로 분리하는 것을 권장합니다. 또한 시간대 의존을 줄이려면Clock주입 후LocalDate.now(clock)로 계산하면 테스트/운영 일관성이 좋아집니다.참고 수정 예시(선택):
- public QuestionInfo getQuestionInfoForToday() { - int dayOfYear = LocalDate.now().getDayOfYear(); + private final java.time.Clock clock = java.time.Clock.systemDefaultZone(); // 또는 생성자 주입 + public QuestionInfo getQuestionInfoForToday() { + int dayOfYear = LocalDate.now(clock).getDayOfYear(); int questionIndex = (dayOfYear - 1) % QUESTIONS.size(); String question = QUESTIONS.get(questionIndex); return new QuestionInfo((long) questionIndex, question); }장기적으로는 고정 ID 맵핑 테이블(예: Map<Long, String> 또는 영속 테이블)로 전환을 검토해 주세요.
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java (1)
45-53: 익명 사용자 처리 기본값 false 처리 OK, 시간대 일관성만 고려비로그인 시 false 반환은 컨트롤러 시나리오에 맞습니다. 동일 서비스 내
saveAnswer에서LocalDate.now()도 사용하므로, 테스트/운영 일관성을 위해Clock주입 사용을 고려해 주세요.src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java (1)
30-40: 응답 조립 흐름 깔끔, 단 Swagger/JSON 필드명 일치 여부 확인컨트롤러 구성은 명확합니다. DTO에서
@JsonProperty("isAnswered")로 고정하지 않으면 문서/런타임에서answered로 노출될 수 있습니다. DTO 쪽 수정 후 스키마를 재확인해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java (2)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
Getter(5-16)src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
Getter(8-17)
🔇 Additional comments (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java (1)
35-40: 내부 DTO 분리 LGTM
QuestionInfo(id, text)로 응답 계층 의존을 줄인 점 좋습니다.
Comment on lines
+7
to
+15
| private final Long id; | ||
| private final String question; | ||
| private final boolean isAnswered; | ||
|
|
||
| public DailyQuestionResponse(Long id, String question, boolean isAnswered) { | ||
| this.id = id; | ||
| this.question = question; | ||
| this.isAnswered = isAnswered; | ||
| } |
There was a problem hiding this comment.
boolean 필드 직렬화 명시 필요: JSON에서 'isAnswered'가 'answered'로 노출될 수 있음
Lombok @Getter가 boolean 필드명을 isAnswered로 두면 Jackson이 기본적으로 프로퍼티명을 answered로 해석할 수 있습니다. API 계약이 isAnswered를 요구한다면 명시적으로 고정하세요.
다음과 같이 수정 제안합니다.
package com.example.cp_main_be.domain.member.daily_question.dto;
+import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter;
@Getter
public class DailyQuestionResponse {
private final Long id;
private final String question;
- private final boolean isAnswered;
+ @JsonProperty("isAnswered")
+ private final boolean isAnswered;
public DailyQuestionResponse(Long id, String question, boolean isAnswered) {
this.id = id;
this.question = question;
this.isAnswered = isAnswered;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private final Long id; | |
| private final String question; | |
| private final boolean isAnswered; | |
| public DailyQuestionResponse(Long id, String question, boolean isAnswered) { | |
| this.id = id; | |
| this.question = question; | |
| this.isAnswered = isAnswered; | |
| } | |
| package com.example.cp_main_be.domain.member.daily_question.dto; | |
| import com.fasterxml.jackson.annotation.JsonProperty; | |
| import lombok.Getter; | |
| @Getter | |
| public class DailyQuestionResponse { | |
| private final Long id; | |
| private final String question; | |
| @JsonProperty("isAnswered") | |
| private final boolean isAnswered; | |
| public DailyQuestionResponse(Long id, String question, boolean isAnswered) { | |
| this.id = id; | |
| this.question = question; | |
| this.isAnswered = isAnswered; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java
around lines 7 to 15, the boolean field name isDeclared as 'isAnswered' which
Jackson may serialize as 'answered' by default; annotate the field or its getter
with @JsonProperty("isAnswered") (import
com.fasterxml.jackson.annotation.JsonProperty) to force the JSON property name
to 'isAnswered' and keep the constructor and field assignment unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/dto/MissionCountPerDay.java (1)
3-14: DTO를 record로 단순화 가능 (선택)Lombok와 수동 생성자를 제거하고 Java record를 사용하면 더 간결해집니다. 직렬화나 프록시 요구가 없다면 고려해주세요.
-package com.example.cp_main_be.domain.mission.user_daily_mission.dto; - -import lombok.Getter; - -@Getter -public class MissionCountPerDay { - private final int day; - private final long count; - - public MissionCountPerDay(int day, long count) { - this.day = day; - this.count = count; - } -} +package com.example.cp_main_be.domain.mission.user_daily_mission.dto; + +public record MissionCountPerDay(int day, long count) {}src/main/java/com/example/cp_main_be/domain/mission/MissionAggregationService.java (2)
54-56: 반환 리스트 정렬 추가 제안 (가독성/일관성)Map→List 변환 시 일자 오름차순 정렬을 보장하면 소비 측에서 가공이 단순해집니다.
- return combinedCounts.entrySet().stream() - .map(entry -> new MissionCountPerDay(entry.getKey(), entry.getValue())) - .collect(Collectors.toList()); + return combinedCounts.entrySet().stream() + .sorted(Map.Entry.comparingByKey()) + .map(entry -> new MissionCountPerDay(entry.getKey(), entry.getValue())) + .collect(Collectors.toList());
46-56: 월 경계 호출 안전성 확인됨
- CalendarService 에서
startDate = yearMonth.atDay(1).atStartOfDay()및
endDate = yearMonth.atEndOfMonth().atTime(23, 59, 59)으로 생성되어
항상 단일 월(해당 월의 1일부터 말일) 범위 내에서만getMissionCountsPerDay를 호출합니다.
따라서MissionCountPerDay::getDay(1~31) 만으로 그룹핑해도 현재 호출 범위에서는 왜곡이 발생하지 않습니다.추가 검토 권장 사항:
- 향후 월 경계를 넘는 기간 조회가 필요해질 경우,
• 그룹핑 키를LocalDate전체 날짜로 변경
• 또는 Repository 레벨에서yyyy-MM-dd기준으로 그룹핑해 내려받는 구조로 리팩토링
등의 방안을 검토하세요.src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (1)
7-8: LocalDateTime 사용은 불필요해 보입니다엔티티 필드가 LocalDate로 보이고(existsByUserAndAnsweredDate 시그니처 참고), 쿼리에서도 CAST로 date만 사용하고 있습니다. 파라미터 타입을 LocalDate로 통일하면 CAST 제거로 가독성과 이식성이 좋아집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java(1 hunks)src/main/java/com/example/cp_main_be/domain/mission/MissionAggregationService.java(3 hunks)src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/dto/MissionCountPerDay.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/dto/MissionCountPerDay.java (1)
7-8: 불변 필드 전환, long 유지 모두 적절합니다일(day) 정수와 집계 수(count)를 불변으로 고정하고 count를 long으로 둔 선택이 집계 시 오버플로·잘림 위험을 줄입니다.
src/main/java/com/example/cp_main_be/domain/mission/MissionAggregationService.java (1)
38-39: dailyQuestionCounts 연동 👍일일 질문 집계를 기존 일기·퀴즈와 동일한 흐름으로 합산하는 접근이 일관되고 명확합니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (1)
16-25: PostgreSQLDialect 사용 확인 및 HQL DAY() 함수 지원 검증 완료
src/main/resources/application.yml에서org.hibernate.dialect.PostgreSQLDialect설정 확인- Hibernate HQL의
DAY()함수는 내부적으로EXTRACT(DAY FROM ...)로 변환되어 PostgreSQL에서도 정상 작동합니다
Comment on lines
+16
to
+25
| @Query( | ||
| "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(DAY(dqa.answeredDate), COUNT(dqa.id)) " | ||
| + "FROM DailyQuestionAnswer dqa " | ||
| + "WHERE dqa.user = :user " | ||
| + " AND dqa.answeredDate BETWEEN CAST(:startDate AS date) AND CAST(:endDate AS date) " | ||
| + "GROUP BY DAY(dqa.answeredDate)") | ||
| List<MissionCountPerDay> findCompletedCountsPerDay( | ||
| @Param("user") User user, | ||
| @Param("startDate") LocalDateTime startDate, | ||
| @Param("endDate") LocalDateTime endDate); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
쿼리의 날짜 처리/그룹핑 재검토 권장 (이식성·정확성)
- BETWEEN CAST(:startDate AS date) 구문과 DAY()는 벤더 의존(Hibernate/MySQL 확장)입니다. JPA 표준/이식성을 높이려면 LocalDate 파라미터 + BETWEEN 조합을 권장합니다.
- DAY로만 그룹핑하면 월 경계에서 동일 일자가 합쳐지는 문제가 있습니다. 사용 시나리오가 단일 월로 고정되지 않는다면 날짜 단위 그룹핑 또는 ORDER BY 추가를 고려하세요.
제안 diff:
- @Query(
- "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(DAY(dqa.answeredDate), COUNT(dqa.id)) "
- + "FROM DailyQuestionAnswer dqa "
- + "WHERE dqa.user = :user "
- + " AND dqa.answeredDate BETWEEN CAST(:startDate AS date) AND CAST(:endDate AS date) "
- + "GROUP BY DAY(dqa.answeredDate)")
- List<MissionCountPerDay> findCompletedCountsPerDay(
- @Param("user") User user,
- @Param("startDate") LocalDateTime startDate,
- @Param("endDate") LocalDateTime endDate);
+ @Query(
+ "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(" +
+ " DAY(dqa.answeredDate), COUNT(dqa.id)) " +
+ "FROM DailyQuestionAnswer dqa " +
+ "WHERE dqa.user = :user " +
+ " AND dqa.answeredDate BETWEEN :startDate AND :endDate " +
+ "GROUP BY DAY(dqa.answeredDate) " +
+ "ORDER BY DAY(dqa.answeredDate)")
+ List<MissionCountPerDay> findCompletedCountsPerDay(
+ @Param("user") User user,
+ @Param("startDate") LocalDate startDate,
+ @Param("endDate") LocalDate endDate);참고: 월 경계 집계 문제가 우려된다면 DTO를 (int day) 대신 (LocalDate date)로 바꾸는 근본적 개선을 고려해주세요. 이 경우 AggregationService의 키도 LocalDate로 전환해야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Query( | |
| "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(DAY(dqa.answeredDate), COUNT(dqa.id)) " | |
| + "FROM DailyQuestionAnswer dqa " | |
| + "WHERE dqa.user = :user " | |
| + " AND dqa.answeredDate BETWEEN CAST(:startDate AS date) AND CAST(:endDate AS date) " | |
| + "GROUP BY DAY(dqa.answeredDate)") | |
| List<MissionCountPerDay> findCompletedCountsPerDay( | |
| @Param("user") User user, | |
| @Param("startDate") LocalDateTime startDate, | |
| @Param("endDate") LocalDateTime endDate); | |
| @Query( | |
| "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(" + | |
| " DAY(dqa.answeredDate), COUNT(dqa.id)) " + | |
| "FROM DailyQuestionAnswer dqa " + | |
| "WHERE dqa.user = :user " + | |
| " AND dqa.answeredDate BETWEEN :startDate AND :endDate " + | |
| "GROUP BY DAY(dqa.answeredDate) " + | |
| "ORDER BY DAY(dqa.answeredDate)") | |
| List<MissionCountPerDay> findCompletedCountsPerDay( | |
| @Param("user") User user, | |
| @Param("startDate") LocalDate startDate, | |
| @Param("endDate") LocalDate endDate); |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java
around lines 16–25: the query uses vendor-specific DAY() and CAST(... AS date)
which harms portability and grouping by DAY() merges same day-of-month across
months; change the query to compare and group by the date portion (e.g., use
DATE(dqa.answeredDate) BETWEEN :startDate AND :endDate and GROUP BY
DATE(dqa.answeredDate)) and change the method params to LocalDate startDate,
LocalDate endDate; additionally consider changing the DTO from int day to
LocalDate date (and update any AggregationService keys) if you need correct
per-calendar-day aggregation across month boundaries.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (1)
16-21: DAY() 기반 그룹핑은 월 경계에서 오집계 가능 + 벤더 의존 쿼리 제거 권장
- DAY()만 그룹핑하면 예: 8/01과 9/01이 합쳐질 수 있습니다.
- CAST(... AS date), DAY()는 벤더/방언 의존도가 높습니다. 날짜 단위(LocalDate)로 그룹핑하거나 최소한 YEAR/MONTH까지 포함해 그룹핑하세요.
최소 수정(안정성↑, 호환성↑):
- + " AND dqa.answeredDate BETWEEN CAST(:startDate AS date) AND CAST(:endDate AS date) " - + "GROUP BY DAY(dqa.answeredDate)") + + " AND dqa.answeredDate BETWEEN :startDate AND :endDate " + + "GROUP BY YEAR(dqa.answeredDate), MONTH(dqa.answeredDate), DAY(dqa.answeredDate) " + + "ORDER BY YEAR(dqa.answeredDate), MONTH(dqa.answeredDate), DAY(dqa.answeredDate)")근본 수정(권장, DTO 변경 필요: day:int → date:LocalDate):
- @Query( - "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(DAY(dqa.answeredDate), COUNT(dqa.id)) " - + "FROM DailyQuestionAnswer dqa " - + "WHERE dqa.user = :user " - + " AND dqa.answeredDate BETWEEN CAST(:startDate AS date) AND CAST(:endDate AS date) " - + "GROUP BY DAY(dqa.answeredDate)") + @Query( + "SELECT new com.example.cp_main_be.domain.mission.user_daily_mission.dto.MissionCountPerDay(" + + " FUNCTION('date', dqa.answeredDate), COUNT(dqa.id)) " + + "FROM DailyQuestionAnswer dqa " + + "WHERE dqa.user = :user " + + " AND dqa.answeredDate >= :startDateInclusive " + + " AND dqa.answeredDate < :endDateExclusive " + + "GROUP BY FUNCTION('date', dqa.answeredDate) " + + "ORDER BY FUNCTION('date', dqa.answeredDate)")DTO 예시(참고):
public record MissionCountPerDay(LocalDate date, long count) {}
🧹 Nitpick comments (2)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (2)
22-25: 기간 경계는 [start, end) 패턴 사용 및 파라미터 명확화 권장
- 시간 포함 비교는 포함/제외 경계 혼동을 유발합니다. endExclusive를 도입해 오프바이원/시간대 이슈를 최소화하세요.
적용 예:
- List<MissionCountPerDay> findCompletedCountsPerDay( - @Param("user") User user, - @Param("startDate") LocalDateTime startDate, - @Param("endDate") LocalDateTime endDate); + List<MissionCountPerDay> findCompletedCountsPerDay( + @Param("user") User user, + @Param("startDateInclusive") LocalDateTime startDateInclusive, + @Param("endDateExclusive") LocalDateTime endDateExclusive);서비스 단에서 endExclusive = endDateInclusive.plusDays(1).atStartOfDay()로 전달 권장.
16-21: 쿼리 성능 메모: 인덱스 확인
- 본 쿼리는 user 조건 + 날짜 범위 필터입니다.
(user_id, answered_date)복합 인덱스가 있으면 범위 스캔 최적화에 도움됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java(2 hunks)src/main/java/com/example/cp_main_be/domain/mission/MissionAggregationService.java(3 hunks)src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/dto/MissionCountPerDay.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/dto/MissionCountPerDay.java
- src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java
- src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java
- src/main/java/com/example/cp_main_be/domain/mission/MissionAggregationService.java
- src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java
- src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java
🔇 Additional comments (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (1)
14-15: 확인 결과:answeredDate필드 타입이LocalDate입니다.
DailyQuestionAnswer 엔티티의answeredDate가LocalDate로 선언되어 있으므로existsByUserAndAnsweredDate(User, LocalDate)시그니처는 필드 타입과 일치하여 별도 변경이 필요 없습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/example/cp_main_be/domain/mission/diary/dto/response/DiaryInfoResponse.java (4)
21-23: createdAt/updatedAt 시간대 명시 고려 + isPublic 키 정책 확인
LocalDateTime은 타임존 정보가 없어 소비자가 서로 다른 시간대로 해석할 수 있습니다. API 계약에 따라OffsetDateTime/Instant로 승격하거나, 유지 시@JsonFormat으로 포맷/타임존을 고정하는 방안을 검토하세요.isPublic만 camelCase로 고정되었습니다. 글로벌 네이밍 전략과의 일관성(예: snake_case 응답)을 팀 차원에서 확정하시길 권장합니다.예시(외부 변경 참고용):
import com.fasterxml.jackson.annotation.JsonFormat; // ... @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss") LocalDateTime createdAt, @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss") LocalDateTime updatedAt,또는
import java.time.OffsetDateTime; // ... OffsetDateTime createdAt, OffsetDateTime updatedAt,
49-56: 댓글 DTO 필드명 컨벤션 일치 제안(writer → writerName)상위 DTO는
writerName을 쓰고, 댓글 DTO는writer를 사용합니다. 응답 스키마의 일관성을 위해writerName으로 통일을 제안합니다. 브레이킹 체인지이므로 클라이언트 확인 필요.- public record CommentResponseDTO( - Long commentId, String profileImageUrl, String writer, String content) { + public record CommentResponseDTO( + Long commentId, String profileImageUrl, String writerName, String content) { public static CommentResponseDTO from(Comment comment) { - return new CommentResponseDTO( + return new CommentResponseDTO( comment.getId(), comment.getWriter().getProfileImageUrl(), comment.getWriter().getNickname(), comment.getContent()); }
26-27: 댓글 로딩 시 N+1/LazyInitialization 리스크 점검
diary.getComments().stream()에서comment.getWriter()의 프로필/닉네임까지 접근합니다. 연관관계가 LAZY고 트랜잭션 범위 밖이면 N+1 또는LazyInitializationException이 발생할 수 있습니다. 조회 계층에서JOIN FETCH/@EntityGraph/DTO 프로젝션 적용 여부를 확인하세요.권장 점검:
- Diary 조회 쿼리에
comments및comment.writerfetch 계획 존재 여부- Repository/QueryDSL에
join fetch또는@EntityGraph(attributePaths = {"comments", "comments.writer"})적용 여부
10-17: 레코드 컴포넌트 순서 변경 시 Factory 메서드만 호출하므로 내부 호출부도 함께 업데이트하세요
DiaryInfoResponse레코드 컴포넌트 순서를 변경할 경우, 동일 파일 내from()메서드(src/main/java/com/example/cp_main_be/domain/mission/diary/dto/response/DiaryInfoResponse.java라인 32)에 있는
new DiaryInfoResponse(...)호출부의 인자 순서도 반드시 같이 수정해야 합니다. 외부에 직접new DiaryInfoResponse(...)를 호출하는 코드는 없으므로, 다른 파일에선 영향이 없습니다.- 현재
@JsonProperty("isPublic")만 명시되어 있어,isLiked는 Jackson 기본 네이밍 전략(기본 lowerCamelCase →"liked")에 따라 직렬화됩니다. 응답 키 일관성을 위해isLiked에도@JsonProperty("isLiked")를 추가할 것을 권장합니다.적용 예시:
public record DiaryInfoResponse( Long id, Long writerId, String writerName, String profileImageUrl, String title, String content, String imageUrl, - boolean isLiked, + @JsonProperty("isLiked") boolean isLiked, @JsonProperty("isPublic") boolean isPublic ) { public static DiaryInfoResponse from(Diary diary) { return new DiaryInfoResponse( diary.getId(), diary.getUser().getId(), diary.getUser().getName(), diary.getUser().getProfileImageUrl(), diary.getTitle(), diary.getContent(), diary.getImageUrl(), - diary.isLiked(), + diary.isLiked(), diary.isPublic() ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/com/example/cp_main_be/domain/mission/diary/dto/response/DiaryInfoResponse.java(2 hunks)
🔇 Additional comments (1)
src/main/java/com/example/cp_main_be/domain/mission/diary/dto/response/DiaryInfoResponse.java (1)
35-36: 생성자 인자 순서 변경 반영 확인 — LGTM
writerName,profileImageUrl위치가 레코드 컴포넌트 순서와 일치하도록 올바르게 수정되었습니다. 별도 이슈 없습니다. (직접 생성자 호출부 존재 여부는 앞선 스크립트로 확인 권장)
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.
📝 개요
이번 PR의 핵심 내용을 한 줄로 요약해 주세요.
💻 작업 내용
이번 PR에서 작업한 내용을 상세히 설명해 주세요.
작업 내용 1
작업 내용 2
...
✅ PR 체크리스트
PR을 보내기 전에 아래 체크리스트를 확인해 주세요.
커밋 메시지는 포맷에 맞게 작성했나요?
스스로 코드를 다시 한번 검토했나요?
관련 이슈를 연결했나요?
빌드 및 테스트가 로컬에서 성공했나요?
🔗 관련 이슈
이번 PR과 관련된 이슈 번호를 기재해 주세요. 예: Closes #129
스크린샷 (선택)
UI 변경 사항이 있다면 스크린샷을 첨부해 주세요.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation