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

로그인 인증 #40

Merged
merged 5 commits into from
Sep 5, 2024
Merged

로그인 인증 #40

merged 5 commits into from
Sep 5, 2024

Conversation

jun0811
Copy link
Contributor

@jun0811 jun0811 commented Sep 3, 2024

next auth 로그인 구현입니다

[로그인 로직]

  • 카카오 로그인 api 요청하여 subId, name, accessToken을 받아옵니다
  • 우리 서비스 backend에 api를 요청해서 서비스와 세션을 만듭니다
  • isSignup은 회원가입인지 로그인인지 구분하는 플래그입니다

@jun0811 jun0811 changed the title Feature/#33 next auth 로그인 인증 Sep 3, 2024
Copy link
Contributor

@kichul7493 kichul7493 left a comment

Choose a reason for hiding this comment

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

수고 많으셨습니다!

자잘한 의견 달아두었으니 검토 해주시면 감사하겠습니다.

isLoading,
})
rerender(<LoginPage />)
expect(screen.getByRole('button')).not.toHaveTextContent('카카오로 시작하기')
})

it('로그인 성공시 signup 라우팅', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트의 설명이 직접 구현하거나 코드의 작동 구조를 면밀히 살피지 않으면 이해하기 어려운 거 같습니다.
테스트 해야 될 대상에 대해 조금 더 알기 쉽게 작성하면 유지보수에도 좋을 거 같습니다.
(다른 사람이 작성한 거라 이해하기 힘든가 싶어서 제가 작성한 걸 보고 왔는데, 제가 작성한 것도 시간이 지나니 뭔소린지 모르겠네요ㅋㅋ 저도 고쳐놔야겠습니다)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러게요 ㅠㅠ 어려운 일인 것 같습니다

isSignup: boolean
}

export const kakaoLogin = async ({
Copy link
Contributor

Choose a reason for hiding this comment

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

함수 이름을 봤을 때는 카카오 로그인을 하는 함수인거 같은데, 구현을 보면 카카오 로그인 이후에 받아온 정보로 백엔드 서버에 로그인하는 기능인 것 같네요. 함수 이름을 조금 더 구체적으로 변경해보는건 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OAuthLogin으로 바꿔봤습니다

}
}, [session.accessToken])

const signInKakao = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

setIsLoading 순서가 요청이 시작될 때 true, 끝날때 false일거 같은데, 반대로 되어 있는거 같아요
그리고 성공했을때 ,실패했을때 작동하는 공통로직(setIsLoading(false))을 finally로 처리하면 반복을 줄일 수 있을거 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 좋은 것 같습니다

오류 찾아주셔서 감사합니다 :)

@jun0811 jun0811 force-pushed the feature/#33-next-auth branch 2 times, most recently from d9dbc5b to 19e6c21 Compare September 5, 2024 01:58
@jun0811 jun0811 merged commit 785ddce into dev Sep 5, 2024
1 check passed
@jun0811 jun0811 deleted the feature/#33-next-auth branch September 5, 2024 08:39
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.

2 participants