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

🔀 :: (ENTRY-203)공지사항 상세조회 API 변경 #49

Merged
merged 4 commits into from
Jun 30, 2024

Conversation

rudeh2926
Copy link
Member

image

@rudeh2926 rudeh2926 changed the title 공지사항 상세조회 API 변경 🔀 :: (ENTRY-203)공지사항 상세조회 API 변경 Jun 30, 2024
Copy link

linear bot commented Jun 30, 2024

@@ -20,10 +21,13 @@ class QueryDetailsNoticeService(
val notice = noticeRepository.findByIdOrNull(noticeId) ?: throw NoticeNotFoundException
val imageURL = notice.fileName?.let { getUrl(it, PathList.NOTICE) }

val attachFileUrls =
val attachFile =
Copy link
Member

Choose a reason for hiding this comment

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

내가 왜 확인 못했지...싶긴 한데 굳이 generateObjectUrl을 getUrl 함수로 감쌀 필요가 있나 싶음

Copy link
Member Author

Choose a reason for hiding this comment

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

attachfile이랑 image에 똑같이 fileUtil.generateObjectUrl 하는것보다 함수로 따로뺴서 하는게 좋다고 생각했기 때문입니다

@@ -9,5 +9,10 @@ data class QueryDetailsNoticeResponse(
val createdAt: LocalDateTime,
val type: NoticeType,
val imageURL: String?,
val attachFileUrl: List<String> = emptyList()
val attachFile: List<AttachFile>
Copy link
Member

Choose a reason for hiding this comment

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

attachFiles가 맞을듯 그래도. 타입은 복수형이니까 네이밍 지켜야지

Copy link
Member Author

Choose a reason for hiding this comment

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

변경하겠습니다

Comment on lines 26 to 30
AttachFile(
attachFileUrl = getUrl(it.attachFileName, PathList.ATTACH_FILE),
attachFileName = it.attachFileName
)
} ?: emptyList()
Copy link
Member

Choose a reason for hiding this comment

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

이거 nullcheck 안해도 자연스럽게 emptyList 되지 않나?

Copy link
Member

Choose a reason for hiding this comment

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

여기다가 주석으로 왜 클래스타입을 List로 변환하는지 설명하는게 좋을듯

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@rudeh2926 rudeh2926 merged commit 09a1e81 into develop Jun 30, 2024
1 check passed
@rudeh2926 rudeh2926 deleted the 공지사항조회변경 branch June 30, 2024 10:45
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.

3 participants