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

구현 - 현재 상영중인 영화 목록 추가 #26

Merged
merged 10 commits into from
Oct 6, 2024

Conversation

ag502
Copy link
Collaborator

@ag502 ag502 commented Sep 29, 2024

PR 타입

  • 기능 추가
  • 버그 수정
  • 의존성, 환경 변수, 빌드 관련 코드 업데이트
  • 문서 수정
  • 기타

연관 이슈

resolve #24

개요

상영중인 영화 목록 리스트 추가

변경 사항

  • 현재 상영중인 영화 API 추가
  • 영화 장르 API 추가
  • 현재 상영중인 영화 목록 모델 추가
  • 영화 아이템 모델에 장르 필드 추가

리뷰시 참고할 사항

@ag502 ag502 added the feature label Sep 29, 2024
@ag502 ag502 requested a review from f-lab-james September 29, 2024 03:43
@ag502 ag502 self-assigned this Sep 29, 2024
Copy link

@@ -9,6 +9,7 @@ import { MovieListReqParams } from "./request-types";
export const movieListQueryKeys = {
popularMovieList: () => ["popular-movie-list"] as const,
upcomingMovieList: () => ["upcoming-movie-list"] as const,
nowPlayingMovieList: () => ["now-playing-movie-list"] as const,
};
Copy link

Choose a reason for hiding this comment

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

아래 팩토리 함수를 구현해서 all을 무조건 받을 수 있도록 만들어보시면 좋을 것 같습니다.

makeKeysFactory('movie-list', {
    popularMovieList: () => ["popular-movie-list"] as const,
    upcomingMovieList: () => ["upcoming-movie-list"] as const,
    nowPlayingMovieList: () => ["now-playing-movie-list"] as const,
})

return this.movieList.results.filter(({ adult }) => !adult).map((item) => new MovieListItem(item));
return this.movieList.results
.filter(({ adult }) => !adult)
.map((item) => new MovieListItem(item, this.genresModel));
}
Copy link

Choose a reason for hiding this comment

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

MovieListItem을 위해 genresModel을 넣어주는게 싫다면 외부에서 주입하는 것도 방법일 것 같습니다.

new MovieList().setXxx().setXxx().build()

@ag502 ag502 merged commit 381105d into develop Oct 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] 상영중인 영화 목록 추가
2 participants