-
Notifications
You must be signed in to change notification settings - Fork 8
feat: 어드민에서 파견 대학을 관리하도록 #633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feat: 어드민에서 파견 대학을 관리하도록 #633
Conversation
- HOST_UNIVERSITY_HAS_REFERENCES : 파견 대학 삭제 시 해당 대학을 참조하는 UnivApplyInfo가 존재하는 경우
Walkthrough호스트 대학 관리를 위한 관리자 REST API를 새로 추가합니다. 다음 13개 파일에서 CRUD 기능을 구현했습니다:
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java`:
- Around line 58-82: Add a DB migration to add a UNIQUE constraint on
host_university.korean_name and update the service to translate DB uniqueness
violations into the existing domain error; specifically, keep
createHostUniversity(...) and validateKoreanNameNotExists(...) but wrap
hostUniversityRepository.save(hostUniversity) in a try/catch for
DataIntegrityViolationException and rethrow or map it to your
HOST_UNIVERSITY_ALREADY_EXISTS error (or appropriate custom exception/response);
also update the HostUniversity entity to reflect uniqueness if you maintain JPA
metadata (add unique=true on koreanName) so the intent is clear in both the
schema and the model.
- Around line 91-115: The updateHostUniversity method currently performs a full
overwrite including nullable fields (homepageUrl, englishCourseUrl,
accommodationUrl, detailsForLocal) which can unintentionally null out existing
data; change the implementation to perform a merge-style update by either (A)
checking each nullable request field in updateHostUniversity and only calling
HostUniversity.update with values present, or (B) modify HostUniversity.update
to treat null inputs as "no-op" for those specific fields so existing values are
preserved; ensure you still validate required fields via
validateKoreanNameNotDuplicated and findCountryByCode/findRegionByCode and
reference updateHostUniversity and HostUniversity.update when applying the
change.
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/admin/university/controller/AdminHostUniversityController.java (2)
47-53: 1. 생성 응답은 201로 맞추는 편이 좋습니다.
- 클라이언트 계약을 명확히 하려면 201 Created(가능하면 Location 헤더)로 응답하는 게 REST 관례입니다.
🔧 제안 변경
- return ResponseEntity.ok(response); + return ResponseEntity.status(201).body(response);
64-69: 1. 삭제 응답은 204가 더 명확합니다.
- 리소스 삭제는 본문 없는 204 No Content가 일반적입니다.
🔧 제안 변경
- return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build();
| @Transactional | ||
| public AdminHostUniversityDetailResponse createHostUniversity(AdminHostUniversityCreateRequest request) { | ||
| validateKoreanNameNotExists(request.koreanName()); | ||
|
|
||
| Country country = findCountryByCode(request.countryCode()); | ||
| Region region = findRegionByCode(request.regionCode()); | ||
|
|
||
| HostUniversity hostUniversity = new HostUniversity( | ||
| null, | ||
| request.koreanName(), | ||
| request.englishName(), | ||
| request.formatName(), | ||
| request.homepageUrl(), | ||
| request.englishCourseUrl(), | ||
| request.accommodationUrl(), | ||
| request.logoImageUrl(), | ||
| request.backgroundImageUrl(), | ||
| request.detailsForLocal(), | ||
| country, | ||
| region | ||
| ); | ||
|
|
||
| HostUniversity savedHostUniversity = hostUniversityRepository.save(hostUniversity); | ||
| return AdminHostUniversityDetailResponse.from(savedHostUniversity); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# 데이터베이스 마이그레이션 및 스키마 파일 찾기
find . -type f \( -name "*.sql" -o -name "*.ddl" -o -name "*.xml" -o -name "*.yaml" -o -name "*.yml" \) | head -20Repository: solid-connection/solid-connect-server
Length of output: 1169
🏁 Script executed:
# HostUniversity 엔티티 클래스 찾기
find . -type f -name "*.java" | xargs grep -l "class HostUniversity" | head -5Repository: solid-connection/solid-connect-server
Length of output: 275
🏁 Script executed:
# host_university 테이블 관련 스키마 검색
rg "host_university|korean_name" -t sql -t xml -t yamlRepository: solid-connection/solid-connect-server
Length of output: 2334
🏁 Script executed:
# V1__init.sql에서 host_university 또는 university 테이블 정의 보기
sed -n '/CREATE TABLE.*university/,/^$/p' src/main/resources/db/migration/V1__init.sql | head -30Repository: solid-connection/solid-connect-server
Length of output: 1205
🏁 Script executed:
# HostUniversity 엔티티 클래스 내용 확인
cat src/main/java/com/example/solidconnection/university/domain/HostUniversity.javaRepository: solid-connection/solid-connect-server
Length of output: 2484
🏁 Script executed:
# V42 마이그레이션 파일에서 host_university 테이블 정의 확인
cat src/main/resources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sqlRepository: solid-connection/solid-connect-server
Length of output: 953
🏁 Script executed:
# V43 이후 마이그레이션 파일에서 host_university unique constraint 추가 여부 확인
ls -1 src/main/resources/db/migration/V*.sql | sort -V | tail -20Repository: solid-connection/solid-connect-server
Length of output: 1554
🏁 Script executed:
# 모든 마이그레이션 파일에서 korean_name이나 host_university unique 관련 검색
grep -r "UNIQUE.*korean_name\|host_university.*UNIQUE\|ADD.*UNIQUE.*host_university" src/main/resources/db/migration/ 2>/dev/null || echo "No UNIQUE constraint found for korean_name"Repository: solid-connection/solid-connect-server
Length of output: 123
🏁 Script executed:
# AdminHostUniversityService 전체 파일 확인
cat src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.javaRepository: solid-connection/solid-connect-server
Length of output: 6789
DB 유니크 제약 부재로 인한 동시 요청 중복 생성 가능성 확인됨
-
현 상태
- host_university 테이블의 korean_name 컬럼에 UNIQUE 제약이 없습니다.
- HostUniversity 엔티티의 koreanName도
@Column(unique = true)속성이 없습니다.
-
발생 가능한 문제
- createHostUniversity() 메서드는 validateKoreanNameNotExists()로 조회 후 저장하는 체크-액트 패턴을 사용 중입니다.
- 이 패턴은 동시 요청에서 원자성이 없어, 두 요청이 모두 검증 단계를 통과한 후 중복 생성될 수 있습니다.
-
필요한 조치
- DB 마이그레이션 파일을 추가하여 host_university.korean_name에 UNIQUE 제약을 추가해주세요.
- DataIntegrityViolationException 발생 시 HOST_UNIVERSITY_ALREADY_EXISTS로 매핑하는 예외 처리를 추가해주세요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java`
around lines 58 - 82, Add a DB migration to add a UNIQUE constraint on
host_university.korean_name and update the service to translate DB uniqueness
violations into the existing domain error; specifically, keep
createHostUniversity(...) and validateKoreanNameNotExists(...) but wrap
hostUniversityRepository.save(hostUniversity) in a try/catch for
DataIntegrityViolationException and rethrow or map it to your
HOST_UNIVERSITY_ALREADY_EXISTS error (or appropriate custom exception/response);
also update the HostUniversity entity to reflect uniqueness if you maintain JPA
metadata (add unique=true on koreanName) so the intent is clear in both the
schema and the model.
| @Transactional | ||
| public AdminHostUniversityDetailResponse updateHostUniversity(Long id, AdminHostUniversityUpdateRequest request) { | ||
| HostUniversity hostUniversity = hostUniversityRepository.findById(id) | ||
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | ||
|
|
||
| validateKoreanNameNotDuplicated(request.koreanName(), id); | ||
|
|
||
| Country country = findCountryByCode(request.countryCode()); | ||
| Region region = findRegionByCode(request.regionCode()); | ||
|
|
||
| hostUniversity.update( | ||
| request.koreanName(), | ||
| request.englishName(), | ||
| request.formatName(), | ||
| request.homepageUrl(), | ||
| request.englishCourseUrl(), | ||
| request.accommodationUrl(), | ||
| request.logoImageUrl(), | ||
| request.backgroundImageUrl(), | ||
| request.detailsForLocal(), | ||
| country, | ||
| region | ||
| ); | ||
|
|
||
| return AdminHostUniversityDetailResponse.from(hostUniversity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -n "AdminHostUniversityUpdateRequest" -e java --exec cat -n {} +Repository: solid-connection/solid-connect-server
Length of output: 311
🏁 Script executed:
rg -n "hostUniversity\.update\(" -A 15 -B 5 src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.javaRepository: solid-connection/solid-connect-server
Length of output: 900
🏁 Script executed:
rg -n "public void update|void update" -A 20 src/main/java/com/example/solidconnection/entity/HostUniversity.javaRepository: solid-connection/solid-connect-server
Length of output: 189
🏁 Script executed:
fd -n "HostUniversity" -e java | head -5Repository: solid-connection/solid-connect-server
Length of output: 311
🏁 Script executed:
find . -name "*AdminHostUniversityUpdateRequest*" -type fRepository: solid-connection/solid-connect-server
Length of output: 183
🏁 Script executed:
find . -name "*HostUniversity*" -type f | grep -E "\.(java)$"Repository: solid-connection/solid-connect-server
Length of output: 1397
🏁 Script executed:
rg "record AdminHostUniversityUpdateRequest|class AdminHostUniversityUpdateRequest" -g "*.java"Repository: solid-connection/solid-connect-server
Length of output: 229
🏁 Script executed:
cat src/main/java/com/example/solidconnection/admin/university/dto/AdminHostUniversityUpdateRequest.javaRepository: solid-connection/solid-connect-server
Length of output: 1498
🏁 Script executed:
cat src/main/java/com/example/solidconnection/university/domain/HostUniversity.javaRepository: solid-connection/solid-connect-server
Length of output: 2484
필수 필드는 검증으로 보호되고 있습니다.
DTO의 필수 필드(koreanName, englishName, formatName, logoImageUrl, backgroundImageUrl, countryCode, regionCode)는 모두 @NotBlank 제약으로 보호되므로, 요청 검증 단계에서 null 값이 거부됩니다.
다만 설계 관점에서 명확히 할 점:
-
현재 코드는 전체 업데이트(PUT) -
homepageUrl,englishCourseUrl,accommodationUrl,detailsForLocal같은 nullable 필드는 null로 덮어써질 수 있으며, 이는 DTO와 엔티티 스키마에서 의도적으로 허용한 설계입니다. -
부분 업데이트(PATCH) 필요 시 - 기존 데이터를 유지하면서 특정 필드만 수정하려면 업데이트 로직에서 null 체크를 추가하고, 해당 필드만 선택적으로 업데이트하는 merge 패턴을 고려하세요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java`
around lines 91 - 115, The updateHostUniversity method currently performs a full
overwrite including nullable fields (homepageUrl, englishCourseUrl,
accommodationUrl, detailsForLocal) which can unintentionally null out existing
data; change the implementation to perform a merge-style update by either (A)
checking each nullable request field in updateHostUniversity and only calling
HostUniversity.update with values present, or (B) modify HostUniversity.update
to treat null inputs as "no-op" for those specific fields so existing values are
preserved; ensure you still validate required fields via
validateKoreanNameNotDuplicated and findCountryByCode/findRegionByCode and
reference updateHostUniversity and HostUniversity.update when applying the
change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어드민 기능이니 프론트에서 똑바로 보내준다고 가정해도 문제 없을 거 같긴 하네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gyuhyeok99, 네, 맞습니다! 어드민 기능의 특성상 신뢰할 수 있는 환경에서 사용되므로 현재 구조로도 충분합니다. 향후 프론트엔드에서 부분 업데이트가 필요한 경우에만 고려하시면 될 것 같습니다. 👍
🧠 Learnings used
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.462Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Gyuhyeok99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오우.... 깔끔하게 너무 잘 작성해주셨네요.. 로직에는 문제 없는 거 같아서 간단한 궁금증만 남겨두었습니다!
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @GetMapping("/{id}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 너무 길어져서 그냥 id로 하신건가요? 그동안은 어떤 id인지 명시했던 거 같아서요!
| private final AdminHostUniversityService adminHostUniversityService; | ||
|
|
||
| @GetMapping | ||
| public ResponseEntity<AdminHostUniversityListResponse> getHostUniversities( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PageResponse 라는 공통객체가 있는데 혹시 사용안하신 이유가 있을까요? response객체보니 따로
boolean hasNext,
boolean hasPrevious
를 정의하신 거 같아서요!
관련 이슈
작업 내용
어드민에서 파견 대학 관련 CRUD 로직을 작성했습니다. 이미지는 별도의 PR에서 작업하겠습니다 !
검색은 기존의 QueryDSL로 작성된 지원 대학 필터 검색을 참고했습니다.
특이 사항
리뷰 요구사항 (선택)