Fix/#232 프로필에 동일한 활동을 중복해서 등록할 수 있는 버그 #250
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
added 2 commits
August 7, 2023 22:11
- 온보딩을 완료한 사용자가 온보딩을 할 경우 예외 처리 - 활동 id 중 이미 존재하는 활동의 id가 포함되어 있을 경우 예외 반환 - 활동 id 목록에 중복 id가 포함되어 있을 경우 예외 반환 #232
- 사용자 정보가 변경됨에 따라 깨진 테스트 코드를 일부 수정하였음. #232
java-saeng
requested changes
Aug 8, 2023
|
|
||
| private boolean hasDuplicateId(final List<MemberActivity> memberActivities, | ||
| final List<Long> activityIds) { | ||
| return activityIds.stream().distinct().count() != memberActivities.size(); |
There was a problem hiding this comment.
activityIds가 중복으로 들어오는 경우도 있는건가요??
distinct 를 해주신 이유가 궁긍합니다.
There was a problem hiding this comment.
현재 UI 상으로는 활동을 중복으로 선택하는 것이 불가능하기에 정상적인 경우 중복 데이터가 요청으로 들어올 일은 없을겁니다!
하지만 악의적으로 중복된 활동 id를 포함한 데이터를 담아 API 호출을 하는 경우, 예외를 반환하는 것이 옳은 대처라고 생각해서 유효성 검증을 추가해주었습니다!
There was a problem hiding this comment.
넵 그러면 distinct 보다는 Set 어떠신가요? Long 사용하는거라 자료구조를 사용하면 더 좋아보여요!!
backend/emm-sale/src/main/java/com/emmsale/member/application/MemberActivityService.java
Outdated
Show resolved
Hide resolved
hong-sile
reviewed
Aug 8, 2023
| INVALID_ACTIVITY_IDS( | ||
| HttpStatus.BAD_REQUEST, | ||
| "요청한 activity id들 중에 유효하지 않은 값이 존재합니다" | ||
| ), | ||
|
|
||
| ALREADY_EXIST_ACTIVITY( |
There was a problem hiding this comment.
여기서 이미 존재한다고 하는 것 보다, 이미 등록된 활동입니다. 정도로 표현하는건 어떨까요?
java-saeng
approved these changes
Aug 8, 2023
hong-sile
approved these changes
Aug 8, 2023
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.
#️⃣연관된 이슈
#232
📝작업 내용
프로필에 동일한 활동을 중복해서 등록할 수 없도록 유효성 검증 코드를 추가하였습니다.
예상 소요 시간 및 실제 소요 시간
30분/2시간
💬리뷰 요구사항(선택)
온보딩된 사용자가 한 번 더 온보딩 API를 호출하는 것을 막아야 해서 테스트를 위해 test-data.sql에서 insert해준 member의 name을 null로 수정했습니다.