-
-
Notifications
You must be signed in to change notification settings - Fork 13
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/cache detail #58
Conversation
data/database/src/main/java/io/filmtime/data/database/DatabaseModule.kt
Outdated
Show resolved
Hide resolved
data/database/src/main/java/io/filmtime/data/database/FilmTimeDatabase.kt
Outdated
Show resolved
Hide resolved
data/database/src/main/java/io/filmtime/data/database/MovieDetailEntity.kt
Outdated
Show resolved
Hide resolved
data/tmdb-movies/src/main/java/io/fimltime/data/tmdb/movies/MovieDetailEntityExt.kt
Outdated
Show resolved
Hide resolved
data/tmdb-movies/src/main/java/io/fimltime/data/tmdb/movies/TmdbMovieRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
data/tmdb-movies/src/main/java/io/fimltime/data/tmdb/movies/TmdbMovieRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
@hadi-norouzi let me know when your PR is ready for review |
data/tmdb-movies/src/main/java/io/fimltime/data/tmdb/movies/TmdbMovieRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
@TypeConverter | ||
fun stringToList(stringList: String): List<String> { | ||
return stringList.split(",") | ||
} | ||
|
||
@TypeConverter | ||
fun listToString(list: List<String>): String { | ||
return list.joinToString(",") | ||
} | ||
} |
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.
It may seem like I'm splitting hairs, but this approach has some corner cases. e.g: You try to store
listOf("A,B", "C", "D")
and you restore as:
listOf("A", "B", "C", "D")
No need to change it right now.
feature/movie-detail/src/main/java/io/filmtime/feature/movie/detail/MovieDetailScreen.kt
Outdated
Show resolved
Hide resolved
is Success -> { | ||
_state.update { state -> | ||
state.copy(videoDetail = result.data, isLoading = false) | ||
_state.update { it.copy(isLoading = true) } |
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 is a known bug here that we should reset error as well. otherwise our offline implementation does not work well.
_state.update { it.copy(isLoading = true) } | |
_state.update { it.copy(isLoading = true, error = null) } |
feature/movie-detail/src/main/java/io/filmtime/feature/movie/detail/MovieDetailViewModel.kt
Outdated
Show resolved
Hide resolved
It's completed. |
Description
Cache permanent movie details in the Room database.
Checklist
Fixes #56