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

Fix/#225 오픈 프로필 url 없으면 exception 발생하도록 수정 #233

Conversation

java-saeng
Copy link
Collaborator

#️⃣연관된 이슈

📝작업 내용

알림 보낼 사람의 오픈 프로필 URL이 없으면 알림을 보낼 수 없게 Exception 추가

예상 소요 시간 및 실제 소요 시간

1시간 / 1시간

@java-saeng java-saeng added 버그 개발자가 의도하지 않은 상황 Backend 백엔드 관련 이슈 High Priority 리뷰 우선순위가 높은 PR labels Aug 7, 2023
@java-saeng java-saeng added this to the 4차 스프린트 1주차 milestone Aug 7, 2023
@java-saeng java-saeng self-assigned this Aug 7, 2023
Copy link
Collaborator

@hyeonjerry hyeonjerry left a comment

Choose a reason for hiding this comment

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

크게 수정할 점은 보이지 않네요.
수고하셨습니다.

@@ -29,6 +30,7 @@ public class Member extends BaseEntity {
@Column(nullable = false)
private String description;
@Column
@Getter(value = AccessLevel.PRIVATE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 AccessLevel을 PRIVATE으로 설정하고 밑에 구현하신 이유가 있으신가요?
밑에 직접 getter를 구현하면 자동으로 override가 되지 않나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현장 리뷰 완료!

Copy link
Collaborator

@amaran-th amaran-th left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 크게 수정할 점은 보이지 않네요!
approve하겠습니다~

Copy link
Collaborator

@hong-sile hong-sile left a comment

Choose a reason for hiding this comment

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

크게 바뀔부분은 없네요. 수고하셨습니다!

@@ -108,7 +113,7 @@ private String getAccessToken() {
try {
final GoogleCredentials googleCredentials = GoogleCredentials
.fromStream(new ClassPathResource(firebaseConfigPath).getInputStream())
.createScoped(List.of("https://www.googleapis.com/auth/cloud-platform"));
.createScoped(List.of(GOOGLE_AUTH_URL));

googleCredentials.refreshIfExpired();
Copy link
Collaborator

Choose a reason for hiding this comment

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

ControllerAdvice가 로그를 남기고 있으니까, log.error를 굳이 찍을 필요가 없지 않나요?

@@ -84,15 +86,18 @@ private String makeMessage(final String targetToken, final Notification notifica
final Member sender = memberRepository.findById(senderId)
.orElseThrow(() -> new MemberException(MemberExceptionType.NOT_FOUND_MEMBER));

final String openProfileUrl = sender.getOptionalOpenProfileUrl()
Copy link
Collaborator

Choose a reason for hiding this comment

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

request changed가 여기밖에 없어서, 여기 남깁니다.
78번째 라인의 log.error를 찍는 부분이 있는데,

이 부분에 다른 CustomException을 만드는 게 좋지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현장 리뷰 완료!

@java-saeng java-saeng merged commit 0ceafa5 into backend-main Aug 8, 2023
1 check failed
@java-saeng java-saeng deleted the Fix/#225-오픈_프로필_URL_없으면_Exception_발생하도록_수정 branch August 8, 2023 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend 백엔드 관련 이슈 High Priority 리뷰 우선순위가 높은 PR 버그 개발자가 의도하지 않은 상황
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants