feat(problem): 문제 등록 및 수정 시, 풀이 이미지 필수 아니도록 수정#271
Conversation
|
Warning Rate limit exceeded@huhdy32 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 42 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 (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRelaxed validation on solutionImage across API and domain DTOs (removed Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @huhdy32, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 문제 등록 및 수정 과정에서 풀이 이미지의 필수 여부를 해제하여, 사용자가 풀이 이미지를 첨부하지 않고도 문제를 생성하거나 업데이트할 수 있도록 유연성을 제공합니다. 이는 시스템의 사용 편의성을 높이고, 특정 상황에서 풀이 이미지가 필요하지 않을 때 불필요한 제약을 제거합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
이번 PR은 문제 등록 및 수정 시 풀이 이미지를 선택적으로 제출할 수 있도록 변경하는 내용이네요. 전반적으로 DTO와 Command 객체에서 solutionImage 필드의 유효성 검사 어노테이션을 제거하여 의도한 대로 잘 수정되었습니다.
다만, 코드의 가독성과 유지보수성을 위해 몇 가지 개선점을 제안합니다. 각 파일에 남긴 리뷰 댓글을 확인해주세요.
추가적으로, solutionImage가 null 또는 빈 값일 경우에도 문제 등록 및 수정이 성공하는지에 대한 테스트 케이스를 추가하면 코드의 안정성을 더욱 높일 수 있을 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/api/mathrank-problem-api/src/main/java/kr/co/mathrank/app/api/problem/Requests.java (2)
19-23: Consider removing commented-out code and clarifying documentation.The commented-out annotation and explanatory comment work, but could be cleaner:
- Commented-out code is typically removed since version control preserves history
- The comment explains what changed rather than documenting the current state
Apply this diff to clean up the code:
- /* - 문제 등록시, 풀이 이미지 필수 아니도록 수정 - */ - // @NotBlank - String solutionImage, + String solutionImage, // Optional: solution image is not required during registrationAlternatively, if you prefer to keep the Korean comment for team communication:
- /* - 문제 등록시, 풀이 이미지 필수 아니도록 수정 - */ - // @NotBlank - String solutionImage, + // 풀이 이미지는 선택 사항입니다 (solution image is optional) + String solutionImage,
60-64: Consider removing commented-out code and clarifying documentation.Same suggestion as for
ProblemRegisterRequest: the commented-out annotation and change-focused comment could be replaced with clearer current-state documentation.Apply this diff:
- /* - 문제 수정시, 풀이 이미지 필수 아니도록 수정 - */ - // @NotBlank - String solutionImage, + String solutionImage, // Optional: solution image is not required during updatedomain/mathrank-problem-domain/src/main/java/kr/co/mathrank/domain/problem/dto/ProblemRegisterCommand.java (2)
17-21: Consider removing commented-out code for cleaner documentation.As with the API layer, the commented-out
@NotBlankannotation can be removed since version control tracks this change.Apply this diff:
- /* - 문제 등록시, 풀이 이미지 필수 아니도록 수정 - */ - // @NotBlank - String solutionImage, + // 풀이 이미지는 선택 사항입니다 (solution image is optional) + String solutionImage,
5-5: Remove unusedNotBlankimport.The
@NotBlankannotation on line 20 is commented out and not used elsewhere in the file. The import on line 5 can be safely removed.domain/mathrank-problem-domain/src/main/java/kr/co/mathrank/domain/problem/dto/ProblemUpdateCommand.java (1)
18-22: Consider removing commented-out code for cleaner documentation.Consistent with the other files, the commented-out
@NotNullannotation can be removed in favor of current-state documentation.Apply this diff:
- /* - 문제 수정시, 풀이 이미지 필수 아니도록 수정 - */ - // @NotNull - String solutionImage, + // 풀이 이미지는 선택 사항입니다 (solution image is optional) + String solutionImage,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/mathrank-problem-api/src/main/java/kr/co/mathrank/app/api/problem/Requests.java(2 hunks)domain/mathrank-problem-domain/src/main/java/kr/co/mathrank/domain/problem/dto/ProblemRegisterCommand.java(1 hunks)domain/mathrank-problem-domain/src/main/java/kr/co/mathrank/domain/problem/dto/ProblemUpdateCommand.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
app/api/mathrank-problem-api/src/main/java/kr/co/mathrank/app/api/problem/Requests.java (1)
23-23: Concerns remain valid but cannot be fully verified—manual database schema confirmation required.The code changes allow
solutionImageto be null (validation@NotBlankis commented out), but the codebase lacks defensive null handling:
- No validation: Lines 23 and 64 in Requests.java show
// @NotBlankcommented out- No entity constraints: Problem.java field has no JPA nullability annotations
- No service-layer guards: ProblemService.java:125 and ProblemQueryResult.java:43 call
getSolutionImage()without null checks- Database schema unverifiable: Migration files were not found in searchable locations
Before merging, verify:
- Database migration explicitly allows NULL for the
solution_imagecolumn (or confirm schema management is external)- Add null-safety checks in ProblemService and consumer layers (e.g., around line 125, or use Optional)
- Consider uncommenting
@NotBlankif null should not be permitted
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
domain/mathrank-problem-domain/src/test/java/kr/co/mathrank/domain/problem/service/ProblemServiceTest.java (1)
118-126: Consider adding an explicit test case for optional solutionImage.The validation test cases correctly verify that null
solutionImagedoesn't trigger validation errors (all cases inargumentsStream()have null solutionImage while testing other required fields). However, the successful registration tests (lines 55, 76, 89, 109) all provide non-null values forsolutionImage.Consider adding an explicit positive test case that demonstrates successful problem registration with null
solutionImageto clearly document this behavioral change.Example test to add:
@Test void solutionImage가_null이어도_등록_성공() { entityManager.flush(); entityManager.clear(); final ProblemRegisterCommand command = new ProblemRegisterCommand( 1L, "image.jpeg", null, // solutionImage is null AnswerType.MULTIPLE_CHOICE, "testPath", Difficulty.KILLER, PastProblem.NONE, "testCode", Set.of("answer"), 1001, null, null ); final Long problemId = problemService.save(command); final Problem savedProblem = problemRepository.findById(problemId) .orElseThrow(); assertNotNull(savedProblem); assertNull(savedProblem.getSolutionImage()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
domain/mathrank-problem-domain/src/test/java/kr/co/mathrank/domain/problem/service/ProblemServiceTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
Summary by CodeRabbit
Enhancements
Tests