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] 카톡 로그인 기능 구현 #13

Merged
merged 9 commits into from
Jan 7, 2024
Merged

[feat] 카톡 로그인 기능 구현 #13

merged 9 commits into from
Jan 7, 2024

Conversation

pkl0912
Copy link
Contributor

@pkl0912 pkl0912 commented Jan 5, 2024

관련 이슈번호

해결하는 데 얼마나 걸렸나요? (예상 작업 시간 / 실제 작업 시간)

  • 하루

해결하려는 문제가 무엇인가요?

  • 카톡 로그인

일단 코드가 많이 안 바뀔 거 같아서 일단 merge 하겠습니다!

@pkl0912 pkl0912 requested review from KWY0218 and hellozo0 January 5, 2024 16:54
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다!
전체적으로 record 적용 및 불필요한 코드 제거 작업만 해주시면 좋을 것 같습니다!

.gitignore Outdated
Comment on lines 40 to 41
application.yml
Copy link
Member

Choose a reason for hiding this comment

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

p3:
application.yml 파일을 커밋하지 않으려고 추가하신 것이라 생각드는데,
첫번째 라인을 보시면 src/main/resources/*.yml 으로 이미 추가가 안될 것이라 생각합니다..!

혹시 해당 라인이 없으면 application.yml 파일이 커밋이 되고 있을까요?

build.gradle Outdated
Comment on lines 3 to 4
id 'org.springframework.boot' version '3.0.12'
id 'io.spring.dependency-management' version '1.1.3'
Copy link
Member

Choose a reason for hiding this comment

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

p3:
스프링 부트 버전을 바꾼 이유가 궁금합니다..!

build.gradle Outdated
Comment on lines 28 to 31
// jwt
implementation group: "io.jsonwebtoken", name: "jjwt-api", version: "0.11.2"
implementation group: "io.jsonwebtoken", name: "jjwt-impl", version: "0.11.2"
implementation group: "io.jsonwebtoken", name: "jjwt-jackson", version: "0.11.2"
Copy link
Member

Choose a reason for hiding this comment

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

p3
해당 이슈에서는 jwt 기능이 없는 것으로 보입니다.
제거하면 좋을 것 같습니다

Comment on lines 6 to 16
@ToString
@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
@AllArgsConstructor(access = AccessLevel.PRIVATE)

public class KakaoUserResponse {

private Long id;


}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@ToString
@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public class KakaoUserResponse {
private Long id;
}
@ToString
@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public record KakaoUserResponse(Long id) {
}

p3
java 17 부터는 record를 사용해서 표현할 수 있습니다!
record를 통해 위 어노테이션을 전부 없앨 수 있습니다
참고해주세요

Comment on lines 6 to 19
@ToString
@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
@AllArgsConstructor(access = AccessLevel.PRIVATE)

public class KakaoAccessTokenResponse {

private String accessToken;
private String refreshToken;

public static KakaoAccessTokenResponse of(String accessToken, String refreshToken) {
return new KakaoAccessTokenResponse(accessToken, refreshToken);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

p3:
해당 코드도 record를 적용하면 좋을 것 같습니다

Comment on lines 2 to 5




Copy link
Member

Choose a reason for hiding this comment

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

p2
불필요한 공백인 것 같아 보입니다!

Comment on lines 9 to 13
public static SocialLoginResponse of(String userId) {
return new SocialLoginResponse(
userId = userId
);
}
Copy link
Member

Choose a reason for hiding this comment

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

p3
record 타입이기 때문에 of 를 사용하지 않아도 new SocialLoginResponse("string") 을 통해 할 수 있다고 생각합니다..!
혹시 of 를 사용한 이유가 있을까요?

Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다!!

@hellozo0 hellozo0 assigned hellozo0 and pkl0912 and unassigned hellozo0 Jan 7, 2024
@hellozo0 hellozo0 added the feat label Jan 7, 2024
Copy link
Member

@hellozo0 hellozo0 left a comment

Choose a reason for hiding this comment

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

짱짱~ 고생하셨습니다!...!!!!

public record SocialLoginRequest(
String code
) {

Copy link
Member

Choose a reason for hiding this comment

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

P4:
요런 빈공백들도 없애면 좋을거 같아요!

@Getter
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
public enum SocialPlatform {
KAKAO("카카오"),
Copy link
Member

Choose a reason for hiding this comment

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

P4:
enum 작성할때 KAKAO("카카오");이렇게 끝낼 수 있나요..? (갑자기 enum 작성 이렇게 해도 되는지 까먹음 이슈)

SocialPlatform 카카오 말고 더 안쓰니까 KAKAO("카카오"); 이렇게 할 수 있으면 바꾸면 좋을거 같아요!

@pkl0912 pkl0912 merged commit 30ad90c into develop Jan 7, 2024
1 check passed
@pkl0912 pkl0912 deleted the feat/#11 branch January 7, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] 카톡 로그인 기능 구현
3 participants