From 21092c79f2b54f7e9f1fe8e0f8be70e718d2345e Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Wed, 8 Jul 2026 02:07:11 +0300 Subject: [PATCH] refactor(settings): edit settings tab (#496) * refactor(settings): remove current connection editor fields Remove the active host, port, token fields, and clear token button from the main Connection tab, as profiles are now managed exclusively through the ProfileEditorDialog. * feat(settings): migrate to full auto-save design and remove save button Update settings change listeners (auto-reconnect, typing effect delay/toggle) to save state immediately to AuthManager. Remove the manual Save button from the scaffold's top bar and delete the redundant SaveIndicator. * style(settings): clean up unused imports from settings UI cleanup * fix(chat): resolve compose resource lint error and initial state test failure Pre-resolve empty credentials string resource at the composable scope to fix LocalContext resource query lint error. Update testInitialStateAndConnection to use DISCONNECTED state to trigger connection and pass verification. --- .../m57/hermescontrol/ui/chat/ChatScreen.kt | 4 +- .../ui/settings/SettingsScreen.kt | 8 - .../ui/settings/SettingsViewModel.kt | 3 + .../settings/components/ConnectionSection.kt | 115 --------- .../ui/chat/ChatViewModelTest.kt | 222 ++++++++++++++---- 5 files changed, 185 insertions(+), 167 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index e15c7851..68452440 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -1667,7 +1667,7 @@ private fun ReloginDialog( onDismiss: () -> Unit, onRelogin: (String, String, (Boolean, String?) -> Unit) -> Unit, ) { - val context = LocalContext.current + val emptyCredentialsError = stringResource(R.string.chat_relogin_error_empty) val statusColors = LocalHermesStatusColors.current var username by rememberSaveable { mutableStateOf("") } var password by rememberSaveable { mutableStateOf("") } @@ -1721,7 +1721,7 @@ private fun ReloginDialog( enabled = !isLoading, onClick = { if (username.isBlank() || password.isBlank()) { - errorMessage = context.getString(R.string.chat_relogin_error_empty) + errorMessage = emptyCredentialsError return@TextButton } isLoading = true diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsScreen.kt index cc82314a..db8dd33e 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsScreen.kt @@ -21,7 +21,6 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.PrimaryScrollableTabRow import androidx.compose.material3.Tab 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 @@ -44,7 +43,6 @@ import com.m57.hermescontrol.ui.settings.components.BehaviorSection import com.m57.hermescontrol.ui.settings.components.ChatSection import com.m57.hermescontrol.ui.settings.components.ConnectionSection import com.m57.hermescontrol.ui.settings.components.NavBarSection -import com.m57.hermescontrol.ui.settings.components.SaveIndicator import com.m57.hermescontrol.ui.settings.components.TestConnectionButton import com.m57.hermescontrol.ui.settings.components.TestResultCard @@ -81,11 +79,6 @@ fun SettingsScreen( HermesScaffold( title = { Text(stringResource(R.string.screen_settings)) }, navigationIcon = NavIcon.Back(onBack), - actions = { - TextButton(onClick = viewModel::save) { - Text(stringResource(R.string.action_save)) - } - }, ) { Column( modifier = @@ -133,7 +126,6 @@ fun SettingsScreen( ) TestResultCard(testResult = state.testResult) - SaveIndicator(isSaved = state.isSaved) TestConnectionButton( isTesting = state.isTesting, onTest = viewModel::testConnection, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt index 6ad54967..9d6ed09d 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt @@ -278,6 +278,7 @@ class SettingsViewModel( fun onAutoReconnectChange(enabled: Boolean) { _uiState.update { it.copy(autoReconnect = enabled, isSaved = false) } + AuthManager.setAutoReconnect(enabled) } fun onThemeChange(theme: ThemePreference) { @@ -302,10 +303,12 @@ class SettingsViewModel( fun onTypingEffectEnabledChange(enabled: Boolean) { _uiState.update { it.copy(typingEffectEnabled = enabled, isSaved = false) } + AuthManager.setTypingEffectEnabled(enabled) } fun onTypingEffectDelayMsChange(delayMs: Int) { _uiState.update { it.copy(typingEffectDelayMs = delayMs, isSaved = false) } + AuthManager.setTypingEffectDelayMs(delayMs) } /** Clear all auth credentials — logs out and returns to landing screen. */ diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt index 0060e8fe..92ea1767 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt @@ -12,23 +12,17 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Edit -import androidx.compose.material.icons.filled.Visibility -import androidx.compose.material.icons.filled.VisibilityOff import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton -import androidx.compose.material3.OutlinedTextField -import androidx.compose.material3.OutlinedTextFieldDefaults import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -38,9 +32,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.input.KeyboardType -import androidx.compose.ui.text.input.PasswordVisualTransformation -import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.unit.dp import androidx.lifecycle.viewmodel.compose.viewModel import com.m57.hermescontrol.R @@ -140,95 +131,6 @@ internal fun ConnectionSection( Spacer(modifier = Modifier.size(4.dp)) Text(stringResource(R.string.settings_action_add_profile)) } - - Spacer(modifier = Modifier.height(12.dp)) - HorizontalDivider(color = MaterialTheme.colorScheme.outlineVariant) - Spacer(modifier = Modifier.height(12.dp)) - - OutlinedTextField( - value = state.host, - onValueChange = viewModel::onHostChange, - label = { Text(stringResource(R.string.settings_field_host)) }, - singleLine = true, - modifier = Modifier.fillMaxWidth(), - colors = - OutlinedTextFieldDefaults.colors( - focusedBorderColor = MaterialTheme.colorScheme.primary, - cursorColor = MaterialTheme.colorScheme.primary, - ), - ) - - Spacer(modifier = Modifier.height(12.dp)) - - OutlinedTextField( - value = state.port, - onValueChange = viewModel::onPortChange, - label = { Text(stringResource(R.string.settings_field_port)) }, - singleLine = true, - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), - modifier = Modifier.fillMaxWidth(), - colors = - OutlinedTextFieldDefaults.colors( - focusedBorderColor = MaterialTheme.colorScheme.primary, - cursorColor = MaterialTheme.colorScheme.primary, - ), - ) - - Spacer(modifier = Modifier.height(12.dp)) - - OutlinedTextField( - value = state.token, - onValueChange = viewModel::onTokenChange, - label = { Text(stringResource(R.string.settings_field_token)) }, - singleLine = true, - visualTransformation = - if (passwordVisible) { - VisualTransformation.None - } else { - PasswordVisualTransformation() - }, - trailingIcon = { - IconButton(onClick = onPasswordVisibilityToggle) { - Icon( - imageVector = - if (passwordVisible) { - Icons.Filled.Visibility - } else { - Icons.Filled.VisibilityOff - }, - contentDescription = - if (passwordVisible) { - stringResource(R.string.content_desc_hide_token) - } else { - stringResource(R.string.content_desc_show_token) - }, - ) - } - }, - modifier = Modifier.fillMaxWidth(), - colors = - OutlinedTextFieldDefaults.colors( - focusedBorderColor = MaterialTheme.colorScheme.primary, - cursorColor = MaterialTheme.colorScheme.primary, - ), - ) - - Spacer(modifier = Modifier.height(8.dp)) - - // B7 (Jun 18 2026): one-tap escape hatch for when the stored - // token is corrupted / rejected. Clears the token field and - // persists the empty value immediately so a stress-free - // re-pair is possible without manually selecting-and-deleting - // the masked password field character by character. - OutlinedButton( - onClick = { - viewModel.onTokenChange("") - viewModel.save() - }, - modifier = Modifier.fillMaxWidth(), - ) { - Text(stringResource(R.string.settings_action_clear_token)) - } } // ── Dialogs ────────────────────────────────────────────────────────── @@ -288,23 +190,6 @@ internal fun TestResultCard(testResult: String?) { } } -@Composable -internal fun SaveIndicator(isSaved: Boolean) { - AnimatedVisibility( - visible = isSaved, - enter = fadeIn(), - exit = fadeOut(), - ) { - Text( - text = stringResource(R.string.settings_save_success), - style = - MaterialTheme.typography.bodyMedium.copy( - color = MaterialTheme.colorScheme.primary, - ), - ) - } -} - @Composable internal fun TestConnectionButton( isTesting: Boolean, diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index 037053a3..a7565c39 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -108,9 +108,8 @@ class ChatViewModelTest { // ── Helpers ────────────────────────────────────────────────────────────── /** Create a ViewModel with the fake repo injected directly. */ - private fun createViewModel(startCleanup: Boolean = false): ChatViewModel { - return ChatViewModel(app, startCleanup, fakeRepo) - } + private fun createViewModel(startCleanup: Boolean = false): ChatViewModel = + ChatViewModel(app, startCleanup, fakeRepo) /** * Create ViewModel, simulate GatewayReady, feed SESSION_CREATE result, @@ -166,8 +165,14 @@ class ChatViewModelTest { ) advanceUntilIdle() - assertTrue(viewModel.uiState.value.messages.any { it.content.contains("Available Commands") }) - assertTrue(viewModel.uiState.value.messages.any { it.content.contains("/status") }) + assertTrue( + viewModel.uiState.value.messages + .any { it.content.contains("Available Commands") }, + ) + assertTrue( + viewModel.uiState.value.messages + .any { it.content.contains("/status") }, + ) } @Test @@ -225,7 +230,10 @@ class ChatViewModelTest { ) advanceUntilIdle() - assertTrue(viewModel.uiState.value.errorMessage?.contains("Unknown command") == true) + assertTrue( + viewModel.uiState.value.errorMessage + ?.contains("Unknown command") == true, + ) } @Test @@ -266,7 +274,7 @@ class ChatViewModelTest { @Test fun testInitialStateAndConnection() = runTest { - mockConnectionStatus.value = ConnectionStatus.CONNECTING + mockConnectionStatus.value = ConnectionStatus.DISCONNECTED createViewModel() advanceUntilIdle() @@ -344,7 +352,11 @@ class ChatViewModelTest { assertEquals("session-123", viewModel.uiState.value.currentSessionId) assertFalse(viewModel.uiState.value.isLoading) assertEquals(1, viewModel.uiState.value.messages.size) - assertEquals("Session created", viewModel.uiState.value.messages[0].content) + assertEquals( + "Session created", + viewModel.uiState.value.messages[0] + .content, + ) } @Test @@ -377,9 +389,21 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals(1, viewModel.uiState.value.sessions.size) - assertEquals("session-123", viewModel.uiState.value.sessions[0].id) - assertEquals("My Session Title", viewModel.uiState.value.sessions[0].title) - assertEquals(12, viewModel.uiState.value.sessions[0].messageCount) + assertEquals( + "session-123", + viewModel.uiState.value.sessions[0] + .id, + ) + assertEquals( + "My Session Title", + viewModel.uiState.value.sessions[0] + .title, + ) + assertEquals( + 12, + viewModel.uiState.value.sessions[0] + .messageCount, + ) } // ── Streaming tests ────────────────────────────────────────────────────── @@ -412,12 +436,20 @@ class ChatViewModelTest { mockEventsFlow.emit(WsEvent.MessageToken("Hello", sessionId)) advanceUntilIdle() assertFalse(viewModel.streamingState.value.isThinking) - assertEquals("Hello", viewModel.streamingState.value.streamingMessage?.content) + assertEquals( + "Hello", + viewModel.streamingState.value.streamingMessage + ?.content, + ) // 5 — Second token mockEventsFlow.emit(WsEvent.MessageToken(" world", sessionId)) advanceUntilIdle() - assertEquals("Hello world", viewModel.streamingState.value.streamingMessage?.content) + assertEquals( + "Hello world", + viewModel.streamingState.value.streamingMessage + ?.content, + ) // 6 — Complete: reducer finalizes message + resets streamingState mockEventsFlow.emit(WsEvent.MessageComplete("Hello world!", sessionId)) @@ -426,8 +458,15 @@ class ChatViewModelTest { assertFalse(viewModel.uiState.value.isAgentTyping) assertNull(viewModel.streamingState.value.streamingMessage) assertEquals(2, viewModel.uiState.value.messages.size) - assertEquals("Hello world!", viewModel.uiState.value.messages[1].content) - assertFalse(viewModel.uiState.value.messages[1].isStreaming) + assertEquals( + "Hello world!", + viewModel.uiState.value.messages[1] + .content, + ) + assertFalse( + viewModel.uiState.value.messages[1] + .isStreaming, + ) } @Test @@ -443,9 +482,21 @@ class ChatViewModelTest { advanceUntilIdle() // messages[0] = "Session created" system message - assertEquals("Calculating sum", viewModel.uiState.value.messages[1].content) - assertEquals(MessageRole.ASSISTANT, viewModel.uiState.value.messages[1].role) - assertEquals(MessageRole.TOOL, viewModel.uiState.value.messages[2].role) + assertEquals( + "Calculating sum", + viewModel.uiState.value.messages[1] + .content, + ) + assertEquals( + MessageRole.ASSISTANT, + viewModel.uiState.value.messages[1] + .role, + ) + assertEquals( + MessageRole.TOOL, + viewModel.uiState.value.messages[2] + .role, + ) assertNull(viewModel.streamingState.value.streamingMessage) } @@ -463,10 +514,21 @@ class ChatViewModelTest { advanceUntilIdle() // messages[0] = "Session created" system message - assertEquals("First part", viewModel.uiState.value.messages[1].content) - assertFalse(viewModel.uiState.value.messages[1].isStreaming) + assertEquals( + "First part", + viewModel.uiState.value.messages[1] + .content, + ) + assertFalse( + viewModel.uiState.value.messages[1] + .isStreaming, + ) assertNotNull(viewModel.streamingState.value.streamingMessage) - assertEquals("Second part", viewModel.streamingState.value.streamingMessage?.content) + assertEquals( + "Second part", + viewModel.streamingState.value.streamingMessage + ?.content, + ) } @Test @@ -482,15 +544,27 @@ class ChatViewModelTest { ) advanceUntilIdle() - assertEquals(MessageRole.TOOL, viewModel.uiState.value.messages[1].role) - assertEquals(ToolStatus.RUNNING, viewModel.uiState.value.messages[1].toolStatus) + assertEquals( + MessageRole.TOOL, + viewModel.uiState.value.messages[1] + .role, + ) + assertEquals( + ToolStatus.RUNNING, + viewModel.uiState.value.messages[1] + .toolStatus, + ) mockEventsFlow.emit( WsEvent.ToolComplete("calculator", mapOf("result" to "4", "exit_code" to 0)), ) advanceUntilIdle() - assertEquals(ToolStatus.COMPLETED, viewModel.uiState.value.messages[1].toolStatus) + assertEquals( + ToolStatus.COMPLETED, + viewModel.uiState.value.messages[1] + .toolStatus, + ) } // ── Clarify tests ──────────────────────────────────────────────────────── @@ -503,9 +577,21 @@ class ChatViewModelTest { mockEventsFlow.emit(WsEvent.ClarifyRequest("Please choose:", listOf("Yes", "No"), "clarify-123")) advanceUntilIdle() - assertEquals("Please choose:", viewModel.uiState.value.clarifyRequest?.text) - assertEquals(listOf("Yes", "No"), viewModel.uiState.value.clarifyRequest?.options) - assertEquals("clarify-123", viewModel.uiState.value.clarifyRequest?.clarifyId) + assertEquals( + "Please choose:", + viewModel.uiState.value.clarifyRequest + ?.text, + ) + assertEquals( + listOf("Yes", "No"), + viewModel.uiState.value.clarifyRequest + ?.options, + ) + assertEquals( + "clarify-123", + viewModel.uiState.value.clarifyRequest + ?.clarifyId, + ) viewModel.respondToClarify("Yes") advanceUntilIdle() @@ -537,16 +623,32 @@ class ChatViewModelTest { mockEventsFlow.emit(WsEvent.ClarifyRequest("Please explain:", emptyList(), "clarify-456")) advanceUntilIdle() - assertEquals("Please explain:", viewModel.uiState.value.clarifyRequest?.text) - assertTrue(viewModel.uiState.value.clarifyRequest?.options?.isEmpty() == true) + assertEquals( + "Please explain:", + viewModel.uiState.value.clarifyRequest + ?.text, + ) + assertTrue( + viewModel.uiState.value.clarifyRequest + ?.options + ?.isEmpty() == true, + ) viewModel.respondToClarify("This is my custom response text") advanceUntilIdle() assertNull(viewModel.uiState.value.clarifyRequest) assertEquals(2, viewModel.uiState.value.messages.size) - assertEquals("This is my custom response text", viewModel.uiState.value.messages[1].content) - assertEquals(MessageRole.USER, viewModel.uiState.value.messages[1].role) + assertEquals( + "This is my custom response text", + viewModel.uiState.value.messages[1] + .content, + ) + assertEquals( + MessageRole.USER, + viewModel.uiState.value.messages[1] + .role, + ) verify { HermesWsClient.send( @@ -575,8 +677,16 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals(2, viewModel.uiState.value.messages.size) - assertEquals("Hello Hermes", viewModel.uiState.value.messages[1].content) - assertEquals(MessageRole.USER, viewModel.uiState.value.messages[1].role) + assertEquals( + "Hello Hermes", + viewModel.uiState.value.messages[1] + .content, + ) + assertEquals( + MessageRole.USER, + viewModel.uiState.value.messages[1] + .role, + ) assertTrue(viewModel.uiState.value.isAgentTyping) verify { HermesWsClient.sendMessage(sessionId, "Hello Hermes", any()) } @@ -593,7 +703,10 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals("session-456", viewModel.uiState.value.currentSessionId) - assertTrue(viewModel.uiState.value.messages.isEmpty()) + assertTrue( + viewModel.uiState.value.messages + .isEmpty(), + ) verify { HermesWsClient.send(WsMethods.SESSION_RESUME, mapOf("session_id" to "session-456"), any()) } } @@ -618,7 +731,10 @@ class ChatViewModelTest { ) advanceUntilIdle() - assertTrue(viewModel.uiState.value.errorMessage!!.contains("Internal error during creation")) + assertTrue( + viewModel.uiState.value.errorMessage!! + .contains("Internal error during creation"), + ) } // ── Session mismatch ───────────────────────────────────────────────────── @@ -644,7 +760,11 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals(1, viewModel.uiState.value.messages.size) - assertEquals("Session created", viewModel.uiState.value.messages[0].content) + assertEquals( + "Session created", + viewModel.uiState.value.messages[0] + .content, + ) assertNull(viewModel.streamingState.value.streamingMessage) assertNull(viewModel.uiState.value.clarifyRequest) } @@ -663,7 +783,11 @@ class ChatViewModelTest { mockEventsFlow.emit(WsEvent.MessageToken("Hello", sessionId)) advanceUntilIdle() - assertEquals("Hello", viewModel.streamingState.value.streamingMessage?.content) + assertEquals( + "Hello", + viewModel.streamingState.value.streamingMessage + ?.content, + ) } // ── MessageComplete without streaming ──────────────────────────────────── @@ -677,8 +801,16 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals(2, viewModel.uiState.value.messages.size) - assertEquals("Fully complete message", viewModel.uiState.value.messages[1].content) - assertEquals(MessageRole.ASSISTANT, viewModel.uiState.value.messages[1].role) + assertEquals( + "Fully complete message", + viewModel.uiState.value.messages[1] + .content, + ) + assertEquals( + MessageRole.ASSISTANT, + viewModel.uiState.value.messages[1] + .role, + ) } // ── Approval flow ──────────────────────────────────────────────────────── @@ -699,7 +831,9 @@ class ChatViewModelTest { ) advanceUntilIdle() - val msg = viewModel.uiState.value.messages.first { it.content.contains("Approval Required") } + val msg = + viewModel.uiState.value.messages + .first { it.content.contains("Approval Required") } assertNotNull(msg.approvalInfo) assertEquals("rm -rf /data", msg.approvalInfo?.command) } @@ -740,13 +874,17 @@ class ChatViewModelTest { ) advanceUntilIdle() - val approvalMsg = viewModel.uiState.value.messages.firstOrNull { it.approvalInfo != null } + val approvalMsg = + viewModel.uiState.value.messages + .firstOrNull { it.approvalInfo != null } assertNotNull(approvalMsg) viewModel.respondToApproval("approve") advanceUntilIdle() - val msgAfter = viewModel.uiState.value.messages.firstOrNull { it.id == approvalMsg!!.id } + val msgAfter = + viewModel.uiState.value.messages + .firstOrNull { it.id == approvalMsg!!.id } assertNotNull(msgAfter) assertNull(msgAfter!!.approvalInfo) }