fix: QA에서 발견한 오류 수정 #75
Conversation
Walkthrough앨범 정보 조회가 메이커(MAKER) 사용자 조회를 포함하도록 변경되었고(메이커 미존재 시 예외), AlbumInfoResponse에 메이커 이름 필드가 추가되었습니다. AlbumValidator의 이벤트 날짜 검증은 한국 표준시(KST, +09:00)로 고정되었고, 프로필 이미지 경로 문자열과 S3 URL 추출 로직 일부가 수정되었습니다. PhotoMapper의 album→응답 매핑 메서드는 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as 클라이언트
participant AlbumService as AlbumService
participant UserAlbumRepo as UserAlbumRepository
participant AlbumMapper as AlbumMapper
participant ClientResp as 클라이언트응답
rect rgba(203,232,255,0.35)
note right of UserAlbumRepo: 신규/변경된 흐름: 메이커 조회 및 메이커 기반 매핑
end
Client->>AlbumService: getAlbumInfo(albumId)
AlbumService->>UserAlbumRepo: findMakerByAlbumId(albumId, Role.MAKER)
alt maker found
UserAlbumRepo-->>AlbumService: UserAlbum (with User)
AlbumService->>AlbumMapper: toAlbumInfoResponse(album, makerUser)
AlbumMapper-->>AlbumService: AlbumInfoResponse (name 포함)
AlbumService-->>ClientResp: 200 OK (AlbumInfoResponse)
else maker not found
UserAlbumRepo-->>AlbumService: empty
AlbumService-->>ClientResp: 오류 (USER_NOT_MAKER)
end
sequenceDiagram
autonumber
participant Client as 클라이언트
participant AlbumService as AlbumService
participant Validator as AlbumValidator
rect rgba(220,247,200,0.35)
note right of Validator: 변경: KST_ZONE(ZoneOffset +09:00) 사용
end
Client->>AlbumService: 앨범 생성 요청 (eventDate)
AlbumService->>Validator: validate(eventDate)
Validator->>Validator: now = LocalDate.now(KST_ZONE)
alt eventDate < now
Validator-->>AlbumService: validation fail
AlbumService-->>Client: 오류 응답
else
Validator-->>AlbumService: validation pass
AlbumService-->>Client: 생성 성공
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code graph analysis (1)src/main/java/com/cheeeese/album/application/AlbumService.java (1)
🔇 Additional comments (6)
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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/user/domain/type/ProfileImageType.java(1 hunks)
🔇 Additional comments (1)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
18-18: LGTM! 타임존 처리를 위한 적절한 import입니다.서울 시간대를 명시적으로 사용하기 위해
ZoneId를 import한 것은 올바른 접근입니다.
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java
Outdated
Show resolved
Hide resolved
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 (1)
src/main/java/com/cheeeese/user/dto/response/UserProfileImageResponse.java (1)
39-39: Swagger 예시 문자열 불일치 수정 필요Line 39의
@Schema예시가 여전히 이전 경로 형식(signup_profile_5.jpg)을 사용하고 있습니다. Lines 17, 21에서 수정한 것과 동일하게sign_up_profile_5.jpg로 업데이트해야 합니다.다음 diff를 적용하여 예시를 일관되게 수정하세요:
@Schema( description = "프로필 이미지의 CDN URL", - example = "https://say-cheese-profile.edge.naverncp.com/profile/signup_profile_5.jpg" + example = "https://say-cheese-profile.edge.naverncp.com/profile/sign_up_profile_5.jpg" ) String profileImageUrl
♻️ Duplicate comments (1)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
27-27: KST 타임존 상수 정의 개선 확인KST 타임존 상수를 정의한 것은 좋은 개선입니다. 다만, 이전 리뷰에서 지적된 것처럼 이 상수를 공통 유틸 클래스로 이동하여
Album.isExpired(),AlbumService,AlbumQueryService,PhotoService등 코드베이스 전체에서 일관되게 사용하는 것이 좋습니다.현재는 이 클래스에만 국한된 변경이므로 점진적 개선으로 볼 수 있지만, 향후 리팩토링을 통해 전사적으로 타임존 처리를 통일하는 것을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/user/dto/response/UserProfileImageResponse.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (2)
44-44: KST 기준 날짜 검증 로직 개선 승인이벤트 날짜 검증에 KST 타임존을 명시적으로 적용한 것은 올바른 수정입니다. 이를 통해 한국 표준시 기준으로 일관된 날짜 비교가 가능합니다.
79-81: 업로드 권한 검증 메서드 추가 승인
validateUploadPermission메서드를 통해 업로드 권한을 명시적으로 검증하는 로직이 추가되었습니다. 기존validateAlbumParticipant메서드를 재사용하여 적절히 구현되었습니다.src/main/java/com/cheeeese/user/dto/response/UserProfileImageResponse.java (1)
17-22: 프로필 이미지 URL 경로 수정 승인CDN 캐시 오류 해결을 위해 프로필 이미지 URL 경로를
signup_profile에서sign_up_profile로 수정한 것이 올바릅니다. 이는 실제 CDN에 저장된 파일명과 일치하도록 하는 필수적인 수정입니다.
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
25-25: KST_ZONE 상수를 공통 클래스로 분리하는 것을 권장합니다.
AlbumExpirationRedisRepository에도 동일한 KST_ZONE 상수가 존재하여 중복이 발생하고 있습니다. 타임존 상수를 여러 곳에서 중복 정의하면 유지보수가 어려워집니다.공통 유틸 클래스(예:
TimeZoneConstants또는DateTimeUtils)에 KST_ZONE을 정의하고, 이를AlbumValidator와AlbumExpirationRedisRepository모두에서 참조하도록 리팩토링하는 것을 고려해주세요.예시:
// com.cheeeese.common.util.TimeZoneConstants public final class TimeZoneConstants { public static final ZoneOffset KST_ZONE = ZoneOffset.of("+09:00"); private TimeZoneConstants() { throw new UnsupportedOperationException(); } }그런 다음 이 파일에서:
- private static final ZoneOffset KST_ZONE = ZoneOffset.of("+09:00"); + private static final ZoneOffset KST_ZONE = TimeZoneConstants.KST_ZONE;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/user/dto/response/UserProfileImageResponse.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cheeeese/user/dto/response/UserProfileImageResponse.java
🔇 Additional comments (1)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
18-18: 필요한 import 추가입니다.KST_ZONE 상수 사용을 위한 필수 import이며, 이전 리뷰에서 언급된 미사용 import들은 정리된 것으로 확인됩니다.
| } | ||
|
|
||
| if (request.eventDate().isAfter(LocalDate.now())) { | ||
| if (request.eventDate().isAfter(LocalDate.now(KST_ZONE))) { |
There was a problem hiding this comment.
🧩 Analysis chain
이벤트 날짜 검증에 KST 적용이 올바르게 수정되었습니다.
LocalDate.now(KST_ZONE)를 사용하여 한국 표준시 기준으로 날짜를 비교하도록 변경한 것은 PR 목적에 부합하는 올바른 수정입니다.
다만, 이전 리뷰에서 지적된 대로 Album.isExpired(), AlbumService, AlbumQueryService, PhotoService 등 다른 곳에서도 타임존 없이 LocalDate.now() 또는 LocalDateTime.now()를 사용하는 부분들이 있는지 확인이 필요합니다. 특히 Album.isExpired() 메서드는 우선적으로 동일한 타임존 처리가 적용되어야 합니다.
다음 스크립트로 남아있는 타임존 미지정 케이스를 확인해주세요:
🏁 Script executed:
#!/bin/bash
# 타임존이 지정되지 않은 LocalDate.now() 및 LocalDateTime.now() 사용 확인
echo "=== 타임존 미지정 LocalDate.now() 확인 ==="
rg -n --type=java 'LocalDate\.now\(\)' -C2
echo -e "\n=== 타임존 미지정 LocalDateTime.now() 확인 ==="
rg -n --type=java 'LocalDateTime\.now\(\)' -C2
echo -e "\n=== Album.isExpired() 메서드 확인 ==="
ast-grep --pattern $'class Album {
$$$
isExpired($_) {
$$$
}
$$$
}'Length of output: 7436
타임존 미지정 문제가 여러 곳에서 발견되어 추가 수정이 필요합니다.
이 변경사항은 올바르지만, 검증 결과 다음 핵심 파일들에서도 타임존 없이 LocalDateTime.now() 및 LocalDate.now()를 사용하고 있습니다:
- Album.java line 66:
isExpired()메서드에서LocalDateTime.now()사용 - AlbumService.java line 75: 앨범 만료 시간 계산에서
LocalDateTime.now().plusDays(7)사용 - AlbumQueryService.java lines 52, 67: 활성 앨범 조회 시
LocalDateTime.now()사용 - AlbumService.java lines 256-257: 주간 앨범 개수 조회에서
LocalDate.now()사용
이들은 모두 KST_ZONE을 포함하여 동일하게 수정되어야 데이터 일관성이 보장됩니다.
🤖 Prompt for AI Agents
In src/main/java/com/cheeeese/album/domain/Album.java around line 66,
src/main/java/com/cheeeese/album/application/AlbumService.java around lines 75
and 256-257, and
src/main/java/com/cheeeese/album/application/AlbumQueryService.java around lines
52 and 67, various uses of LocalDateTime.now() and LocalDate.now() lack a
timezone; update each occurrence to use the shared KST_ZONE (e.g.,
LocalDateTime.now(KST_ZONE) or LocalDate.now(KST_ZONE)) so all time computations
and comparisons use the same ZoneId constant, and ensure the KST_ZONE
constant/import is referenced where needed.
🔗 연관된 이슈
🚀 변경 유형
📝 작업 내용
💬 리뷰 요구사항
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.
Summary by CodeRabbit
신규 기능
버그 수정