Feature/#901 온보딩에서 활동 설정 시 관심태그도 함께 설정 #903
Open
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
hong-sile
approved these changes
Jan 28, 2024
| @@ -33,6 +33,17 @@ public List<InterestTagResponse> findInterestTags(final Long memberId) { | |||
| return InterestTagResponse.convertAllFrom(interestTags); | |||
| } | |||
|
|
|||
| public void initializeInterestTags(final Member member, final List<String> tagNames) { | |||
| if (member.isOnboarded()) { | |||
There was a problem hiding this comment.
멤버의 온보딩 여부를 interestTag에서 해주고 있는데,
Member를 initializae 시키는 initializeMember에서 예외처리하는게 적절해보여요.
그러면 온보딩이 된 멤버의 경우 불필요한 activity 조회 등록도 안 일어날 것 같고요.
| @@ -46,6 +46,11 @@ insert into member(id, name, image_url, open_profile_url, github_id, github_user | |||
| values (2, 'member2', 'https://imageurl.com', 'https://openprofileurl.com', 2, 'amaran-th22', | |||
| CURRENT_TIMESTAMP(), | |||
| CURRENT_TIMESTAMP()); | |||
| insert into member(id, name, image_url, open_profile_url, github_id, github_username, created_at, | |||
There was a problem hiding this comment.
혹시 해당 로직은 왜 추가한건지 알 수 있을까요? 저희가 되도록이면 data-test.sql을 안 건들기로 했었어서요
There was a problem hiding this comment.
이런 데이터 하나 추가로 변경해야될 테스트 코드도 있는거 같아서요
There was a problem hiding this comment.
엥 그러네요... MemberFixture를 추가하면 된다는 생각을 못 했었나봅니다,,, 다시 삭제해두겠습니다
|
|
||
| //when & then | ||
| assertAll( | ||
| () -> assertDoesNotThrow( |
There was a problem hiding this comment.
요 assertDoesNotThrow 부분은 따로 분리해보면 어떤가요?
저 부분에서 에러가 나지 않는 것을 굳이 assertAll로 감싸고 체크할 필요는 없다고 생각해요.
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.
#️⃣ 연관된 이슈
📝 작업 내용
온보딩에서 직무 활동을 설정할 때 직무활동 name과 같은 이름을 갖는 태그가 관심태그로 설정되도록 했습니다.
예상 소요 시간 및 실제 소요 시간 (일 / 시간 / 분)
예상 소요 시간 : 1/24(수)
실제 소요 시간 : 1/24(수)
💬 리뷰어 요구사항 (선택)
생각보다 건드린 부분이 많네요...!
먼저 기존에는 api가 MemberActivityCommandService의 메서드를 호출하고 있었는데, 이 서비스에서 InterestTag 도메인에 대한 작업을 수행하는 게 부자연스러운 것 같아서 로직 흐름 구조를 아래 사진처럼 변경해주었습니다.
그리고 추가 테스트를 작성하는 과정에서, 온보딩을 하지 않았고 활동, 관심태그가 설정되지 않은 멤버가 필요해서 data-test.sql에 회원 한 명을 추가했습니다. 이로 인해 기존에 memberId 3을 사용하던 몇몇 테스트가 깨져서 그 코드도 수정을 가한 상태입니다.
기존 로직 흐름
변경 후 로직 흐름
