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

[FEAT] 선택지별 투표 결과 API 호출 시, 기권자 정보도 함께 조회 #211

Merged
merged 9 commits into from
Aug 20, 2024

Conversation

PgmJun
Copy link
Member

@PgmJun PgmJun commented Aug 19, 2024

Issue Number

close #208

As-Is

선택지 별 투표 결과 조회 시 기권자 정보가 표시되지 않아 누가 기권했는 지 확인할 수 없다.

To-Be

선택지별 투표 결과 API Response DTO에 기권자 정보 추가

Check List

  • 테스트가 전부 통과되었나요?
  • 모든 commit이 push 되었나요?
  • merge할 branch를 확인했나요?
  • Assignee를 지정했나요?
  • Label을 지정했나요?

Test Screenshot

스크린샷 2024-08-19 19 49 22

(Optional) Additional Description

@PgmJun PgmJun added ♻️ refactor 리팩토링 ✨ feat 기능 추가 🍃 BE back end labels Aug 19, 2024
@PgmJun PgmJun added this to the BE Sprint4 milestone Aug 19, 2024
@PgmJun PgmJun self-assigned this Aug 19, 2024
Copy link
Contributor

@leegwichan leegwichan left a comment

Choose a reason for hiding this comment

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

이든 빠르게 구현하느라 고생했어~!
코멘트 달았으니 확인해 줘!

Comment on lines 50 to +58
@Transactional(readOnly = true)
public List<RoomBalanceVote> getVotesInRoom(Room room, BalanceOption balanceOption) {
public List<RoomBalanceVote> getVotesInRoomByOption(Room room, BalanceOption balanceOption) {
return roomVoteRepository.findByMemberRoomAndBalanceOption(room, balanceOption);
}

@Transactional(readOnly = true)
public List<RoomBalanceVote> getVotesInRoomByContent(Room room, BalanceContent balanceContent) {
return roomVoteRepository.findByMemberRoomAndBalanceOptionBalanceContent(room, balanceContent);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

사소한 의견) 해당 메서드 내부에서 한 번만 조회하는데, @Transactional을 붙인 이유가 있을까요?
(그냥 붙어있으니까 붙인건가?)

Comment on lines 97 to 107
private ContentRoomBalanceVoteResponse getContentRoomBalanceVoteResponse(Room room,
BalanceOptions balanceOptions,
List<Member> giveUpMembers) {
List<RoomBalanceVote> firstOptionVotes = roomBalanceVoteService
.getVotesInRoom(room, balanceOptions.getFirstOption());
.getVotesInRoomByOption(room, balanceOptions.getFirstOption());
List<RoomBalanceVote> secondOptionVotes = roomBalanceVoteService
.getVotesInRoom(room, balanceOptions.getSecondOption());
return ContentRoomBalanceVoteResponse.create(balanceOptions, firstOptionVotes, secondOptionVotes);
.getVotesInRoomByOption(room, balanceOptions.getSecondOption());

return ContentRoomBalanceVoteResponse.create(balanceOptions, firstOptionVotes, secondOptionVotes,
giveUpMembers);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

의견) 받은 giveUpMembers를 해당 메서드에서 받아서 그대로 ContentRoomBalanceVoteResponse로 넘겨주는데,
giveUpMembers를 만드는 메서드를 getContentRoomBalanceVoteResponse 내부에서 호출해서 사용하는 것이 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

좋습니다~! ContentRoomBalanceVoteResponse의 생성에 사용하는 값이니 그게 맞겠네요~ 반영완료요!

Copy link
Contributor

Choose a reason for hiding this comment

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

의견) 해당 사항도 반영해 주실 수 있으실까요? (바쁘시면 제가 하겠습니다...)

  • 그룹 내 투표자가 0명인 경우, 그룹 결과 퍼센트를 0, 0으로 반환
    전체 투표자가 0명인 경우, 전체 결과 퍼센트는 50, 50으로 반환

근거

image
image
image

Copy link
Contributor

Choose a reason for hiding this comment

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

위 테스크를 '마루' 의견 반영해서 "투표자가 0명인 경우, 그룹 결과 퍼센트를 0, 0으로 반환"하도록 하는 것 확인했습니다~

Copy link
Member

@GIVEN53 GIVEN53 left a comment

Choose a reason for hiding this comment

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

고생했어요 이든 😊
문서 생성은 -D가 아닌 -P로 해주시면 됩니다 관련PR: #128

./gradlew bootJar -PcreateRestDocs

Property의 약자로 기억 오네가이시마스~~

import ddangkong.domain.room.member.Member;
import java.util.List;

public record GiveUpVoteMemberResponse(List<String> members, int memberCount) {
Copy link
Member

Choose a reason for hiding this comment

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

의견)
컨벤션에 맞춰 개행 부탁드립니다!

@@ -90,6 +95,11 @@ class 방_투표_결과_조희 {
.description("선택지를 선택한 사람 수"),
fieldWithPath("group.secondOption.percent").type(NUMBER)
.description("선택지를 선택한 퍼센트"),
fieldWithPath("group.giveUp").type(OBJECT).description("기권한 멤버 정보"),
fieldWithPath("group.giveUp.members").type(ARRAY)
.description("기권한 멤버 이름들"),
Copy link
Member

Choose a reason for hiding this comment

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

사소한 의견)
'기권한 멤버 닉네임들'은 어떨까요??

Copy link
Member Author

@PgmJun PgmJun Aug 20, 2024

Choose a reason for hiding this comment

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

좋아유~ 변경할게요!
근데 제가 이렇게 처리한 이유가 위에 이미 구현된 (from 커찬) description에 맞춰서 설정한 건데 위에 내용도 같이 변경해두겠습니다~!
image

@PgmJun
Copy link
Member Author

PgmJun commented Aug 19, 2024

고생했어요 이든 😊

문서 생성은 -D가 아닌 -P로 해주시면 됩니다 관련PR: #128

./gradlew bootJar -PcreateRestDocs

Property의 약자로 기억 오네가이시마스~~

아 맞네여.. 피곤해서 능지이슈옴.. 감삼다

Copy link
Contributor

@leegwichan leegwichan left a comment

Choose a reason for hiding this comment

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

변경 사항 확인했습니다~!
CI 확인하고 Merge 해주세요~~~

Copy link
Member

@jhon3242 jhon3242 left a comment

Choose a reason for hiding this comment

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

든든 수고했어요~~ 사소한 의견만 남겼는데 확인해보고 납득되면 반영해주세요~

Comment on lines 82 to 94
private List<Member> getGiveUpVoteMemberResponse(Room room, BalanceContent balanceContent) {
List<Member> roomMembers = memberService.findRoomMembers(room);
List<RoomBalanceVote> votesInRoomByContent = roomBalanceVoteService.getVotesInRoomByContent(room,
balanceContent);

List<Member> voteMembers = votesInRoomByContent.stream()
.map(RoomBalanceVote::getMember)
.toList();

return roomMembers.stream()
.filter(roomMember -> !voteMembers.contains(roomMember))
.toList();
}
Copy link
Member

Choose a reason for hiding this comment

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

사소한 의견) 밑에 메서드랑 위치를 바꿔주시면 더 가독성이 좋을것 같아요~

Comment on lines 84 to 89
List<RoomBalanceVote> votesInRoomByContent = roomBalanceVoteService.getVotesInRoomByContent(room,
balanceContent);

List<Member> voteMembers = votesInRoomByContent.stream()
.map(RoomBalanceVote::getMember)
.toList();
Copy link
Member

Choose a reason for hiding this comment

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

사소한 의견) 이 부분은 따로 메서드 분리하면 더 좋을 것 같아요!

@PgmJun PgmJun merged commit 4f00072 into develop Aug 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍃 BE back end ✨ feat 기능 추가 ♻️ refactor 리팩토링
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEAT] 선택지별 투표 결과 (라운드 투표 통계) API 호출 시, 기권자 함께 조회
4 participants