Skip to content

epic/뉴스리스트조회 #74

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.

Sign up for GitHub

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

Jump to bottom
Merged
net79736 merged 14 commits into from
Jan 31, 2025
Merged

epic/뉴스리스트조회 #74

net79736 merged 14 commits into from
Jan 31, 2025

Conversation

@SangkiHan
Copy link
Contributor

@SangkiHan SangkiHan commented Jan 24, 2025 •
edited
Loading

기능 설명

  • 뉴스 리스트 조회 기능
  • 페이징 적용
  • 카테고리, 정렬 기준에 요청값 받아 대응되도록 구현
  • 페이징에 필요한 DTO 생성해뒀으니 참고하셔서 가져다 쓰시면됩니다
  • DTO를 Controller Service 이곳 저곳에 가져다 쓰면 Service에 기능이 추가되거나 수정이 될 시 DTO수정시 여기저기서 수정이 대거 일어나서 Controller Service에 쓰이는 DTO를 분리하여 생성하였습니다. 어떠신가요? 의견 부탁드립니다!

작업 내용

  • QueryDsl 세팅
  • NewsController
  • NewsReadService
  • NewsQueryRepository
  • 테스트코드

수정 사항

  • 댓글작업 진행 예정

추가 작업 예정

  • 아직 크롤링 배치 구현 X

테스트

1,2,3 테스트코드로 구현 확인완료

  • 단위 테스트 확인(포스트맨 등..)
  • 통합 테스트 확인(서버 빌드되는지 확인)
  • 비정상 입력 시 오류 메시지 확인
  • AWS에 서버 올라가는지 or Swagger 확인

Zy0ung
Zy0ung approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@Zy0ung Zy0ung left a comment

Choose a reason for hiding this comment

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

코드 확인했습니다! 코드 리뷰까지 쓰시는거 대단하세요ㅠㅠ 보고 배우겠습니다 ✨

Kyoungwoong
Kyoungwoong reviewed Jan 25, 2025
View reviewed changes
src/main/java/org/myteam/server/news/repository/NewsCountRepository.java
import org.myteam.server.news.entity.NewsCount;
import org.springframework.data.jpa.repository.JpaRepository;

public interface NewsCountRepository extends JpaRepository<NewsCount, Long> {
Copy link
Contributor

@Kyoungwoong Kyoungwoong Jan 25, 2025

Choose a reason for hiding this comment

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

저희 JpaRepository를 상속받으면 @Repository 어노테이션 삭제하는 것이 정책인가요?
오히려 있는 것이 가독성 측면에서 좋지 않을까요?

Copy link
Contributor Author

@SangkiHan SangkiHan Jan 28, 2025

Choose a reason for hiding this comment

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

저는 JpaRepository를 상속했으니 Repository라는건 정확하게 인지가 된다고 생각해서 제거를 한겁니다
근데 영웅님 말씀대로 다른분들도 @repository가 있는게 더 명확하다 하시면 명시해줘도 무방하다고 생각합니다!

@Zy0ung @net79736 어떠신가요???

Copy link
Contributor

@Zy0ung Zy0ung Jan 28, 2025

Choose a reason for hiding this comment

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

저는 두분 말씀 둘 다 맞다고 생각해서 다수 의견으로 따르겠습니당 :)

Copy link
Contributor Author

@SangkiHan SangkiHan Jan 28, 2025

Choose a reason for hiding this comment

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

확실하게 붙히는걸로 가시죠!

Copy link
Contributor Author

@SangkiHan SangkiHan Jan 28, 2025

Choose a reason for hiding this comment

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

그리고 현재 제가 Entity클래스들은 entity라는 패키지에 넣고 있는데 Entity뿐만 아니라 다른 객체들도 포함될 확률이 있어서 영웅님 처럼 domain 패키지가 더 좋아보이네요!

domain패키지로 다들 통일하는거 어떤가요?

@Zy0ung @Kyoungwoong @net79736

Copy link
Contributor

@Kyoungwoong Kyoungwoong Jan 28, 2025

Choose a reason for hiding this comment

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

의견 들어주셔서 감사합니다😊
domain도 역시 좋습니다!

Copy link
Contributor

@Zy0ung Zy0ung Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

그럼 domain 패키지안에 Enum이나 엔티티 넣으면 될까용?
저도 다음 PR 올리면서 작업 했던 Repository 클래스에 어노테이션 붙여놓겠습니당 !

Copy link
Contributor

@net79736 net79736 Jan 31, 2025

Choose a reason for hiding this comment

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

저도 domain 패키지로 통일 하는 것에 동의합니다.

net79736
net79736 approved these changes Jan 31, 2025
View reviewed changes
Copy link
Contributor

@net79736 net79736 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다^^

src/main/java/org/myteam/server/news/repository/NewsCountRepository.java
import org.myteam.server.news.entity.NewsCount;
import org.springframework.data.jpa.repository.JpaRepository;

public interface NewsCountRepository extends JpaRepository<NewsCount, Long> {
Copy link
Contributor

@net79736 net79736 Jan 31, 2025

Choose a reason for hiding this comment

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

저도 domain 패키지로 통일 하는 것에 동의합니다.

@net79736 net79736 merged commit c61bd57 into dev Jan 31, 2025
@SangkiHan SangkiHan deleted the feat-뉴스-기능 branch January 31, 2025 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@Kyoungwoong Kyoungwoong Kyoungwoong approved these changes

@Zy0ung Zy0ung Zy0ung approved these changes

@net79736 net79736 net79736 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@SangkiHan @net79736 @Kyoungwoong @Zy0ung