From 96e43f354f3400aa406efd20f90cad37450aff80 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:16:34 +0300 Subject: [PATCH] feat(system): update progress popup with live action log (issue #863) (#872) Co-authored-by: M57 --- .../ui/common/ActionProgressController.kt | 162 ++++++++++++++ .../ui/common/ActionProgressDialog.kt | 180 +++++++++++++++ .../hermescontrol/ui/system/SystemScreen.kt | 9 + .../ui/system/SystemViewModel.kt | 46 +++- app/src/main/res/values/strings.xml | 6 + .../ui/common/ActionProgressControllerTest.kt | 206 ++++++++++++++++++ 6 files changed, 605 insertions(+), 4 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressController.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressDialog.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/common/ActionProgressControllerTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressController.kt b/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressController.kt new file mode 100644 index 00000000..7f2bd7c9 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressController.kt @@ -0,0 +1,162 @@ +package com.m57.hermescontrol.ui.common + +import com.m57.hermescontrol.data.model.ActionStatusResponse +import com.m57.hermescontrol.data.remote.ApiClient +import com.m57.hermescontrol.data.remote.NetworkResult +import com.m57.hermescontrol.data.remote.safeApiCall +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch + +/** Lifecycle of a backend action tracked through [ActionProgressController]. */ +enum class ActionProgressPhase { + /** The trigger request is in flight (dialog shown, nothing to poll yet). */ + STARTING, + + /** The action is running; the controller is polling its status log. */ + RUNNING, + + /** The action exited with code 0. */ + SUCCEEDED, + + /** The action exited non-zero, the trigger was rejected, or tracking was lost. */ + FAILED, +} + +/** + * UI state backing [ActionProgressDialog]. Driven exclusively by + * [ActionProgressController]; screens read it via `controller.state`. + */ +data class ActionProgressState( + val visible: Boolean = false, + val phase: ActionProgressPhase = ActionProgressPhase.STARTING, + val actionName: String? = null, + val lines: List = emptyList(), + val exitCode: Int? = null, + val error: String? = null, +) + +/** + * Reusable tracker for backend actions spawned through the action API. + * + * The backend pattern is: the trigger POST (`/api/.../action`) returns + * immediately with `{ok, name, ...}` and the work runs in the background; + * `GET /api/actions/{name}/status?lines=N` exposes a live log tail + * (`{running, exit_code, lines}`). This controller opens the dialog, polls + * the status every [pollIntervalMs] while it runs, and settles on + * [ActionProgressPhase.SUCCEEDED] / [ActionProgressPhase.FAILED] when the + * action exits. [onFinished] fires once with the final status so the host + * can refresh whatever the action mutated. + * + * Cancel-safe: [dismiss] and [open]/[markStarted] re-entry cancel the poll + * job, and the host's [CoroutineScope] (e.g. `viewModelScope`) stops the + * loop when the screen leaves composition. The backend action itself keeps + * running — dismissing only stops tracking it. + * + * Host a dialog with [ActionProgressDialog] off [state]. + */ +class ActionProgressController( + private val scope: CoroutineScope, + private val pollIntervalMs: Long = 1_500L, + private val onFinished: ((ActionStatusResponse?) -> Unit)? = null, +) { + private val _state = MutableStateFlow(ActionProgressState()) + val state: StateFlow = _state.asStateFlow() + + private var pollJob: Job? = null + + /** Show the dialog (starting phase) before firing the trigger request. */ + fun open() { + pollJob?.cancel() + _state.value = ActionProgressState(visible = true) + } + + /** + * Trigger accepted; start polling the named action's status log. + * + * @param actionName action name returned by the trigger response. + */ + fun markStarted(actionName: String) { + pollJob?.cancel() + _state.update { + it.copy( + phase = ActionProgressPhase.RUNNING, + actionName = actionName, + lines = emptyList(), + exitCode = null, + error = null, + ) + } + pollJob = + scope.launch { + var consecutiveFailures = 0 + while (isActive) { + delay(pollIntervalMs) + when ( + val result = + safeApiCall { ApiClient.hermesApi.getActionStatus(actionName) } + ) { + is NetworkResult.Success -> { + consecutiveFailures = 0 + val status = result.data + val done = status.running != true + _state.update { + it.copy( + lines = status.lines ?: it.lines, + exitCode = status.exit_code, + phase = + when { + !done -> ActionProgressPhase.RUNNING + status.exit_code == 0 -> ActionProgressPhase.SUCCEEDED + else -> ActionProgressPhase.FAILED + }, + ) + } + if (done) { + onFinished?.invoke(status) + break + } + } + + is NetworkResult.Failure -> { + // Transient poll failures are ignored (the action may + // briefly restart the gateway); give up after a run of + // consecutive failures so the dialog can't spin forever. + consecutiveFailures++ + if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { + _state.update { + it.copy( + phase = ActionProgressPhase.FAILED, + error = result.error.message, + ) + } + break + } + } + } + } + } + } + + /** Trigger was rejected — surface the error in the dialog. */ + fun fail(error: String) { + pollJob?.cancel() + _state.update { it.copy(phase = ActionProgressPhase.FAILED, error = error) } + } + + /** Stop tracking and hide the dialog. Safe mid-run (action keeps going). */ + fun dismiss() { + pollJob?.cancel() + _state.value = ActionProgressState() + } + + companion object { + private const val MAX_CONSECUTIVE_FAILURES = 5 + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressDialog.kt b/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressDialog.kt new file mode 100644 index 00000000..581027f4 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/common/ActionProgressDialog.kt @@ -0,0 +1,180 @@ +package com.m57.hermescontrol.ui.common + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.heightIn +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.CheckCircle +import androidx.compose.material.icons.filled.Error +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.dp +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.m57.hermescontrol.R +import com.m57.hermescontrol.theme.LocalHermesStatusColors +import com.m57.hermescontrol.theme.LocalSpacing + +/** + * Reusable progress popup for backend actions tracked by + * [ActionProgressController]: spinner while running, the live log tail + * (monospace, auto-scrolled to the newest lines), then a success or failure + * state with the exit code when the action finishes. + * + * Dismissing is always allowed and is cancel-safe — it stops tracking via + * [ActionProgressController.dismiss]; the backend action keeps running. + * + * @param controller the tracker backing this dialog (e.g. owned by a + * ViewModel with `viewModelScope`). + * @param title short human title for the action being tracked, e.g. + * "Software update". + * @param onDismiss optional override; defaults to [ActionProgressController.dismiss]. + */ +@Composable +fun ActionProgressDialog( + controller: ActionProgressController, + title: String, + onDismiss: (() -> Unit)? = null, +) { + val state by controller.state.collectAsStateWithLifecycle() + if (!state.visible) return + + val dismiss = onDismiss ?: controller::dismiss + val spacing = LocalSpacing.current + val statusColors = LocalHermesStatusColors.current + + AlertDialog( + onDismissRequest = dismiss, + title = { Text(title) }, + text = { + Column(verticalArrangement = Arrangement.spacedBy(spacing.sm)) { + // Status row: spinner / success / failure + phase text + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(spacing.sm), + ) { + when (state.phase) { + ActionProgressPhase.STARTING, + ActionProgressPhase.RUNNING, + -> { + CircularProgressIndicator( + modifier = Modifier.size(18.dp), + strokeWidth = 2.dp, + ) + } + + ActionProgressPhase.SUCCEEDED -> { + Icon( + Icons.Filled.CheckCircle, + contentDescription = null, + modifier = Modifier.size(20.dp), + tint = statusColors.success, + ) + } + + ActionProgressPhase.FAILED -> { + Icon( + Icons.Filled.Error, + contentDescription = null, + modifier = Modifier.size(20.dp), + tint = statusColors.error, + ) + } + } + // Delegated property — capture locally so the smart cast works. + val exitCode = state.exitCode + Text( + text = + when (state.phase) { + ActionProgressPhase.STARTING -> + stringResource(R.string.action_progress_starting) + + ActionProgressPhase.RUNNING -> + stringResource(R.string.action_progress_running) + + ActionProgressPhase.SUCCEEDED -> + stringResource(R.string.action_progress_succeeded) + + ActionProgressPhase.FAILED -> + if (exitCode != null) { + stringResource(R.string.action_progress_failed_exit, exitCode) + } else { + stringResource(R.string.action_progress_failed) + } + }, + style = MaterialTheme.typography.bodyMedium, + fontWeight = FontWeight.SemiBold, + ) + } + + // Live log tail (newest lines; backend returns the last `lines=N`) + if (state.lines.isNotEmpty()) { + val scrollState = rememberScrollState() + Card( + modifier = Modifier.fillMaxWidth(), + colors = + CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.surfaceVariant, + ), + ) { + Column( + modifier = + Modifier + .fillMaxWidth() + .heightIn(max = 220.dp) + .verticalScroll(scrollState) + .padding(spacing.sm), + verticalArrangement = Arrangement.spacedBy(2.dp), + ) { + state.lines.takeLast(15).forEach { line -> + Text( + text = line, + style = MaterialTheme.typography.bodySmall, + fontFamily = FontFamily.Monospace, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } + // Keep the newest lines in view as the tail grows. + LaunchedEffect(state.lines.size) { + scrollState.scrollTo(scrollState.maxValue) + } + } + + state.error?.let { error -> + Text( + text = error, + style = MaterialTheme.typography.bodySmall, + color = statusColors.error, + ) + } + } + }, + confirmButton = { + TextButton(onClick = dismiss) { + Text(stringResource(R.string.action_done)) + } + }, + ) +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemScreen.kt index 5caa3a53..482a508b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemScreen.kt @@ -81,6 +81,7 @@ import com.m57.hermescontrol.R import com.m57.hermescontrol.theme.LocalHermesStatusColors import com.m57.hermescontrol.theme.LocalSpacing import com.m57.hermescontrol.ui.chat.MediaImageStore +import com.m57.hermescontrol.ui.common.ActionProgressDialog import com.m57.hermescontrol.ui.common.EmptyState import com.m57.hermescontrol.ui.common.ErrorState import com.m57.hermescontrol.ui.common.HermesScaffold @@ -192,6 +193,14 @@ fun SystemScreen( ) } + // Update progress popup (issue #863): replaces the old bottom-of-screen + // action log for the update flow — spinner + log tail while running, + // success/failure state once it exits. + ActionProgressDialog( + controller = viewModel.actionProgress, + title = stringResource(R.string.system_update_progress_title), + ) + // Credential remove confirmation var credToRemove by remember { mutableStateOf?>(null) } if (credToRemove != null) { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt index f60b0354..afcd40e4 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/system/SystemViewModel.kt @@ -21,6 +21,7 @@ import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.NetworkError import com.m57.hermescontrol.data.remote.NetworkResult import com.m57.hermescontrol.data.remote.safeApiCall +import com.m57.hermescontrol.ui.common.ActionProgressController import com.m57.hermescontrol.ui.common.ToastHost import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -97,6 +98,18 @@ class SystemViewModel : private val _uiState = MutableStateFlow(SystemUiState()) val uiState: StateFlow = _uiState.asStateFlow() + /** + * Progress popup for the update flow (issue #863): after the trigger POST + * returns, [ActionProgressController] polls the action status log until + * the update exits, then refreshes the whole screen so the version row + * and update badge reflect the new build. + */ + val actionProgress = + ActionProgressController( + scope = viewModelScope, + onFinished = { loadAll() }, + ) + private var actionPollingJob: Job? = null private var downloadJob: Job? = null @@ -266,11 +279,36 @@ class SystemViewModel : } } + /** + * Run the update and surface its progress in [ActionProgressController]'s + * popup: the trigger POST returns immediately (`{ok, name}`) while the + * actual update runs in the background, so the dialog polls the action + * status log until it exits (success/failure + tail shown to the user). + */ fun applyUpdate() { - runOperation( - apiCall = { safeApiCall { ApiClient.hermesApi.updateHermes() } }, - label = "Update", - ) + actionProgress.open() + viewModelScope.launch { + val result = + withContext(Dispatchers.IO) { + safeApiCall { ApiClient.hermesApi.updateHermes() } + } + when (result) { + is NetworkResult.Success -> { + val name = result.data.name + if (name != null) { + actionProgress.markStarted(name) + } else { + actionProgress.fail( + "Update started but the backend did not report an action name", + ) + } + } + + is NetworkResult.Failure -> { + actionProgress.fail("Failed to start update: ${result.error.message}") + } + } + } } fun openUpdateConfirm() { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a5a39df8..58eea04f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -26,6 +26,11 @@ Confirm Clear OK + Starting… + Running… + Completed + Failed + Failed (exit %1$d) Delete @@ -354,6 +359,7 @@ Update now Update Hermes? This will run hermes update and restart the gateway. + Software update Nous Portal diff --git a/app/src/test/java/com/m57/hermescontrol/ui/common/ActionProgressControllerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/common/ActionProgressControllerTest.kt new file mode 100644 index 00000000..72512fa8 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/common/ActionProgressControllerTest.kt @@ -0,0 +1,206 @@ +package com.m57.hermescontrol.ui.common + +import com.m57.hermescontrol.data.model.ActionStatusResponse +import com.m57.hermescontrol.data.remote.ApiClient +import com.m57.hermescontrol.data.remote.HermesApiService +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkAll +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import retrofit2.Response + +/** + * NOTE: no mockkStatic(Dispatchers) here — a static Dispatchers mock bleeds + * into later test classes in the same JVM. Real Dispatchers are fine: the + * network layer is mocked, so the poll loop is driven by the test dispatcher + * only (the controller calls the suspend API directly, no IO hops). + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ActionProgressControllerTest { + private val dispatcher = StandardTestDispatcher() + private val scope = TestScope(dispatcher) + private lateinit var mockApi: HermesApiService + private var finishedExitCode: Int? = null + + @Before + fun setup() { + mockkObject(ApiClient) + mockApi = mockk(relaxed = true) + every { ApiClient.hermesApi } returns mockApi + finishedExitCode = null + } + + @After + fun teardown() { + unmockkAll() + } + + private fun controller(onFinished: ((com.m57.hermescontrol.data.model.ActionStatusResponse?) -> Unit)? = null) = + ActionProgressController( + scope = scope, + onFinished = onFinished ?: { finishedExitCode = it?.exit_code }, + ) + + private fun tick() { + scope.advanceTimeBy(1_500) + scope.runCurrent() + } + + @Test + fun `open shows the dialog in starting phase`() { + val controller = controller() + + controller.open() + + val state = controller.state.value + assertTrue(state.visible) + assertEquals(ActionProgressPhase.STARTING, state.phase) + assertNull(state.actionName) + assertTrue(state.lines.isEmpty()) + } + + @Test + fun `markStarted polls the status log until exit and reports success`() { + val controller = controller() + coEvery { mockApi.getActionStatus("hermes-update") } returnsMany + listOf( + Response.success( + ActionStatusResponse(name = "hermes-update", running = true, lines = listOf("l1")), + ), + Response.success( + ActionStatusResponse( + name = "hermes-update", + running = true, + lines = listOf("l1", "l2"), + ), + ), + Response.success( + ActionStatusResponse( + name = "hermes-update", + running = false, + exit_code = 0, + lines = listOf("l1", "l2", "l3"), + ), + ), + ) + + controller.open() + controller.markStarted("hermes-update") + + assertEquals(ActionProgressPhase.RUNNING, controller.state.value.phase) + assertEquals("hermes-update", controller.state.value.actionName) + + tick() + assertEquals(ActionProgressPhase.RUNNING, controller.state.value.phase) + assertEquals(listOf("l1"), controller.state.value.lines) + + tick() + assertEquals(listOf("l1", "l2"), controller.state.value.lines) + + tick() + val state = controller.state.value + assertEquals(ActionProgressPhase.SUCCEEDED, state.phase) + assertEquals(0, state.exitCode) + assertTrue(state.visible) + assertEquals(0, finishedExitCode) + } + + @Test + fun `non-zero exit reports failure with the exit code`() { + val controller = controller() + coEvery { mockApi.getActionStatus("hermes-update") } returns + Response.success( + ActionStatusResponse(name = "hermes-update", running = false, exit_code = 7), + ) + + controller.open() + controller.markStarted("hermes-update") + tick() + + val state = controller.state.value + assertEquals(ActionProgressPhase.FAILED, state.phase) + assertEquals(7, state.exitCode) + assertEquals(7, finishedExitCode) + } + + @Test + fun `fail surfaces the trigger error in the dialog`() { + val controller = controller() + + controller.open() + controller.fail("Failed to start update: boom") + + val state = controller.state.value + assertTrue(state.visible) + assertEquals(ActionProgressPhase.FAILED, state.phase) + assertEquals("Failed to start update: boom", state.error) + } + + @Test + fun `repeated poll failures settle on failed with the error message`() { + val controller = controller() + // 404 is not retried by safeApiCall, so each tick is exactly one poll + // iteration; 5 consecutive failures end the loop. + coEvery { mockApi.getActionStatus("hermes-update") } returns + Response.error(404, "not found".toResponseBody()) + + controller.open() + controller.markStarted("hermes-update") + repeat(5) { tick() } + + val state = controller.state.value + assertEquals(ActionProgressPhase.FAILED, state.phase) + assertTrue(state.error != null) + assertNull(finishedExitCode) + } + + @Test + fun `dismiss stops polling and hides the dialog`() { + val controller = controller() + var calls = 0 + coEvery { mockApi.getActionStatus("hermes-update") } answers { + calls++ + Response.success(ActionStatusResponse(name = "hermes-update", running = true)) + } + + controller.open() + controller.markStarted("hermes-update") + tick() + val callsAfterFirstPoll = calls + + controller.dismiss() + assertFalse(controller.state.value.visible) + + repeat(3) { tick() } + assertEquals(callsAfterFirstPoll, calls) + } + + @Test + fun `onFinished is not called when dismissed mid-run`() { + val controller = controller() + coEvery { mockApi.getActionStatus("hermes-update") } returns + Response.success(ActionStatusResponse(name = "hermes-update", running = true)) + + controller.open() + controller.markStarted("hermes-update") + tick() + controller.dismiss() + repeat(3) { tick() } + + assertNull(finishedExitCode) + } +}