Skip to content

✨ refactor: Notice 도메인 리팩터링(공지 추가/공지 조회)#32

Merged
5nam merged 5 commits intodevelopfrom
refactor/29-notice
Apr 24, 2025
Merged

✨ refactor: Notice 도메인 리팩터링(공지 추가/공지 조회)#32
5nam merged 5 commits intodevelopfrom
refactor/29-notice

Conversation

@5nam
Copy link
Collaborator

@5nam 5nam commented Apr 14, 2025

✨ refactor: Notice 도메인 리팩터링(공지 추가/공지 조회)

  • refactor: Notice 도메인 리팩터링(공지 추가/공지 조회)

📌 변경 사항 (What’s changed?)

  • Notice 공지 조회 페이징 처리(Slice, 무한 스크롤로 구현)
  • Notice 공지 추가 권한 확인 로직 추가

⚙️ 변경 이유 (Why?)

  • 공지 조회 기능 페이징 처리 : 한번에 많은 공지 데이터를 불러와야 할 경우 성능 저하를 대비하여, 그리고 스크롤 형식이므로 더 적합하다고 생각
  • 공지 추가 권한 확인 : 공지는 운영진만 남길 수 있도록 변경해야 하므로(권한)

✅ 테스트 방법 (How to test?)

  • notice test 코드로 테스트
  • controller 수동 테스트 완료

🤔 기타 참고 사항

  • 알림 기능 merge 후 공지 생성 로직에 알림 전송되는 로직 추가해야 함

💬리뷰 요구사항(선택)

  • 코드 보시고 편하게 피드백 남겨주세요!

@5nam 5nam added the ♻️ refactor Refactor code label Apr 14, 2025
@5nam 5nam self-assigned this Apr 14, 2025
@5nam 5nam requested review from hojooo and y3binchoi as code owners April 14, 2025 05:14
@5nam 5nam linked an issue Apr 14, 2025 that may be closed by this pull request
Copy link
Collaborator

@y3binchoi y3binchoi left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~!! 추가로 고려할 점을 꼼꼼히 TODO나 FIXME로 남겨두신 점도 너무 좋아요!!

private final JPAQueryFactory queryFactory;

@Override
public List<Notice> findByCursor(Long cursor, int size, Club club) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

저희 클라이언트가 모바일이기 때문에 스크롤 할 것을 고려하여 커서 방식의 페이지네이션을 선택하신 점 너무 좋아요!


public interface NoticeRepository extends JpaRepository<Notice, Long> {
List<Notice> findByClub(Club club);
public interface NoticeRepository extends JpaRepository<Notice, Long>, NoticeRepositoryCustom {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NotificationRepositoryImplNotificationRepository의 구현체로 해석될 위험은 없을까요? 혹시 이름을 NotificationJpaRepository로 변경하는건 불필요한 일일까요?

Copy link
Collaborator Author

@5nam 5nam Apr 24, 2025

Choose a reason for hiding this comment

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

아 네이밍에 대한 생각을 못했네요. Jpa로 바꾸는게 좋을 거 같습니다! 반영해서 머지하겠습니다! 좋은 피드백 감사합니다 ㅎㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

지금 오류가 나서 찾아보니까, JPA의 커스텀 리포지토리 규칙이 있다고 합니다! 그래서

XxxRepositoryCustom → 구현체명은 반드시 XxxRepositoryImpl이어야 한다고 합니다!

https://imprint.tistory.com/142

링크 공유 드립니다!

@hojooo
Copy link
Collaborator

hojooo commented Apr 20, 2025

커서 기반 페이지네이션이라는 방법을 처음 알았네요. 수고하셨습니다!

@5nam 5nam merged commit 1b32e5d into develop Apr 24, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notice 도메인 리팩터링

3 participants