Conversation
Deploying p-pick-front with
|
| Latest commit: |
21720ee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://187c4de1.front-7g6.pages.dev |
| Branch Preview URL: | https://feature--112-review.front-7g6.pages.dev |
| const newReviewData = { | ||
| userId: userData.uid, | ||
| user: { | ||
| uid: userData.uid, | ||
| displayName: userData.displayName, | ||
| email: userData.email, | ||
| photoURL: userData.photoURL, | ||
| }, |
There was a problem hiding this comment.
@rhehfl firebase쪽에서 uesrId를 가지고 검증을해서 최상위 경로에 하나가 존재해야 합니다 만약 그게 필요없다면 UID하나로 처리하도록 바꾸겠습니다
There was a problem hiding this comment.
firebase 규칙 보니까 userId로만 판별해서 user객체는 필요 없을 것 같아용
| mutation.mutate({ | ||
| user: auth, | ||
| contentId, | ||
| rating: newReview.rating, | ||
| contents: newReview.contents, | ||
| images: newReview.images || [], | ||
| }); | ||
| setIsOpen(false); |
There was a problem hiding this comment.
mutate 성공후 콜백으로 모달 상태를 관리하는건 어떨까요?
|
@zzzRYT 코드 확인이 늦어 죄송합니다! |
| const newReviewData = { | ||
| userId: userData.uid, | ||
| user: { | ||
| uid: userData.uid, | ||
| displayName: userData.displayName, | ||
| email: userData.email, | ||
| photoURL: userData.photoURL, | ||
| }, |
There was a problem hiding this comment.
firebase 규칙 보니까 userId로만 판별해서 user객체는 필요 없을 것 같아용
| onValue( | ||
| reviewRef, | ||
| snapshot => { | ||
| const reviews: ReviewResponse[] = []; | ||
| snapshot.forEach(childSnapshot => { | ||
| const childKey = childSnapshot.key; | ||
| const childData = childSnapshot.val(); | ||
| reviews.push({ | ||
| id: childKey, | ||
| ...childData, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
firebase를 잘 아는게 아니라 그러는데 해당 부분은 서버에서 받아와서 프론트에서 filter하는 로직인가요?
| export interface ReviewResponse { | ||
| id: string; | ||
| contents: string; | ||
| userId: string; | ||
| user: { | ||
| uid: string; | ||
| displayName: string; | ||
| email: string; | ||
| photoURL: string; | ||
| }; | ||
| rating: number; | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| images?: string[]; | ||
| } |
There was a problem hiding this comment.
요거 리뷰 리스폰스도 user를 보내지 않으면 저부분은 없애도 될것같습니다
There was a problem hiding this comment.
아 리뷰 보여줄때 user 닉네임 등을 써야해서 이렇게 해둔건가요?
객체가 통으로 들어가서 순간 이상했네요 ㅋㅋㅋㅋ 이러면 user 전체를 저장하는게 맞는 것 같습니다
혼동드려서 죄송합니당😓
There was a problem hiding this comment.
@rhehfl 늦게 답장드리네여죄송합니다.
- 우선 user객체 내부에 들어가 있는 userId정보를 firebase에서 감지하지 못합니다. 그래서 최상단으로
userId에 대한 정보를 하나 빼 둔것이구요, - reivew에서 사용할 user정보를 참조해서 사용하는형식을 사용하지 못하는것 같습니다(제가 찾은 바로는 그렇습니다😂)
| images: [], | ||
| }); | ||
|
|
||
| const auth = getAuth(); |
There was a problem hiding this comment.
인증상태 체크 부분 Provider로 인증 정보를 요청하여 무한 캐싱 해놓고
따로 훅을 하나 만들어서 queryClient.getQuery(유저정보) 하는 훅을 만드는건 어떨까요
zustand 안써도 메모리에 캐싱해놓고 바로바로 가져올 수 있을 것 같아요
There was a problem hiding this comment.
@rhehfl 저도 분리가 필요하다 생각했습니다.
무한 캐싱 나쁘지 않은 것 같네여, 다만 걱정되는 부분은, firebase 요청을 보낼 때, 네티워크탭에 아무것도 뜨지 않더군요, 이게 내부적인 로직으로 처리하는건 아닐텐데 무한캐싱을하는게 도움이 될까요?
There was a problem hiding this comment.
@zzzRYT 데이터베이스에 대한 요청은 네트워크탭에 별도로 뜨진 않는데
getAuth를 호출하면 네트워크탭에 뜨더라구요 어느정도 효과가 있는거 아닐까요?
There was a problem hiding this comment.
그럼 괜찮아 보이네요 이 부분은 이미 getAuth를 사용하는 부분이 많아서 이슈 새로 파서 진행하는게 좋아보이네요
|
@rhehfl 다른부분은 이상 없으먄 머지할까여? |
|
@zzzRYT 넵 머지해주셔도 될것같습니다 |
2 participants
🔗 관련 이슈
#112
📝작업 내용
기능 추가
firebase연결: 이 부분은 전체적인 필드 정의가 필요해 보임앞으로 추가
🔍 변경 사항
💬리뷰 요구사항 (선택사항)
getAuth를 핸들링할 수 잇는 hook이나 상태가 있으면 좋을 것 같습니다. 모든 부분에서 getAuth를 가져다 쓰면,auth.currentUser에 대한 정보로 잇는지 없는지도 판별하고 하는데, 가독성이 좀 떨어지는 느낌입니다.