Feature/#633 행사 추가 api 요청 명세 변경 #634
Conversation
amaran-th
added
Backend
amaran-th
added
Low Priority
| /events?name=인프콘 2023&location=코엑스&informationUrl=https://~~~&startDateTime=2023:06:01:12:00:00&endDateTime=2023:09:01:12:00:00&applyStartDateTime=2023:05:01:12:00:00&applyEndDateTime=2023:06:01:12:00:00&tags=백엔드,안드로이드&imageUrl=https://image.url&type=CONFERENCE&eventMode=ON_OFFLINE&paymentType=FREE | ||
| ---- | ||
| .HTTP request | ||
| include::{snippets}/add-event/http-request.adoc[] |
There was a problem hiding this comment.
제가 알기로는 request 스니펫이 이해하기 너무 어려워서 의도적으로 문자열로 하드코딩 해 둔 것으로 아는데 혹시 수정하신 이유가 있으신가요
There was a problem hiding this comment.
스니펫이 수정되기도 했고, 수정 후의 request 스니펫은 그대로 사용해도 이해하기 어렵지 않을거라 생각해서 수정해주었습니다!
There was a problem hiding this comment.
multipart 로 adoc 만들면 엄청 지저분해지지 않아요??
| @@ -89,7 +87,7 @@ class EventApiTest extends MockMvcTestHelper { | |||
| fieldWithPath("imageUrls[]").description("이미지 URL들").optional(), | |||
| fieldWithPath("organization").description("행사기관") | |||
| ); | |||
|
|
|||
There was a problem hiding this comment.
이 부분 컨벤션이 다른 것 같아요.
저만 그런지 모르겠는데 저는 빈 라인에 공백을 추가하지 않는데 혹시 다른 분들은 어떤가요?
There was a problem hiding this comment.
확인하고 수정했습니다! 코드 스타일을 수정한 적이 없는데 설정이 다르게 되어 있더라구요🤔
알려주셔서 감사합니다~!
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| public enum ImageType { | |||
| FEED(5), | |||
| EVENT(2); | |||
| EVENT(200); | |||
There was a problem hiding this comment.
이건 그냥 개인적인 의견인에 이벤트는 이미지 업로드에 제한이 없으니까 무제한으로 둔다는 의미로 0을 두고 아래 isOver 메서드에서 0은 무조건 false를 반환하게 하는건 어떠신가요?
0은 매직넘버이니 상수로 빼서 의미를 부여하고요.
There was a problem hiding this comment.
오 너무 좋은 의견인 것 같아요! 그렇게 반영하겠습니다!
| return eventService.addEvent(request, LocalDate.now()); | ||
| public EventDetailResponse addEvent(@RequestPart @Valid final EventDetailRequest request, | ||
| @RequestPart final List<MultipartFile> images) { | ||
| return eventService.addEvent(request, images, LocalDate.now()); | ||
| } | ||
|
|
||
| @PutMapping("/{eventId}") |
There was a problem hiding this comment.
(consumes = MediaType.MULTIPART_FORM_DATA_VALUE) 가 왜 한 쪽에만 있는건가요?
There was a problem hiding this comment.
updateEvent에 consumes 속성을 설정해주는 걸 깜빡하고 누락한 것 같습니다.
그런데 해당 설정을 해주지 않았는데도 조금 전에 했던 테스트가 잘 동작해서, 왜 그런지 개인적으로 알아보았는데요.
consumes 설정을 해주지 않은 경우, 스프링이 요청 헤더의 Content-Type을 보고 컨텐츠 타입을 지정해줘서 받아들이기 때문에 필수적인 속성은 아니라고 합니다.
하지만 요청으로 받을 컨텐츠 타입을 제한해주는 역할을 해서, API의 요청 컨텐츠의 타입이 확실한 경우 명시해주는 것이 좋다고 하더라구요. 그래서 updateEvent에도 addEvent와 동일하게 consumes 속성을 설정해주었습니다.
| @@ -196,7 +198,7 @@ public EventDetailResponse addEvent(final EventDetailRequest request, final Loca | |||
| } | |||
|
|
|||
| public EventDetailResponse updateEvent(final Long eventId, final EventDetailRequest request, | |||
| final LocalDate today) { | |||
| final List<MultipartFile> images, final LocalDate today) { | |||
| final Event event = eventRepository.findById(eventId) | |||
There was a problem hiding this comment.
update에서는 images 파라미터가 안 사용 되는 것 같은데 파라미터로 넣으신 이유가 있나요?
There was a problem hiding this comment.
관리자 페이지(프론트)를 미리 완성시켜놓으려고 요청 포맷만 미리 수정해두었습니다...!
지금 생각해보니 이슈를 분리하고 했어야 했는데 혼동하게 한 것 같네요. 제 실수입니다🥲
새로 판 피쳐 브랜치에서는 해당 images 데이터를 사용하도록 구현해둔 상태입니다!
#️⃣연관된 이슈
#633
📝작업 내용
관리자페이지에서 행사 정보를 추가할 수 있도록 행사 생성/수정 API의 요청 데이터 형식을 수정했습니다.
+)행사 당 첨부할 수 있는 이미지의 최대 개수가 2개로 제한이 걸려있어서 상한을 200개로 늘렸습니다.
예상 소요 시간 및 실제 소요 시간
9/23 9/23
💬리뷰 요구사항(선택)
왜인진 모르겠는데 쿼리 최적화 작업했던 커밋 일부가 섞여 들어갔네요,,,죄송합니다🥲
추후에 별도로 PR을 날릴 예정이니 무시해주셔도 될 것 같아요...!