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

Feature/#133 fcm서버에 알림 정보 보내기 #161

Merged

Conversation

java-saeng
Copy link
Collaborator

#️⃣연관된 이슈

📝작업 내용

FCM에게 알람 요청 보내는 기능 구현

빨리 문서화해서 올리도록 할게요

@java-saeng java-saeng added Backend 백엔드 관련 이슈 기능 추가 새로운 기능 추가 및 기존 기능 변경 labels Jul 31, 2023
@java-saeng java-saeng self-assigned this Jul 31, 2023
@java-saeng java-saeng added the High Priority 리뷰 우선순위가 높은 PR label Jul 31, 2023
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.

크게 수정할 부분은 없는 것 같아요.


@Bean
public RestTemplate restTemplate() {
return new RestTemplate();
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 이렇게 별도로 분리한 이유가 있을까요?

public class FcmMessage {

@JsonProperty("validate_only")
private final boolean validateOnly;
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 이게 무엇을 의미하는지 간략하게 알려주실 수 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현장 리뷰 완료요~

//when
final NotificationResponse expected = notificationCommandService.create(request);
final NotificationResponse expected = mockingNotificationCommandService.create(request);
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금 다시 보니까 expected랑 actual 이 바뀌어야 할 것 같아요.(어차피 모킹이라 큰 의미 없지만, 그래도...)

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

@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.

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

@@ -28,6 +28,8 @@ public class GithubClient {
@Value("${github.url.profile}")
private String profileUrl;

private final RestTemplate restTemplate;
Copy link
Collaborator

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하겠습니다~

@java-saeng java-saeng merged commit 6adedd7 into backend-main Jul 31, 2023
1 check passed
@amaran-th amaran-th deleted the Feature/#133-FCM서버에_알림_정보_보내기 branch August 4, 2023 00:30
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: Archive
Development

Successfully merging this pull request may close these issues.

4 participants