Push: improve Push history screen, log and stored data (#4601)

* Add adb tools to help with doze mode and app standby * Add info about the device state when an error occurs in push. * Keep more events in the DB. * Push history: add confirmation dialog when resetting the data * Push history: add a filter to see only the errors * Update screenshots * Push history: print out invalid/ignored data received. * Increase log level for push, to make such log more visible. It also appears that sometimes Timber.d are not present in the rageshakes. * Log priority * Do not include device state for invalid/ignored event. * Fix tests. * Fix format issue. * Fix mistake in code blocks and do not filter when not necessary. * Improve formatting and add missing unit test. * Reduce nesting of blocks. --------- Co-authored-by: ElementBot <android@element.io>
2025-04-16 16:37:32 +02:00 · 2025-04-16 16:37:32 +02:00 · 7ed362b9db
commit 7ed362b9db
parent 653416fa34
30 changed files with 388 additions and 58 deletions
--- a/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryEvents.kt
+++ b/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryEvents.kt
@ -8,5 +8,7 @@
 package io.element.android.libraries.troubleshoot.impl.history

 sealed interface PushHistoryEvents {
-    data object Reset : PushHistoryEvents
+    data class SetShowOnlyErrors(val showOnlyErrors: Boolean) : PushHistoryEvents
+    data class Reset(val requiresConfirmation: Boolean) : PushHistoryEvents
+    data object ClearDialog : PushHistoryEvents
 }
--- a/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryPresenter.kt
+++ b/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryPresenter.kt
@ -10,11 +10,15 @@ package io.element.android.libraries.troubleshoot.impl.history
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.setValue
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.push.api.PushService
 import kotlinx.collections.immutable.toImmutableList
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
 import javax.inject.Inject

@ -25,14 +29,36 @@ class PushHistoryPresenter @Inject constructor(
    override fun present(): PushHistoryState {
        val coroutineScope = rememberCoroutineScope()
        val pushCounter by pushService.pushCounter.collectAsState(0)
-        val pushHistory by remember {
-            pushService.getPushHistoryItemsFlow()
+        var showOnlyErrors: Boolean by remember { mutableStateOf(false) }
+        val pushHistory by remember(showOnlyErrors) {
+            pushService.getPushHistoryItemsFlow().map {
+                if (showOnlyErrors) {
+                    it.filter { item -> item.hasBeenResolved.not() }
+                } else {
+                    it
+                }
+            }
        }.collectAsState(emptyList())
+        var resetAction: AsyncAction<Unit> by remember { mutableStateOf(AsyncAction.Uninitialized) }

        fun handleEvents(event: PushHistoryEvents) {
            when (event) {
-                PushHistoryEvents.Reset -> coroutineScope.launch {
-                    pushService.resetPushHistory()
+                is PushHistoryEvents.SetShowOnlyErrors -> {
+                    showOnlyErrors = event.showOnlyErrors
+                }
+                is PushHistoryEvents.Reset -> {
+                    if (event.requiresConfirmation) {
+                        resetAction = AsyncAction.ConfirmingNoParams
+                    } else {
+                        resetAction = AsyncAction.Loading
+                        coroutineScope.launch {
+                            pushService.resetPushHistory()
+                            resetAction = AsyncAction.Uninitialized
+                        }
+                    }
+                }
+                PushHistoryEvents.ClearDialog -> {
+                    resetAction = AsyncAction.Uninitialized
                }
            }
        }
@ -40,6 +66,8 @@ class PushHistoryPresenter @Inject constructor(
        return PushHistoryState(
            pushCounter = pushCounter,
            pushHistoryItems = pushHistory.toImmutableList(),
+            showOnlyErrors = showOnlyErrors,
+            resetAction = resetAction,
            eventSink = ::handleEvents
        )
    }
--- a/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryState.kt
+++ b/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryState.kt
@ -7,11 +7,14 @@

 package io.element.android.libraries.troubleshoot.impl.history

+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.push.api.history.PushHistoryItem
 import kotlinx.collections.immutable.ImmutableList

 data class PushHistoryState(
    val pushCounter: Int,
    val pushHistoryItems: ImmutableList<PushHistoryItem>,
+    val showOnlyErrors: Boolean,
+    val resetAction: AsyncAction<Unit>,
    val eventSink: (PushHistoryEvents) -> Unit,
 )
--- a/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryStateProvider.kt
+++ b/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryStateProvider.kt
@ -8,6 +8,7 @@
 package io.element.android.libraries.troubleshoot.impl.history

 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.core.SessionId
@ -36,16 +37,23 @@ open class PushHistoryStateProvider : PreviewParameterProvider<PushHistoryState>
                    )
                )
            ),
+            aPushHistoryState(
+                resetAction = AsyncAction.ConfirmingNoParams,
+            ),
        )
 }

 fun aPushHistoryState(
    pushCounter: Int = 0,
    pushHistoryItems: List<PushHistoryItem> = emptyList(),
+    showOnlyErrors: Boolean = false,
+    resetAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
    eventSink: (PushHistoryEvents) -> Unit = {},
 ) = PushHistoryState(
    pushCounter = pushCounter,
    pushHistoryItems = pushHistoryItems.toImmutableList(),
+    showOnlyErrors = showOnlyErrors,
+    resetAction = resetAction,
    eventSink = eventSink,
 )

--- a/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryView.kt
+++ b/libraries/troubleshoot/impl/src/main/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryView.kt
@ -24,6 +24,10 @@ import androidx.compose.foundation.lazy.items
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.HorizontalDivider
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.style.TextOverflow
@ -31,16 +35,20 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.compound.tokens.generated.CompoundIcons
+import io.element.android.libraries.designsystem.components.async.AsyncActionView
 import io.element.android.libraries.designsystem.components.button.BackButton
+import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
 import io.element.android.libraries.designsystem.components.list.ListItemContent
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.aliasScreenTitle
+import io.element.android.libraries.designsystem.theme.components.DropdownMenu
+import io.element.android.libraries.designsystem.theme.components.DropdownMenuItem
 import io.element.android.libraries.designsystem.theme.components.Icon
+import io.element.android.libraries.designsystem.theme.components.IconButton
 import io.element.android.libraries.designsystem.theme.components.ListItem
 import io.element.android.libraries.designsystem.theme.components.Scaffold
 import io.element.android.libraries.designsystem.theme.components.Text
-import io.element.android.libraries.designsystem.theme.components.TextButton
 import io.element.android.libraries.designsystem.theme.components.TopAppBar
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
@ -57,6 +65,8 @@ fun PushHistoryView(
    onItemClick: (SessionId, RoomId, EventId) -> Unit,
    modifier: Modifier = Modifier,
 ) {
+    var showMenu by remember { mutableStateOf(false) }
+
    Scaffold(
        modifier = modifier
            .fillMaxSize()
@ -77,12 +87,42 @@ fun PushHistoryView(
                    )
                },
                actions = {
-                    TextButton(
-                        text = stringResource(CommonStrings.action_reset),
-                        onClick = {
-                            state.eventSink(PushHistoryEvents.Reset)
-                        },
-                    )
+                    IconButton(onClick = { showMenu = !showMenu }) {
+                        Icon(
+                            imageVector = CompoundIcons.OverflowVertical(),
+                            contentDescription = stringResource(id = CommonStrings.a11y_user_menu),
+                        )
+                    }
+                    DropdownMenu(
+                        expanded = showMenu,
+                        onDismissRequest = { showMenu = false },
+                    ) {
+                        DropdownMenuItem(
+                            text = { Text("Show only errors") },
+                            trailingIcon = if (state.showOnlyErrors) {
+                                {
+                                    Icon(
+                                        imageVector = CompoundIcons.CheckCircleSolid(),
+                                        contentDescription = null,
+                                        modifier = Modifier.size(16.dp),
+                                    )
+                                }
+                            } else {
+                                null
+                            },
+                            onClick = {
+                                showMenu = false
+                                state.eventSink(PushHistoryEvents.SetShowOnlyErrors(state.showOnlyErrors.not()))
+                            },
+                        )
+                        DropdownMenuItem(
+                            text = { Text(stringResource(id = CommonStrings.action_reset)) },
+                            onClick = {
+                                showMenu = false
+                                state.eventSink(PushHistoryEvents.Reset(requiresConfirmation = true))
+                            },
+                        )
+                    }
                }
            )
        },
@ -95,6 +135,22 @@ fun PushHistoryView(
            onItemClick = onItemClick,
        )
    }
+
+    AsyncActionView(
+        async = state.resetAction,
+        onSuccess = {},
+        confirmationDialog = {
+            ConfirmationDialog(
+                content = "",
+                title = stringResource(CommonStrings.dialog_title_confirmation),
+                submitText = stringResource(CommonStrings.action_reset),
+                cancelText = stringResource(CommonStrings.action_cancel),
+                onSubmitClick = { state.eventSink(PushHistoryEvents.Reset(requiresConfirmation = false)) },
+                onDismiss = { state.eventSink(PushHistoryEvents.ClearDialog) },
+            )
+        },
+        onErrorDismiss = {},
+    )
 }

@Composable
--- a/libraries/troubleshoot/impl/src/test/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryPresenterTest.kt
+++ b/libraries/troubleshoot/impl/src/test/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryPresenterTest.kt
@ -10,12 +10,12 @@
 package io.element.android.libraries.troubleshoot.impl.history

 import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.push.api.PushService
 import io.element.android.libraries.push.test.FakePushService
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.test
 import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.junit.Test

@ -27,6 +27,8 @@ class PushHistoryPresenterTest {
            val initialState = awaitItem()
            assertThat(initialState.pushCounter).isEqualTo(0)
            assertThat(initialState.pushHistoryItems).isEmpty()
+            assertThat(initialState.showOnlyErrors).isFalse()
+            assertThat(initialState.resetAction).isEqualTo(AsyncAction.Uninitialized)
        }
    }

@ -49,7 +51,7 @@ class PushHistoryPresenterTest {
    }

    @Test
-    fun `present - reset`() = runTest {
+    fun `present - reset and cancel`() = runTest {
        val resetPushHistoryResult = lambdaRecorder<Unit> { }
        val pushService = FakePushService(
            resetPushHistoryResult = resetPushHistoryResult,
@ -59,12 +61,64 @@ class PushHistoryPresenterTest {
        )
        presenter.test {
            val initialState = awaitItem()
-            initialState.eventSink(PushHistoryEvents.Reset)
-            runCurrent()
+            initialState.eventSink(PushHistoryEvents.Reset(requiresConfirmation = true))
+            assertThat(awaitItem().resetAction).isEqualTo(AsyncAction.ConfirmingNoParams)
+            initialState.eventSink(PushHistoryEvents.ClearDialog)
+            assertThat(awaitItem().resetAction).isEqualTo(AsyncAction.Uninitialized)
+            resetPushHistoryResult.assertions().isNeverCalled()
+        }
+    }
+
+    @Test
+    fun `present - reset and confirm`() = runTest {
+        val resetPushHistoryResult = lambdaRecorder<Unit> { }
+        val pushService = FakePushService(
+            resetPushHistoryResult = resetPushHistoryResult,
+        )
+        val presenter = createPushHistoryPresenter(
+            pushService = pushService,
+        )
+        presenter.test {
+            val initialState = awaitItem()
+            initialState.eventSink(PushHistoryEvents.Reset(requiresConfirmation = true))
+            assertThat(awaitItem().resetAction).isEqualTo(AsyncAction.ConfirmingNoParams)
+            initialState.eventSink(PushHistoryEvents.Reset(requiresConfirmation = false))
+            assertThat(awaitItem().resetAction).isEqualTo(AsyncAction.Loading)
+            assertThat(awaitItem().resetAction).isEqualTo(AsyncAction.Uninitialized)
            resetPushHistoryResult.assertions().isCalledOnce()
        }
    }

+    @Test
+    fun `present - set show only errors`() = runTest {
+        val pushService = FakePushService()
+        val presenter = createPushHistoryPresenter(
+            pushService = pushService,
+        )
+        presenter.test {
+            val initialState = awaitItem()
+            assertThat(initialState.showOnlyErrors).isFalse()
+            val item = aPushHistoryItem(hasBeenResolved = true)
+            val itemError = aPushHistoryItem(hasBeenResolved = false)
+            pushService.emitPushHistoryItems(listOf(item, itemError))
+            awaitItem().let { state ->
+                assertThat(state.pushHistoryItems).containsExactly(item, itemError)
+                state.eventSink(PushHistoryEvents.SetShowOnlyErrors(showOnlyErrors = true))
+            }
+            skipItems(1)
+            awaitItem().let { state ->
+                assertThat(state.showOnlyErrors).isTrue()
+                assertThat(state.pushHistoryItems).containsExactly(itemError)
+                state.eventSink(PushHistoryEvents.SetShowOnlyErrors(showOnlyErrors = false))
+            }
+            skipItems(1)
+            awaitItem().let { state ->
+                assertThat(state.showOnlyErrors).isFalse()
+                assertThat(state.pushHistoryItems).containsExactly(item, itemError)
+            }
+        }
+    }
+
    private fun createPushHistoryPresenter(
        pushService: PushService = FakePushService(),
    ): PushHistoryPresenter {
--- a/libraries/troubleshoot/impl/src/test/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryViewTest.kt
+++ b/libraries/troubleshoot/impl/src/test/kotlin/io/element/android/libraries/troubleshoot/impl/history/PushHistoryViewTest.kt
@ -10,6 +10,7 @@ package io.element.android.libraries.troubleshoot.impl.history
 import androidx.activity.ComponentActivity
 import androidx.compose.ui.test.junit4.AndroidComposeTestRule
 import androidx.compose.ui.test.junit4.createAndroidComposeRule
+import androidx.compose.ui.test.onNodeWithContentDescription
 import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
 import androidx.test.ext.junit.runners.AndroidJUnit4
@ -46,12 +47,44 @@ class PushHistoryViewTest {
                eventSink = eventsRecorder,
            ),
        )
+        val menuContentDescription = rule.activity.getString(CommonStrings.a11y_user_menu)
+        rule.onNodeWithContentDescription(menuContentDescription).performClick()
        rule.clickOn(CommonStrings.action_reset)
-        eventsRecorder.assertSingle(PushHistoryEvents.Reset)
+        eventsRecorder.assertSingle(PushHistoryEvents.Reset(requiresConfirmation = true))
        // Also check that the push counter is rendered
        rule.onNodeWithText("123").assertExists()
    }

+    @Test
+    fun `clicking on show only errors sends a PushHistoryEvents(true)`() {
+        val eventsRecorder = EventsRecorder<PushHistoryEvents>()
+        rule.setPushHistoryView(
+            aPushHistoryState(
+                showOnlyErrors = false,
+                eventSink = eventsRecorder,
+            ),
+        )
+        val menuContentDescription = rule.activity.getString(CommonStrings.a11y_user_menu)
+        rule.onNodeWithContentDescription(menuContentDescription).performClick()
+        rule.onNodeWithText("Show only errors").performClick()
+        eventsRecorder.assertSingle(PushHistoryEvents.SetShowOnlyErrors(showOnlyErrors = true))
+    }
+
+    @Test
+    fun `clicking on show only errors sends a PushHistoryEvents(false)`() {
+        val eventsRecorder = EventsRecorder<PushHistoryEvents>()
+        rule.setPushHistoryView(
+            aPushHistoryState(
+                showOnlyErrors = true,
+                eventSink = eventsRecorder,
+            ),
+        )
+        val menuContentDescription = rule.activity.getString(CommonStrings.a11y_user_menu)
+        rule.onNodeWithContentDescription(menuContentDescription).performClick()
+        rule.onNodeWithText("Show only errors").performClick()
+        eventsRecorder.assertSingle(PushHistoryEvents.SetShowOnlyErrors(showOnlyErrors = false))
+    }
+
    @Test
    fun `clicking on an invalid event has no effect`() {
        val eventsRecorder = EventsRecorder<PushHistoryEvents>(expectEvents = false)