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

[CAT-313] Timer Service 구현 및 SharedViewModel 관리 #109

Merged
merged 22 commits into from
Sep 11, 2024

Conversation

lee-ji-hoon
Copy link
Collaborator

@lee-ji-hoon lee-ji-hoon commented Sep 4, 2024

작업 내용

  • Timer 관련 Service 구현
  • Timer 의 전반적인 Model을 갖고 있는 SharedViewModel 추가
    • Rest / Focus의 각각의 ViewModel: 클릭 등 각 Screen의 고유한 상태 및 이벤트 관리
    • TimerViewModel: Timer에 대한 상태 및 전반적인 이벤트 관리

체크리스트

  • 빌드 확인
  • 포커스 / 휴식 타이머 정상 동작 확인
  • 포커스 화면에서 특정 시간 (릴리즈 60분 / 디버그 5초?)지난 후 휴식 대기 화면으로 이동 되는지
  • 휴식 대기 화면에서 특정 시간(릴리즈 60분 / 디버그 10초) 이 지난 후 강제로 홈화면 이동 후 다이얼로그 뜨는지
  • 휴식이 끝나면 Room 에서 데이터 정상적으로 제거가 되는지

동작 화면

전체적으로 화면은 똑같아서 첨부는 안했읍니다

살려주세요

@lee-ji-hoon lee-ji-hoon added the bug 버-그 label Sep 4, 2024
Copy link

linear bot commented Sep 4, 2024

CAT-313

Comment on lines 16 to 18
@Query("SELECT *FROM pomodoro_timer WHERE focusTimeId =:timerId")
fun getCurrentTimer(timerId: String): Flow<PomodoroTimerEntity?>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이제 Room에서 가장 하단 Data를 바라보는게 아닌 ViewModel에서 시작한 Id를 기반으로 바라보게 수정

Copy link
Collaborator Author

@lee-ji-hoon lee-ji-hoon left a comment

Choose a reason for hiding this comment

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

아예 ViewModel을 갈아 엎고 Service를 만들어서 로직을 옮기고 이것저것을 하다보니 change가 많아서 전체적인 흐름을 봐주면 좋을거 같아

Comment on lines 78 to 84
init {
viewModelScope.launch {
combinedTimerData.collect { timerData ->
timerData?.let { updateTimerState(it) }
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://proandroiddev.com/loading-initial-data-in-launchedeffect-vs-viewmodel-f1747c20ce62 이 글을 읽다보니 가장 베스트는 SaveStateHandle과 같은 것을 사용해서 데이터를 Lazy하게 관리하는 것인데 우선은 init을 쓰고 이 형태는 다음번에...

Comment on lines 51 to 56
sealed interface PomodoroTimerEffect : ViewSideEffect {
data class GoToPomodoroRest(
val title: String,
val focusTime: Int,
val exceededTime: Int
) : PomodoroTimerEffect

data object GoToPomodoroSetting : PomodoroTimerEffect
data object StartFocusAlarm : PomodoroTimerEffect
data object StopFocusAlarm : PomodoroTimerEffect
data object SendEndFocusAlarm : PomodoroTimerEffect
sealed interface PomodoroFocusEffect : PomodoroTimerEffect {
data object SendEndFocusAlarm : PomodoroFocusEffect
data object ForceGoRest : PomodoroFocusEffect
}

sealed interface PomodoroRestEffect : PomodoroTimerEffect {
data object SendEndRestAlarm : PomodoroRestEffect
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Timer에 Focus랑 Rest 각각 다른 Effect로 분리를 해서 각 Screen에서 알맞은 Effect만 받을 수 있게 구조 변경

suspend fun updateFocusTime(focusedTime: Int)
suspend fun updateRestedTime(restedTime: Int)
suspend fun insertPomodoroTimerInitData(categoryNo: Int, focusTimeId: String)
suspend fun incrementFocusedTime()
Copy link
Member

Choose a reason for hiding this comment

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

기획에 맞춰서 increase -> decrease 되는 걸로 바꾸어야 될듯!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://discord.com/channels/1259883776474087638/1263826433969491998/1276752685890015243

위 디코 코멘트를 보면 지금 현재 상태가 맞지 않아? 0 -> N 분으로 가는거

Copy link
Member

Choose a reason for hiding this comment

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

image

피그마에는 이렇고, iOS 도 여기 따라가던데 머지

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이거 지금 보니까 1. 사용자가 설정한 시간에서 1초씩 감소함 인데 시간 초과시의 UI를 보면 감소를 했는데 30:00 일수가 없어서 내가 저번에 디자인과 기획에 문의해보니 초과랑 헷갈리지 않게끔 1초씩 증가 하는 것으로 이야기가 됐는데, 피그마가 업데이트가 안된거라고 하더라고, 다음번 회의때 이거 어떻게 할지 이야기 해봅시다

exceededTime = state.value.exceededTime
)
)
private fun initPomodoroData() {
Copy link
Member

Choose a reason for hiding this comment

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

코드를 내가 이해한게 맞으면, viewmodel이 init 될때마다 focusTimerId를 만들고 해당 타이머 사이클에 대한 정보를 entity로 만들어서 관리하는거지?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

viewModel이 init될때마다는 아닌 상태야 PomodoroTimer의 LaunchedEffect로 init 이벤트가 실행이 될 때 release가 된다고 보면 되긴해!..

내가 원하는건 PomodoroFocusScreen을 기반으로 FocusScreen으로 들어올때마다 ViewModel이 아예 재생성 됐으면 하는데 그러면 휴식 대기화면으로 갔을 때 FocusScreen을 pop하면 안되다보니 backstack이 좀 꼬여서 어떻게 관리할지 좀 고민중이야

@HyomK
Copy link
Member

HyomK commented Sep 5, 2024

스크린샷 2024-09-05 오후 8 49 47

휴식하기에서 너무 오래 머물러서 홈으로 돌아갈 때 doneAt이 빈채로 room 에 남아있는데 이게 나중에 다른 타이머 entitiy랑 같이 서버에 전송될때 빈값이라 400 에러 발생 -> room 안지워짐 버그 있어요

@lee-ji-hoon
Copy link
Collaborator Author

#109 (comment) 이거

아래 2개로 수정해서 방어해놨어~!

Copy link
Member

Choose a reason for hiding this comment

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

확실히 포그라운드로 전환하니까 정확성이 올라간다bb
서비스라 브로드캐스트 리시버, presentation 쪽 비즈니스 로직이 좀 엉켜있게 된 거 같은데, 나중에 리팩토링할 때 어떻게 깔끔하게 가져갈지 이야기 해보자~~

Copy link
Collaborator Author

@lee-ji-hoon lee-ji-hoon Sep 10, 2024

Choose a reason for hiding this comment

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

포그라운드 서비스를 알람들 처럼 그냥 app 모듈로 올리고 로컬 브로드캐스트 해버릴까

Copy link
Member

Choose a reason for hiding this comment

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

요건 좀 생각해보고 뒤에 리팩토링하자

@@ -62,7 +62,7 @@ fun PomodoroFocusRoute(
// NOTHING
}

pomodoroFocusViewModel.effects.collectWithLifecycle(minActiveState = Lifecycle.State.CREATED) { effect ->
Copy link
Member

Choose a reason for hiding this comment

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

라이프 사이클의 중요성 많이 배워갑니다

@lee-ji-hoon lee-ji-hoon merged commit 160e3f6 into develop Sep 11, 2024
1 check passed
@lee-ji-hoon lee-ji-hoon deleted the feature/cat-313 branch September 11, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 버-그
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants