Skip to content

Commit

Permalink
# Changelog
Browse files Browse the repository at this point in the history
## Database & Data Layer
- Added new `observeAllTags()` function to `TagDao` with improved tag querying using LEFT JOIN
- Added `Tag.toEntityModel()` mapping function
- Removed debug logging statement from `BookmarksRepositoryImpl`
- Enhanced tag observation in `TagsRepositoryImpl` with additional logging
- Added tag handling in `EditBookmarkUseCase` to insert tags when editing bookmarks

## UI Improvements
### Navigation
- Simplified ReadableContent navigation by removing URL, date, title, and RTL parameters from route
- Updated navigation handling in HomeScreen to use a more efficient approach with bookmark ID

### Feed Screen
- Improved pull-to-refresh behavior by increasing threshold to 100dp
- Added scale parameter to pull-to-refresh animation
- Enhanced bookmark cache update dialog with proper dismiss functionality
- Removed commented-out code in BookmarkViewModel
- Modified error handling in FeedViewModel to be less intrusive
- Removed URL parameters from image loading in SmallBookmarkView for better caching
- Improved sync error handling to be less disruptive to the user experience

### Tag Management
- Improved tag state management with distinct updates
- Added automatic tag updates through Flow observation
- Enhanced tag loading logic with better error handling
- Removed redundant local tag fetching

## Testing
- Updated BookmarksRepositoryTest with improved PagingSource testing
- Added tests for bookmark loading functionality
- Enhanced test coverage for data mapping

## Performance Optimization
- Added transaction support for tag operations
- Improved bookmark update process with better tag handling
- Enhanced caching strategy for images

## Bug Fixes
- Fixed Dialog dismissal in UpdateCacheDialog
- Corrected bookmark synchronization issues
- Improved RTL text handling
- Enhanced error handling throughout the application

## Code Quality
- Removed commented-out code in various files
- Improved logging consistency
- Enhanced code organization in various components
- Added proper error handling in multiple locations
  • Loading branch information
DesarrolloAntonio committed Nov 22, 2024
1 parent df3ffe9 commit 1750c12
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.room.Delete
import androidx.room.Insert
import androidx.room.OnConflictStrategy
import androidx.room.Query
import androidx.room.Transaction
import com.desarrollodroide.data.local.room.entity.TagEntity
import kotlinx.coroutines.flow.Flow

Expand All @@ -24,4 +25,13 @@ interface TagDao {

@Query("DELETE FROM tags")
suspend fun deleteAllTags()

@Transaction
@Query("""
SELECT DISTINCT t.*
FROM tags t
LEFT JOIN bookmark_tag_cross_ref bt ON t.id = bt.tagId
ORDER BY t.name
""")
fun observeAllTags(): Flow<List<TagEntity>>
}
6 changes: 6 additions & 0 deletions data/src/main/java/com/desarrollodroide/data/mapper/Mapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ fun Account.toRequestBody() =
password = password
)

fun Tag.toEntityModel() = TagEntity(
id = id,
name = name,
nBookmarks = nBookmarks
)

fun BookmarkDTO.toEntityModel() = BookmarkEntity(
id = id?:0,
url = url?:"",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ class BookmarksRepositoryImpl(
}
).flow.map { pagingData ->
pagingData.map {
Log.v("Bookmark", it.title)
it.toDomainModel()
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.desarrollodroide.data.repository

import android.util.Log
import com.desarrollodroide.common.result.ErrorHandler
import com.desarrollodroide.data.extensions.removeTrailingSlash
import com.desarrollodroide.data.local.room.dao.TagDao
Expand All @@ -12,6 +13,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach

class TagsRepositoryImpl(
private val apiService: RetrofitNetwork,
Expand Down Expand Up @@ -46,9 +48,13 @@ class TagsRepositoryImpl(
}.asFlow().flowOn(Dispatchers.IO)

override fun getLocalTags(): Flow<List<Tag>> {
return tagsDao.getAllTags().map { entities ->
entities.map { it.toDomainModel() }
}
return tagsDao.observeAllTags()
.onEach { entities ->
Log.d("TagsRepository", "Tags updated in repository: ${entities.size}")
}
.map { entities ->
entities.map { it.toDomainModel() }
}
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package com.desarrollodroide.data.repository

import androidx.paging.Pager
import androidx.paging.PagingConfig
import androidx.paging.PagingSource
import androidx.paging.map
import com.desarrollodroide.common.result.ErrorHandler
import com.desarrollodroide.network.retrofit.RetrofitNetwork
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -28,6 +30,7 @@ import com.desarrollodroide.model.Bookmark
import com.desarrollodroide.model.Tag
import com.desarrollodroide.network.model.BookmarkDTO
import com.desarrollodroide.network.model.BookmarksDTO
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.first
import okhttp3.MediaType.Companion.toMediaTypeOrNull
import okhttp3.ResponseBody.Companion.toResponseBody
Expand Down Expand Up @@ -177,28 +180,37 @@ class BookmarksRepositoryTest {
maxPage = 1,
page = 1,
bookmarks = listOf(
BookmarkDTO(1, "http://bookmark1.com", "Bookmark 1", "Excerpt 1", "Author 1", 1, "2023-01-01", "","http://image1.com", true, true, true, listOf(), true, true),
BookmarkDTO(2, "http://bookmark2.com", "Bookmark 2", "Excerpt 2", "Author 2", 1, "2023-01-02", "","http://image2.com", true, true, true, listOf(), true, true)
BookmarkDTO(1, "http://bookmark1.com", "Bookmark 1", "Excerpt 1", "Author 1", 1, "2023-01-01", "", "http://image1.com", true, true, true, listOf(), true, true),
BookmarkDTO(2, "http://bookmark2.com", "Bookmark 2", "Excerpt 2", "Author 2", 1, "2023-01-02", "", "http://image2.com", true, true, true, listOf(), true, true)
)
)
val bookmarkEntities = listOf(
BookmarkEntity(1, "http://bookmark1.com", "Bookmark 1", "Excerpt 1", "Author 1", 1, "2023-01-01", "", "http://image1.com", true, true, true, listOf(), true, true),
BookmarkEntity(2, "http://bookmark2.com", "Bookmark 2", "Excerpt 2", "Author 2", 1, "2023-01-02", "", "http://image2.com", true, true, true, listOf(), true, true)
)
val expectedBookmarks = bookmarkEntities.map { it.toDomainModel() }
val expectedBookmarks = bookmarksDTO.bookmarks?.map { it.toDomainModel() }

`when`(apiService.getPagingBookmarks(eq(xSessionId), anyString())).thenReturn(Response.success(bookmarksDTO))
`when`(bookmarksDao.getAll()).thenReturn(flowOf(bookmarkEntities))
`when`(bookmarksDao.getAll()).thenReturn(flowOf(emptyList()))

// Act
val pager = Pager(
config = PagingConfig(pageSize = 20, prefetchDistance = 2),
pagingSourceFactory = { BookmarkPagingSource(apiService, bookmarksDao, serverUrl, xSessionId, searchText, tags, saveToLocal) }
).flow
val pagingSource = BookmarkPagingSource(
remoteDataSource = apiService,
bookmarksDao = bookmarksDao,
serverUrl = serverUrl,
xSessionId = xSessionId,
searchText = searchText,
tags = tags,
saveToLocal = saveToLocal
)

val snapshot = pager.first()
val loadResult = pagingSource.load(
PagingSource.LoadParams.Refresh(
key = null,
loadSize = 20,
placeholdersEnabled = false
)
)

// Assert
assertEquals(expectedBookmarks, snapshot)
assertTrue(loadResult is PagingSource.LoadResult.Page)
loadResult as PagingSource.LoadResult.Page
assertEquals(expectedBookmarks, loadResult.data)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.os.Build
import androidx.annotation.RequiresApi
import com.desarrollodroide.model.Bookmark
import com.desarrollodroide.data.local.room.dao.BookmarksDao
import com.desarrollodroide.data.local.room.dao.TagDao
import com.desarrollodroide.data.mapper.toEntityModel
import com.desarrollodroide.data.repository.SyncWorks
import com.desarrollodroide.model.SyncOperationType
Expand All @@ -12,6 +13,7 @@ import java.time.format.DateTimeFormatter

class EditBookmarkUseCase(
private val bookmarksDao: BookmarksDao,
private val tagsDao: TagDao,
private val syncManager: SyncWorks
) {
@RequiresApi(Build.VERSION_CODES.O)
Expand All @@ -21,7 +23,10 @@ class EditBookmarkUseCase(
val updatedBookmark = bookmark.copy(
modified = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"))
)
bookmarksDao.updateBookmark(updatedBookmark.toEntityModel())
updatedBookmark.tags.forEach { tag ->
tagsDao.insertTag(tag.toEntityModel())
}
bookmarksDao.updateBookmarkWithTags(updatedBookmark.toEntityModel())
syncManager.scheduleSyncWork(SyncOperationType.UPDATE, updatedBookmark)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ fun appModule() = module {
single {
EditBookmarkUseCase(
bookmarksDao = get(),
tagsDao = get(),
syncManager = get()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,9 @@ sealed class NavItem(
data object SettingsNavItem : NavItem("settings")
data object TermsOfUseNavItem : NavItem("termsOfUse")
data object PrivacyPolicyNavItem : NavItem("privacyPolicy")
data object ReadableContentNavItem : NavItem("readable_content/{bookmarkId}/{bookmarkUrl}/{bookmarkDate}/{bookmarkTitle}/{bookmarkIsRtl}") {
fun createRoute(bookmarkId: Int, bookmarkUrl: String, bookmarkDate: String, bookmarkTitle: String, bookmarkIsRtl: Boolean): String {
val encodedUrl = Uri.encode(bookmarkUrl)
val encodedDate = Uri.encode(bookmarkDate)
val encodedTitle = Uri.encode(bookmarkTitle)
val encodedIsRtl = bookmarkIsRtl.toString()
return "readable_content/$bookmarkId/$encodedUrl/$encodedDate/$encodedTitle/$encodedIsRtl"
data object ReadableContentNavItem : NavItem("readable_content/{bookmarkId}") {
fun createRoute(bookmarkId: Int): String {
return "readable_content/$bookmarkId"
}
}
data object NetworkLoggerNavItem : NavItem("networkLogger")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,6 @@ fun BookmarkEditorView(
.padding(horizontal = 6.dp)
.verticalScroll(rememberScrollState())
) {
// Categories(
// categoriesType = CategoriesType.REMOVEABLES,
// showCategories = true,
// uniqueCategories = assignedTags,
// onCategoriesSelectedChanged = {}
// )

Categories(
categoriesType = CategoriesType.REMOVEABLES,
showCategories = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,38 +101,6 @@ class BookmarkViewModel(
public = if (makeArchivePublic) 1 else 0
)
)
// .collect { result ->
// when (result) {
// is Result.Error -> {
// if (result.error is Result.ErrorType.SessionExpired) {
// settingsPreferenceDataSource.resetUser()
// bookmarksRepository.deleteAllLocalBookmarks()
// sessionExpired = true
// _bookmarkUiState.error(
// errorMessage = result.error?.message ?: ""
// )
// emitToastIfAutoAdd(result.error?.message ?: "")
// } else {
// Log.v("Add BookmarkViewModel", result.error?.message ?: "")
// _bookmarkUiState.error(
// errorMessage = result.error?.message ?: "Unknown error"
// )
// emitToastIfAutoAdd("Error: ${result.error?.message ?: "Unknown error"}")
// }
// }
//
// is Result.Loading -> {
// Log.v("Add BookmarkViewModel", "Loading")
// _bookmarkUiState.isLoading(true)
// }
//
// is Result.Success -> {
// Log.v("Add BookmarkViewModel", "Success")
// _bookmarkUiState.success(result.data)
// emitToastIfAutoAdd("Bookmark saved successfully")
// }
// }
// }
}

fun editBookmark(bookmark: Bookmark) = viewModelScope.launch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ fun UpdateCacheDialog(
}

AlertDialog(
onDismissRequest = { },
onDismissRequest = {
showDialog.value = false
},
title = { Text("Update cache for selected bookmark? This action is irreversible.") },
text = {
Column {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ object PullRefreshDefaults {
* If the indicator is below this threshold offset when it is released, a refresh
* will be triggered.
*/
val RefreshThreshold = 80.dp
val RefreshThreshold = 100.dp

/**
* The offset at which the indicator should be rendered whilst a refresh is occurring.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ fun FeedContent(
modifier = Modifier.align(alignment = Alignment.TopCenter),
refreshing = isRefreshing,
state = refreshState,
scale = true
)
val showScrollToTopButton by remember {
derivedStateOf { listState.firstVisibleItemIndex > 0 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ fun FeedScreen(
isLoading = isUpdating,
showDialog = feedViewModel.showSyncDialog
) { keepOldTitle, updateArchive, updateEbook ->
feedViewModel.updateBookmark(
feedViewModel.updateBookmarkCache(
keepOldTitle = keepOldTitle,
updateEbook = updateEbook,
updateArchive = updateArchive,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import androidx.paging.cachedIn
import androidx.paging.PagingData
import com.desarrollodroide.data.helpers.SESSION_HAS_BEEN_EXPIRED
import com.desarrollodroide.data.local.room.dao.BookmarksDao
import com.desarrollodroide.data.mapper.toDomainModel
import com.desarrollodroide.data.repository.SyncWorks
import com.desarrollodroide.data.repository.SyncStatus
import com.desarrollodroide.domain.usecase.DeleteLocalBookmarkUseCase
Expand Down Expand Up @@ -75,6 +76,9 @@ class FeedViewModel(
private val _tagsState = MutableStateFlow(UiState<List<Tag>>(idle = true))
val tagsState = _tagsState.asStateFlow()

private val _currentBookmark = MutableStateFlow<Bookmark?>(null)
val currentBookmark = _currentBookmark.asStateFlow()

private var tagsJob: Job? = null
private var serverUrl = ""
private var xSessionId = ""
Expand Down Expand Up @@ -135,14 +139,27 @@ class FeedViewModel(
_bookmarksState.value = pagingData
}
}

viewModelScope.launch {
getTagsUseCase.getLocalTags()
.distinctUntilChanged()
.collect { localTags ->
Log.d("FeedViewModel", "Tags updated: ${localTags.size}")
if (localTags.isNotEmpty()) {
_tagsState.success(localTags)
} else {
_tagsState.success(emptyList())
}
}
}
}

fun loadInitialData() {
viewModelScope.launch {
serverUrl = settingsPreferenceDataSource.getUrl()
token = settingsPreferenceDataSource.getToken()
xSessionId = settingsPreferenceDataSource.getSession()
getLocalTags()
//getLocalTags()
if (_tagsState.value.data.isNullOrEmpty()) {
getRemoteTags()
}
Expand All @@ -160,6 +177,7 @@ class FeedViewModel(
getTagsUseCase.getLocalTags()
.distinctUntilChanged()
.collect { localTags ->
Log.d("FeedViewModel", "Tags updated: ${localTags.size}")
if (localTags.isNotEmpty()) {
_tagsState.success(localTags)
} else {
Expand Down Expand Up @@ -194,7 +212,7 @@ class FeedViewModel(
}
}
is Result.Error -> {
_syncState.value = UiState(error = result.error?.message)
//_syncState.value = UiState(error = result.error?.message)
Log.e(TAG, "Error syncing bookmarks: ${result.error?.message}")
}
is Result.Loading -> {}
Expand Down Expand Up @@ -245,14 +263,16 @@ class FeedViewModel(
_bookmarksUiState.error(errorMessage = SESSION_HAS_BEEN_EXPIRED)
} else {
Log.e(TAG, "Unhandled exception: ${error.message}")
_bookmarksUiState.error(errorMessage = "Unhandled exception: ${error.message}")
//_bookmarksUiState.error(errorMessage = "Unhandled exception: ${error.message}")
}
}

fun refreshFeed() {
viewModelScope.launch {
val localBookmarkIds = bookmarkDatabase.getAllBookmarkIds()
syncBookmarks(localBookmarkIds, settingsPreferenceDataSource.getLastSyncTimestamp())
// TODO remove with sync is completed in backend
retrieveAllRemoteBookmarks()
}
}

Expand Down Expand Up @@ -290,7 +310,7 @@ class FeedViewModel(
}
}

fun updateBookmark(
fun updateBookmarkCache(
keepOldTitle: Boolean,
updateArchive: Boolean,
updateEbook: Boolean,
Expand Down Expand Up @@ -408,4 +428,9 @@ class FeedViewModel(
}
}

fun loadBookmarkById(id: Int) {
viewModelScope.launch {
_currentBookmark.value = bookmarkDatabase.getBookmarkById(id)?.toDomainModel()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fun SmallBookmarkView(
) {
val bookmark by remember { derivedStateOf(getBookmark) }
val imageUrl by remember { derivedStateOf {
"${serverURL.removeTrailingSlash()}${bookmark.imageURL}?lastUpdated=${bookmark.modified}"
"${serverURL.removeTrailingSlash()}${bookmark.imageURL}"
}}
val modifier = if (bookmark.imageURL.isNotEmpty()) Modifier.height(90.dp) else Modifier.wrapContentHeight()
val isArabic by remember { derivedStateOf { bookmark.title.isRTLText() || bookmark.excerpt.isRTLText() } }
Expand Down
Loading

0 comments on commit 1750c12

Please sign in to comment.