-
Notifications
You must be signed in to change notification settings - Fork 2
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] 초대페이지 로직 수정 QA_2 #482
base: develop
Are you sure you want to change the base?
Conversation
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.
보우니 고생햇서 !! LGTM ~~
minHeight: path === PATH.ARCHIVING ? 'calc(100vh - 53rem)' : '', | ||
width: path !== PATH.DRIVE ? '100%' : '', |
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.
요거 else의 경우 아무런 값도 지정해주지 않으니 &&
사용해줘도 될 것 같아유 !
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.
감사합니당!!
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.
고생하셨습니다 ~!
navigate(PATH.ONBOARDING); | ||
}, | ||
} | ||
); | ||
clearInvitation(); |
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.
오잉 이거 호출하고있는 함수는 handleApproveInvitation
이고
함수 clearInvitation
에 대한 정의는 그 위에 있습니다!
@@ -17,16 +17,18 @@ interface ErrorResponse { | |||
/* 토큰 여부 확인 */ | |||
export const authMiddleware: Middleware = { | |||
async onRequest({ request }) { | |||
const accessToken = localStorage.getItem(STORAGE_KEY.ACCESS_TOKEN_KEY); | |||
if (localStorage.getItem(STORAGE_KEY.ACCESS_TOKEN_KEY) && !localStorage.getItem(STORAGE_KEY.INVITATION_ID)) { |
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.
ㅎㅎ 뭐하는 로직인지 생각해보다가 로직이 잘못됨을 깨달았습니다!!
변경한 로직은
const isInvitationIdNone =
!localStorage.getItem(STORAGE_KEY.INVITATION_ID) || localStorage.getItem(STORAGE_KEY.INVITATION_ID) === '';
if (isInvitationIdNone) {
//...
요것인데요, 초대 아이디가 존재한다면 로그인을 안해도(토큰이 없어도) 로그인화면으로 가지 않도록 했습니다.
왜냐면 로그인을 안한 상태로 초대를 받았을 경우, '로그인 전 초대 페이지'가 나오지 않고 바로 로그인화면으로 이동되었기 때문에
이 현상을 막기 위해 초대 아이디가 있다면 토큰 확인을 하지 않도록 수정했습니다.
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.
몇 개 궁금한 거 코멘트로 남겨봤슴다 !
@@ -17,16 +17,21 @@ interface ErrorResponse { | |||
/* 토큰 여부 확인 */ | |||
export const authMiddleware: Middleware = { | |||
async onRequest({ request }) { | |||
const accessToken = localStorage.getItem(STORAGE_KEY.ACCESS_TOKEN_KEY); | |||
const isInvitationIdNone = | |||
!localStorage.getItem(STORAGE_KEY.INVITATION_ID) || localStorage.getItem(STORAGE_KEY.INVITATION_ID) === ''; |
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.
INVITATION_ID가 ''로 저장될 수도 있나요 ??
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.
빈 경우도 처리를 해봤는데 그럴 경우가 없겠네용 !! ㅎㅎ 감삼다
const isInvitationIdNone = | ||
!localStorage.getItem(STORAGE_KEY.INVITATION_ID) || localStorage.getItem(STORAGE_KEY.INVITATION_ID) === ''; |
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.
이 반대의 경우에 토큰 검사 하지 않고 얼리 리턴해주면 안대나요 ??
해당 이슈 번호
closed #481
체크리스트
💎 PR Point
cal(100vh - 어쩌구)
로 반응형으로 해결할 수 있었습니다.middleware.ts
에서 토큰여부확인하는 로직을 수정했습니다.accessToken
만 없으면 바로 로그인페이지로 이동하게 되어있었는데,📌스크린샷 (선택)