-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 공지사항 기능 구현 #18
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: 공지사항 기능 구현 #18
Conversation
Uralauah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 깔끔하게 작성된 코드 같습니다.
| public void update(NotificationReq req){ | ||
| this.title = req.title(); | ||
| this.content = req.content(); | ||
| this.isFixed = Boolean.TRUE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 isFixed를 수정되었냐라는 의미로 사용하신 것 같습니다.
사실 엔티티를 설계할 때, 와이어프레임이 제대로 완성되지 않아서 isFixed를 상단 고정 여부로 만들었어서
필요없는 필드이긴합니다.
수정 여부도 BaseTimeEntity에서 modifiedAt으로 자동으로 관리되기 때문에
isFixed필드는 삭제하셔도 무방할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 넵 삭제해두겠습니다
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class NotificationRepositoryImpl implements NotificationRepositoryCustom { | ||
|
|
||
| private final JPAQueryFactory queryFactory; | ||
|
|
||
| @Override | ||
| public Page<Notification> getNotifications(String keyword, Pageable pageable, NotificationSortType sort) { | ||
| BooleanExpression condition = (keyword != null && !keyword.isBlank()) ? | ||
| notification.title.containsIgnoreCase(keyword) : null; | ||
|
|
||
| List<Notification> notifications = queryFactory. | ||
| selectFrom(notification). | ||
| where(condition). | ||
| orderBy(sort.getOrder()). | ||
| offset(pageable.getOffset()). | ||
| limit(pageable.getPageSize()). | ||
| fetch(); | ||
|
|
||
| Long total = Optional.ofNullable( | ||
| queryFactory. | ||
| select(notification.count()). | ||
| from(notification). | ||
| where(condition). | ||
| fetchOne() | ||
| ).orElse(0L); | ||
|
|
||
| return new PageImpl<>(notifications, pageable, total); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이 코드는 공지사항을 페이지로 리턴해주는 코드를 QueryDSL로 작성한 것 같습니다.
근데 JPA로 Page<Notification> notifications = findByTitleContainingIgnoreCase(String keyword, Pageable pageable);
이렇게 한줄로 해결할 수 있는데 이렇게 짜신 이유가 있는지 여쭤보고 싶습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 제가 JPA 숙련도가 떨어지기도 하고, 그냥 가장 먼저 생각났던 방법이 QueryDSL이여서 이렇게 작성했습니다..!
간단하게 한 줄로 해결하고 싶다면 JPA가, 추후에 유지보수성을 고려했을 때 QueryDSL이 나아보입니다.
아직은 프로젝트 규모가 크지 않으니, JPA 한 줄만 쓰는걸로 수정하겟습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 예전 프로젝트에서도 이런 방식으로 QueryDSL을 직접 작성해서 사용했었는데,
그때는 Mysql의 Full-Text-Index를 도입하기 위해서 작성하였습니다.
우리 프로젝트에도 그 기능을 도입하게 된다면 이 코드를 조금 수정해서 적용하면 될 것 같습니다.
Full-Text-Index를 잘 모르신다면 아래 링크를 한번 참고해보시는 것도 좋을 것 같습니다.
Full-Text-Index
| @PostMapping | ||
| public ResponseEntity<?> createNotification( | ||
| @Valid @RequestBody NotificationReq notificationReq, | ||
| @AuthenticationPrincipal SecurityUserDetails userDetails){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AuthenticationPrincipal 애노테이션을 통해 관리자 권한 인증을 하고, 정확한 커스텀 예외 클래스를 던져주는 건 좋은 로직인 것 같습니다!!
권한 인증은 다음과 같은 방법도 있으니 추후 참고해보셔도 좋을 듯 합니다!
@PreAuthorize
공지사항 기능 구현
📝 개요
공지사항 기능을 구현하였습니다.
⚙️ 구현 내용
/admin/notification 엔드포인트에서는 생성/수정/삭제,
/notifications 엔드포인트에서는 조회가 가능하도록 구현했습니다.
두 엔드포인트에 들어오는 요청을 각각 AdminNotification / Notification의 파일로 구분하여 작성했습니다.
📎 기타
AdminNotificationService에서 중복 사용되는 검증 로직을 하나의 메소드로 묶었습니다.
이 때,
요청을 날린 사용자가 ADMIN Role을 가지고 있는지를 검증해주고 있지만, 이는 /admin/ 엔드포인트에 들어오는 요청은 SecurityFilterChain에서 이미 ADMIN인지 검증해주고 있으므로 불필요한 로직일수도 있다는 생각이 듭니다. 혹시 모르니 더블체크를 위해 Service 계층에 검증 로직을 넣어두었습니다.
같은 ADMIN이라도 해당 공지글을 작성한 관리자만이 수정/삭제할 수 있도록 구현했습니다.
QueryDSL 작성을 위한 JPAQueryFactory를 Bean으로 등록하기 위해 JpaConfig 파일을 수정했습니다.
🧪 테스트 결과
관리자 - 공지사항 생성
관리자 - 공지사항 생성 실패
관리자 - 공지사항 수정
관리자 - 공지사항 수정 실패
관리자 - 공지사항 삭제
관리자 - 공지사항 삭제 실패
일반 사용자 - 관리자 권한 접근
일반 사용자 - 공지사항 목록
일반 사용자 - 공지사항 조회
일반 사용자 - 공지사항 조회 실패