-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 교환학생 준비 상태의 컬럼명 변경 #351
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
refactor: 교환학생 준비 상태의 컬럼명 변경 #351
Conversation
Walkthrough
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (2)
src/main/resources/db/migration/V17__rename_preparation_status.sql (1)
1-1: 2. 데이터베이스 스키마 변경이 적절하게 구현되었습니다- preparation_stage에서 exchange_status로의 컬럼명 변경이 domain model 변경과 일치합니다 - 단순하고 명확한 ALTER TABLE 구문으로 안전하게 처리되었습니다 - 만약 롤백이 필요한 경우를 대비해 역방향 마이그레이션 계획도 고려해두시면 좋겠습니다src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)
14-14: 8. 동적 픽스처에서 새로운 enum 사용이 일관되게 적용되었습니다- ExchangeStatus.CONSIDERING을 사용하도록 적절히 변경되었습니다 - TODO 주석에 언급된 대로 향후 test fixture 개선 작업 시 이 클래스 삭제를 고려해주세요
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/example/solidconnection/auth/dto/SignUpRequest.java(4 hunks)src/main/java/com/example/solidconnection/siteuser/domain/ExchangeStatus.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java(4 hunks)src/main/resources/db/migration/V17__rename_preparation_status.sql(1 hunks)src/test/java/com/example/solidconnection/e2e/DynamicFixture.java(2 hunks)src/test/java/com/example/solidconnection/security/authentication/SiteUserAuthenticationTest.java(2 hunks)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java(2 hunks)src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (12)
src/main/java/com/example/solidconnection/siteuser/domain/ExchangeStatus.java (1)
3-9: 1. 체계적인 enum 이름 변경 작업이 잘 수행되었습니다- PreparationStatus에서 ExchangeStatus로의 이름 변경이 명확하고 의미있게 처리되었습니다 - enum 상수들이 그대로 유지되어 기존 데이터와의 호환성이 보장됩니다 - 마지막 상수 뒤의 trailing comma 추가는 향후 새로운 상수 추가 시 git diff를 깔끔하게 유지하는 좋은 관례입니다src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (2)
4-4: 3. import 문이 새로운 enum으로 정확히 업데이트되었습니다- PreparationStatus에서 ExchangeStatus로의 import 변경이 올바르게 처리되었습니다
65-65: 4. 테스트 픽스처에서 enum 사용이 일관되게 업데이트되었습니다- ExchangeStatus.CONSIDERING을 기본값으로 사용하는 것이 적절합니다 - 테스트 데이터 생성 로직이 새로운 enum과 일치하게 변경되었습니다src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java (2)
4-4: 5. 리포지토리 테스트의 import가 올바르게 업데이트되었습니다- ExchangeStatus로의 import 변경이 정확히 처리되었습니다
86-86: 6. 테스트 헬퍼 메서드가 새로운 enum을 적절히 사용합니다- createSiteUser 메서드에서 ExchangeStatus.CONSIDERING을 사용하도록 일관되게 변경되었습니다 - 기존 테스트 로직의 의도가 그대로 유지되었습니다src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)
3-3: 7. E2E 테스트 픽스처의 import가 정확히 변경되었습니다- ExchangeStatus로의 import 업데이트가 올바르게 처리되었습니다src/test/java/com/example/solidconnection/security/authentication/SiteUserAuthenticationTest.java (1)
4-4: 테스트 코드 업데이트가 올바르게 적용되었습니다.
- import문이
ExchangeStatus로 정확히 변경됨- 테스트 헬퍼 메서드에서도 새로운 enum 값 사용
변경사항이 일관되게 적용되어 좋습니다.
Also applies to: 66-66
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (2)
67-67: 도메인 모델의 필드명 변경이 올바르게 적용되었습니다.
preparationStage에서exchangeStatus로의 필드명 변경이 적절합니다.
97-109: 변경사항 요약
- 생성자들의 매개변수명이
ExchangeStatus exchangeStatus로 일관되게 업데이트되었습니다.- 필드 할당도 모두
this.exchangeStatus = exchangeStatus형태로 동일하게 반영되었습니다.- 총 세 개의 생성자에 동일한 수정사항이 적용된 것을 확인했습니다.
- 마이그레이션 파일(
src/main/resources/db/migration/V17__rename_preparation_status.sql)이 존재함을 확인했습니다.데이터베이스에 해당 마이그레이션이 실제로 적용되었는지 한 번 더 검증해 주세요.
src/main/java/com/example/solidconnection/auth/dto/SignUpRequest.java (3)
4-4: 필요한 import문이 올바르게 추가되었습니다.
ExchangeStatusimport 추가@JsonProperty어노테이션을 위한 Jackson import 추가적절한 의존성 추가입니다.
Also applies to: 7-7
17-18: JSON 호환성을 유지하면서 내부 필드명을 개선한 우수한 접근 방식입니다.
- 필드명을
exchangeStatus로 변경하여 의미를 명확히 함@JsonProperty("preparationStatus")어노테이션으로 기존 API 호환성 유지- 클라이언트 코드 수정 없이 내부 구조 개선 가능
이런 방식의 점진적 리팩토링이 매우 바람직합니다.
25-34: 1. 검색 결과
rg "PreparationStatus"명령으로는 어떠한 참조도 찾아지지 않았습니다.
- 검증 요청
- 혹시 숨어있는 참조가 있을 수 있으니, 코드베이스 전반에서
PreparationStatus가 완전히 제거되었는지 수동으로 한 번 더 확인 부탁드립니다.
whqtker
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.
빠른 변경 좋습니다 !!!!!
| @@ -0,0 +1 @@ | |||
| ALTER TABLE site_user RENAME COLUMN preparation_stage TO exchange_status; No newline at end of file | |||
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.
- PreparationStatus -> ExchangeStudentStatus chore: 컬럼명 변경 flyway 스크립트 추가
cada438 to
c587fef
Compare
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.
확인했습니다!
| @@ -0,0 +1 @@ | |||
| ALTER TABLE site_user RENAME COLUMN preparation_stage TO exchange_status; | |||
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.
어떤게 빨리 approve 될지 몰라서 가장 최신 버전으로 등록해두었습니다.
머지할 때 맞추려고요!
제 생각을 미리 공유드렸어야 했는데 속으로만 생각했네요.. 😅
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.
아하 그렇군요!! 다시 approve했습니다!
lsy1307
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.
확인했습니다!
whqtker
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.
개행 추가 확인했습니다 !
…e-preparation-status # Conflicts: # src/test/java/com/example/solidconnection/e2e/DynamicFixture.java

관련 이슈
작업 내용
회의에서 논의된 내용에 따라 컬럼명을
exchange_student로 바꿉니다.JSON key 의 이름은 JsonProperty 를 사용하여 유지합니다.