Skip to content

Conversation

@leebs0521
Copy link
Collaborator

resolved :

📌 과제 설명

  • 봉사 모집글 조회 기능

👩‍💻 요구 사항과 구현 내용

  • 특정 봉사 모집글 상세 조회
  • 봉사 모집글 리스트 조회
  • 봉사 모집글 리스트 검색 조회
  • 위치 기반 봉사 모집글 리스트 조회
  • 특정 기관 봉사 모집글 리스트 조회

✅ PR 포인트 & 궁금한 점

  • 조인 조회를 위해 mapping 클래스 적절한지 궁금합니다.
  • 동적 쿼리를 위한 condition dto 적절한지 궁금합니다.
  • 레포지토리에서 최대한 테스트해서 서비스는 좀 가볍게 만들었는데 괜찮을지 궁금합니다.
  • Haversine을 이용해서 위치(경도, 위도)기반 조회를 만들었는데 오차가 있어서 나중에 고치겠습니다..

import org.springframework.data.domain.Pageable;

@Builder
public record RecruitBoardNearByCondition(
Copy link
Collaborator

Choose a reason for hiding this comment

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

컨디션 클래스는 가져올 필드들을 명시하는 클래스인건가요?
컨디션 클래스 사용이 처음이라 여쭤봐요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

동적 쿼리 파라미터를 위한 레코드 클래스입니다.
위치 기반 조회 때, keyword = 제목 검색이 있을 수도 없을 수도 있어서 만들어 놨어요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RecruitBoardSearchCondition 레코드 클래스 보시는게 더 이해하시기 좋을거에요

@Builder
@JsonNaming(SnakeCaseStrategy.class)
@Schema(description = "봉사 모집글 위치 포함 응답 DTO")
public record RecruitBoardWithLocationResponseDto(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Response에서 도메인이 많이 엮여있다는게 느껴졌습니다 고생하셨습니다

QCenter center = QCenter.center;

Pageable pageable = condition.pageable();
BooleanExpression predicate = isNotDeleted(recruitBoard)
Copy link
Collaborator

Choose a reason for hiding this comment

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

메서드화 시키는것도 괜찮을 것 같아요

public Optional<RecruitBoard> findById(Long id) {
return recruitBoardRepository.findById(id);
public RecruitBoardResponseDto getById(Long id) {
RecruitBoard recruitBoard = recruitBoardRepository.findById(id).orElseThrow(
Copy link
Collaborator

Choose a reason for hiding this comment

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

재사용성이 높아보이는 부분이라 메서드로 분리시켜도 좋을것 같다는 생각이 들었습니다


import com.somemore.center.domain.Center;

public class CenterFixture {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixture 사용이 편리하지만 몇가지 주의점이 있어서 관련 포스팅 남겨두도록 하겠습니다
범수님이 아닌 다른 팀원분들을 위해 남긴 리뷰입니다!
Fixture 주의사항

Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니다.

Copy link
Collaborator

@ayoung-dev ayoung-dev 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 56 to 63
public static RecruitBoardDetailResponseDto from(RecruitBoardDetail recruitBoardDetail) {
RecruitBoard board = recruitBoardDetail.recruitBoard();
RecruitmentInfo info = board.getRecruitmentInfo();
CenterInfoResponse center = CenterInfoResponse.of(board.getCenterId(),
recruitBoardDetail.centerName());
LocationResponseDto location = LocationResponseDto.of(
recruitBoardDetail.address(), recruitBoardDetail.latitude(),
recruitBoardDetail.longitude());
Copy link
Collaborator

Choose a reason for hiding this comment

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

엮인게 진짜 많네요... 고생하셨어요

Copy link
Collaborator

@m-a-king m-a-king left a comment

Choose a reason for hiding this comment

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

수고하셨어요 범수 님! 좋은 코드 많이 보고 갑니다.

  1. 쿼리는 리팩토링이 필요하다고 생각했지만, 우선순위가 아닌 것을 알기에 따로 멘트를 남기지 않았습니다.
  2. 저는 사실 테스트 코드를 본격적으로 배운 적은 없지만 테스트만을 위한 코드는 존재해서는 안 된다라고 배웠는데 본격적인 테스트를 위해서는 Fixture가 필요한지도 여쭤보고 싶습니다.
  3. mapper 패키지는 repository에서 사용하는 것 같은데 위치가 repository 아래로 가면 어색할까요?


import com.somemore.center.domain.Center;

public class CenterFixture {
Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

왜 JPA로 바꾸셨는지 여쭤보고 싶습니다

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

빌드에 실패했습니다.

@github-actions github-actions bot changed the title Feature/49 봉사 모집글 조회 기능 [BUILD FAIL] Feature/49 봉사 모집글 조회 기능 Nov 29, 2024
@github-actions github-actions bot closed this Nov 29, 2024
@leebs0521 leebs0521 reopened this Nov 29, 2024
@leebs0521 leebs0521 force-pushed the feature/49-add-recruit-board-query branch from 5c20dd4 to 3ee132e Compare November 29, 2024 03:25
@sonarqubecloud
Copy link

@leebs0521 leebs0521 merged commit d4d2790 into main Nov 29, 2024
2 checks passed
@leebs0521 leebs0521 deleted the feature/49-add-recruit-board-query branch November 29, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants