feat: 지원 대학 관련 CRUD 추가#779
Merged
Merged
Conversation
- 기존 벌크 insert 에 import 추가
- 학기, 국내 대학 id 추가
whqtker
requested review from
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
|
Warning Review limit reached
More reviews will be available in 6 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Walkthrough이번 PR에서는 크게 두 가지 중요한 변경이 이루어졌습니다. 1️⃣ 관리자 UnivApplyInfo CRUD 기능 추가
2️⃣ UnivApplyInfo 텍스트 검색 필터 확장
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1b6fb9a35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (1)
226-239: ⚡ Quick win1)
homeUniversityId필터 테스트에 대조 데이터가 없어 회귀를 놓칠 수 있습니다.
- Line 226-239 테스트는 현재 데이터 1건만 생성해서, 필터가 실제로 적용되지 않아도 통과할 수 있습니다.
- 다른
homeUniversityId데이터를 함께 넣고 배제되는지까지 검증하면 회귀 탐지력이 올라갑니다.보강 예시
void homeUniversityId를_지정하면_해당_국내대학의_지원_정보만_반환한다() { // given UnivApplyInfo 괌대학_지원_정보 = univApplyInfoFixture.괌대학_A_지원_정보(term.getId()); + UnivApplyInfo 메이지대학_지원_정보 = univApplyInfoFixture.메이지대학_지원_정보(term.getId()); Long homeUniversityId = 괌대학_지원_정보.getHomeUniversity().getId(); + assertThat(메이지대학_지원_정보.getHomeUniversity().getId()).isNotEqualTo(homeUniversityId); // when UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(null, homeUniversityId, null); // then assertThat(response.univApplyInfoPreviews()) .hasSize(1) - .allMatch(r -> r.id() == 괌대학_지원_정보.getId()); + .extracting(UnivApplyInfoPreviewResponse::id) + .containsExactly(괌대학_지원_정보.getId()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java` around lines 226 - 239, The test method homeUniversityId를_지정하면_해당_국내대학의_지원_정보만_반환한다() only creates a single UnivApplyInfo record with one homeUniversityId, which means the filter could be broken and the test would still pass since there's nothing to exclude. Add a second UnivApplyInfo fixture with a different homeUniversityId using univApplyInfoFixture, then verify that when searchUnivApplyInfoByText() is called with the first homeUniversityId, only the first record is returned and the second record with the different homeUniversityId is correctly filtered out. Update the assertion to verify the response size and that the excluded record is not present in the results.src/test/java/com/example/solidconnection/admin/university/service/AdminUnivApplyInfoServiceTest.java (1)
545-565: ⚡ Quick win1) 부분 수정 계약을 고정하는 회귀 테스트를 추가해 주세요.
Line 545-565는 언어요건 교체 성공만 검증하고, nullable 필드 전달 시 기존 값 보존/삭제의 기대 동작을 잠그지 못합니다.
languageRequirements = null(미전달)과languageRequirements = [](명시적 비움)을 분리해 검증하는 케이스를 추가하면 데이터 소거 회귀를 빠르게 잡을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/example/solidconnection/admin/university/service/AdminUnivApplyInfoServiceTest.java` around lines 545 - 565, The current test in the 수정_시_언어_요건이_기존_것과_교체된다 method only validates successful replacement of language requirements but does not lock down the expected behavior for partial updates with null or empty fields. Add two separate regression test cases to the AdminUnivApplyInfoServiceTest class: one that passes languageRequirements as null (to verify existing language requirements are preserved when the field is not provided), and another that passes languageRequirements as an empty list (to verify existing language requirements are explicitly cleared/deleted). This will establish a clear contract for the updateUnivApplyInfo method's behavior when handling nullable collection fields during partial updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoCreateRequest.java`:
- Line 21: The `languageRequirements` field in `AdminUnivApplyInfoCreateRequest`
class currently uses `@Valid` on the List, but this does not prevent null
elements within the list (e.g., `[null]` will pass validation), causing
NullPointerException later when the service accesses properties like
`languageTestType()`. Add element-level validation constraints to the
`languageRequirements` field to ensure null elements are rejected at the request
validation stage. Apply the same validation pattern to the corresponding update
DTO class to maintain consistency and prevent similar NPE issues.
In
`@src/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoUpdateRequest.java`:
- Around line 9-17: The DTO in AdminUnivApplyInfoUpdateRequest contains nullable
fields (studentCapacity, semesterAvailableForDispatch, semesterRequirement,
detailsForLanguage, gpaRequirement, gpaRequirementCriteria,
detailsForAccommodation, extraInfo, languageRequirements) that are being
directly applied to the entity in the PUT update flow, which creates a data loss
risk when clients omit fields. You need to clarify the update contract: if this
should be a complete replacement, add `@NotNull` validation annotations to
required fields to enforce full updates; if this should be a partial/patch
update, modify the update service logic to implement null-aware merging that
preserves existing entity values when the DTO fields are null instead of
overwriting them.
In
`@src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java`:
- Around line 107-109: The clearLanguageRequirements() method is currently being
called unconditionally in the update flow, which causes existing language
requirements to be deleted even when the field is not provided in the request.
To fix this, locate where clearLanguageRequirements() is being invoked and wrap
the call with a null check to ensure it only executes when languageRequirements
are explicitly provided in the request. This way, missing fields in the request
will not trigger deletion of existing data, while explicitly provided (even if
empty) languageRequirements will be properly cleared and recreated.
- Around line 87-105: The update method in UnivApplyInfo directly assigns all
parameters without null checks, which causes existing values to be overwritten
with null when fields are omitted from the update request. To maintain a partial
update contract, add null checks before each assignment in the update method
(lines 97-104). For each field including studentCapacity,
semesterAvailableForDispatch, semesterRequirement, detailsForLanguage,
gpaRequirement, gpaRequirementCriteria, detailsForAccommodation, and extraInfo,
only assign the new value if the parameter is not null, preserving the existing
value when null is passed.
In
`@src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java`:
- Around line 46-50: The cache key in the `@ThunderingHerdCaching` annotation for
the searchUnivApplyInfoByText method only includes {0} (text parameter), which
causes different requests with different homeUniversityId and termId values to
reuse the same cached result. Update the cache key to include all parameters
that affect the search results: {0} for text, {1} for homeUniversityId, and {2}
for termId. This ensures that requests with different filter combinations
generate distinct cache entries and do not return mixed or incorrect results.
---
Nitpick comments:
In
`@src/test/java/com/example/solidconnection/admin/university/service/AdminUnivApplyInfoServiceTest.java`:
- Around line 545-565: The current test in the 수정_시_언어_요건이_기존_것과_교체된다 method
only validates successful replacement of language requirements but does not lock
down the expected behavior for partial updates with null or empty fields. Add
two separate regression test cases to the AdminUnivApplyInfoServiceTest class:
one that passes languageRequirements as null (to verify existing language
requirements are preserved when the field is not provided), and another that
passes languageRequirements as an empty list (to verify existing language
requirements are explicitly cleared/deleted). This will establish a clear
contract for the updateUnivApplyInfo method's behavior when handling nullable
collection fields during partial updates.
In
`@src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java`:
- Around line 226-239: The test method
homeUniversityId를_지정하면_해당_국내대학의_지원_정보만_반환한다() only creates a single
UnivApplyInfo record with one homeUniversityId, which means the filter could be
broken and the test would still pass since there's nothing to exclude. Add a
second UnivApplyInfo fixture with a different homeUniversityId using
univApplyInfoFixture, then verify that when searchUnivApplyInfoByText() is
called with the first homeUniversityId, only the first record is returned and
the second record with the different homeUniversityId is correctly filtered out.
Update the assertion to verify the response size and that the excluded record is
not present in the results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 44a49eb7-c0fe-4798-a96d-2bbcb5716ea5
📒 Files selected for processing (16)
src/main/java/com/example/solidconnection/admin/university/controller/AdminUnivApplyInfoController.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoCreateRequest.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoLanguageRequirementRequest.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoResponse.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminUnivApplyInfoUpdateRequest.javasrc/main/java/com/example/solidconnection/admin/university/service/AdminUnivApplyInfoService.javasrc/main/java/com/example/solidconnection/application/repository/ApplicationRepository.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.javasrc/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.javasrc/main/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepository.javasrc/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.javasrc/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.javasrc/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.javasrc/test/java/com/example/solidconnection/admin/university/service/AdminUnivApplyInfoServiceTest.javasrc/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java
whqtker
force-pushed
the
feat/778-univ-apply-info-crud
branch
from
707cce4 to
9963bf6
Compare
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.
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)