Skip to content

Commit

Permalink
Fixing a bug where an already kept local group triggered a dialog eve…
Browse files Browse the repository at this point in the history
…ry time app restarts.

Fixing WebSocket message parsing failing for a group deletion event.
Moving DB changes listener responsible for watching over group deletion events to DashboardViewModel to ensure it doesn't stop listening to events after navigating through screens as was happening previously.
  • Loading branch information
Dima-Android committed Jun 27, 2024
1 parent 4ded7bd commit 1ba5e4e
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ChangeWsResponseMapper @Inject constructor(private val gson: Gson) {
}
val libraryId = LibraryIdentifier.from(topic)
if (libraryId != null) {
val version = json["version"].asInt
val version = json["version"]?.asString?.toIntOrNull()
return ChangeWsResponse(type = ChangeWsResponse.Kind.library(libraryId, version))
}
throw ChangeWsResponse.Error.unknownChange(topic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ import org.zotero.android.architecture.navigation.toZoteroWebViewScreen
import org.zotero.android.architecture.navigation.toolbar.SyncToolbarScreen
import org.zotero.android.architecture.navigation.videoPlayerScreen
import org.zotero.android.architecture.navigation.zoterWebViewScreen
import org.zotero.android.architecture.ui.CustomLayoutSize
import org.zotero.android.pdf.pdfReaderNavScreensForPhone
import org.zotero.android.pdf.toPdfScreen
import org.zotero.android.screens.collectionedit.collectionEditNavScreens
import org.zotero.android.screens.collectionedit.toCollectionEditScreen
import org.zotero.android.screens.collectionpicker.CollectionPickerScreen
import org.zotero.android.screens.creatoredit.creatorEditNavScreens
import org.zotero.android.screens.creatoredit.toCreatorEdit
import org.zotero.android.screens.dashboard.DashboardViewEffect
import org.zotero.android.screens.dashboard.DashboardViewModel
import org.zotero.android.screens.dashboard.DashboardViewState
import org.zotero.android.screens.filter.FilterScreenPhone
Expand All @@ -70,15 +72,27 @@ internal fun DashboardRootPhoneNavigation(
wasPspdfkitInitialized: Boolean,
) {
val viewState by viewModel.viewStates.observeAsState(DashboardViewState())
val isTablet = CustomLayoutSize.calculateLayoutType().isTablet()
LaunchedEffect(key1 = viewModel) {
viewModel.init()
viewModel.init(isTablet = isTablet)
}

val navController = rememberNavController()
val dispatcher = LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher
val navigation = remember(navController) {
ZoteroNavigation(navController, dispatcher)
}

val viewEffect by viewModel.viewEffects.observeAsState()
LaunchedEffect(key1 = viewEffect) {
when (viewEffect?.consume()) {
null -> Unit
DashboardViewEffect.NavigateToCollectionsScreen -> navigateToCollectionsScreen(
navController
)
}
}

val context = LocalContext.current

Box {
Expand All @@ -102,9 +116,7 @@ internal fun DashboardRootPhoneNavigation(
)
librariesScreen(
navigateToCollectionsScreen = {
navController.popBackStack(navController.graph.id, inclusive = true)
navController.navigate(CommonScreenDestinations.LIBRARIES_SCREEN)
navController.navigate(CommonScreenDestinations.COLLECTIONS_SCREEN)
navigateToCollectionsScreen(navController)
},
onSettingsTapped = { navigation.toSettingsScreen() }
)
Expand Down Expand Up @@ -230,6 +242,12 @@ internal fun DashboardRootPhoneNavigation(
}
}

private fun navigateToCollectionsScreen(navController: NavHostController) {
navController.popBackStack(navController.graph.id, inclusive = true)
navController.navigate(CommonScreenDestinations.LIBRARIES_SCREEN)
navController.navigate(CommonScreenDestinations.COLLECTIONS_SCREEN)
}

private object DashboardRootPhoneDestinations {
const val SINGLE_PICKER = "singlePicker"
const val ADD_BY_IDENTIFIER = "addByIdentifier"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.zotero.android.architecture.navigation.CommonScreenDestinations
import org.zotero.android.architecture.navigation.DashboardTopLevelDialogs
import org.zotero.android.architecture.navigation.ZoteroNavigation
import org.zotero.android.architecture.navigation.toolbar.SyncToolbarScreen
import org.zotero.android.architecture.ui.CustomLayoutSize
import org.zotero.android.screens.dashboard.DashboardViewModel
import org.zotero.android.screens.dashboard.DashboardViewState
import org.zotero.android.uicomponents.misc.NewDivider
Expand All @@ -39,8 +40,9 @@ internal fun DashboardRootTabletNavigationScreen(
viewModel: DashboardViewModel,
) {
val viewState by viewModel.viewStates.observeAsState(DashboardViewState())
val isTablet = CustomLayoutSize.calculateLayoutType().isTablet()
LaunchedEffect(key1 = viewModel) {
viewModel.init()
viewModel.init(isTablet)
}

val rightPaneNavController = rememberNavController()
Expand All @@ -61,6 +63,7 @@ internal fun DashboardRootTabletNavigationScreen(
Row(modifier = Modifier.fillMaxSize()) {
Box(modifier = Modifier.weight(0.35f)) {
TabletLeftPaneNavigation(
viewModel = viewModel,
navigateAndPopAllItemsScreen = navigateAndPopAllItemsScreen,
onOpenWebpage = onOpenWebpage
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ import android.net.Uri
import androidx.activity.compose.LocalOnBackPressedDispatcherOwner
import androidx.compose.foundation.layout.navigationBarsPadding
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.livedata.observeAsState
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.navigation.NavHostController
import androidx.navigation.compose.rememberNavController
import org.zotero.android.architecture.navigation.CommonScreenDestinations
import org.zotero.android.architecture.navigation.ZoteroNavigation
Expand All @@ -15,11 +19,14 @@ import org.zotero.android.architecture.navigation.dialogDynamicHeight
import org.zotero.android.architecture.navigation.dialogFixedMaxHeight
import org.zotero.android.architecture.navigation.librariesScreen
import org.zotero.android.screens.collectionedit.CollectionEditNavigation
import org.zotero.android.screens.dashboard.DashboardViewEffect
import org.zotero.android.screens.dashboard.DashboardViewModel
import org.zotero.android.screens.settings.SettingsNavigation
import org.zotero.android.uicomponents.navigation.ZoteroNavHost

@Composable
internal fun TabletLeftPaneNavigation(
viewModel: DashboardViewModel,
onOpenWebpage: (uri: Uri) -> Unit,
navigateAndPopAllItemsScreen: () -> Unit,
) {
Expand All @@ -28,6 +35,13 @@ internal fun TabletLeftPaneNavigation(
val navigation = remember(navController) {
ZoteroNavigation(navController, dispatcher)
}
val viewEffect by viewModel.viewEffects.observeAsState()
LaunchedEffect(key1 = viewEffect) {
when (viewEffect?.consume()) {
null -> Unit
DashboardViewEffect.NavigateToCollectionsScreen -> navigateToCollectionsScreen(navController)
}
}
ZoteroNavHost(
navController = navController,
startDestination = CommonScreenDestinations.COLLECTIONS_SCREEN,
Expand All @@ -44,9 +58,7 @@ internal fun TabletLeftPaneNavigation(

librariesScreen(
navigateToCollectionsScreen = {
navController.popBackStack(navController.graph.id, inclusive = true)
navController.navigate(CommonScreenDestinations.LIBRARIES_SCREEN)
navController.navigate(CommonScreenDestinations.COLLECTIONS_SCREEN)
navigateToCollectionsScreen(navController)
},
onSettingsTapped = navigation::toSettingsNavigation
)
Expand All @@ -64,6 +76,12 @@ internal fun TabletLeftPaneNavigation(
}
}

private fun navigateToCollectionsScreen(navController: NavHostController) {
navController.popBackStack(navController.graph.id, inclusive = true)
navController.navigate(CommonScreenDestinations.LIBRARIES_SCREEN)
navController.navigate(CommonScreenDestinations.COLLECTIONS_SCREEN)
}

private object TabletLeftPaneDestinations {
const val COLLECTION_EDIT_NAVIGATION = "collectionEditNavigation"
const val SETTINGS_NAVIGATION = "settingsNavigation"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ fun <T> RealmQuery<T>.name(name: String): RealmQuery<T> {
return rawPredicate("name = $0", name)
}

fun <T> RealmQuery<T>.groupId(identifier: Int): RealmQuery<T> {
return rawPredicate("identifier == $0", identifier)
}

fun <T> RealmQuery<T>.name(name: String, libraryId: LibraryIdentifier): RealmQuery<T> {
return name(name)
.and()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,11 @@ class SyncGroupVersionsDbRequest(private val versions: Map<Int, Int>) :
database: Realm,
): ResultSyncVersions {
val allKeys = versions.keys.toMutableList()

val toRemove: List<RGroup> =
database
.where<RGroup>()
.rawPredicate("isLocalOnly == false")
.findAll()
.filter { !allKeys.contains(it.identifier) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import android.content.ClipboardManager
import android.content.Context
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
import io.realm.OrderedCollectionChangeSet
import io.realm.RealmResults
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
Expand All @@ -24,10 +26,13 @@ import org.zotero.android.architecture.logging.debug.DebugLoggingDialogDataEvent
import org.zotero.android.architecture.logging.debug.DebugLoggingInterface
import org.zotero.android.database.DbWrapper
import org.zotero.android.database.objects.RCustomLibraryType
import org.zotero.android.database.objects.RGroup
import org.zotero.android.database.requests.DeleteGroupDbRequest
import org.zotero.android.database.requests.ReadAllGroupsDbRequest
import org.zotero.android.database.requests.ReadCollectionDbRequest
import org.zotero.android.database.requests.ReadLibraryDbRequest
import org.zotero.android.database.requests.ReadSearchDbRequest
import org.zotero.android.database.requests.groupId
import org.zotero.android.files.FileStore
import org.zotero.android.screens.allitems.data.AllItemsArgs
import org.zotero.android.screens.allitems.data.InitialLoadData
Expand Down Expand Up @@ -63,6 +68,9 @@ class DashboardViewModel @Inject constructor(
) : BaseViewModel2<DashboardViewState, DashboardViewEffect>(DashboardViewState()),
DebugLoggingInterface {

var isTablet: Boolean = false
var groupLibraries: RealmResults<RGroup>? = null

@Subscribe(threadMode = ThreadMode.MAIN)
fun onEvent(deleteGroupDialogData: DeleteGroupDialogData) {
updateState {
Expand Down Expand Up @@ -127,13 +135,15 @@ class DashboardViewModel @Inject constructor(
}
}

fun init() = initOnce {
fun init(isTablet: Boolean) = initOnce {
this.isTablet = isTablet
EventBus.getDefault().register(this)
ScreenArguments.collectionsArgs = CollectionsArgs(libraryId = fileStore.getSelectedLibrary(), fileStore.getSelectedCollectionId())

debugLogging.debugLoggingInterface = this
setupDebugLoggingDialogDataEventStream()
setupCrashShareDataEventStream()
listenToGroupDeletionEvents()

if (sessionController.isInitialized && debugLogging.isEnabled) {
setDebugWindow(true)
Expand Down Expand Up @@ -365,6 +375,77 @@ class DashboardViewModel @Inject constructor(
}
}
}

private fun listenToGroupDeletionEvents() {
dbWrapper.realmDbStorage.perform { coordinator ->
this.groupLibraries = coordinator.perform(request = ReadAllGroupsDbRequest())

this.groupLibraries?.addChangeListener { _, changeSet ->
when (changeSet.state) {
OrderedCollectionChangeSet.State.INITIAL -> {
//no-op
}

OrderedCollectionChangeSet.State.UPDATE -> {
val deletions = changeSet.deletions
if (deletions.isNotEmpty()) {
showDefaultLibraryIfNeeded()
}
}

OrderedCollectionChangeSet.State.ERROR -> {
Timber.e(changeSet.error, "DashboardViewModel: could not listen to Group Events")
}
else -> {
//no-op
}
}
}
}
}

private fun showDefaultLibraryIfNeeded() {
when (val visibleLibraryId = fileStore.getSelectedLibrary()) {
is LibraryIdentifier.custom -> {
//no-op
}

is LibraryIdentifier.group -> {
val groupId = visibleLibraryId.groupId

if (this.groupLibraries?.where()?.groupId(groupId)?.findFirst() == null) {
showCollections(LibraryIdentifier.custom(RCustomLibraryType.myLibrary))
}
}
}
}

fun showCollections(libraryId: LibraryIdentifier) {
val collectionId = storeIfNeeded(libraryId = libraryId)

ScreenArguments.collectionsArgs = CollectionsArgs(
libraryId = libraryId,
selectedCollectionId = collectionId,
shouldRecreateItemsScreen = this.isTablet
)
triggerEffect(DashboardViewEffect.NavigateToCollectionsScreen)
}

private fun storeIfNeeded(libraryId: LibraryIdentifier, collectionId: CollectionIdentifier? = null): CollectionIdentifier {
if (fileStore.getSelectedLibrary() == libraryId) {
if (collectionId != null) {
fileStore.setSelectedCollectionId(collectionId)
return collectionId
}
return fileStore.getSelectedCollectionId()
}

val collectionId = collectionId ?: CollectionIdentifier.custom(CollectionIdentifier.CustomType.all)
fileStore.setSelectedLibrary(libraryId)
fileStore.setSelectedCollectionId(collectionId)
return collectionId

}
}

data class DashboardViewState(
Expand All @@ -379,6 +460,7 @@ data class DashboardViewState(
) : ViewState

sealed class DashboardViewEffect : ViewEffect {
object NavigateToCollectionsScreen : DashboardViewEffect()
}

sealed class ConflictDialogData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ internal class LibrariesViewModel @Inject constructor(
}

OrderedCollectionChangeSet.State.UPDATE -> {
val deletions = changeSet.deletions
if (deletions.isNotEmpty()) {
showDefaultLibraryIfNeeded()
}
generateLibraryRows()
}

Expand All @@ -78,10 +74,6 @@ internal class LibrariesViewModel @Inject constructor(
}
}

private fun showDefaultLibraryIfNeeded() {
showCollections(LibraryIdentifier.custom(RCustomLibraryType.myLibrary))
}

private fun generateLibraryRows() {
updateState {
copy(
Expand Down

0 comments on commit 1ba5e4e

Please sign in to comment.