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

FCM device registration 구현 #170

Merged
merged 4 commits into from
Nov 21, 2022
Merged

FCM device registration 구현 #170

merged 4 commits into from
Nov 21, 2022

Conversation

shp7724
Copy link
Contributor

@shp7724 shp7724 commented Nov 12, 2022

  • 로그인 시 생성되는 fcmToken을 서버로 저장하도록 구현했습니다.

@shp7724 shp7724 self-assigned this Nov 12, 2022
@shp7724 shp7724 requested a review from a team as a code owner November 12, 2022 09:22
@shp7724
Copy link
Contributor Author

shp7724 commented Nov 12, 2022

@JSKeum 아 #168 이 진섭님 태스크로 되어 있었군요
바통터치 받아주시면 감사하겠습니다.

@shp7724
Copy link
Contributor Author

shp7724 commented Nov 12, 2022

@JSKeum 특히 shouldDeleteFCMInfos 부분이 잘 이해가 안되는데, 뭔가 맥락 알고계신게 있으시다면 공유 부탁드려요.

@keumartist
Copy link
Member

keumartist commented Nov 19, 2022

@JSKeum 특히 shouldDeleteFCMInfos 부분이 잘 이해가 안되는데, 뭔가 맥락 알고계신게 있으시다면 공유 부탁드려요.

저도 잘 모르겠네요.. 기존 SNUTT에 appdelegate에서 shouldDeleteFCMInfos 값에 따라 logout하는 로직이 들어 있는데, 왜인지 모르곘습니다

Comment on lines +65 to +85
func addDevice(fcmToken: String) async throws {
userDefaultsRepository.set(String.self, key: .fcmToken, value: fcmToken)

if appState.user.accessToken == nil {
// defer until sign in
return
}

try await userRepository.addDevice(fcmToken: fcmToken)
}

func deleteDevice(fcmToken: String) async throws {
userDefaultsRepository.set(String.self, key: .fcmToken, value: nil)

if appState.user.accessToken == nil {
return
}

try await userRepository.deleteDevice(fcmToken: fcmToken)
}

Copy link
Member

Choose a reason for hiding this comment

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

UserService에서도 addDevice, deleteDevice 메소드를 구현할 필요가 있나요?
login / logout 할 때만 addDevice / deleteDevice 하면 되지 않나요??

같은 맥락에서, bootstrap 안에 services.userService.addDevice(fcmToken: fcmToken) 요 부분도 빼도 되지 않을까요?
Screen Shot 2022-11-19 at 3 43 48 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

토큰이 변경되는 케이스인데, 앱을 복원한 경우에는 로그인된 상태가 유지되지만 토큰이 변경되므로 앱이 켜질 때마다 할때마다 확인하는게 안전할 것 같습니다.

Copy link
Member

Choose a reason for hiding this comment

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

"앱을 복원한 경우"가 정확히 어떤 경우인가요? access token도 userdefault에서 저장되는 걸로 알고 있는데, 새 경우 모두 userdefault가 초기화되지 않나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아마 새로운 디바이스에서 마이그레이션 하는 경우를 의미하는 것 같습니다.
이때는 userdefault를 포함해서 앱의 모든 데이터가 보존되는 것으로 알고 있어요.

Copy link
Member

Choose a reason for hiding this comment

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

아하 그렇군요. 그러면 그 경우를 위해서, 앱 실행 마다 addevice 요청을 최소 두 번씩 보내는 것 맞을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 왜 두번인가요?

Copy link
Member

Choose a reason for hiding this comment

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

아 앱 실행 마다 한 번 호출하게 되겠네요

Copy link
Member

@keumartist keumartist left a comment

Choose a reason for hiding this comment

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

머지해도 될 것 같습니다. 머지하시면 dev, prod 둘다 테플 배포 부탁드릴게요! 푸시알림 다시 테스트해보죠

@shp7724 shp7724 merged commit 30464a7 into master Nov 21, 2022
@shp7724 shp7724 deleted the shp7724/fcm-device-register branch November 21, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants