Skip to content

[FEAT] NoticeCategory 추가#254

Closed
rinarina0429 wants to merge 8 commits intodevfrom
feat/#253
Closed

[FEAT] NoticeCategory 추가#254
rinarina0429 wants to merge 8 commits intodevfrom
feat/#253

Conversation

@rinarina0429
Copy link
Copy Markdown
Member

@rinarina0429 rinarina0429 commented Jan 9, 2025

Related Issue

Key Changes

공지를 내려주는 방식에 변경사항이 있었습니다.

  • 기존방식: 공지에서 모두 같은 아이콘을 사용해 클라에서 처리
  • 변경된방식: 공지마다 다른 아이콘을 사용해 서버에서 이미지값 함께 반환

이에 NoticeCategory를 추가하고 이를 기존 코드에 반영했습니다.

To Reviewers

테이블 변경사항은 DEV DB에만 반영해뒀습니다! 추후 머지되면 PROD DB에도 반영하겠습니다~

References

@rinarina0429 rinarina0429 self-assigned this Jan 9, 2025
@github-actions github-actions Bot requested review from ChaeAg and Kim-TaeUk January 9, 2025 00:43
@rinarina0429 rinarina0429 marked this pull request as draft January 9, 2025 01:26
@rinarina0429 rinarina0429 changed the title [FEAT] Notice 안에 noticeIcon 추가 [FEAT] NoticeCategory 추가 Jan 9, 2025
@rinarina0429 rinarina0429 marked this pull request as ready for review January 9, 2025 03:15
Copy link
Copy Markdown
Member

@ChaeAg ChaeAg left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 이런 태스크가 남몰래 있었군용 🤔
제가 남긴 코멘트 한 번 확인 부탁드립니다~!~!

.noticeTitle(noticePostRequest.noticeTitle())
.noticeContent(noticePostRequest.noticeContent())
.userId(noticePostRequest.userId())
.noticeCategory(getNoticeCategoryOrException(noticePostRequest.noticeCategoryId()))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P3; 해당 로직에서 공지 카테고리 ID를 받아 공지 카테고리 엔티티를 조회하도록 구현하셨는데, NoticeCategory 엔티티의 noticeCategoryName 컬럼을 활용해보는 건 어떨까요? 현재 방식은 카테고리 ID를 이용해서 카테고리 종류가 많아질 경우 클라이언트가 각 ID를 일일이 외워야 한다는 점에서 다소 불편할 수 있을 것 같다는 생각이 듭니당
카테고리 이름을 활용하면 클라이언트 입장에서 더 유연한 접근이 가능해질 것 같아요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

아하! 클라에서 사용하지 않는 api라 그냥 이렇게 만들었는데 저희가 그냥 포스트맨에서 쓰기에도 그게 편하겠네요. 수정하겠슴다~!👍🏻👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] NoticeCategory 추가

2 participants