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

공지사항과 이벤트 목록을 조회한다. #39

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jundonghyuk
Copy link
Contributor

👍관련 이슈

🤔세부 내용

GET /announcements?type=[notice, event]&page=0&sort=createdAt,[desc, asc] 

🫵리뷰 중점사항

공지사항과 이벤트 모두 어차피 정적페이지를 조회하는거라 하나로 묶어도 괜찮을 듯 합니다.
어떻게 생각하시는지요 ?

그리고 html파일은 서버에서 정적파일로 갖고있는게 나을 것 같습니다. 본문을 디비에 저장하는것보다요.
어떻게 생각하시는지요 ?

@jundonghyuk jundonghyuk added the 📝feature 기능 추가 label Mar 7, 2024
@jundonghyuk jundonghyuk self-assigned this Mar 7, 2024
Copy link

github-actions bot commented Mar 7, 2024

Test Results

106 tests   106 ✅  1s ⏱️
 21 suites    0 💤
 21 files      0 ❌

Results for commit 248aa3d.

Copy link
Contributor

@youngh0 youngh0 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~ 코멘트 남겼으니 확인 부탁드려요 ㅎ

@GetMapping("/announcements")
public ResponseEntity<Page<Announcement>> findAnnouncements(@RequestParam String type, Pageable pageable) {
AnnouncementType announcementType = AnnouncementType.findType(type);
Page<Announcement> announcements = announcementRepository.findAllByAnnouncementType(announcementType, pageable);
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 응답이 나가면 정적파일이 아니라 파일의 filename 이 담긴 json 이 나가게 되는거 아닌가요?? html 파일이 나가나요??


public interface AnnouncementRepository extends JpaRepository<Announcement, Long> {

Page<Announcement> findAllByAnnouncementType(AnnouncementType announcementType, Pageable pageable);
Copy link
Contributor

Choose a reason for hiding this comment

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

Page 로 하신 이유가 있나요?? 제가 Page 를 잘몰라서..

Copy link
Contributor

@This2sho This2sho left a comment

Choose a reason for hiding this comment

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

공지사항과 이벤트 모두 어차피 정적페이지를 조회하는거라 하나로 묶어도 괜찮을 듯 합니다.
어떻게 생각하시는지요 ?

좋습니다.

그리고 html파일은 서버에서 정적파일로 갖고있는게 나을 것 같습니다. 본문을 디비에 저장하는것보다요.
어떻게 생각하시는지요 ?

좋습니다. 근데 그러면 저희가 엔티티로 관리할 필요가 없을거 같은데 어떻게 생각하시나여?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝feature 기능 추가
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

공지사항을 조회한다.
3 participants