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] feat: 모임 상세 조회 API 구현 및 검색 필터링 조건 추가 #269

Merged
merged 8 commits into from
Aug 1, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.woowacourse.friendogly.club.dto.request.FindSearchingClubRequest;
import com.woowacourse.friendogly.club.dto.request.SaveClubMemberRequest;
import com.woowacourse.friendogly.club.dto.request.SaveClubRequest;
import com.woowacourse.friendogly.club.dto.response.FindClubResponse;
import com.woowacourse.friendogly.club.dto.response.FindSearchingClubResponse;
import com.woowacourse.friendogly.club.dto.response.SaveClubMemberResponse;
import com.woowacourse.friendogly.club.dto.response.SaveClubResponse;
Expand Down Expand Up @@ -34,9 +35,18 @@ public ClubController(ClubCommandService clubCommandService, ClubQueryService cl
this.clubQueryService = clubQueryService;
}

@GetMapping("/{id}")
public ApiResponse<FindClubResponse> findById(@Auth Long memberId, @PathVariable Long id) {
return ApiResponse.ofSuccess(clubQueryService.findById(memberId, id));
}

// TODO: @ModelAttribute
Copy link
Contributor

@takoyakimchi takoyakimchi Aug 1, 2024

Choose a reason for hiding this comment

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

@ModelAttribute 이건 어디에 쓸 예정인가요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

현재 여러 개의 쿼리 파라미터를 DTO 객체로 한 번에 받고 있습니다.
이 경우 @ModelAttribute 를 생략할 수 있지만, 해당 요청에 필요한 값을 path parameter로 받는지, query parameter로 받는지, 혹은 body로 받는지 혼동이 있을 수 있다고 생각했어요. 실제로 저희도 개발 중에 헷갈리기도 했구요 ㅎㅎ;

쿼리 파라미터로 요청 받음을 명확하게 표현하기 위해 해당 어노테이션을 붙이는 것을 고려하고 있어요! 트레와 도도의 의견도 궁금합니다~

Copy link
Contributor

Choose a reason for hiding this comment

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

굿입니다~~!!

@GetMapping("/searching")
public ApiResponse<List<FindSearchingClubResponse>> findSearching(@Valid FindSearchingClubRequest request) {
return ApiResponse.ofSuccess(clubQueryService.findSearching(request));
public ApiResponse<List<FindSearchingClubResponse>> findSearching(
@Auth Long memberId,
@Valid FindSearchingClubRequest request
) {
return ApiResponse.ofSuccess(clubQueryService.findSearching(memberId, request));
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 findByFilter 하면 안 되나요 (5트)
찾다 찾다
검색을 찾다
와닿으시는지...

Copy link
Member

Choose a reason for hiding this comment

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

수정 완료 했습니다.!!

}

@PostMapping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,16 @@ public void addClubMember(Member member) {
}

private void validateAlreadyExists(Member member) {
if (clubMembers.stream()
.anyMatch(clubMember -> clubMember.isSameMember(member))) {
if (isAlreadyJoined(member)) {
throw new FriendoglyException("이미 참여 중인 모임입니다.");
}
Comment on lines +140 to 142
Copy link
Contributor

Choose a reason for hiding this comment

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

메서드 분리 좋네요

}

public boolean isAlreadyJoined(Member member) {
return clubMembers.stream()
.anyMatch(clubMember -> clubMember.isSameMember(member));
}

private void validateMemberCapacity() {
if (memberCapacity.isCapacityReached(countClubMember())) {
throw new FriendoglyException("최대 인원을 초과하여 모임에 참여할 수 없습니다.");
Expand All @@ -158,11 +162,23 @@ public void addClubPet(List<Pet> pets) {
}

private void validateParticipatePet(Pet pet) {
if (!allowedGenders.contains(pet.getGender()) || !allowedSizes.contains(pet.getSizeType())) {
if (canNotJoin(pet)) {
throw new FriendoglyException("모임에 데려갈 수 없는 강아지가 있습니다.");
}
}

private boolean canNotJoin(Pet pet) {
return !allowedGenders.contains(pet.getGender()) || !allowedSizes.contains(pet.getSizeType());
}
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
if (canNotJoin(pet)) {
throw new FriendoglyException("모임에 데려갈 수 없는 강아지가 있습니다.");
}
}
private boolean canNotJoin(Pet pet) {
return !allowedGenders.contains(pet.getGender()) || !allowedSizes.contains(pet.getSizeType());
}
if (!canJoin(pet)) {
throw new FriendoglyException("모임에 데려갈 수 없는 강아지가 있습니다.");
}
}
private boolean canJoin(Pet pet) {
return allowedGenders.contains(pet.getGender()) && allowedSizes.contains(pet.getSizeType());
}

not 절이 많아서 읽기 어려울 수 있겠네요
not이 2개인 것 보다는 하나인 게 나을 것 같은데, 이렇게 바꾸는 건 어떨까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

이건 그냥 제안인데
자연어에 가까운 메서드 네이밍을 해보자면
club.canBeJoinedBy(pet) 이런 이름도 괜찮을 것 같아요

Copy link
Member

Choose a reason for hiding this comment

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

canJoinWith로 변경했습니다.


public boolean isJoinable(Member member, List<Pet> pets) {
boolean hasJoinablePet = pets.stream()
.anyMatch(pet -> !canNotJoin(pet));
boolean isNotFull = !this.memberCapacity.isCapacityReached(countClubMember());

return hasJoinablePet && isNotFull && isAlreadyJoined(member) && isOpen();
}
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
public boolean isJoinable(Member member, List<Pet> pets) {
boolean hasJoinablePet = pets.stream()
.anyMatch(pet -> !canNotJoin(pet));
boolean isNotFull = !this.memberCapacity.isCapacityReached(countClubMember());
return hasJoinablePet && isNotFull && isAlreadyJoined(member) && isOpen();
}
public boolean isJoinable(Member member, List<Pet> pets) {
boolean hasJoinablePet = pets.stream()
.anyMatch(pet -> canJoin(pet));
boolean isNotFull = !this.memberCapacity.isCapacityReached(countClubMember());
return hasJoinablePet && isNotFull && isAlreadyJoined(member) && isOpen();
}

아까 리뷰 반영하신다면 이렇게 쓸 수 있어서 더 깔끔할 것 같아요!


public void removeClubMember(Member member) {
ClubMember targetClubMember = findTargetClubMember(member);
clubMembers.remove(targetClubMember);
Expand Down Expand Up @@ -197,14 +213,22 @@ public boolean isEmpty() {
return clubMembers.isEmpty();
}

public boolean isOwner(ClubMember target) {
return findOwner().isSameMember(target.getClubMemberPk().getMember());
public boolean isOpen() {
return this.status == Status.OPEN;
}

public boolean isOwner(Member targetMember) {
return findOwner().isSameMember(targetMember);
}

public Name findOwnerName() {
return findOwner().getClubMemberPk().getMember().getName();
}

public String findOwnerImageUrl() {
return findOwner().getClubMemberPk().getMember().getImageUrl();
}

private ClubMember findOwner() {
return clubMembers.stream()
.min(Comparator.comparing(ClubMember::getCreatedAt))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.woowacourse.friendogly.club.domain;

import com.woowacourse.friendogly.exception.FriendoglyException;

public enum FilterCondition {

ALL,
OPEN,
ABLE_TO_JOIN;

public static FilterCondition toFilterCondition(String filterCondition) {
try {
return valueOf(filterCondition);
} catch (IllegalArgumentException e) {
throw new FriendoglyException("존재하지 않는 FilterCondition 입니다.");
}
}
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
public static FilterCondition toFilterCondition(String filterCondition) {
try {
return valueOf(filterCondition);
} catch (IllegalArgumentException e) {
throw new FriendoglyException("존재하지 않는 FilterCondition 입니다.");
}
}
public static FilterCondition from(String filterCondition) {
try {
return valueOf(filterCondition);
} catch (IllegalArgumentException e) {
throw new FriendoglyException("존재하지 않는 FilterCondition 입니다.");
}
}

Copy link
Member

Choose a reason for hiding this comment

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

DTO 컨밴션이랑 헷갈렸습니다. 반영완료요

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import java.util.Set;

public record FindSearchingClubRequest(
@NotBlank(message = "필터링 조건은 필수입니다.")
String filterCondition,

@NotBlank(message = "주소 정보는 필수입니다.")
String address,

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.woowacourse.friendogly.club.dto.response;

import com.woowacourse.friendogly.member.domain.Member;

public record ClubMemberDetailResponse(
Long id,
String name,
String imageUrl
) {

public ClubMemberDetailResponse(Member member) {
this(member.getId(), member.getName().getValue(), member.getImageUrl());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.woowacourse.friendogly.club.dto.response;

import com.woowacourse.friendogly.pet.domain.Pet;

public record ClubPetDetailResponse(
Long id,
String name,
String imageUrl,
boolean isMine
){

public ClubPetDetailResponse(Pet pet, boolean isMine) {
this(pet.getId(),pet.getName().getValue(), pet.getImageUrl(), isMine);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.woowacourse.friendogly.club.dto.response;

import com.woowacourse.friendogly.club.domain.Club;
import com.woowacourse.friendogly.club.domain.Status;
import com.woowacourse.friendogly.member.domain.Member;
import com.woowacourse.friendogly.pet.domain.Gender;
import com.woowacourse.friendogly.pet.domain.Pet;
import com.woowacourse.friendogly.pet.domain.SizeType;
import java.time.LocalDateTime;
import java.util.List;
import java.util.Set;

public record FindClubResponse(
Long id,
String title,
String content,
String ownerMemberName,
String address,
Status status,
LocalDateTime createdAt,
Set<Gender> allowedGender,
Set<SizeType> allowedSize,
int memberCapacity,
int currentMemberCount,
String imageUrl,
String ownerImageUrl,
boolean isMine,
boolean alreadyParticipate,
boolean canParticipate,
List<ClubMemberDetailResponse> memberDetails,
List<ClubPetDetailResponse> petDetails
) {

public FindClubResponse(Club club, Member member, List<Pet> myPets) {
this(
club.getId(),
club.getTitle().getValue(),
club.getContent().getValue(),
club.findOwnerName().getValue(),
club.getAddress().getValue(),
club.getStatus(),
club.getCreatedAt(),
club.getAllowedGenders(),
club.getAllowedSizes(),
club.getMemberCapacity().getValue(),
club.countClubMember(),
club.getImageUrl(),
club.findOwnerImageUrl(),
club.isOwner(member),
club.isAlreadyJoined(member),
club.isJoinable(member, myPets),
club.getClubMembers().stream()
.map(clubMember -> clubMember.getClubMemberPk().getMember())
.map(ClubMemberDetailResponse::new)
.toList(),
club.getClubPets().stream()
.map(clubPet -> clubPet.getClubPetPk().getPet())
.map(pet -> new ClubPetDetailResponse(pet, pet.isOwner(member)))
.toList()
);
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package com.woowacourse.friendogly.club.service;

import com.woowacourse.friendogly.club.domain.Club;
import com.woowacourse.friendogly.club.domain.FilterCondition;
import com.woowacourse.friendogly.club.dto.request.FindSearchingClubRequest;
import com.woowacourse.friendogly.club.dto.response.FindClubResponse;
import com.woowacourse.friendogly.club.dto.response.FindSearchingClubResponse;
import com.woowacourse.friendogly.club.repository.ClubRepository;
import com.woowacourse.friendogly.club.repository.ClubSpecification;
import com.woowacourse.friendogly.member.domain.Member;
import com.woowacourse.friendogly.member.repository.MemberRepository;
import com.woowacourse.friendogly.pet.domain.Pet;
import com.woowacourse.friendogly.pet.repository.PetRepository;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand All @@ -18,23 +23,56 @@
public class ClubQueryService {

private final ClubRepository clubRepository;
private final MemberRepository memberRepository;
private final PetRepository petRepository;

public ClubQueryService(ClubRepository clubRepository) {
public ClubQueryService(
ClubRepository clubRepository,
MemberRepository memberRepository,
PetRepository petRepository
) {
this.clubRepository = clubRepository;
this.memberRepository = memberRepository;
this.petRepository = petRepository;
}

public List<FindSearchingClubResponse> findSearching(FindSearchingClubRequest request) {
public List<FindSearchingClubResponse> findSearching(Long memberId, FindSearchingClubRequest request) {
Member member = memberRepository.getById(memberId);
List<Pet> pets = petRepository.findByMemberId(memberId);

Specification<Club> spec = ClubSpecification.where()
.equalsAddress(request.address())
.hasGenders(request.genderParams())
.hasSizeTypes(request.sizeParams())
.build();

return clubRepository.findAll(spec).stream()
List<Club> clubs = clubRepository.findAll(spec);

if (FilterCondition.toFilterCondition(request.filterCondition()) == FilterCondition.ABLE_TO_JOIN) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if문안에 많은 괄호가 있어서 if문의 조건이 흐려지는 것 같아요!

Suggested change
if (FilterCondition.toFilterCondition(request.filterCondition()) == FilterCondition.ABLE_TO_JOIN) {
FilterCondition filterConditon = FilterCondition.toFilterCondition(request.filterCondition());
if (filterCondition == FilterCondition.ABLE_TO_JOIN) {
// OR if (filterCondition.isJoinable()) {

Copy link
Contributor

Choose a reason for hiding this comment

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

오 enum class 안에 로직을 넣어버리는게 제일 깔끔하겠네요

Copy link
Member

Choose a reason for hiding this comment

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

오 트레 피드백 반영하면서, 적용했습니다.

return clubs.stream()
.filter(club -> club.isJoinable(member, pets))
.map(club -> new FindSearchingClubResponse(club, collectOverviewPetImages(club)))
.toList();
}
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
if (FilterCondition.toFilterCondition(request.filterCondition()) == FilterCondition.ABLE_TO_JOIN) {
return clubs.stream()
.filter(club -> club.isJoinable(member, pets))
.map(club -> new FindSearchingClubResponse(club, collectOverviewPetImages(club)))
.toList();
}
if (FilterCondition.from(request.filterCondition()) == ABLE_TO_JOIN) {
return clubs.stream()
.filter(club -> club.isJoinable(member, pets))
.map(club -> new FindSearchingClubResponse(club, collectOverviewPetImages(club)))
.toList();
}

filter condition이라는 말이 너무 많아서 헷갈리는데
메서드명 개선하고 static import 하면 어떨까요?

Copy link
Member

@jimi567 jimi567 Aug 1, 2024

Choose a reason for hiding this comment

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

동일성 비교를 enum 내부로 보내면서 개선해봤습니다.

if (FilterCondition.toFilterCondition(request.filterCondition()) == FilterCondition.OPEN) {
return clubs.stream()
.filter(Club::isOpen)
.map(club -> new FindSearchingClubResponse(club, collectOverviewPetImages(club)))
.toList();
}
return clubs.stream()
.map(club -> new FindSearchingClubResponse(club, collectOverviewPetImages(club)))
.toList();
}

public FindClubResponse findById(Long memberId, Long id) {
Club club = clubRepository.getById(id);
Member member = memberRepository.getById(memberId);
List<Pet> pets = petRepository.findByMemberId(memberId);

return new FindClubResponse(club, member, pets);
}

private List<String> collectOverviewPetImages(Club club) {
Map<Long, List<Pet>> groupPetsByMemberId = club.getClubPets()
.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,8 @@ private void validateMember(Member member) {
throw new FriendoglyException("member는 null일 수 없습니다.");
}
}

public boolean isOwner(Member member) {
return this.member.getId().equals(member.getId());
}
}
25 changes: 25 additions & 0 deletions backend/src/main/resources/data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,28 @@ VALUES (1, '부', '곰돌이 컷 원조가 저에요~!', '2010-04-01', 'SMALL',
'https://mblogthumb-phinf.pstatic.net/MjAxNzA0MjhfMjg5/MDAxNDkzMzQzOTgxMDQy.4u283s2rrib7xSWR5IdL1r_yiipEITnM6VofjaJsZPUg.oTUM6w8LeF-orpapq_S8j45mjRxjofwrpq8jhQ5LC5Eg.PNG.fjvfeewt/20170428_104509.png?type=w800'),
(8, '초코', '매일 저녁에 나와요!', '2017-12-01', 'MEDIUM', 'FEMALE',
'https://cdn.pixabay.com/photo/2019/12/08/21/09/animal-4682251_960_720.jpg');

INSERT INTO club (title, content, member_capacity, address, image_url, created_at, status)
VALUES ('전국구 강아지 모임', '주먹이 가장 매운 강이지 모임', 3, '서울 송파구 신천동', 'http://example.com/image.jpg', '2023-08-01 12:00:00', 'OPEN'),
('미녀 강아지 모임', '예쁜 강아지 모임', 5, '서울 송파구 신천동', 'http://example.com/image.jpg', '2023-07-31 01:00:00', 'OPEN');

INSERT INTO club_gender (club_id, allowed_gender)
VALUES (1, 'MALE'),
(1, 'FEMALE'),
(2, 'MALE_NEUTERED'),
(2, 'FEMALE_NEUTERED');

INSERT INTO club_size (club_id, allowed_size)
VALUES (1, 'LARGE'),
(2, 'SMALL'),
(2, 'MEDIUM');

INSERT INTO club_member (club_id, member_id, created_at)
VALUES (1, 6, '2023-07-31 01:00:00'),
(2, 5, '2023-07-31 01:00:00'),
(2, 7, '2023-07-31 02:00:00');

INSERT INTO club_pet (club_id, pet_id)
VALUES (1, 6),
(2, 5),
(2, 7);
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import static org.junit.jupiter.api.Assertions.assertAll;

import com.woowacourse.friendogly.club.domain.Club;
import com.woowacourse.friendogly.club.domain.ClubMember;
import com.woowacourse.friendogly.club.dto.request.SaveClubMemberRequest;
import com.woowacourse.friendogly.club.dto.request.SaveClubRequest;
import com.woowacourse.friendogly.club.dto.response.SaveClubResponse;
Expand Down Expand Up @@ -166,7 +165,7 @@ void deleteClubMember() {

assertAll(
() -> assertThat(savedClub.countClubMember()).isEqualTo(1),
() -> assertThat(savedClub.isOwner(ClubMember.create(savedClub, savedNewMember))).isTrue()
() -> assertThat(savedClub.isOwner(savedNewMember)).isTrue()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.junit.jupiter.api.Assertions.assertAll;

import com.woowacourse.friendogly.club.domain.Club;
import com.woowacourse.friendogly.club.domain.FilterCondition;
import com.woowacourse.friendogly.club.dto.request.FindSearchingClubRequest;
import com.woowacourse.friendogly.club.dto.response.FindSearchingClubResponse;
import com.woowacourse.friendogly.member.domain.Member;
Expand Down Expand Up @@ -43,12 +44,13 @@ void findSearching() {
);

FindSearchingClubRequest request = new FindSearchingClubRequest(
FilterCondition.ALL.name(),
address,
Comment on lines 46 to 48
Copy link
Contributor

Choose a reason for hiding this comment

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

오 굿

Set.of(Gender.FEMALE),
Set.of(SizeType.SMALL)
);

List<FindSearchingClubResponse> responses = clubQueryService.findSearching(request);
List<FindSearchingClubResponse> responses = clubQueryService.findSearching(savedMember.getId(), request);
List<FindSearchingClubResponse> expectedResponses = List.of(
new FindSearchingClubResponse(club, List.of(petImageUrl))
);
Expand Down Expand Up @@ -82,12 +84,13 @@ void findSearching_Nothing() {
);

FindSearchingClubRequest request = new FindSearchingClubRequest(
FilterCondition.ALL.name(),
address,
Set.of(Gender.MALE),
Set.of(SizeType.SMALL)
);

List<FindSearchingClubResponse> responses = clubQueryService.findSearching(request);
List<FindSearchingClubResponse> responses = clubQueryService.findSearching(savedMember.getId(), request);

assertThat(responses.isEmpty()).isTrue();
}
Expand Down
Loading
Loading