Conversation
- ArticlesPageResponse에서 getAllArticles 전용 DTO 분리 - GetAllArticlesResponse 신규 생성 - getAllArticles 메소드의 반환 타입 변경 - 관련 테스트 코드 업데이트 getAllArticlesByCategory는 기존 ArticlesPageResponse 유지
- User.isNewUser(LocalDateTime now) 메소드 추가 - 생성일로부터 3일 이내 사용자인지 판별 - 외부에서 현재 시간을 주입받아 테스트 용이성 확보
- GetAllArticlesResponse에 isNewUser 필드 추가 - getAllArticles에서 User.isNewUser() 메소드 활용 - 사용자 생성일로부터 3일 이내인지 판별하여 응답에 포함 - 관련 테스트 코드 업데이트
WalkthroughThe article “get all” flow now returns a new aggregated DTO (GetAllArticlesResponse) including an isNewUser flag. Controller, port, and use case signatures were updated. A new User.isNewUser(LocalDateTime) method was added. Tests were adjusted. No other logic changes beyond computing and returning the new flag. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant AC as ArticleController
participant UC as GetArticleUsecase
participant P as GetArticlePort
note over C,AC: GET /articles
C->>AC: Request all articles (paged)
AC->>UC: getAllArticles(user, pageQuery)
UC->>P: fetchArticlesWithUnreadCount(user, pageQuery)
P-->>UC: Articles + unreadCount + total
UC->>UC: isNewUser = user.isNewUser(now)
UC-->>AC: GetAllArticlesResponse(total, unread, isNewUser, articles)
AC-->>C: ResponseDto<GetAllArticlesResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional changes identified) 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. 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: 1
🧹 Nitpick comments (7)
domain/src/main/java/com/pinback/domain/user/entity/User.java (1)
3-3: Consider time zone explicitly for durabilityLocalDateTime ignores zone/offset. If createdAt is stored in UTC (or DB default) but “now” is server-local, edge cases around DST/time zone shifts can surface. Consider Instant/OffsetDateTime in BaseEntity and compute using a consistent ZoneId.
api/src/main/java/com/pinback/api/article/controller/ArticleController.java (1)
20-20: API response shape change: coordinate with FE and update Swagger schemaThe endpoint now returns GetAllArticlesResponse including isNewUser. Ensure FE expects this new shape and that OpenAPI schema documents the new field; otherwise clients will break.
Optionally, mention “isNewUser 포함” in the @operation description for discoverability.
Also applies to: 81-81, 87-87
application/src/main/java/com/pinback/application/article/usecase/query/GetArticleUsecase.java (1)
51-67: Inject time source for testability and consistency with other flowsUsing LocalDateTime.now() in application layer makes tests/time-travel brittle and diverges from your pattern in getRemindArticles (which takes now). Prefer Clock injection or pass now from controller.
Apply within this method:
- boolean isNewUser = user.isNewUser(LocalDateTime.now()); + boolean isNewUser = user.isNewUser(LocalDateTime.now(clock));Also add (outside this hunk):
// imports import java.time.Clock; // field private final Clock clock;Provide a Clock bean:
// e.g., in a @Configuration class @Bean public Clock clock() { return Clock.systemDefaultZone(); }Alternative: add LocalDateTime now param to the port/usecase, mirroring getRemindArticles.
I can prepare a minimal patch wiring Clock across the constructor and Spring config if you want.
application/src/main/java/com/pinback/application/article/dto/response/GetAllArticlesResponse.java (1)
5-13: Stabilize JSON field name for boolean and document via OpenAPIIn some Jackson setups, boolean “isXxx” can be normalized unexpectedly. Records usually preserve the component name, but to be explicit add @JsonProperty. Also consider @Schema docs for FE.
Apply:
+package com.pinback.application.article.dto.response; + +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import java.util.List; + -public record GetAllArticlesResponse( - long totalArticle, - long totalUnreadArticle, - boolean isNewUser, - List<ArticleResponse> articles -) { - public static GetAllArticlesResponse of(long totalArticle, long totalUnreadArticle, boolean isNewUser, List<ArticleResponse> articles) { - return new GetAllArticlesResponse(totalArticle, totalUnreadArticle, isNewUser, articles); - } -} +@Schema(name = "GetAllArticlesResponse") +public record GetAllArticlesResponse( + @Schema(description = "전체 아티클 수") long totalArticle, + @Schema(description = "읽지 않은 아티클 수") long totalUnreadArticle, + @JsonProperty("isNewUser") @Schema(description = "가입 후 3일 이내 여부") boolean isNewUser, + @Schema(description = "아티클 목록") List<ArticleResponse> articles +) { + public static GetAllArticlesResponse of(long totalArticle, long totalUnreadArticle, boolean isNewUser, List<ArticleResponse> articles) { + return new GetAllArticlesResponse(totalArticle, totalUnreadArticle, isNewUser, articles); + } +}application/src/test/java/com/pinback/application/article/usecase/query/GetArticleUsecaseTest.java (3)
55-55: Avoid time-based flakiness in tests.Don’t use LocalDateTime.now() in fixtures; pin to a fixed instant so assertions can be precise.
Apply this diff:
- ReflectionTestUtils.setField(user, "createdAt", LocalDateTime.now().minusDays(5)); + // Fixed timestamp to keep tests deterministic + ReflectionTestUtils.setField(user, "createdAt", LocalDateTime.of(2025, 8, 15, 12, 0));Additionally, consider injecting a java.time.Clock into GetArticleUsecase (and User.isNewUser) so you can pass a fixed clock in tests and eliminate direct uses of LocalDateTime.now().
124-124: Assert the actual value, not just presence.Capture the expected isNewUser from the domain before invoking the use case and assert equality.
Apply this diff:
- GetAllArticlesResponse response = getArticleUsecase.getAllArticles(user, pageQuery); + boolean expectedIsNew = user.isNewUser(LocalDateTime.now()); + GetAllArticlesResponse response = getArticleUsecase.getAllArticles(user, pageQuery);
128-129: Strengthen assertions (and verify aggregates).Check the computed flag and, optionally, the aggregate counters to lock behavior.
Apply this diff:
- assertThat(response.isNewUser()).isNotNull(); + assertThat(response.isNewUser()).isEqualTo(expectedIsNew); + // Optional but recommended: verify totals mapped from ArticlesWithUnreadCountDto + assertThat(response.totalArticle()).isEqualTo(5L); + assertThat(response.totalUnreadArticle()).isEqualTo(3L);Optional extra boundary test to clarify the 3-day rule semantics:
@Test void getAllArticles_isNewUser_boundaryAtExactly3Days() { // given User u = user(); ReflectionTestUtils.setField(u, "id", java.util.UUID.randomUUID()); LocalDateTime fixedNow = LocalDateTime.of(2025, 8, 20, 15, 0); ReflectionTestUtils.setField(u, "createdAt", fixedNow.minusDays(3)); // exactly 3 days PageQuery pageQuery = new PageQuery(0, 1); PageRequest pageRequest = PageRequest.of(0, 1); Page<Article> page = new PageImpl<>(List.of(article), pageRequest, 1); when(articleGetServicePort.findAll(u, pageRequest)).thenReturn(new ArticlesWithUnreadCountDto(0L, page)); // when GetAllArticlesResponse resp = getArticleUsecase.getAllArticles(u, pageQuery); // then: decide expected per spec (exactly 3 days = new or not) // assertThat(resp.isNewUser()).isFalse(); // or isTrue(); pick and document the rule }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/src/main/java/com/pinback/api/article/controller/ArticleController.java(2 hunks)application/src/main/java/com/pinback/application/article/dto/response/GetAllArticlesResponse.java(1 hunks)application/src/main/java/com/pinback/application/article/port/in/GetArticlePort.java(2 hunks)application/src/main/java/com/pinback/application/article/usecase/query/GetArticleUsecase.java(2 hunks)application/src/test/java/com/pinback/application/article/usecase/query/GetArticleUsecaseTest.java(3 hunks)domain/src/main/java/com/pinback/domain/user/entity/User.java(2 hunks)
🔇 Additional comments (3)
application/src/main/java/com/pinback/application/article/port/in/GetArticlePort.java (1)
8-8: All getAllArticles implementors and callers updated to GetAllArticlesResponse No leftover ArticlesPageResponse references in the use case, controller, or tests; API annotations will generate the updated payload schema.application/src/main/java/com/pinback/application/article/usecase/query/GetArticleUsecase.java (1)
61-66: Confirm count semantics match paging contentYou’re returning result.article().getTotalElements() with a mapped page content. This is fine; just confirm FE understands totalArticle is total elements, not current page size.
application/src/test/java/com/pinback/application/article/usecase/query/GetArticleUsecaseTest.java (1)
28-28: LGTM: import updated to new response type.Import aligns with the API change to GetAllArticlesResponse.
| public boolean isNewUser(LocalDateTime now) { | ||
| return getCreatedAt().isAfter(now.minusDays(3)); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify 3-day boundary and guard against null createdAt
Spec says “3일이 지났는지”; current isAfter(now.minusDays(3)) treats exactly 72h as NOT new and NPEs if createdAt is null. Confirm boundary (strict vs inclusive). If inclusive “within first 3 days” is intended, use !isBefore(threshold) and null-guard.
Apply:
- public boolean isNewUser(LocalDateTime now) {
- return getCreatedAt().isAfter(now.minusDays(3));
- }
+ public boolean isNewUser(LocalDateTime now) {
+ LocalDateTime createdAt = getCreatedAt();
+ if (createdAt == null) return false; // or true based on biz rule
+ LocalDateTime threshold = now.minusDays(3);
+ // true when createdAt is within the last 3 days (inclusive of exactly 3 days ago)
+ return !createdAt.isBefore(threshold);
+ }If the business rule is “strictly more than 3 days have passed,” then rename method or invert accordingly:
- hasPassed3Days = createdAt.isBefore(now.minusDays(3)) (strict)
- isNewUser = !hasPassed3Days
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate User.java
user_file=$(fd -t f -p 'User.java' domain/src/main/java/com/pinback/domain/user/entity)
echo "User file: $user_file"
# Show createdAt field and surrounding annotations
echo -e "\n=== Field Declaration and Annotations ==="
rg -nP '^(?:@.+\r?\n)*\s*private\s+LocalDateTime\s+createdAt' "$user_file"
# Show getCreatedAt() method
echo -e "\n=== getCreatedAt() Method ==="
rg -nP 'public\s+LocalDateTime\s+getCreatedAt' -C2 "$user_file"
# Show isNewUser() method context
echo -e "\n=== isNewUser() Method ==="
rg -nP 'public\s+boolean\s+isNewUser' -C3 "$user_file"Length of output: 549
🏁 Script executed:
#!/bin/bash
# Locate User.java without exiting on no match
file=$(find domain/src/main/java/com/pinback/domain/user/entity -type f -name User.java)
echo "User file: $file"
echo -e "\n=== Class Declaration ==="
rg -nP 'class\s+User' -C2 "$file" || true
echo -e "\n=== createdAt Field or Inheritance ==="
# Check for direct declaration or inherited auditing fields
rg -nP 'LocalDateTime\s+createdAt' -C2 "$file" || true
rg -nP 'extends?\s+\w+' -C1 "$file" || true
echo -e "\n=== getCreatedAt() Method ==="
rg -nP 'public\s+LocalDateTime\s+getCreatedAt' -C3 "$file" || true
echo -e "\n=== isNewUser() Method ==="
rg -nP 'public\s+boolean\s+isNewUser' -C3 "$file" || trueLength of output: 1500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate BaseEntity.java
base_file=$(fd -t f -p BaseEntity.java domain/src/main/java/com/pinback)
echo "BaseEntity file: $base_file"
# Show class declaration
echo -e "\n=== Class Declaration ==="
rg -nP 'class\s+BaseEntity' -C2 "$base_file" || true
# Show createdAt field and annotations
echo -e "\n=== createdAt Field Declaration ==="
rg -nP 'LocalDateTime\s+createdAt' -C2 "$base_file" || true
# Show getCreatedAt() method implementation
echo -e "\n=== getCreatedAt() Method ==="
rg -nP 'public\s+LocalDateTime\s+getCreatedAt' -C3 "$base_file" || true
# Check for JPA auditing annotations on createdAt
echo -e "\n=== JPA Auditing Annotations (@CreatedDate) ==="
rg -nP '@CreatedDate' -C2 "$base_file" || trueLength of output: 1489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="domain/src/main/java/com/pinback/domain/common/BaseEntity.java"
echo "=== Searching for getCreatedAt definition in $file ==="
rg -nP 'public\s+LocalDateTime\s+getCreatedAt' -C3 "$file" || true
echo -e "\n=== Showing first 80 lines of $file ==="
sed -n '1,80p' "$file"Length of output: 1433
Guard null createdAt and clarify 3-day boundary
Current isNewUser(now) will NPE if createdAt is null and treats exactly 72 hours as “not new.” Add a null check and use an inclusive threshold if you intend “within first 3 days”:
public boolean isNewUser(LocalDateTime now) {
- return getCreatedAt().isAfter(now.minusDays(3));
+ LocalDateTime createdAt = getCreatedAt();
+ if (createdAt == null) {
+ return false; // or true per business rule
+ }
+ LocalDateTime threshold = now.minusDays(3);
+ // inclusive: true when createdAt is within last 3 days (>= exactly 3 days ago)
+ return !createdAt.isBefore(threshold);
}🤖 Prompt for AI Agents
In domain/src/main/java/com/pinback/domain/user/entity/User.java around lines
59-61, the isNewUser(LocalDateTime now) method can NPE if createdAt is null and
currently treats exactly 72 hours as not new; add a null check to return false
when createdAt is null and change the comparison to include the 3-day boundary
(e.g., compute threshold = now.minusDays(3) and return
!createdAt.isBefore(threshold) or use isAfter(threshold) || isEqual(threshold))
so users created exactly 3 days ago are considered new.
2 participants
🚀 PR 요약
대시보드 조회시 사용자의 최초 가입일을 기준으로 요청일로 부터 3일이 지났는지 클라이언트에 반환하도록 코드를 작성했습니다.
close #95
✨ PR 상세 내용
User는 LocalDateTime now를 받아 자신이 새로운 유저인지 판별하고 결과 값을 boolean으로 반환하도록 코드를 수정했습니다.
모든 아티클 조회시 해당 결과값을 반영하도록 dto 필드를 수정했습니다.
🚨 주의 사항
없습니다.
✅ 체크 리스트
Summary by CodeRabbit