[BUG] 챌린지 인증 게시물 로직 수정 #190
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough인증 게시물의 이미지 구조를 단일 Changes
Sequence DiagramsequenceDiagram
participant Client as 클라이언트
participant Service as VerificationService
participant Repo as DB (Verification/Challenge)
participant Verification as Verification (엔티티)
Client->>Service: update/delete 요청 (verificationId, payload)
rect rgb(230,245,255)
Note over Service: 검증 윈도우 확인 시작
Service->>Repo: 조회 getVerification(verificationId)
Repo-->>Service: Verification (createdAt, images...)
Service->>Repo: getChallenge(challengeId)
Repo-->>Service: Challenge (startTime, endTime, days...)
Service->>Service: getWindowAnchorDate(challenge, now)
Service->>Service: isVerificationDay(challenge, anchorDate)
Service->>Service: isWithinVerificationTime(challenge, now)
end
alt 윈도우 내 && 동일 윈도우에서 생성된 게시물
Service->>Verification: update(... textImage1/2/3 ...) / delete()
Verification->>Repo: save() / delete()
Repo-->>Client: 200 OK
else 외부 시도
Service-->>Client: 400 VERIFICATION_EDIT_DELETE_NOT_ALLOWED
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 (5)
src/main/resources/db/migration/V2.24__add_text_image_to_verification.sql (1)
1-4: VARCHAR(255) 길이 제한 검토 권장텍스트 이미지 URL을 저장하는 컬럼의 길이가 255자로 제한되어 있습니다. 현대의 CDN URL이나 서명된 URL(query parameter 포함)은 이 길이를 초과할 수 있습니다.
향후 URL 길이 초과로 인한 데이터 손실을 방지하려면
VARCHAR(512)또는TEXT타입 사용을 고려해보세요.🔎 제안하는 수정안
ALTER TABLE verification - ADD COLUMN text_image1 VARCHAR(255) NULL, - ADD COLUMN text_image2 VARCHAR(255) NULL, - ADD COLUMN text_image3 VARCHAR(255) NULL; + ADD COLUMN text_image1 VARCHAR(512) NULL, + ADD COLUMN text_image2 VARCHAR(512) NULL, + ADD COLUMN text_image3 VARCHAR(512) NULL;src/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.java (1)
15-17: 텍스트 이미지 필드 추가 확인세 개의 텍스트 이미지 필드가 적절하게 추가되었습니다. 필드가 선택적(nullable)으로 설정되어 유연성을 제공합니다.
선택적 개선사항: URL 형식 검증을 추가하면 잘못된 데이터 입력을 사전에 방지할 수 있습니다. 예를 들어
@Pattern또는 커스텀 URL 검증 어노테이션을 고려해보세요.🔎 URL 검증 예시
import jakarta.validation.constraints.Pattern; @Pattern(regexp = "^https?://.*", message = "올바른 URL 형식이 아닙니다.") private String textImage1; @Pattern(regexp = "^https?://.*", message = "올바른 URL 형식이 아닙니다.") private String textImage2; @Pattern(regexp = "^https?://.*", message = "올바른 URL 형식이 아닙니다.") private String textImage3;src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
84-88: 반복되는 null-check 패턴을 헬퍼 메서드로 추출하면 좋겠습니다.
field != null ? s3UrlUtil.toFullUrl(field) : null패턴이toResponseDto,toDetailDto,toHistoryDto에서 반복됩니다.firstNonNull처럼 헬퍼 메서드로 추출하면 가독성이 향상됩니다.🔎 헬퍼 메서드 예시
private String toFullUrlOrNull(String key) { return key != null ? s3UrlUtil.toFullUrl(key) : null; }사용 예:
-String fullPhotoUrl = verification.getPhotoUrl() != null ? s3UrlUtil.toFullUrl(verification.getPhotoUrl()) : null; +String fullPhotoUrl = toFullUrlOrNull(verification.getPhotoUrl());src/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.java (2)
121-123: API 문서화를 위해@Schema어노테이션 추가를 권장합니다.다른 필드들과 달리
textImage1/2/3필드에@Schema어노테이션이 누락되어 있습니다. Swagger 문서 일관성을 위해 추가하면 좋겠습니다.🔎 예시
+ @Schema(description = "텍스트 인증 이미지 1", example = "https://example.com/image1.jpg") private String textImage1; + @Schema(description = "텍스트 인증 이미지 2", example = "https://example.com/image2.jpg") private String textImage2; + @Schema(description = "텍스트 인증 이미지 3", example = "https://example.com/image3.jpg") private String textImage3;
175-178:HistoryDto에도 동일하게@Schema어노테이션이 누락되어 있습니다.위
CreateResponseDto와 마찬가지로 API 문서화 일관성을 위해 추가해 주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationDetailResponseDto.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationRequestDto.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.javasrc/main/java/com/hrr/backend/domain/verification/entity/Verification.javasrc/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/resources/db/migration/V2.24__add_text_image_to_verification.sql
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.java (1)
VerificationResponseDto(16-200)
🔇 Additional comments (12)
src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
126-126: 에러 코드 추가 확인 완료인증 시간 제약 위반을 나타내는 에러 코드가 적절하게 추가되었습니다.
BAD_REQUEST사용은 타이밍 검증 실패를 나타내기에 적합합니다.참고: 일부 REST API 설계 가이드에서는 시간 기반 접근 제한을
403 FORBIDDEN으로 처리하기도 하지만, 현재 코드베이스의 다른 타이밍 제약(예:ROUND_NOT_STARTED)도BAD_REQUEST를 사용하므로 일관성이 유지됩니다.src/main/java/com/hrr/backend/domain/verification/dto/VerificationDetailResponseDto.java (1)
34-36: 응답 DTO 필드 추가 완료 👍텍스트 이미지 필드가 깔끔하게 추가되었습니다. 기존 이미지 관련 필드들과 논리적으로 그룹화되어 있어 가독성이 좋습니다.
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
225-230: 깔끔한 헬퍼 메서드입니다! 👍간결하고 의도가 명확합니다.
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (5)
386-389: BLOCKED 상태 체크 추가 - 좋습니다!수정/삭제 시 차단된 게시글에 대한 접근 제어가 적절히 추가되었습니다.
Also applies to: 448-451
536-538:start.equals(end)케이스 검토가 필요합니다.
start == end인 경우 인증 윈도우가 사실상 0분이 됩니다. 이 조건에서!now.isBefore(start) && !now.isAfter(end)는 정확히 해당 시각에만true를 반환합니다. 의도된 동작인지 확인해 주세요. 만약 24시간 인증을 의미한다면 별도 처리가 필요합니다.
551-565: 자정을 넘기는 인증 윈도우 처리 로직이 잘 구현되었습니다.22:00~02:00 같은 overnight 케이스에서 기준 날짜를 정확히 계산합니다.
578-588: Switch expression 사용 - 깔끔합니다!Java 14+ switch expression을 활용해 간결하게 매핑했습니다. 만약
ChallengeDays에fromDayOfWeek정적 메서드가 있다면 활용할 수 있지만, 현재 구현도 명확합니다.
203-206:textImage1/2/3필드 연동이 올바르게 구현되었습니다.DTO에서 엔티티로의 데이터 전달이 정확합니다.
src/main/java/com/hrr/backend/domain/verification/entity/Verification.java (4)
57-59: 새로운textImage필드 추가 - 구조가 명확합니다.텍스트 인증에 최대 3개 이미지를 지원하는 요구사항이 잘 반영되었습니다.
178-186: 이미지 필드 삭제(초기화) 기능이 없습니다.현재 로직에서
null은 "변경하지 않음"을 의미합니다. 사용자가 업로드한 이미지를 제거하고 싶은 경우 처리 방법이 없습니다. 의도된 동작인지 확인해 주세요.만약 이미지 삭제가 필요하다면:
- 빈 문자열(
"")을 "삭제" 신호로 사용하거나- 별도
clearTextImages()메서드를 추가하는 방안을 고려해 주세요.
84-111: TEXT 인증 팩토리 메서드가 올바르게 구현되었습니다.
textImage1/2/3필드가 적절히 빌더에 전달되고,photoUrl은 TEXT 타입에서 사용되지 않습니다.
160-172: 부분 업데이트 패턴이 잘 적용되었습니다.
null은 "변경 없음", 빈 문자열은 에러 처리로 명확하게 구분됩니다.
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java
Outdated
Show resolved
Hide resolved
| // textImage1/2/3 (null 허용, 최대 3개 필드) | ||
| private String textImage1; | ||
| private String textImage2; | ||
| private String textImage3; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for VerificationService implementation and textImage field usage
rg -n "textImage|VerificationService" --type java | head -50Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 6155
🏁 Script executed:
# Find the VerificationService file(s)
fd -t f "VerificationService" --extension javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 247
🏁 Script executed:
# Check the DTO file to understand context
cat -n src/main/java/com/hrr/backend/domain/verification/dto/VerificationRequestDto.java 2>/dev/null || echo "File not found, searching..."
fd -t f "VerificationRequestDto" --extension javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1057
🏁 Script executed:
cat -n src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 28679
🏁 Script executed:
# Also check for any validation annotations or custom validators
rg -n "@Valid|validate|textImage" src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 292
🏁 Script executed:
# Check if there's a custom validator for textImage fields
rg -n "@Validated|@Valid|Validator" src/main/java/com/hrr/backend/domain/verification/ --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 680
🏁 Script executed:
# Search for any validation logic on textImage fields
rg -n "textImage|isEmpty|isBlank|validate" src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 292
🏁 Script executed:
# Check if Verification entity has any validation logic
cat -n src/main/java/com/hrr/backend/domain/verification/entity/Verification.java | grep -A 20 "createTextVerification"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 958
🏁 Script executed:
# Check if there are any other constraints or validators in the codebase
rg -n "@Size|@Length|@Max|maxItems" src/main/java/com/hrr/backend/domain/verification/dto/ --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 224
🏁 Script executed:
# Check VerificationUpdateRequestDto for textImage field validation
cat -n src/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 638
DTO 주석의 "최대 3개 필드" 제약이 실제로 강제되지 않습니다.
검증 결과, VerificationRequestDto와 VerificationUpdateRequestDto의 textImage1/2/3 필드에 대한 유효성 검사가 전혀 구현되어 있지 않습니다. DTO 주석에서 "최대 3개 필드"를 명시했으나, 서비스 레이어에서 이 제약을 강제하는 검증 로직이 없고, 세 필드 모두 동시에 값을 가질 수 있습니다.
다음과 같이 개선하세요:
- DTO에
@Size또는 커스텀 validator를 추가하여 최대 3개 제약을 강제합니다. 예: 모든 이미지 필드가 null이 아닌 경우 유효성 검사 실패 - 인증 타입별 이미지 필드 사용 규칙(TEXT vs CAMERA)이 필요하다면, 서비스 레이어에서 명시적으로 검증합니다
- 빈 문자열(
"")과 null을 구분하여 처리하는 로직을 추가합니다 (현재는 모두 허용 중)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationRequestDto.java (1)
23-28: DTO 레벨에서 이미지 필드 개수 제약 검증이 여전히 누락되어 있습니다.이전 리뷰에서 지적된 내용이 아직 해결되지 않았습니다.
textImage1,textImage2,textImage3필드 모두에 대한 개수 제한이나 상호 배타적 검증 로직이 없습니다. 엔티티의normalizeOptionalImageKey는 개별 필드의 유효성만 검사하며, "최대 3개"라는 제약은 실제로 강제되지 않습니다.다음 중 하나를 선택하여 개선하세요:
- 커스텀 validator를 생성하여 클래스 레벨에서 검증 (
@ImageFieldsValidator)- 서비스 레이어에서 명시적으로 검증하고 적절한 에러 메시지 반환
🔎 커스텀 validator 예시
@Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Constraint(validatedBy = ImageFieldsValidator.class) public @interface ValidImageFields { String message() default "이미지 필드는 최대 3개까지만 허용됩니다"; Class<?>[] groups() default {}; Class<? extends Payload>[] payload() default {}; } public class ImageFieldsValidator implements ConstraintValidator<ValidImageFields, VerificationRequestDto> { @Override public boolean isValid(VerificationRequestDto dto, ConstraintValidatorContext context) { if (dto == null) return true; int count = 0; if (dto.getTextImage1() != null && !dto.getTextImage1().isBlank()) count++; if (dto.getTextImage2() != null && !dto.getTextImage2().isBlank()) count++; if (dto.getTextImage3() != null && !dto.getTextImage3().isBlank()) count++; return count <= 3; } }그런 다음 DTO 클래스에 애노테이션 추가:
+@ValidImageFields @Schema(description = "글 인증 생성 요청 DTO") public class VerificationRequestDto {
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
226-231: 표준 라이브러리 메서드 활용을 고려해보세요.
firstNonNull헬퍼 메서드는 정상적으로 작동하지만, Java 표준 라이브러리의Stream.of(...).filter(Objects::nonNull).findFirst().orElse(null)또는 Guava의Iterables.getFirst()같은 검증된 유틸리티를 활용하면 코드 중복을 줄이고 가독성을 높일 수 있습니다.🔎 표준 라이브러리 활용 예시
+import java.util.Objects; +import java.util.stream.Stream; + private String firstNonNull(String a, String b, String c) { - if (a != null) return a; - if (b != null) return b; - if (c != null) return c; - return null; + return Stream.of(a, b, c) + .filter(Objects::nonNull) + .findFirst() + .orElse(null); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationRequestDto.javasrc/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.javasrc/main/java/com/hrr/backend/domain/verification/entity/Verification.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.java (1)
VerificationResponseDto(16-200)
🔇 Additional comments (3)
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
38-51: 이전 리뷰의 enum 비교 이슈가 올바르게 수정되었습니다!Line 38에서
.name().equals("CAMERA")대신 직접 enum 비교(== VerificationPostType.CAMERA)를 사용하도록 개선되었습니다. 타입 안전성이 향상되었고 리팩토링에도 강건해졌습니다. 👍src/main/java/com/hrr/backend/domain/verification/entity/Verification.java (2)
173-181: 이미지 필드 업데이트 로직이 명확합니다.각
textImage필드에 대해 null 체크와normalizeOptionalImageKey를 통한 검증을 수행하는 방식이 일관적이고 안전합니다. 빈 문자열이나 "null" 문자열을 적절히 필터링하고 있습니다.
187-196: 유효성 검증 헬퍼 메서드가 잘 구현되었습니다.
normalizeOptionalImageKey는 빈 문자열과 "null" 문자열을 명시적으로 검증하여 잘못된 이미지 키 입력을 효과적으로 방지합니다.trim()처리도 적절하며,VERIFICATION_INVALID_IMAGE_KEY에러 코드(VERIFICATION4008)가ErrorCode에HttpStatus.BAD_REQUEST와 함께 명확한 에러 메시지로 정의되어 있습니다. 구현이 일관성 있게 이루어졌습니다.
| public void update( | ||
| String title, | ||
| String content, | ||
| String textUrl, | ||
| String textImage1, | ||
| String textImage2, | ||
| String textImage3 | ||
| ) { |
There was a problem hiding this comment.
Critical: update 메서드 시그니처에 isQuestion 파라미터가 누락되었습니다.
Lines 183-185에서 isQuestion 변수를 참조하지만, 메서드 시그니처에는 해당 파라미터가 선언되어 있지 않습니다. 이는 컴파일 에러를 발생시키거나 의도하지 않은 필드 접근을 유발할 수 있습니다.
🔎 수정 제안
public void update(
String title,
String content,
String textUrl,
String textImage1,
String textImage2,
- String textImage3
+ String textImage3,
+ Boolean isQuestion
) {또는 Lines 183-185의 isQuestion 처리 로직이 불필요하다면 해당 블록을 제거하세요.
📝 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.
| public void update( | |
| String title, | |
| String content, | |
| String textUrl, | |
| String textImage1, | |
| String textImage2, | |
| String textImage3 | |
| ) { | |
| public void update( | |
| String title, | |
| String content, | |
| String textUrl, | |
| String textImage1, | |
| String textImage2, | |
| String textImage3, | |
| Boolean isQuestion | |
| ) { |
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/verification/entity/Verification.java
around lines 152-159 (method signature) and lines 183-185 (usage), the
update(...) method is missing the boolean isQuestion parameter referenced later,
causing compilation errors; add a boolean isQuestion parameter to the method
signature and propagate it into the method body to set the corresponding field
(or, if that logic is truly unnecessary, remove the lines 183-185 that reference
isQuestion instead). Ensure the method signature, any callers, and the
assignment inside the method are updated consistently.
yc3697
left a comment
yc3697
left a comment
There was a problem hiding this comment.
update 메서드에 isQuestion 누락된 거랑, 에러 코드 중복된 거 수정해주시고
마이그레이션 파일은 멱등성 만족하게 해당 칼럼들이 없을 경우에만 추가될 수 있도록 수정해주세요!
…hrr-server into bug/#182-verification-update-and-delete
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @src/main/resources/db/migration/V2.25__add_text_image_to_verification.sql:
- Around line 1-44: The new migration procedure add_verification_text_images
creates text_image1/2/3 as VARCHAR(255) which is inconsistent with existing S3
key columns (photo_url, text_url) defined as VARCHAR(512); change the column
definitions inside the procedure to VARCHAR(512) for text_image1, text_image2,
and text_image3 so they match the existing schema and can safely store S3 object
keys.
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
502-527: 시간 윈도우 검증 로직이 정교합니다! 엣지 케이스 방어 제안인증 수정/삭제 시간 제한 로직이 매우 체계적입니다:
- 현재가 인증 시간대인지 확인
- 현재 윈도우의 기준 날짜 계산 (자정 넘김 케이스 포함)
- 해당 날짜가 인증 요일인지 검증
- 게시글 작성 윈도우와 현재 윈도우 일치 여부 확인
다만, Line 573의
challenge.getChallengeDays()가 빈 리스트를 반환할 경우anyMatch는 false를 반환하지만, null을 반환하면 NPE가 발생할 수 있습니다.🔎 방어적 null 체크 추가 제안
Lines 571-577의
isVerificationDay메서드에 방어 코드를 추가하면 더 안전합니다:private boolean isVerificationDay(Challenge challenge, LocalDate date) { ChallengeDays targetDay = mapToChallengeDays(date.getDayOfWeek()); List<ChallengeDayJoin> days = challenge.getChallengeDays(); + + if (days == null || days.isEmpty()) { + return false; // 또는 적절한 예외 발생 + } return days.stream() .anyMatch(join -> join.getDayOfWeek() == targetDay); }챌린지 엔티티에서
getChallengeDays()가 항상 non-null을 보장한다면 이 체크는 불필요하지만, 방어적 프로그래밍 관점에서 권장합니다.관련 자료:
src/main/java/com/hrr/backend/domain/verification/entity/Verification.java (1)
188-197: 이미지 키 검증 로직이 엄격하고 명확합니다
normalizeOptionalImageKey헬퍼 메서드가 다음을 방지합니다:
- 빈 문자열 (
"") - Line 190-192- 리터럴 문자열
"null"- Line 193-195이는 잘못된 S3 키가 저장되는 것을 사전에 차단하는 좋은 방어 로직입니다.
다만, Lines 174-182의 호출 지점을 보면 이미지 필드를 제거(clear)하는 메커니즘은 제공되지 않습니다. 클라이언트가 이미지를 제거하려면:
- DTO에서 해당 필드를 보내지 않으면 → 기존 값 유지
- 빈 문자열을 보내면 → 예외 발생
- 새 S3 키를 보내면 → 교체
이미지는 교체만 가능하고 제거는 불가능한 것으로 보이는데, 이것이 의도된 비즈니스 요구사항이라면 문제없습니다.
📝 선택적 제안: 이미지 제거 기능이 필요한 경우
만약 향후 이미지를 명시적으로 제거해야 하는 요구사항이 생긴다면, 다음과 같은 접근을 고려할 수 있습니다:
방법 1: 별도의 삭제 플래그 사용
// DTO에 추가 private Boolean clearTextImage1; // update 메서드에서 if (Boolean.TRUE.equals(clearTextImage1)) { this.textImage1 = null; } else if (textImage1 != null) { this.textImage1 = normalizeOptionalImageKey(textImage1); }방법 2: 특별한 키워드 사용 (권장하지 않음)
if ("REMOVE".equals(textImage1)) { this.textImage1 = null; }현재 구조에서는 이미지 제거가 불가능하므로, API 문서나 Swagger에 "이미지는 교체만 가능하며 제거는 불가능합니다"라고 명시하는 것을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.javasrc/main/java/com/hrr/backend/domain/verification/entity/Verification.javasrc/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/resources/db/migration/V2.25__add_text_image_to_verification.sql
🔇 Additional comments (9)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationUpdateRequestDto.java (1)
16-22: LGTM! 스키마 문서화가 명확합니다.
@Schema어노테이션으로 각 필드가 nullable임을 명확히 표시했고, S3 Key라는 용도도 잘 설명되어 있습니다. 유효성 검사는 엔티티 계층(Verification.normalizeOptionalImageKey)에서 처리되므로 계층 분리가 적절합니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
126-127: 중복 해결 완료! 에러 코드가 명확합니다.이전 리뷰에서 지적된 중복 에러 코드 이슈가 완벽하게 해결되었습니다.
VERIFICATION_INVALID_IMAGE_KEY가 고유한 코드"VERIFICATION40021"을 사용하며, 두 에러 코드 모두 PR 목적(시간 윈도우 검증 + 이미지 키 검증)에 부합합니다. 에러 메시지도 사용자 친화적이고 구체적입니다. 👍src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (5)
203-205: 텍스트 인증 생성 로직 업데이트 확인단일
photoUrl대신textImage1,textImage2,textImage3을 전달하도록 변경되어 PR 목적에 부합합니다.
386-401: 차단된 게시글 접근 제어와 시간 윈도우 검증 추가Lines 386-389에서 차단된 게시글에 대한 수정 시도를 방지하고, Lines 400-401에서 인증 시간 윈도우 검증을 추가한 것은 PR의 핵심 요구사항을 충족합니다.
532-543: 자정 넘김 케이스 처리가 탁월합니다!Lines 537-538과 Lines 541-542에서 일반 케이스(09:00
22:00)와 자정 넘어가는 케이스(22:0002:00)를 모두 올바르게 처리하고 있습니다. 시간대 비교 로직이 정확합니다.
552-566: Anchor Date 계산 로직이 정확합니다!자정 넘어가는 윈도우(22:00
02:00)에서 01:00의 "인증 요일"을 전날로 간주하는 로직(Line 565의화요일 02:00 윈도우에서 화요일 01:00에 작성된 글은 "월요일 인증"으로 처리되는 것이 맞습니다.minusDays(1))이 비즈니스 요구사항을 정확히 반영합니다. 예를 들어, 월요일 22:00
579-589: Switch expression 활용이 깔끔합니다!Java의 switch expression을 사용해
DayOfWeek를ChallengeDays로 매핑한 것이 가독성이 좋습니다. 모든 요일 케이스가 커버되어 있어 런타임 에러 가능성도 없습니다.src/main/java/com/hrr/backend/domain/verification/entity/Verification.java (2)
84-111: 텍스트 인증 생성자 업데이트 완료Lines 90-92에서
photoUrl대신textImage1,textImage2,textImage3파라미터를 받도록 시그니처가 변경되었고, Lines 104-106에서 빌더에 올바르게 설정되었습니다. PR의 핵심 변경사항이 정확히 반영되었습니다.
152-160: 이전 리뷰 이슈 해결 완료!이전 리뷰에서 지적된
isQuestion파라미터 누락 이슈가 Line 159에 추가되어 완전히 해결되었습니다. 메서드 시그니처가 이제 7개 파라미터를 모두 포함하며, Lines 184-186에서 정상적으로 사용됩니다.
src/main/resources/db/migration/V2.25__add_text_image_to_verification.sql
Show resolved
Hide resolved
yc3697
left a comment
yc3697
left a comment
There was a problem hiding this comment.
varchar 길이만 512로 변경해서 merge 해주세요~
…hrr-server into bug/#182-verification-update-and-delete
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
=> 기존 방식에서는 PhotoUrl로 통합하여 사진인증과 글인증의 사진을 동일하게 취급했지만 textimage1, textImage2, textImage3로 세분화하여 null 일 때도 적용이 가능하도록 변경하였습니다.
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
인증하는 시간대일때 수정과 삭제
인증하는 시간대가 아닐 때의 수정과 삭제
글 인증의 필드를 textImage1,2,3으로 변경한 뒤의 생성과 수정
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
기타
✏️ Tip: You can customize this high-level summary in your review settings.