c4151ca · opencode-android

git.iko.soy/opencode-android

Make Connect the single primary action on the server edit screen, folding the connection test into its probe and dropping the standalone Test connection and Save without connecting buttons

c4151caiko3 months ago

app/src/main/java/soy/iko/opencode/ui/server/ServerEditScreen.kt+2−71

compare with master

unifiedsplit

@@ -262,19 +262,8 @@ private fun ServerEditForm(

262 onImeDone = { if (canSave && !state.saving) vm.connect(onDone) },

263 )

264 }

265− // Pre-save reachability probe: lets the user confirm the URL is reachable before

266− // committing. Extracted to [TestConnectionControl] to keep this function's complexity

267− // under the detekt threshold.

268− TestConnectionControl(

269− canSave = canSave,

270− testing = state.testingConnection,

271− saving = state.saving,

272− testingCredentials = state.testingCredentials,

273− result = state.connectionResult,

274− onTest = vm::testConnection,

275− )

276− // Single primary action: probe-then-save-and-connect. Replaces the former

265+ // Single primary action: probe-then-save-and-connect. The probe IS the connection

277− // Check-connectivity / Save / Save-&-connect trio.

266+ // test — a failed probe keeps the user on this screen with the error shown below.

278 Button(

267 Button(

279 onClick = {

268 onClick = {

280 haptics.performHapticFeedback(androidx.compose.ui.hapticfeedback.HapticFeedbackType.LongPress)

269 haptics.performHapticFeedback(androidx.compose.ui.hapticfeedback.HapticFeedbackType.LongPress)

@@ -304,17 +293,6 @@ private fun ServerEditForm(

304 modifier = Modifier.fillMaxWidth(),

293 modifier = Modifier.fillMaxWidth(),

305 )

294 )

306 }

295 }

307− // Secondary, low-emphasis: persist without connecting (offline setup).

308− TextButton(

309− onClick = {

310− haptics.performHapticFeedback(androidx.compose.ui.hapticfeedback.HapticFeedbackType.LongPress)

311− vm.save(onDone)

312− },

313− enabled = canSave && !state.saving,

314− modifier = Modifier.fillMaxWidth().testTag("server_save"),

315− ) {

316− Text(stringResource(R.string.save_without_connecting))

317− }

318 // De-emphasized optional label at the bottom: displayLabel already falls back to the

296 // De-emphasized optional label at the bottom: displayLabel already falls back to the

319 // URL, so most servers don't need one.

297 // URL, so most servers don't need one.

320 OutlinedTextField(

298 OutlinedTextField(

@@ -421,53 +399,6 @@ private fun AuthFields(

421 }

399 }

422}

400}

423

401

424−/** The "Test connection" button plus its success/failure feedback line. Extracted from

425− * [ServerEditForm] to keep that function's cyclomatic complexity under the detekt threshold. */

426−@Composable

427−private fun TestConnectionControl(

428− canSave: Boolean,

429− testing: Boolean,

430− saving: Boolean,

431− testingCredentials: Boolean,

432− result: ConnectionProbeResult?,

433− onTest: () -> Unit,

434−) {

435− OutlinedButton(

436− onClick = onTest,

437− enabled = canSave && !testing && !saving && !testingCredentials,

438− modifier = Modifier.fillMaxWidth().testTag("server_test_connection"),

439− ) {

440− if (testing) {

441− CircularProgressIndicator(

442− modifier = Modifier.size(20.dp),

443− strokeWidth = 2.dp,

444− )

445− Text(stringResource(R.string.testing_connection), modifier = Modifier.padding(start = 8.dp))

446− } else {

447− Text(stringResource(R.string.test_connection))

448− }

449− }

450− ConnectionProbeFeedback(result)

451−}

452−

453−/** Renders the success/failed feedback line for a "Test connection" probe. Extracted from

454− * [ServerEditForm] to keep that function's cyclomatic complexity under the detekt threshold. */

455−@Composable

456−private fun ConnectionProbeFeedback(result: ConnectionProbeResult?) {

457− if (result == null) return

458− val (msg, color) = when (result) {

459− is ConnectionProbeResult.Success ->

460− stringResource(R.string.test_connection_success, result.latencyMs) to MaterialTheme.colorScheme.primary

461− is ConnectionProbeResult.Failed ->

462− stringResource(R.string.test_connection_failed, result.message) to MaterialTheme.colorScheme.error

463− }

464− Text(

465− msg,

466− style = MaterialTheme.typography.bodySmall,

467− color = color,

468− )

469−}

470−

471/**

402/**

472 * Optional TLS hardening, tucked behind a collapsed "Security" expander so the common

403 * Optional TLS hardening, tucked behind a collapsed "Security" expander so the common

473 * localhost/HTTP setup never sees it. Auto-expands when a loaded profile already uses one of

404 * localhost/HTTP setup never sees it. Auto-expands when a loaded profile already uses one of

app/src/main/java/soy/iko/opencode/ui/server/ServerEditViewModel.kt+1−56

@@ -41,10 +41,6 @@ data class ServerEditState(

41 val testingCredentials: Boolean = false,

42 /** Non-null when the last credential test succeeded (true) or failed (false). */

43 val credentialsResult: Boolean? = null,

44− /** Whether a reachability probe (Test connection) is in progress. */

45− val testingConnection: Boolean = false,

46− /** Latency (ms) of the last successful reachability probe, or an error message. */

47− val connectionResult: ConnectionProbeResult? = null,

48) {

44) {

49 val canSave: Boolean get() = baseUrl.isNotBlank() && isValidUrl(baseUrl) && certPinValid

45 val canSave: Boolean get() = baseUrl.isNotBlank() && isValidUrl(baseUrl) && certPinValid

50 val isNew: Boolean get() = id == null

46 val isNew: Boolean get() = id == null

@@ -63,13 +59,6 @@ data class ServerEditState(

63 }

59 }

64}

60}

66−/** Result of a "Test connection" reachability probe from the edit screen. */

67−@Immutable

68−sealed interface ConnectionProbeResult {

69− data class Success(val latencyMs: Long) : ConnectionProbeResult

70− data class Failed(val message: String) : ConnectionProbeResult

71−}

72−

73/** Snapshot of the profile loaded into the editor, normalized the same way save() stores it. */

62/** Snapshot of the profile loaded into the editor, normalized the same way save() stores it. */

74data class InitialProfile(

63data class InitialProfile(

75 val label: String,

64 val label: String,

@@ -337,50 +326,6 @@ class ServerEditViewModel(

337 }

326 }

338 }

327 }

339

328

340− /** Secondary action: persist the profile without connecting (e.g. setting up offline). */

341− fun save(onDone: () -> Unit) {

342− saveInternal(connectAfter = false, onDone = onDone)

343− }

344−

345− /**

346− * Probe the server's reachability *without* requiring credentials (a pre-save "is this URL

347− * reachable?" check). Surfaces [ServerEditState.connectionResult] so the UI can show

348− * success/latency or a failure without dismissing the form. Complements [testCredentials],

349− * which validates auth once the URL is known to be reachable.

350− */

351− fun testConnection() {

352− val s = _state.value

353− if (!s.canSave || s.testingConnection) return

354− _state.update { it.copy(testingConnection = true, connectionResult = null, error = null) }

355− val probedUrl = normalizeForSave(s.baseUrl)

356− val probedRequireHttps = s.requireHttps

357− val probedCertPin = s.certPin.trim().takeIf { it.isNotBlank() }

358− viewModelScope.launch {

359− val start = System.currentTimeMillis()

360− val result = runCatchingCancellable {

361− withTimeoutOrNull(NetworkConfig.testCredentialsTimeoutMs) {

362− container.probeServer(probedUrl, probedRequireHttps, probedCertPin)

363− } ?: throw java.net.SocketTimeoutException("Probe timed out")

364− }

365− val latency = System.currentTimeMillis() - start

366− _state.update {

367− if (normalizeForSave(it.baseUrl) != probedUrl) return@update it.copy(testingConnection = false)

368− val verdict = result.fold(

369− onSuccess = { probe ->

370− when (probe) {

371− ProbeResult.Reachable, is ProbeResult.NeedsAuth ->

372− ConnectionProbeResult.Success(latency)

373− is ProbeResult.Unreachable ->

374− ConnectionProbeResult.Failed(probe.error)

375− }

376− },

377− onFailure = { e -> ConnectionProbeResult.Failed(container.friendlyError(e)) },

378− )

379− it.copy(testingConnection = false, connectionResult = verdict)

380− }

381− }

382− }

383−

384 /**

329 /**

385 * Delete the profile being edited (only valid for an existing profile, not a new one).

330 * Delete the profile being edited (only valid for an existing profile, not a new one).

386 * Fires [onDone] on success so the screen pops back to the list. The active profile can't

331 * Fires [onDone] on success so the screen pops back to the list. The active profile can't

@@ -389,7 +334,7 @@ class ServerEditViewModel(

389 fun delete(onDone: () -> Unit) {

334 fun delete(onDone: () -> Unit) {

390 val s = _state.value

335 val s = _state.value

391 val id = s.id ?: return

336 val id = s.id ?: return

392− if (s.saving || s.testingConnection || s.testingCredentials) return

337+ if (s.saving || s.testingCredentials) return

393 _state.update { it.copy(saving = true, error = null) }

338 _state.update { it.copy(saving = true, error = null) }

394 viewModelScope.launch {

339 viewModelScope.launch {

395 val result = runCatchingCancellable { container.profileStore.delete(id) }

340 val result = runCatchingCancellable { container.profileStore.delete(id) }

app/src/main/res/values-es/strings.xml−2

@@ -92,7 +92,6 @@

92 <string name="username_optional">Usuario (opcional)</string>

93 <string name="password_optional">Contraseña (opcional)</string>

94 <string name="connect">Conectar</string>

95− <string name="save_without_connecting">Guardar sin conectar</string>

96 <string name="discard_title">¿Descartar cambios?</string>

95 <string name="discard_title">¿Descartar cambios?</string>

97 <string name="discard_text">Se perderán tus cambios.</string>

96 <string name="discard_text">Se perderán tus cambios.</string>

98 <string name="discard">Descartar</string>

97 <string name="discard">Descartar</string>

@@ -100,7 +99,6 @@

100 <string name="show_password">Mostrar contraseña</string>

99 <string name="show_password">Mostrar contraseña</string>

101 <string name="hide_password">Ocultar contraseña</string>

100 <string name="hide_password">Ocultar contraseña</string>

102 <string name="test_credentials">Probar credenciales</string>

101 <string name="test_credentials">Probar credenciales</string>

103− <string name="testing_connection">Probando…</string>

104 <string name="delete_server">Eliminar servidor</string>

102 <string name="delete_server">Eliminar servidor</string>

105 <string name="delete_server_confirm">¿Eliminar este perfil de servidor?</string>

103 <string name="delete_server_confirm">¿Eliminar este perfil de servidor?</string>

106 <string name="testing_credentials">Probando…</string>

104 <string name="testing_credentials">Probando…</string>

app/src/main/res/values/strings.xml−2

@@ -93,7 +93,6 @@

93 <string name="username_optional">Username (optional)</string>

94 <string name="password_optional">Password (optional)</string>

95 <string name="connect">Connect</string>

96− <string name="save_without_connecting">Save without connecting</string>

97 <string name="discard_title">Discard changes?</string>

96 <string name="discard_title">Discard changes?</string>

98 <string name="discard_text">Your edits will be lost.</string>

97 <string name="discard_text">Your edits will be lost.</string>

99 <string name="discard">Discard</string>

98 <string name="discard">Discard</string>

@@ -101,7 +100,6 @@

101 <string name="show_password">Show password</string>

100 <string name="show_password">Show password</string>

102 <string name="hide_password">Hide password</string>

101 <string name="hide_password">Hide password</string>

103 <string name="test_credentials">Test credentials</string>

102 <string name="test_credentials">Test credentials</string>

104− <string name="testing_connection">Testing…</string>

105 <string name="delete_server">Delete server</string>

103 <string name="delete_server">Delete server</string>

106 <string name="delete_server_confirm">Delete this server profile?</string>

104 <string name="delete_server_confirm">Delete this server profile?</string>

107 <string name="testing_credentials">Testing…</string>

105 <string name="testing_credentials">Testing…</string>

app/src/test/java/soy/iko/opencode/ui/server/ServerEditViewModelTest.kt+34−46

@@ -161,15 +161,19 @@ class ServerEditViewModelTest {

161 assertTrue(vm.state.value.canSave)

162 }

163

164− // --- save() ---

164+ // --- connect() save path (probe Reachable → save + connect + ping) ---

165+ // The former save()-specific tests now exercise the same persist path via connect(),

166+ // since the standalone "Save without connecting" action was removed: connect() is the

167+ // single primary action and its probe IS the connection test.

165

168

166 @Test

169 @Test

167− fun save_persistsProfileAndCallsOnDone() = testScope.runTest {

170+ fun connect_persistsProfileAndCallsOnDone() = testScope.runTest {

168 val container = FakeAppContainer()

171 val container = FakeAppContainer()

172+ container.probeResult = ProbeResult.Reachable

169 val vm = makeVm(container)

173 val vm = makeVm(container)

170 vm.update { it.copy(baseUrl = "http://localhost:3000", label = "Test") }

174 vm.update { it.copy(baseUrl = "http://localhost:3000", label = "Test") }

171 var doneCalled = false

175 var doneCalled = false

172− vm.save { doneCalled = true }

176+ vm.connect { doneCalled = true }

173 testScheduler.advanceUntilIdle()

177 testScheduler.advanceUntilIdle()

174 assertTrue(doneCalled)

178 assertTrue(doneCalled)

175 assertNotNull(container.fakeProfileStore.savedProfile)

179 assertNotNull(container.fakeProfileStore.savedProfile)

@@ -178,36 +182,12 @@ class ServerEditViewModelTest {

178 }

182 }

179

183

180 @Test

184 @Test

181− fun save_invalidUrlDoesNotSave() = testScope.runTest {

185+ fun connect_generatesIdForNewProfile() = testScope.runTest {

182− val container = FakeAppContainer()

183− val vm = makeVm(container)

184− vm.update { it.copy(baseUrl = "not a url") }

185− var doneCalled = false

186− vm.save { doneCalled = true }

187− testScheduler.advanceUntilIdle()

188− assertFalse(doneCalled)

189− assertNull(container.fakeProfileStore.savedProfile)

190− }

191−

192− @Test

193− fun save_whileSavingDoesNotSaveAgain() = testScope.runTest {

194− val container = FakeAppContainer()

195− val vm = makeVm(container)

196− vm.update { it.copy(baseUrl = "http://localhost:3000") }

197− var doneCount = 0

198− vm.save { doneCount++ }

199− // While saving, try to save again

200− vm.save { doneCount++ }

201− testScheduler.advanceUntilIdle()

202− assertEquals(1, doneCount)

203− }

204−

205− @Test

206− fun save_generatesIdForNewProfile() = testScope.runTest {

207 val container = FakeAppContainer()

186 val container = FakeAppContainer()

187+ container.probeResult = ProbeResult.Reachable

208 val vm = makeVm(container)

188 val vm = makeVm(container)

209 vm.update { it.copy(baseUrl = "http://localhost:3000") }

189 vm.update { it.copy(baseUrl = "http://localhost:3000") }

210− vm.save { }

190+ vm.connect { }

211 testScheduler.advanceUntilIdle()

191 testScheduler.advanceUntilIdle()

212 val saved = container.fakeProfileStore.savedProfile

192 val saved = container.fakeProfileStore.savedProfile

213 assertNotNull(saved?.id)

193 assertNotNull(saved?.id)

@@ -215,23 +195,25 @@ class ServerEditViewModelTest {

215 }

195 }

216

196

217 @Test

197 @Test

218− fun save_preservesIdForExistingProfile() = testScope.runTest {

198+ fun connect_preservesIdForExistingProfile() = testScope.runTest {

219 val container = FakeAppContainer()

199 val container = FakeAppContainer()

200+ container.probeResult = ProbeResult.Reachable

220 val profile = ServerProfile(id = "existing-id", label = "Old", baseUrl = "http://old")

201 val profile = ServerProfile(id = "existing-id", label = "Old", baseUrl = "http://old")

221 container.fakeProfileStore.setProfiles(listOf(profile))

202 container.fakeProfileStore.setProfiles(listOf(profile))

222 val vm = makeVm(container, profileId = "existing-id")

203 val vm = makeVm(container, profileId = "existing-id")

223 vm.update { it.copy(baseUrl = "http://new") }

204 vm.update { it.copy(baseUrl = "http://new") }

224− vm.save { }

205+ vm.connect { }

225 testScheduler.advanceUntilIdle()

206 testScheduler.advanceUntilIdle()

226 assertEquals("existing-id", container.fakeProfileStore.savedProfile?.id)

207 assertEquals("existing-id", container.fakeProfileStore.savedProfile?.id)

227 }

208 }

228

209

229 @Test

210 @Test

230− fun save_trimsFields() = testScope.runTest {

211+ fun connect_trimsFields() = testScope.runTest {

231 val container = FakeAppContainer()

212 val container = FakeAppContainer()

213+ container.probeResult = ProbeResult.Reachable

232 val vm = makeVm(container)

214 val vm = makeVm(container)

233 vm.update { it.copy(baseUrl = " http://localhost:3000 ", label = " Test ", username = " admin ") }

215 vm.update { it.copy(baseUrl = " http://localhost:3000 ", label = " Test ", username = " admin ") }

234− vm.save { }

216+ vm.connect { }

235 testScheduler.advanceUntilIdle()

217 testScheduler.advanceUntilIdle()

236 val saved = container.fakeProfileStore.savedProfile!!

218 val saved = container.fakeProfileStore.savedProfile!!

237 assertEquals("http://localhost:3000", saved.baseUrl)

219 assertEquals("http://localhost:3000", saved.baseUrl)

@@ -240,11 +222,12 @@ class ServerEditViewModelTest {

240 }

222 }

241

223

242 @Test

224 @Test

243− fun save_appliesHttpsSchemeToBareLanHost() = testScope.runTest {

225+ fun connect_appliesHttpsSchemeToBareLanHost() = testScope.runTest {

244 val container = FakeAppContainer()

226 val container = FakeAppContainer()

227+ container.probeResult = ProbeResult.Reachable

245 val vm = makeVm(container)

228 val vm = makeVm(container)

246 vm.update { it.copy(baseUrl = "localhost:3000", label = "LAN") }

229 vm.update { it.copy(baseUrl = "localhost:3000", label = "LAN") }

247− vm.save { }

230+ vm.connect { }

248 testScheduler.advanceUntilIdle()

231 testScheduler.advanceUntilIdle()

249 val saved = container.fakeProfileStore.savedProfile!!

232 val saved = container.fakeProfileStore.savedProfile!!

250 // A bare host is always schemed to https:// (the safe default). The user can still

233 // A bare host is always schemed to https:// (the safe default). The user can still

@@ -258,35 +241,38 @@ class ServerEditViewModelTest {

258 }

241 }

259

242

260 @Test

243 @Test

261− fun save_appliesHttpsSchemeToBarePublicDomain() = testScope.runTest {

244+ fun connect_appliesHttpsSchemeToBarePublicDomain() = testScope.runTest {

262 val container = FakeAppContainer()

245 val container = FakeAppContainer()

246+ container.probeResult = ProbeResult.Reachable

263 val vm = makeVm(container)

247 val vm = makeVm(container)

264 vm.update { it.copy(baseUrl = "example.com") }

248 vm.update { it.copy(baseUrl = "example.com") }

265− vm.save { }

249+ vm.connect { }

266 testScheduler.advanceUntilIdle()

250 testScheduler.advanceUntilIdle()

267 val saved = container.fakeProfileStore.savedProfile!!

251 val saved = container.fakeProfileStore.savedProfile!!

268 assertEquals("https://example.com", saved.baseUrl)

252 assertEquals("https://example.com", saved.baseUrl)

269 }

253 }

270

254

271 @Test

255 @Test

272− fun save_preservesExplicitHttpSchemeForPublicDomain() = testScope.runTest {

256+ fun connect_preservesExplicitHttpSchemeForPublicDomain() = testScope.runTest {

273 val container = FakeAppContainer()

257 val container = FakeAppContainer()

258+ container.probeResult = ProbeResult.Reachable

274 val vm = makeVm(container)

259 val vm = makeVm(container)

275 // The user typed the scheme explicitly — don't upgrade a public domain to https://

260 // The user typed the scheme explicitly — don't upgrade a public domain to https://

276 // behind their back; preserve their explicit http:// choice.

261 // behind their back; preserve their explicit http:// choice.

277 vm.update { it.copy(baseUrl = "http://example.com") }

262 vm.update { it.copy(baseUrl = "http://example.com") }

278− vm.save { }

263+ vm.connect { }

279 testScheduler.advanceUntilIdle()

264 testScheduler.advanceUntilIdle()

280 val saved = container.fakeProfileStore.savedProfile!!

265 val saved = container.fakeProfileStore.savedProfile!!

281 assertEquals("http://example.com", saved.baseUrl)

266 assertEquals("http://example.com", saved.baseUrl)

282 }

267 }

283

268

284 @Test

269 @Test

285− fun save_emptyUsernameClearsCredentials() = testScope.runTest {

270+ fun connect_emptyUsernameClearsCredentials() = testScope.runTest {

286 val container = FakeAppContainer()

271 val container = FakeAppContainer()

272+ container.probeResult = ProbeResult.Reachable

287 val vm = makeVm(container)

273 val vm = makeVm(container)

288 vm.update { it.copy(baseUrl = "http://localhost:3000", username = "", password = "") }

274 vm.update { it.copy(baseUrl = "http://localhost:3000", username = "", password = "") }

289− vm.save { }

275+ vm.connect { }

290 testScheduler.advanceUntilIdle()

276 testScheduler.advanceUntilIdle()

291 val saved = container.fakeProfileStore.savedProfile!!

277 val saved = container.fakeProfileStore.savedProfile!!

292 assertNull(saved.username)

278 assertNull(saved.username)

@@ -294,13 +280,14 @@ class ServerEditViewModelTest {

294 }

280 }

295

281

296 @Test

282 @Test

297− fun save_failureSetsErrorAndClearsSaving() = testScope.runTest {

283+ fun connect_saveFailureSetsErrorAndClearsSaving() = testScope.runTest {

298 val container = FakeAppContainer()

284 val container = FakeAppContainer()

285+ container.probeResult = ProbeResult.Reachable

299 container.fakeProfileStore.saveException = RuntimeException("disk full")

286 container.fakeProfileStore.saveException = RuntimeException("disk full")

300 val vm = makeVm(container)

287 val vm = makeVm(container)

301 vm.update { it.copy(baseUrl = "http://localhost:3000") }

288 vm.update { it.copy(baseUrl = "http://localhost:3000") }

302 var doneCalled = false

289 var doneCalled = false

303− vm.save { doneCalled = true }

290+ vm.connect { doneCalled = true }

304 testScheduler.advanceUntilIdle()

291 testScheduler.advanceUntilIdle()

305 assertFalse(doneCalled)

292 assertFalse(doneCalled)

306 assertFalse(vm.state.value.saving)

293 assertFalse(vm.state.value.saving)

@@ -310,12 +297,13 @@ class ServerEditViewModelTest {

310 }

297 }

311

298

312 @Test

299 @Test

313− fun save_failureDoesNotClobberUserEdits() = testScope.runTest {

300+ fun connect_saveFailureDoesNotClobberUserEdits() = testScope.runTest {

314 val container = FakeAppContainer()

301 val container = FakeAppContainer()

302+ container.probeResult = ProbeResult.Reachable

315 container.fakeProfileStore.saveException = RuntimeException("disk full")

303 container.fakeProfileStore.saveException = RuntimeException("disk full")

316 val vm = makeVm(container)

304 val vm = makeVm(container)

317 vm.update { it.copy(baseUrl = "http://localhost:3000", label = "Original") }

305 vm.update { it.copy(baseUrl = "http://localhost:3000", label = "Original") }

318− vm.save { }

306+ vm.connect { }

319 // Simulate the user editing the label while the save coroutine is in-flight.

307 // Simulate the user editing the label while the save coroutine is in-flight.

320 vm.update { it.copy(label = "Edited") }

308 vm.update { it.copy(label = "Edited") }

321 testScheduler.advanceUntilIdle()

309 testScheduler.advanceUntilIdle()