-
Notifications
You must be signed in to change notification settings - Fork 3
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/#365] 상세이미지 업로드 서버 연결 #366
Conversation
PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶 |
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.
확인해쯉니다 ~
잘 동작합니다! 아직 제가 상세 페이지 뷰를 안 만들어서 보이지는 않지만,, ㅎㅎ
presignedUrl 받아오는 부분은 반복적으로 사용할 수 있을 것 같아서, 나중에 hook으로 빼면 좋을 것 같네요!
src/pages/register/Register.tsx
Outdated
const S3Urls = extractUrls(data); | ||
console.log("extractUrls", S3Urls); |
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.
p5) 머지할 때 삭제해주세요!
console.log("extractUrls", S3Urls); |
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.
삭제 완료~
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.
와웅,,, 조아욥 PR 확인했씀니다
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.
오래 걸린다더니 생각보다 금방 끝나신 것 같은데요? 고생하셨습니다 ㅎㅎ
코멘트 한번만 확인해주세요 !!
.replace(/staffImages=%5B%5D/g, "staffImages") | ||
.replace(/performanceImages=%5B%5D/g, "performanceImages"); |
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.
p5) 도영이가 만들어 둔 코드에 잘 추가해뒀네요! 굿굿
setPerformanceImages( | ||
gigInfo.performanceImageList.map( | ||
(_, index) => `performance-${index + 1}-${new Date().getTime()}` | ||
) | ||
); | ||
}, [gigInfo.castList.length, gigInfo.staffList.length, gigInfo.performanceImageList.length]); |
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.
p5) 이미 반응형 값으로 존재하는 gigInfo에서 performanceImageList를 돌며 추후 presignedUrl을 받아올 속성값을 생성하는 로직 좋네요! 잘 참고하도록 하겠습니다 👍
|
||
const params = { | ||
posterImage: `poster-${new Date().getTime()}`, | ||
castImages, | ||
staffImages, | ||
performanceImages, | ||
}; | ||
|
||
const { data, refetch } = useGetPresignedUrl(params); |
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.
p5) 궁금한 게 는데 여기서 data를 바로 사용하는 게 아니라 refetch를 사용해서 최신의 presignedUrl을 가져오는 로직을 handleComplete에 구현하신 이유가 있나요? 굳이 다시 한번 refetch를 해야 하는 이유가 있는건지 단순히 궁금해서 코멘트 남깁니다 !
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.
아.. handleComplete가 언제 눌릴지 모르니까, presignedUrl의 유효기간이 만료되는 걸 방지하기 위해 사용하셨던거죠? 그렇다면 👍
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.
보통 리액트쿼리 예시 코드들 보면 버튼같은 사용자 이벤트 관련해서는 refetch를 사용하는 걸로 알고 있습니다!
그리구 일단 저기 사용된 useGetPresignedUrl이 버튼 클릭했을 때 데이터를 가져와야 하고, enabled도 false로 옵션이 설정되어 있는데요. 그래서 저는 완료하기 버튼을 클릭했을 때 refetch를 해서 서버에서 새로 받아오는 방식으로 코드 로직을 이해했습니다.
presignedUrl을 새로 가져오는 거니까 같은 말인 것 같은데여 사실 이부분은 저두 뇌피셜이라 도영오빠 확인 받구 싶네영
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.
마자요!
저도 가장 아래의 아티클을 참고했습니다! 한 번 읽어보시면 좋을 것 같아요!
몇 개의 이미지가 저장이 되야할지 모르니까 처음에는 동작하지 않게 enabled: false
로 설정하고, '등록하기' 라는 버튼을 눌렀을 때, 존재하는 이미지의 갯수를 불러오기 위해서 refetch()
를 통해서 이미지의 갯수가 저장될 만큼의 presigned url을 가져오는 로직이빈다!
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.
역시 리액트쿼리 권위자..... 깔끔한 설명 감사합니다.
performanceImageList: gigInfo.performanceImageList.map((image) => ({ | ||
performanceImage: image.performanceImage, | ||
performanceImageList: gigInfo.performanceImageList.map((image, index) => ({ | ||
performanceImage: performanceUrls[index] || image.performanceImage, |
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.
p3) 근데 || 뒤에 있는 image.performanceImage,는 혹시 왜 작성한건지 여쭤봐도 될까요?
최종적으로 api 요청으로 보낼 form의 속성인 performanceImageList 에는 이미지 url을 담아 보내야 하니 performanceUrls[index] 를 담는 것은 맞는 것 같은데, image.performanceImage를 담는 건 이상해보여서요!
image.performanceImage의 경우에는 제가 위에서 제가 언급한 setPerformanceImages 함수를 통해 속성값들만 넣어주고, 이 속성값에 get 해온 presignedUrl 값(value)를 연결해주는 형식인 것 같은데.. 의도가 궁금합니다!
제가 뭔가 잘못 이해한 거라면 지적 부탁드립니다 ~!
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.
image.performanceImage의 경우에는 제가 위에서 제가 언급한 setPerformanceImages 함수를 통해 속성값들만 넣어주고, 이 속성값에 get 해온 presignedUrl 값(value)를 연결해주는 형식인 것 같은데.. 의도가 궁금합니다!
제가 뭔가 잘못 이해한 거라면 지적 부탁드립니다 ~!
이 부분이 무슨 뜻인지 사실 이해가 잘 안가네여....
그냥 해당 Urls부분에서 문제가 생겼을 경우 대비해서 원래 값을 디폴트로 넣어줘야 한다고 생각했는데.....이상할까요...?
+) 말씀하신 image.performanceImage의 경우가 map을 돌리는 저 상황을 말씀하신걸까요?
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.
엇 아뇨 performanceUrls[index] 과 image.performanceImage 의 의도가 다른 게 아닌가 싶어서 질문드렸어요 !
performanceUrls[index] 는 받아온 presignedUrl 값인거고,
image.performanceImage 는 추후 presignedUrl을 받아오기 위해 만들어둔 속성 값 아니었나요??
만약 제 생각이 맞다면, 둘의 용도(하나는 presigendUrl 값 그 자체, 하나는 속성값)가 다르니까, default 값을 || image.performanceImage 로 설정하는게 이상하지 않나.. 싶어서 여쭤봤습니다! (마치 count : number = 3 || "Three" 이런 느낌..? )
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.
흐으음.... 비어있는 값을 넣어서 서버로 요청하면 문제없이 공연 업로드가 되어서, urls부분에서 문제가 생겼을 경우를 대비해서 원래 사용하던 이미지 값인 image.performanceImage를 넣어둔거였는데, 그렇다면 어떻게 처리해야 할까요?
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.
아아 클라이언트에서 사용하던 이미지 값인 image.performanceImage를 넣는다는 말씀이죠?
제가 performanceImageList 랑 performanceImages, 그리고 performanceUrl을 제대로 이해하지 못하고 있었네요! 그런 의도라면 굳이 다른 값으로 바꾸지 않아도 될 것 같습니다 !
친절한 답변 감사합니다 👍
Quality Gate passedIssues Measures |
performanceImageList: gigInfo.performanceImageList.map((image) => ({ | ||
performanceImage: image.performanceImage, | ||
performanceImageList: gigInfo.performanceImageList.map((image, index) => ({ | ||
performanceImage: performanceUrls[index] || image.performanceImage, |
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.
아아 클라이언트에서 사용하던 이미지 값인 image.performanceImage를 넣는다는 말씀이죠?
제가 performanceImageList 랑 performanceImages, 그리고 performanceUrl을 제대로 이해하지 못하고 있었네요! 그런 의도라면 굳이 다른 값으로 바꾸지 않아도 될 것 같습니다 !
친절한 답변 감사합니다 👍
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes
📢 To Reviewers
업로드 잘됨여
📸 스크린샷
뷰가 없어서 응답으로 캡처본 대신합니다.
🔗 참고 자료