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

feat: 비밀번호 찾기 기능 추가 #19

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

parkmuhyeun
Copy link
Member

📄 Summary

🙋🏻 More

@parkmuhyeun parkmuhyeun self-assigned this Oct 9, 2023
Copy link
Contributor

@wonyongChoi05 wonyongChoi05 left a comment

Choose a reason for hiding this comment

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

무무야 빠르게 ㄱ


public void changePassword(final MemberChangePasswordRequest request) {
final Member member = memberRepository.getByEmail(request.email());

Copy link
Contributor

Choose a reason for hiding this comment

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

요거 붙이는거 어떤가요??

@@ -52,6 +52,10 @@ public void encodePassword(final PasswordEncoder passwordEncoder) {
this.password = passwordEncoder.encode(this.password);
}

public void changeEncodedPassword(final PasswordEncoder passwordEncoder, final String newPassword) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이게 먼가 좀 헷갈리네요.. 2번 생각해야한달까? changeEncodedPassword라고 하니 기존 패스워드를 변경한다는건지 패스워드를 입력받아서 암호화한다는건지 ㅠ

Copy link
Member Author

Choose a reason for hiding this comment

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

길더라도 encodeAndChangePassword 이런 느낌으로 가야겠꾼여

Comment on lines +21 to +27
public class PasswordRequestNotMatchException extends TechHubException {

public PasswordRequestNotMatchException() {
super(new ErrorCode(BAD_REQUEST, "비밀번호가 일치하지 않습니다. 같은 비밀번호인지 확인해주세요."));
}

}
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
Member Author

Choose a reason for hiding this comment

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

이게 request validation이라 여기서만 쓰일꺼 같아서 일단 여기뒀습니당


public class PasswordNotMatchException extends TechHubException {

public PasswordNotMatchException() {
super(new ErrorCode(FORBIDDEN, "유저의 비밀번호가 일치하지 않습니다."));
super(new ErrorCode(UNAUTHORIZED, "유저의 비밀번호가 일치하지 않습니다."));
Copy link
Contributor

Choose a reason for hiding this comment

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

비밀번호가 일치하지 않을 때는 403이 맞지 않나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

403은 권한아닌가여? 401이 인증쪽이고

Copy link
Contributor

Choose a reason for hiding this comment

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

ㅇㅎ 401이 맞는 것 같네여

Comment on lines +15 to +19
public void validateSamePassword() {
if (!newPassword.equals(checkPassword)) {
throw new PasswordRequestNotMatchException();
}
}
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
Member Author

Choose a reason for hiding this comment

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

이거 checkPassword라 도메인이 알필요가 잇을까여

Copy link
Contributor

Choose a reason for hiding this comment

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

네네 도메인 예외를 request가 책임질 필요가 전혀 없어보입니다

Copy link
Member Author

Choose a reason for hiding this comment

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

이거 그래서 예외를 도메인 예외 안쓰고 잘보면 request안에 예외를 쓰고 있읍니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

비밀번호 찾기 기능 추가
2 participants