-
Notifications
You must be signed in to change notification settings - Fork 0
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 ] : 버전 업데이트 다이얼로그 구현 #303
Changes from 13 commits
c0a9ce9
03991a5
2c4a208
b35c6d4
53723bc
5c122d9
e890bf8
77420d2
e88342c
8392d39
eefdecb
2a2c1c4
263b5af
bcf81be
59d7496
69d04a3
1a4c01d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package com.yapp.data.model | ||
|
||
import com.yapp.domain.model.Version | ||
import kotlinx.serialization.SerialName | ||
import kotlinx.serialization.Serializable | ||
|
||
@Serializable | ||
data class VersionEntity( | ||
@SerialName("min_version_code") | ||
val minVersionCode: Long? = null, | ||
@SerialName("max_version_code") | ||
val maxVersionCode: Long? = null, | ||
@SerialName("current_version") | ||
val currentVersion: String? = null, | ||
) | ||
|
||
fun VersionEntity.toDomain(isAlreadyRequestVersionUpdate: Boolean): Version { | ||
return Version( | ||
isAlreadyRequestVersionUpdate = isAlreadyRequestVersionUpdate, | ||
minVersionCode = minVersionCode!!, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. !! 를 사용하는 것 보다, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 현재는 시간이 없으므로.. 추후에 일괄 수정하는 것을 함께 검토해보면 좋을 것 같습니다! |
||
maxVersionCode = maxVersionCode!!, | ||
currentVersion = currentVersion!! | ||
) | ||
} | ||
Comment on lines
+8
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VersionEntity가 전부 nullable 처리 되어있는데 어떤 이슈 때문일까요 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 기존 코드를 많이 참고했습니다! 아마 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,12 @@ import com.yapp.data.datasource.FirebaseRemoteConfigDataSource | |
import com.yapp.data.model.ConfigEntity | ||
import com.yapp.data.model.SessionEntity | ||
import com.yapp.data.model.TeamEntity | ||
import com.yapp.data.model.VersionEntity | ||
import com.yapp.data.model.toDomain | ||
import com.yapp.domain.model.Config | ||
import com.yapp.domain.model.Session | ||
import com.yapp.domain.model.Team | ||
import com.yapp.domain.model.Version | ||
import com.yapp.domain.repository.RemoteConfigRepository | ||
import javax.inject.Inject | ||
|
||
|
@@ -16,6 +18,8 @@ class RemoteConfigRepositoryImpl @Inject constructor( | |
private val firebaseRemoteConfigDataSource: FirebaseRemoteConfigDataSource, | ||
) : RemoteConfigRepository { | ||
|
||
private var isAlreadyRequestUpdate = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repository가 싱글톤으로 생성되다 보니, 매번 앱 진입 시 한번만 업데이트 체크를 해주겠군요! |
||
|
||
override suspend fun getMaginotlineTime(): Result<String> { | ||
return runCatching { | ||
firebaseRemoteConfigDataSource.getMaginotlineTime() | ||
|
@@ -94,4 +98,18 @@ class RemoteConfigRepositoryImpl @Inject constructor( | |
) | ||
} | ||
|
||
override suspend fun getVersionInfo(): Result<Version> { | ||
return runCatching { | ||
firebaseRemoteConfigDataSource.getVersionInfo() | ||
}.fold( | ||
onSuccess = { entity: VersionEntity -> | ||
Result.success(entity.toDomain(isAlreadyRequestUpdate)).also { | ||
isAlreadyRequestUpdate = true | ||
} | ||
}, | ||
onFailure = { exception -> | ||
Result.failure(exception) | ||
} | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.yapp.domain.model | ||
|
||
data class Version( | ||
val isAlreadyRequestVersionUpdate: Boolean, | ||
val minVersionCode: Long, | ||
val maxVersionCode: Long, | ||
val currentVersion: String, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.yapp.domain.model.types | ||
|
||
enum class VersionType { | ||
NOT_REQUIRED, | ||
REQUIRED, | ||
UPDATED_BUT_NOT_REQUIRED | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package com.yapp.domain.usecases | ||
|
||
import com.yapp.domain.model.types.VersionType | ||
import com.yapp.domain.repository.RemoteConfigRepository | ||
import javax.inject.Inject | ||
|
||
class CheckVersionUpdateUseCase @Inject constructor( | ||
private val remoteConfigRepository: RemoteConfigRepository, | ||
) { | ||
|
||
suspend operator fun invoke(currentVersionCode: Long): Result<VersionType> { | ||
return remoteConfigRepository.getVersionInfo().mapCatching { version -> | ||
val minVersionCode = version.minVersionCode | ||
val maxVersionCode = version.maxVersionCode | ||
|
||
return@mapCatching when { | ||
currentVersionCode < minVersionCode -> VersionType.REQUIRED | ||
currentVersionCode == maxVersionCode -> VersionType.NOT_REQUIRED | ||
else -> { | ||
if (version.isAlreadyRequestVersionUpdate) VersionType.NOT_REQUIRED | ||
else VersionType.UPDATED_BUT_NOT_REQUIRED | ||
} | ||
} | ||
Comment on lines
+7
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 하나의 UseCase 단위로 깔끔하게 떨어지니 보기도 좋고 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 평소 유즈케이스를 repository function의 래핑 정도로만 인지하고 있었는데, usecase를 기능 단위로 제공할 수 있다는 점이 인상 깊네요! |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YDSPopupDialog에 들어가는 버튼영역(Row)를 함수의 composable로 받도록 하고 버튼 콜백을 제거한 뒤
YDSPopUpDialog를 이용해서 YDSPopUpOneButtonDialog / YDSPopUpTwoButtonDialog 이렇게 만들면
YDSPopUpDialog는 불필요하게 매번 두가지의 콜백, 버튼Text를 받을 필요가 없고
OneButton / TwoButton 컴포저블들외에 다른 타입을 추가할때 제약이 없을것 같아요!
물론 지금 바로 수정하기엔 시간이 부족한지라 이후에 개선해보면 좋을것 같습니다!
이번에 운영진 화면을 개선하면서 이렇게 하는게 좋은 방법인것 같아 공유해봅니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵ㅎㅎ 이슈로 등록해놓고, 추후에 개선하면 좋을 것 같습니다! 좋은 의견 감사드려요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동의합니다~~ 추후 슬롯 API 의 개념을 참고해서 하면 좋을 듯 합니다!