-
Notifications
You must be signed in to change notification settings - Fork 0
feature/13-layered-to-hexagonal-auth #21
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
Conversation
qkrwndnjs1075
commented
Jul 28, 2025
- auth 관련 비즈니스 로직 PR
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
패키지명 resopnse 오타입니다. response로 수정하세요 @qkrwndnjs1075
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.
수정했습니다
|
|
||
| return htmlBuilder.toString() | ||
| } catch (e: Exception) { | ||
| println(e.message) |
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.
println 말고 로거 사용이 적절해 보이는데 확인부탁드립니다 @qkrwndnjs1075
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.
bbf106f 수정했습니다
| @RedisHash | ||
| class PassInfo( | ||
| @Id | ||
| val name: String, |
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.
사용자 이름을 키로 잡는 로직에서 동명이인이 동시에 사용시 안전한가요? @qkrwndnjs1075
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.
그럼 @id를 그냥 phoneNumber로 할까요 그게 나을 것 같은데
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.
d4ede1b phoneNumberHash로 하도록 변경
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.
passInfo도 암호화 되어야 하기 때문에 Hash 기준으로 find 하도록 수정
4b9ba22
| token: String?, | ||
| ttl: Long, | ||
| ) { | ||
| this.token = token!! |
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.
null이 들어왔을때 대응 로직이 있을까요 ? @qkrwndnjs1075
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.
non-null 캐스트 제거 했습니다. ( 과거의 유산 )