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 : 설정화면 fcm 토큰 관련 api 연동 #216

Merged
merged 7 commits into from
Mar 31, 2024
Merged

Conversation

m6z1
Copy link
Collaborator

@m6z1 m6z1 commented Mar 30, 2024

📮 관련 이슈

✍️ 구현 내용

  • �스위치 알림 버튼 on/off가 api 구현에 따른 로직에 의해 반영되도록 수정했습니다.
  • 학과 / 대학 공지 알림 off 시 delete api 연결했습니다.
  • 학과 / 대학 공지 알림 on 시 update api 연결했습니다.
  • 설정화면에서 학과 수정 시 메인으로 이동하는 로직을 finish()처리만 해서 다시 설정화면으로 돌아오도록 수정했습니다.
  • 설정화면에서 키워드 액티비티 진입 후 완료 버튼 눌렀을 때 키워드 api 가 통신되도록 구현했습니다.

📷 구현 영상

스크린샷 2024-03-30 오후 8 09 26 서버 통신 200 로그로 구현 영상 대체합니다

✔️ 확인 사항

  • 컨벤션에 맞는 PR 타이틀
  • 관련 이슈 연결
  • PR 관련 정보 연결 (작업자, 라벨, 마일스톤 등)
  • Github Action 통과

@m6z1 m6z1 added 🤗 FEATURE Develop this project 💛명지 labels Mar 30, 2024
@m6z1 m6z1 requested review from huiwoo-jo and hoyahozz March 30, 2024 11:09
@m6z1 m6z1 self-assigned this Mar 30, 2024
Copy link
Contributor

@huiwoo-jo huiwoo-jo left a comment

Choose a reason for hiding this comment

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

수고많으셨습니다!
코멘트 확인 부탁드려요 👍

Comment on lines 64 to 68

override fun onNewToken(token: String) {
super.onNewToken(token)
Timber.d("fcm_token_new: $token")
SharedPreference.setFcmToken(token)
Copy link
Contributor

Choose a reason for hiding this comment

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

Timber은 지워도 될거 같아용

Comment on lines 11 to 13
object RetrofitObject {
private const val TIME_OUT_COUNT: Long = 10
private const val TIME_OUT_COUNT: Long = 30

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
Collaborator Author

Choose a reason for hiding this comment

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

넵 키워드 때문에 타임 아웃 에러나서 서버 측에서 로직 수정해주시면 다시 10으로 되돌릴 예정입니당

Comment on lines 66 to 67
viewModel.setDepartment(items[viewModel.selectDepartPosition.value ?: 0])
val intent = Intent(this, MainActivity::class.java)
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
startActivity(intent)
finish()
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
Collaborator Author

Choose a reason for hiding this comment

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

네 이 부분 사용해보셨는지 모르겠는데 불편하기도 하고, 학과가 바꼈는지 확인이 안 되기에 finish처리만 했습니다.

Comment on lines -46 to -50
if (viewModel.isFirstLaunch.value == true) {
viewModel.setFirstLaunch(false)
val intent = Intent(this@KeywordActivity, OnboardingPermissionActivity::class.java)
startActivity(intent)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines 54 to 55
//prefs.edit().putString("fcm_token", token).commit()
Timber.d("fcm_token: $token")
SharedPreference.setFcmToken(token)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 이제 Timber은 지워도 될거같아요!

Comment on lines +24 to 28

override fun initStartView() {
binding.tvSettingAppVersion.text = getAppVersion()
setResultKeyword()
}
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
Collaborator Author

Choose a reason for hiding this comment

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

넴 고고 합시다

Comment on lines 32 to 39
viewModel.myDepartment.observe(viewLifecycleOwner) { department ->
binding.tvSettingDepartment.text = department
}

viewModel.myTopics.observe(viewLifecycleOwner) { myTopics ->
topics = myTopics
}

Copy link
Contributor

Choose a reason for hiding this comment

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

it을 명시할때 department, myTopics라 약간 혼동이 옵니다
통일을 하는게 좋을거 같아요

Comment on lines 82 to 84
_isLoading.postValue(true)
viewModelScope.launch {
when (val result = settingRepository.updateUserDepartment(
Copy link
Contributor

Choose a reason for hiding this comment

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

개인적으로 코드가 viewModelScope.launch처럼 길어질 때 코드 사이에 공백이 있는게 보기 편하더라구요!
효율적인 부분에서는 잘 모르겠어서 의견 말해주시면 컨벤션을 맞추는게 좋을거 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

공백도 컨벤션이라고 했는데 공백 추가하는 걸로 맞추겠슴다

Comment on lines 14 to 16
import kotlinx.coroutines.launch
import timber.log.Timber

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
Contributor

@huiwoo-jo huiwoo-jo left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 👍

@m6z1 m6z1 merged commit c2b2550 into develop Mar 31, 2024
1 check passed
@m6z1 m6z1 deleted the feature/fcm-api branch March 31, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💛명지 🤗 FEATURE Develop this project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

알림 비/활성화 시 api 연동
2 participants