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] 랜덤 참여자들 가끔씩 5명만 보이는 문제 해결(#720) #722

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 5, 2024

📌 관련 이슈

✨ PR 세부 내용

랜덤 참여자들이 한번씩 5명씩 보이는 문제를 해결했습니다.

기존 코드에서 limit() 함수가 filter()보다 먼저 호출되어, 필터링 조건에 맞지 않는 null 값이 포함된 상태에서 제한된 개수가 반환되는 문제가 있었습니다.

List<RoomParticipantResponse> roomParticipantResponses = participants.stream()
            .limit(RANDOM_DISPLAY_PARTICIPANTS_SIZE) // limit() 함수가 filter() 함수 이전에 호출되고 있습니다.
            .map(participation -> getRoomParticipantResponse(roomId, participation))
            .filter(Objects::nonNull)
            .toList();

해결을 위해 filter()를 limit() 위로 위치를 변경했고, 이에 따른 수정 전후 테스트 기록을 커밋에 남겨두고 제거했습니다.
테스트 기록 (해결 된 후에는 굳이 남겨놓을 필요가 없다 생각들어 테스트 제거했습니다!)

또한, 기존 테스트 코드를 보완하여 본인, 리뷰어, pr 미제출자가 잘 제외가 되는지 테스트를 테스트 목적에 맞게 추가했습니다.

@github-actions github-actions bot added BE 백엔드 개발 관련 작업 리팩터링 리팩터링 작업 labels Nov 5, 2024
Copy link
Contributor Author

github-actions bot commented Nov 5, 2024

Test Results

 61 files   61 suites   8s ⏱️
194 tests 178 ✅ 16 💤 0 ❌
209 runs  193 ✅ 16 💤 0 ❌

Results for commit c0af077.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@youngsu5582 youngsu5582 left a comment

Choose a reason for hiding this comment

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

간단한 코드인데 테스트 보강 굳이용 👍

return participationRepository.findAllByRoomId(roomId)
.stream()
.filter(participation -> isValidParticipant(participation, memberId))
.collect(Collectors.toCollection(ArrayList::new));
Copy link
Contributor

Choose a reason for hiding this comment

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

와 이거 까먹고 있었네 👍

.orElse(null);
}

private RoomParticipantResponse toRoomParticipantResponse(MatchResult matchResult) {
return new RoomParticipantResponse(
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 그냥 DTO 가 Participation 받아서 변환하게 하는건 안되나용?

Copy link
Contributor

Choose a reason for hiding this comment

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

-> 이거 그냥 DTO 가 MatchResult 받아서 변환하게 하는건 안되나용?
이 말 맞죠??
그렇네요~ DTO가 변환하도록 수정합니다~ 👍👍

Copy link
Contributor

@ashsty ashsty left a comment

Choose a reason for hiding this comment

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

안녕하세요 뽀로로!

테스트 보강 진짜 짱이네요
유지보수 킹왕짱 >_< 👍

고생하셨습니다!

Room room = roomRepository.save(RoomFixture.ROOM_DOMAIN(manager));
participationRepository.save(new Participation(room, manager, MemberRole.REVIEWER, ParticipationStatus.MANAGER, room.getMatchingSize()));
@Nested
@DisplayName("방에 참여한 사람들에 정보를 알 수 있다.")
Copy link
Contributor

Choose a reason for hiding this comment

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

ㅎㅎ

Suggested change
@DisplayName("방에 참여한 사람들에 정보를 알 수 있다.")
@DisplayName("방에 참여한 사람들의 정보를 알 수 있다.")

Copy link
Contributor

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 솔직히 저건 헷갈릴만 했다. 인정하시죠?

Copy link
Contributor

@hjk0761 hjk0761 left a comment

Choose a reason for hiding this comment

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

테스트 보완까지 이쁘게 된 것 같네요 ㅎㅎ
고생많았습니다!
좋은 것 같아 바로 approve 드려용

@jcoding-play jcoding-play merged commit 9a2e8de into develop Nov 7, 2024
4 checks passed
@jcoding-play jcoding-play deleted the feat/#720 branch November 7, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 개발 관련 작업 리팩터링 리팩터링 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 랜덤 참여자들 가끔씩 5명만 보이는 문제 해결
4 participants