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: 셀프 소개 기능을 구현한다 #46

Merged
merged 28 commits into from
Jul 17, 2024
Merged

feat: 셀프 소개 기능을 구현한다 #46

merged 28 commits into from
Jul 17, 2024

Conversation

eom-tae-in
Copy link
Owner

@eom-tae-in eom-tae-in commented Jul 16, 2024

📄 Summary

셀프 소개 기능을 구현하였습니다.

  • 생성, 조회, 수정, 삭제 기능은 모두 토큰 인증이 된 유저만 요청할 수 있습니다.
  • 조회는 처음 메인페이지 접근 시 사용할 페이징 조회와 사용하는 유저가 필터 조건을 추가하여 조회할 수 있는 페이징 조회 기능이 있습니다.
  • 필터로 추가할 수 있는 조건은 연령대, 성별, 선호 지역이 있습니다.

🙋🏻 More

  • 셀프 소개 기능은 member 컨텍스트 안에 구현하였으며 약간의 파일 수정이 있어 다른 도메인의 코드 수정이 있었습니다.
  • 추가해야 하거나 부족한 부분이 있다면 리뷰 남겨주시면 감사하겠습니다!

close #45

- InvalidContentException
- SelfIntroNotFoundException
- WriterNotEqualsException
- CityRequest
- SelfIntroCreateRequest
- SelfIntroFilterRequest
- SelfIntroUpdateRequest
- SelfIntroRepository
- SelfIntroJpaRepository
- SelfIntroQueryRepository
- SelfIntroRepositoryImpl
Copy link
Collaborator

@devholic22 devholic22 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 몇 가지 궁금한 거랑 수정하면 좋을 만한 것들만 봐주시면 될 것 같습니다.

}

private ResponseEntity<String> getBadRequest(final Exception e) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST)
private ResponseEntity<String> getErrorMessageWithStatus(final Exception e, final HttpStatus status) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 했던 구조처럼으로 재사용하는 방식을 사용하셨군요 좋습니다!

}

public void validateSameWriter(final Long memberId) {
if (!Objects.equals(this.memberId, memberId)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

저였다면 습관적으로 직접 두 필드를 비교하는 방식을 썼을 것 같은데 Objects에 있는 걸 이용하면 더 간단히 작성할 수 있어서 좋은 것 같습니다!

selfIntroRepository.deleteById(foundSelfIntro.getId());
}

private SelfIntro findSelfIntroById(final Long selfIntroId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

updateSelfIntro와 deleteSelfIntro에서 모두 findSelfIntroById를 거친 뒤 validateSameWriter를 호출하고 있는데, 이러한 경우라면 findSelfIntroById 안에 validateSameWriter까지 넣어보는 것은 어떨까요? SelfIntro 조회 시 validateSameWriter를 호출해야 한다는 조건이 들어가있다면 findSelfIntroById에서 넣어둬도 괜찮지 않을까라는 생각이 드는데 의견이 궁금합니다 :)

fieldWithPath("selfIntros[].age").description("작성자의 나이"),
fieldWithPath("selfIntros[].height").description("작성자의 키"),
fieldWithPath("nowPage").description("현재 페이지"),
fieldWithPath("nextPage").description("다음 페이지"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

다른 API 문서들처럼 다음 페이지가 없다면 -1로 나오게 되는 것도 추가해주시면 좋을 것 같습니다!

.extract();
}

protected void 셀프_소개글_페이징_조회_요청_겅즘(final ExtractableResponse<Response> response) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

겅즘 -> 검증으로 고쳐주시면 될 것 같습니다


public class SelfIntroResponseFixture {

private static final Long SELF_INTRO_ID = 1L;
Copy link
Collaborator

Choose a reason for hiding this comment

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

픽스처 객체에서도 반복적으로 사용된다면 작업하신 것 처럼 상수화해두는 게 더 좋은 거 같네요!

public SelfIntrosResponse findAllSelfIntrosWithPaging(final Pageable pageable,
final Long memberId) {
Page<SelfIntroResponse> selfIntroResponses = selfIntroRepository.findAllSelfIntrosWithPaging(pageable,
memberId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

memberId를 위로 올리셔도 좋을 것 같습니다 :)

);
}

private static CityRequest 선호_지역_요청서() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

CityRequest fixture를 만듦에 있어서 간단한 것이다보니 따로 메서드 분리를 하지 않아도 괜찮을 것 같은데, 어떻게 생각하시는지 궁금합니다!

Copy link
Owner Author

Choose a reason for hiding this comment

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

말씀하신대로 분리하지 않아도 괜찮아보이네요 :)
수정하도록 하겠습니다~

@RequestMapping("/api/members/self-intros")
public class SelfIntroController {

private static final String CREATION_TIME = "createAt";
Copy link
Collaborator

Choose a reason for hiding this comment

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

보통 BaseEntity에서나 다른 클래스들에서 사용하고 있는 것들은 createdAt인데, 그것에 맞춰서 정렬하게끔 작성해주시면 좋을 것 같습니다..!

Copy link
Owner Author

Choose a reason for hiding this comment

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

수정하겠습니다

import org.springframework.data.jpa.repository.JpaRepository;

public interface SelfIntroJpaRepository extends JpaRepository<SelfIntro, Long> {
}
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
Owner Author

Choose a reason for hiding this comment

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

jpa에 내장된 메서드는 안해도 무방해보였는데 지정해준다면 도메인에서 사용되는 기능을 파악하기 용이해보이네요.
추가하도록 하겠습니다!

Copy link
Collaborator

@devholic22 devholic22 left a comment

Choose a reason for hiding this comment

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

변경해야 할 것 잘 반영해주신 것 같아서 승인하겠습니다! 수고하셨습니다 :)

@eom-tae-in eom-tae-in removed the request for review from sosow0212 July 17, 2024 03:28
@eom-tae-in eom-tae-in merged commit 0a8d8cb into develop Jul 17, 2024
1 check passed
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.

셀프 소개 기능을 구현한다
2 participants