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 마이그레이션 #201

Merged
merged 23 commits into from
May 31, 2024

Conversation

mikekks
Copy link
Member

@mikekks mikekks commented May 25, 2024

👩‍💻 Contents

  • 맥락 : 모임 신청자 리스트 조회 API 에 기능적 이슈가 있었고, 해당 API에 대한 수정이 필요한 상황이었습니다. 이왕 수정해야하기 때문에 아예 spring으로 마이그레이션하면서 수정하는게 더 낫겠다고 판단해서 진행했습니다.

  • 모임 신청자 리스트 조회 API 마이그레이션 했습니다.

  • 스터디 개설자가 아닌 경우 전하는 말에 빈값이 들어가도록 구현했습니다.

📝 Review Note

페이지네이션

  • 이전 구현은 제가 이해하기론 먼저 메모리에 올린 후, 서비스 레이어에서 페이지네이션 작업을 해준 것으로 이해했습니다! 해당 방식은 쿼리가 1번 나간다는 장점이 있다고 생각이 들었지만, "서비스 레이어 코드의 복잡함 + 테이블 전체 데이터를 메모리에 올린다는 부담감"이 있다고 생각했습니다.
  • 그래서 repository 단에서 querydsl과 offset를 활용해서 페이지네이션을 구현해봤습니다! 카운트 쿼리 1번이 추가됐지만 성능에 큰 차이를 주진 않을 것 같았습니다! 해당 제안에 대해 어떻게 생각하시는지 궁금합니다!
  • 또한, Pageable 객체를 사용해봤습니다! 휴먼에러를 방지할 수 있는 것 같아서 저는 애용하는데요! 현재 ApplySearchRepositoryImpl 에서 파라미터로 받고 있습니다.

querydsl

테스트 코드

  • 이 API는 querydsl를 사용했기 때문에 Repository 쪽 테스트가 가장 중요하다고 생각했습니다.
  • 그래서 그 쪽을 조금 더 꼼꼼히 봤고, 나머지 레이어도 추가하겠습니다..!
  • 또한, mock 데이터를 위해 @Sql 를 사용했습니다. 해당 sql 파일은 오직 ApplyRepositoryTest 클래스에서만 사용하는 것을 의도했습니다.
  • 참고링크 : https://cheese10yun.github.io/spring-about-test/#null
  • sql 파일을 gitignore를 하려다가 말았는데요! 저희 코드가 public여서 db구조는 누구나 볼 수 있기에 큰 의미가 없다고 생각했습니다. 또한, 개발자간 sql파일을 sync 맞추는데도 꽤 리소스가 들기 때문에 그냥 github에 일부러 올려뒀습니다!
  • 테스트 컨테이너를 사용하고 있기 때문에 테스트 코드 성공을 위해선 도커 데몬이 돌아가고 있어야 합니다!

아쉬운 점

  • status(모임 승인, 거절, 대기 상태)를 받아서 Enum으로 변경하는 과정을 MeetingGetApplyListCommand 내에서 구현했는데요! 이 부분이 뭔가 좀 어색하게 느껴져서 더 좋은 방법있으시면 환영입니다..!
  • 뿐만 아니라 jsonb 타입도 db에서 불러오는 과정에서 가장 최근 활동(UserActivity)를 찾는 로직을 ApplicantDto 에 구현했는데요! 이 부분도 어색하다고는 느껴졌지만 더 좋은 방법이 안 떠올라 일단 말씀드려봅니다!
스크린샷 2024-05-26 오전 12 06 00 스크린샷 2024-05-26 오전 12 11 31 스크린샷 2024-05-26 오전 12 11 42

📣 Related Issue

✅ 점검사항

  • docker-compose.yml 파일에 마이그레이션 한 API의 포워딩을 변경해줬나요?
  • Spring Secret 값을 수정하거나 추가했다면 Github Secret에서 수정을 해줬나요?
  • Nestjs Secret 값을 수정하거나 추가했다면 Docker-Compose.yml 파일 및 인스턴스 내부의 .env 파일을 수정했나요?

@mikekks mikekks added the 🎁 feature 새로운 기능 label May 25, 2024
@mikekks mikekks self-assigned this May 25, 2024
Copy link

height bot commented May 25, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@mikekks mikekks changed the title [FEAT] 구인 리스트 조회 API 마이그레이션 [FEAT] 모임 리스트 조회 API 마이그레이션 May 26, 2024
@mikekks mikekks changed the title [FEAT] 모임 리스트 조회 API 마이그레이션 [FEAT] 모임 신청자 리스트 조회 API 마이그레이션 May 26, 2024
Copy link
Member

@yeseul106 yeseul106 left a comment

Choose a reason for hiding this comment

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

테스트 코드까지 너무 꼼꼼하게 잘 작성해주신 것 같아요 !!!! sql문으로 테스트 코드 시, 필요한 셋업 해두는거 좋은거 같아요 ! 많이 배워갑니다 🙇‍♀️🙇‍♀️ 그리고 간단한 제 생각 코멘트로 조금 남겨봤으니 확인해주시면 감사할거 같아요 :) 정말정말 고생하셨습니다 !!

그리고 리뷰 노트에 남겨주신 내용 잘 다 확인했습니다 ! 아래는 남겨주신 부분에 대한 제 생각을 간단하게 정리해봤습니다.

  • status(모임 승인, 거절, 대기 상태)를 받아서 Enum으로 변경하는 과정을 MeetingGetApplyListCommand 내에서 구현했는데요! 이 부분이 뭔가 좀 어색하게 느껴져서 더 좋은 방법있으시면 환영입니다..!

이건 저도 헷갈려서 여쭤보는건데, DB에는 status 값이 정수로 저장이 되어있고 이를 코드단에 Enum으로 관리하기 위해서 엔티티에는 Enum으로 Convert 해놓고 있는데요 !
QueryDsl을 통해 where 절로 해당 값을 기준으로 가져올 때 쿼리로 받은 Integer 그대로 Enum 변환없이 하면 오류가 나는걸까요 ?!

뿐만 아니라 jsonb 타입도 db에서 불러오는 과정에서 가장 최근 활동(UserActivity)를 찾는 로직을 ApplicantDto 에 구현했는데요! 이 부분도 어색하다고는 느껴졌지만 더 좋은 방법이 안 떠올라 일단 말씀드려봅니다!

음,,, 어떤 부분이 고민인지 충분히 공감합니다 ! 제 생각에도 특정 API 응답 형태에 맞춰진 데이터 처리 로직을 DTO 내부에 두어 API 스펙이 변경되더라도 관련 로직을 한 곳에서 관리할 수 있다는 점에서는 Dto 단에 로직을 구현해도 좋을 거 같은데 !

Dto가 데이터 전달의 역할 이상을 하게 되면 SRP(단일 책임 원칙)을 벗어나는 것 같아, 최근 기수 뽑아내는 로직이 다른 곳에서도 중복으로 사용될 가능성이 있다면 객체가 스스로 상태를 관리하도록 엔티티 내에 구현하거나, util 클래스로 분리하는 것도 고려해볼 만하다고 생각합니다 ! 이에 대해서는 다른 분들의 의견도 궁금하네요 !!

@sgh002400
Copy link
Contributor

sgh002400 commented May 27, 2024

많은 고민과 정성을 한 것이 느껴지네요 !! 저도 항상 많이 배우고 있어요 :)
querydsl도 붙이고 싶었는데 여러모로 고생 많으셨습니다 👍🏻

status(모임 승인, 거절, 대기 상태)를 받아서 Enum으로 변경하는 과정을 MeetingGetApplyListCommand 내에서 구현했는데요! 이 부분이 뭔가 좀 어색하게 느껴져서 더 좋은 방법있으시면 환영입니다..!

예슬님 의견대로 그대로 조회하는 것도 가능할 것 같기는 하지만 그렇게 되면 의도치 않은 값이 들어 왔을 때 대처가 안될 것 같다는 생각이 드네요 !
저는 밑에 예슬님이 리뷰 남겨주신 ModelAttribute로 받아서 utils 같은 걸로 빼서 변환해주는 것도 괜찮은 방법이 될 것 같습니다.
이건 민규님의 질문 방향에 살짝 벗어나기는 하지만 가독성, 유지보수, 일관성 관점에서 클라이언트와 논의해서 상수가 아닌 enum 값을 파라미터로 받는 것으로 스펙을 논의해보는게 어떨까 싶습니다! 바뀔 일이 있을까 싶기는 하지만, 지금과 같이 API 요청에 상수를 사용한다면 WAITING이 0이 아닌 1로 변경된다면 클라이언트 측에서 여러 코드를 수정해야 될테니까요. 만약 요청에 enum으로 준다면 서버측에서 굳이 int 타입을 변환하지 않고 enum 그대로 받을 수도 있을테고요. 하지만 문제가 있는 것도 아니고 이 또한 클라이언트 측에 공수가 들테니,, 단지 의견으로만 봐주시면 될 것 같습니다 :)

뿐만 아니라 jsonb 타입도 db에서 불러오는 과정에서 가장 최근 활동(UserActivity)를 찾는 로직을 ApplicantDto 에 구현했는데요! 이 부분도 어색하다고는 느껴졌지만 더 좋은 방법이 안 떠올라 일단 말씀드려봅니다!

이 부분은 예슬님과 동일한 의견입니다~

Copy link
Contributor

@sgh002400 sgh002400 left a comment

Choose a reason for hiding this comment

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

👍🏻👍🏻

@yeseul106 yeseul106 self-requested a review May 30, 2024 10:12
Copy link
Member

@yeseul106 yeseul106 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 !!!!

@mikekks mikekks merged commit 64afced into develop May 31, 2024
1 check passed
@mikekks mikekks deleted the feat/#198 branch May 31, 2024 04:28
sgh002400 pushed a commit that referenced this pull request May 31, 2024
* [ADD] querydsl 의존성 추가

* [ADD] queryFactory 추가

* [ADD] 신청자 정보 관련 Dto 추가

* [FEAT] querydsl를 사용한 신청자 리스트 조회

* [ADD] 신청자 정보 관련 Dto 추가

* [ADD] 컨트롤러 -> 서비스 간의 Dto 추가

* [ADD] 신청자 리스트 조회 API 스웨거 반영

* [FEAT] 신청자 리스트 조회 컨트롤러 구현

* [FEAT] 신청자 리스트 조회 서비스 로직 구현, 페이지네이션 구현

* [ADD] querydsl 사용을 위한 config 추가

* [ADD] ApplyRepositoryTest를 위한 sql 추가

* [REFACTOR] apply와 Meeting 간의 의존성 줄이기

* [TEST] ApplyRepository querydsl 로직 테스트

* [CHORE] type(신청, 초대) 제거

* [CHORE] @ModelAttribute 사용

* [CHORE] 주석제거

* [CHORE] studyCreatorId -> meetingCreatorId 변경

* [CHORE] join문 삭제

* [CHORE] leftjoin -> innerjoin 변경

* [CHORE] Dto 변수명 변경

* [CHORE] Dto 변수명 변경

* [CHORE] jsonb 타입 처리 UserUtil에서 처리

* [CHORE] 디폴트 value 설정
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎁 feature 새로운 기능 size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 모임 신청자 리스트 조회 API 마이그레이션
3 participants