refactor : 회원가입 + 회원정보 수정 형식 일치 #886
Merged
Conversation
This file contains 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
kwoo28
approved these changes
Sep 14, 2024
seongjae6751
approved these changes
Sep 14, 2024
There was a problem hiding this comment.
고생하셨습니다!
첫번째는 무슨 말씀이신지 저도 이해가 잘 안가서 다시 말씀해주시면 좋을 것 같아요..!
두번째는 저는 검증 로직을 별도의 서비스로 분리하는 것에 반대합니다. 검증 로직이 클래스에 포함되어 있다고 해서 반드시 단일 책임 원칙을 위반하는 것은 아니라고 생각합니다. 현재 검증 로직들은 비즈니스 로직과 매우 밀접하게 연관되어 있으며, 변경될 가능성이 적고 해당 서비스 내에서만 사용되기 때문에 SRP 위반이 아니라고 생각합니다. 또한, 검증 로직을 분리할 경우 복잡성이 크게 증가하여 유지보수성이 오히려 저하되지 않을까 우려가 됩니다
|
첫 번째는, 레거시 코드에서 마이그레이션을 진행했을 떄 같은 컬럼들을 다 가져왔는데, 업데이트 하지 않는 항목이라고 [NOT UPDATE]라고 적혀져 있는 코드를 그대로 가져왔었습니다. 근데 이 정보가 굳이 회원정보 수정에서 있어야하는가? 에 대해서 궁금해서 여쭤봤습니다. 그리고 졸업 여부도 사실상 유지를 해야하는 가?에 대해서 궁금했어서, 의견 감사합니다! 단일책임원칙은 좀 더 생각해봐야겠네용 |
* feat: flyway 작성 * feat: 도메인 엔티티 작성 * feat: entity, repository 작성 및 test 약간 * feat: 엔티티 수정 및 테스트 일부 작성 * test: fixture 작성 및 테스트 일부 작성 * feat: 엔티티 수정 및 테스트 완성 * chore: flyway 파일 수정 * chore: flyway 파일 수정 * fix: flyway 버그 수정 * feat: redis entity, repo 작성 * feat: 개수 동기화 로직 구현 * feat: 스케줄링 적용 * feat: assignVariable 함수 작성 * feat: 신규 편입, 자신의 실험군 조회 일부 작성 * feat: 자신의 실험군 조회 시 캐시에 없으면 DB로 최신화 * feat: 엔티티 버그 수정, 기기 정보 수집, 최초 편입 로직 작성 * feat: 최초 편입 api 일부 작성 * feat: 최초 편입 api 완성 * feat: 실험 생성 api 작성 중 * rename: flyway version 수정 * feat: 실험 생성 api 완성 * feat: 실험 단건/목록 조회 api 작성 * feat: AB테스트 수정 api 작성 * refactor: redis key 직렬화 설정 추가 * feat: ab테스트 수정 시도 시 관련 캐시 초기화 * feat: 실험 수정 api 작성 * feat: 실험 삭제 api 작성 * feat: 실험 종료 api 작성 * feat: 이름으로 사용자 조회 api 작성 * feat: 유저 id로 디바이스 목록 조회 api 작성 * feat: 실험군 수동 편입 api 작성 * refactor: 신규 편입 API 리팩토링 * rename: 캐시 엔티티명 변경 * remove: test 관련 코드 제거 * docs: API 설명 수정 * feat: 종료된 실험에 대한 요청 차단 * feat: 실험군 수동 편입 시 기존 캐시 제거 및 캐싱 진행 * feat: 실험군 수동 편입 - 이전/이후 실험군이 동일한 경우 예외처리 * feat: flyway 작성 notification, notification_subscribe 테이블에서 user_id fk 제거, device_id 추가 user_id 컬럼 제거는 추후(안정화) 진행 * feat: fcm Token 관리 주체 변경 * feat: 종료된 실험에 대해 최초편입, 내실험군조회 시 승자 반환 * test: 테스트 수정 중 * test: 테스트 수정 중 * refactor: 실험 수정 api 버그 수정 중 * fix: 각종 버그 수정 * fix: 각종 버그 수정 * feat: 로그인 성공 시 AccessHistory, Device 생성 * feat: login or refresh 성공 시 Device Model 정보 최신화 * fix: flyway default 값 수정 * feat: login or refresh 성공 시 마지막 접속 일시 최신화 * merge: flyway 컨플릭트 수정 * fix: column definition 추가 * fix: 실험군 최초 편입 버그 수정 * fix: 실험군 최초 편입 버그 수정 * merge: 컨플릭트 수정 * fix: 존재하지 않는 실험군에 대한 요청 에러 수정 * refactor: 컬럼 일부 제거 및 이동(ip기반에서 id기반으로 변경 시작) * rollback: user, notification 관련 수정 롤백 * rollback: login 관련 수정 롤백 * feat: API 명세 변경 * feat: ip 기반에서 accessHistoryId 기반으로 변경 * feat: ip -> id 명세 및 로직 일부 수정 * fix: 자잘한 버그 수정 * fix: api 컨벤션 통일 * test: 테스트 수정 * refactor: 코드 개선 * feat: 실험 수정 API 변경된 요구사항 반영 * refactor: 불필요 컬럼 제거 * refactor: User 엔티티 name 필드 unique 제약조건 제거 * fix: 일부 응답 필드 및 테스트 수정 * fix: 스케줄러 예외처리 추가 * test: mockMvc 적용 * refactor: unique 속성 추가 * style: 컨벤션 단일화 --------- Co-authored-by: seongjae6751 <seongjae6751@naver.com>
Co-authored-by: HyeonsuLee <leehyeonsu4888@naver.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔥 연관 이슈
🚀 작업 내용
(01024607469 / 010-2460-7469)-> 통일된 형식이 없어 둘 다 받도록 했습니다. (DB 내엔 둘 다 있습니다. || 사장님은 앞의 형식을 따르고 있는 걸로 알고 있습니다.)
💬 리뷰 중점사항
QA 진행 중 나왔던 이슈를 해결했습니다.
회원정보 수정이랑 회원가입의 형식을 일치시켰습니다.
궁금한 사항으로는, 이번 코드 리팩토링을 진행하면서
is_graduated와userIdentity의 경우는 업데이트에서 NOT UPDATE 컬럼인데 유지를 해야하나, 고민이 있었습니다(기존 레거시 코드와 동일하게 마이그레이션 하는 과정에서 작성했던 코드입니다.) 추가적으로 학생 회원가입에도is_graduated가 있는데, 이도 유지를 해야하나 싶습니다.추가적으로, Service에서 다양한 검증을 진행하고 있는데, 검증단을 Service에서 따로 분리하여 유지하는 것(ex : ValidationService)에 대한 의견도 묻고 싶습니다.(SRP 위배 관련) 긴 의문사항 읽어주셔서 감사합니당 :D