Skip to content
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

[BE] 멤버 정보 수정 API 호출 버그 수정 #610

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Conversation

khabh
Copy link
Contributor

@khabh khabh commented Sep 24, 2024

issue

구현 사항

멤버 정보 수정 API 예외 처리 로직을 수정했습니다.

private void validateUpdatedNamesUnique(List<Member> eventMembers, List<Member> updatedMembers) {
        Set<String> eventMemberNames = eventMembers.stream()
                .map(Member::getName)
                .collect(Collectors.toSet());

        boolean memberNameDuplicated = updatedMembers.stream()
                .map(Member::getName)
                .anyMatch(eventMemberNames::contains);

        if (memberNameDuplicated) {
            throw new HaengdongException(HaengdongErrorCode.MEMBER_NAME_DUPLICATE);
        }
    }

위와 같이 updatedMembers로 들어온 모든 멤버에 대해 이름 중복을 검증하고 있었는데, 이름 변경이 있는 멤버에 대해서만 검증을 수행하도록 수정했습니다.

private void validateUpdatedNamesUnique(List<Member> eventMembers, List<Member> updatedMembers) {
        Map<Long, String> eventMemberNames = eventMembers.stream()
                .collect(toMap(Member::getId, Member::getName));
        Set<String> existNames = new HashSet<>(eventMemberNames.values());

        boolean memberNameDuplicated = updatedMembers.stream()
                .filter(updatedMember -> isMemberNameUpdated(eventMemberNames, updatedMember))
                .map(Member::getName)
                .anyMatch(existNames::contains);

        if (memberNameDuplicated) {
            throw new HaengdongException(HaengdongErrorCode.MEMBER_NAME_DUPLICATE);
        }
    }

@khabh khabh self-assigned this Sep 24, 2024
@khabh khabh added ⌨️ BE Backend 🚨 bug bug labels Sep 24, 2024
@khabh khabh linked an issue Sep 24, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Sep 24, 2024

Test Results

 22 files   22 suites   3s ⏱️
126 tests 126 ✅ 0 💤 0 ❌
130 runs  130 ✅ 0 💤 0 ❌

Results for commit 2702028.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감자 고생했습니다~
에러 메시지 쪽 코멘트 남겨뒀는데 이따가 데일리 스크럼 때 같이 이야기해봐도 좋을 것 같아요

"code": "MEMBER_NOT_FOUND",
"message": "존재하지 않는 참여자입니다."
"code": "MEMBER_UPDATE_MISMATCH",
"message": "업데이트 요청된 참여자 정보와 기존 행사 참여자 정보가 일치하지 않습니다."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래 코드를 보니깐 요청된 멤버 수와 기존의 멤버 수가 다르고 업데이트 된 멤버가 기존 멤버에 모두 포함되어있지 않을 때로 보입니다

멤버를 수정하는데 일치하지 않는다는 메시지를 받으면 "수정하기를 원하는데 일치하지 않는다?" 무슨 의미지?라고 생각할 수도 있을 것 같아요

@@ -212,6 +213,28 @@ void updateMembersTest() {
);
}

@DisplayName("멤버 정보를 수정한다.")
@Test
void updateMembersIsDepositedTest() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

볼 때마다 느끼지만 테스트코드 철저하게 작성하는 모습 멋있어요. 우리는 테스트 ci 터져서 알빠노하고 냅다 삭제해버렸는데;;


boolean memberNameDuplicated = updatedMembers.stream()
.filter(updatedMember -> isMemberNameUpdated(eventMemberNames, updatedMember))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인자로 updatedMember를 받는데, 내부에서 갱신된 멤버인지 확인하는 것이 어색하게 느껴지네요! 외부에서 갱신된 멤버만 받거나, 파라미터명을 수정하면 좋을 것 같아요

@3Juhwan 3Juhwan merged commit 6cc464a into be-dev Sep 25, 2024
3 checks passed
@3Juhwan 3Juhwan deleted the fix/#609 branch September 25, 2024 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 멤버 정보 수정 API 호출 버그 수정
5 participants