From 686a35a0e82dda147a3796960627f49b95a16d03 Mon Sep 17 00:00:00 2001 From: MusTalK <49585280+mustalk@users.noreply.github.com> Date: Sun, 1 Jun 2025 23:01:30 +0200 Subject: [PATCH] fix: prevent invalid input in builder mode numeric fields (#9) - Add real-time input filtering for numeric fields: * Plateau width/height - digits only * Rover start X/Y coordinates - digits only * Filters out minus signs on real devices and letters/symbols on emulators - Enhance validation logic with clearer error messages: * "Must be a valid number" for invalid format * "Must be a positive number" for zero values * "Must be a non-negative number" for negative values - Add comprehensive test coverage: * Input filtering behavior tests * Updated validation tests for filtered inputs * Edge case handling (empty after filtering) - UX improvements: * Consistent behavior across real devices and emulators * Proactive prevention vs reactive error handling * Immediate visual feedback during typing * Eliminates user frustration from invalid input - Fixes builder mode input validation issue #9 - All tests passing --- app/build.gradle.kts | 4 +- .../ui/mission/NewMissionViewModel.kt | 97 +++++++++++++----- .../ui/mission/NewMissionViewModelTest.kt | 99 +++++++++++++++++-- 3 files changed, 164 insertions(+), 36 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 5d4ee65..af2a927 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -15,8 +15,8 @@ android { applicationId = "com.mustalk.seat.marsrover" minSdk = 24 targetSdk = 35 - versionCode = 1 - versionName = "1.0.0" + versionCode = 2 + versionName = "1.0.1" testInstrumentationRunner = "com.mustalk.seat.marsrover.HiltTestRunner" } diff --git a/app/src/main/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModel.kt b/app/src/main/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModel.kt index e66e08e..931ad3f 100644 --- a/app/src/main/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModel.kt +++ b/app/src/main/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModel.kt @@ -136,9 +136,11 @@ private class FieldUpdateHandler( } fun updatePlateauWidth(width: String) { + // Filter to allow only digits + val filteredWidth = width.filter { it.isDigit() } updateState( getCurrentState().copy( - plateauWidth = width, + plateauWidth = filteredWidth, plateauWidthError = null, errorMessage = null ) @@ -146,9 +148,11 @@ private class FieldUpdateHandler( } fun updatePlateauHeight(height: String) { + // Filter to allow only digits + val filteredHeight = height.filter { it.isDigit() } updateState( getCurrentState().copy( - plateauHeight = height, + plateauHeight = filteredHeight, plateauHeightError = null, errorMessage = null ) @@ -156,9 +160,11 @@ private class FieldUpdateHandler( } fun updateRoverStartX(x: String) { + // Filter to allow only digits (no negative sign since we don't allow negative coordinates) + val filteredX = x.filter { it.isDigit() } updateState( getCurrentState().copy( - roverStartX = x, + roverStartX = filteredX, roverStartXError = null, errorMessage = null ) @@ -166,9 +172,11 @@ private class FieldUpdateHandler( } fun updateRoverStartY(y: String) { + // Filter to allow only digits (no negative sign since we don't allow negative coordinates) + val filteredY = y.filter { it.isDigit() } updateState( getCurrentState().copy( - roverStartY = y, + roverStartY = filteredY, roverStartYError = null, errorMessage = null ) @@ -198,6 +206,7 @@ private class FieldUpdateHandler( /** * Handles field validation for the New Mission screen. + * Validates user input and sets appropriate error messages. */ private class ValidationHandler( private val updateState: (NewMissionUiState) -> Unit, @@ -207,12 +216,22 @@ private class ValidationHandler( val width = getCurrentState().plateauWidth if (width.isNotBlank()) { val widthValue = width.toIntOrNull() - if (widthValue == null || widthValue <= 0) { - updateState( - getCurrentState().copy( - plateauWidthError = "Must be a positive number" + when { + widthValue == null -> { + updateState( + getCurrentState().copy( + plateauWidthError = "Must be a valid number" + ) ) - ) + } + + widthValue <= 0 -> { + updateState( + getCurrentState().copy( + plateauWidthError = "Must be a positive number" + ) + ) + } } } } @@ -221,12 +240,22 @@ private class ValidationHandler( val height = getCurrentState().plateauHeight if (height.isNotBlank()) { val heightValue = height.toIntOrNull() - if (heightValue == null || heightValue <= 0) { - updateState( - getCurrentState().copy( - plateauHeightError = "Must be a positive number" + when { + heightValue == null -> { + updateState( + getCurrentState().copy( + plateauHeightError = "Must be a valid number" + ) ) - ) + } + + heightValue <= 0 -> { + updateState( + getCurrentState().copy( + plateauHeightError = "Must be a positive number" + ) + ) + } } } } @@ -235,12 +264,22 @@ private class ValidationHandler( val x = getCurrentState().roverStartX if (x.isNotBlank()) { val xValue = x.toIntOrNull() - if (xValue == null || xValue < 0) { - updateState( - getCurrentState().copy( - roverStartXError = "Must be a non-negative number" + when { + xValue == null -> { + updateState( + getCurrentState().copy( + roverStartXError = "Must be a valid number" + ) ) - ) + } + + xValue < 0 -> { + updateState( + getCurrentState().copy( + roverStartXError = "Must be a non-negative number" + ) + ) + } } } } @@ -249,12 +288,22 @@ private class ValidationHandler( val y = getCurrentState().roverStartY if (y.isNotBlank()) { val yValue = y.toIntOrNull() - if (yValue == null || yValue < 0) { - updateState( - getCurrentState().copy( - roverStartYError = "Must be a non-negative number" + when { + yValue == null -> { + updateState( + getCurrentState().copy( + roverStartYError = "Must be a valid number" + ) ) - ) + } + + yValue < 0 -> { + updateState( + getCurrentState().copy( + roverStartYError = "Must be a non-negative number" + ) + ) + } } } } diff --git a/app/src/test/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModelTest.kt b/app/src/test/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModelTest.kt index d08e5af..2dd2140 100644 --- a/app/src/test/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModelTest.kt +++ b/app/src/test/kotlin/com/mustalk/seat/marsrover/presentation/ui/mission/NewMissionViewModelTest.kt @@ -359,18 +359,95 @@ class NewMissionViewModelTest { assertThat(state.jsonError).isNull() } - // Validation tests + // Input filtering tests + @Test + fun `updatePlateauWidth should filter out non-digit characters`() { + // When + viewModel.updatePlateauWidth("12abc34") + + // Then + val state = viewModel.uiState.value + assertThat(state.plateauWidth).isEqualTo("1234") + assertThat(state.plateauWidthError).isNull() + } + + @Test + fun `updatePlateauHeight should filter out non-digit characters`() { + // When + viewModel.updatePlateauHeight("5#6@7") + + // Then + val state = viewModel.uiState.value + assertThat(state.plateauHeight).isEqualTo("567") + assertThat(state.plateauHeightError).isNull() + } + + @Test + fun `updateRoverStartX should filter out letters and special characters`() { + // When + viewModel.updateRoverStartX("a1b2c3!") + + // Then + val state = viewModel.uiState.value + assertThat(state.roverStartX).isEqualTo("123") + assertThat(state.roverStartXError).isNull() + } + + @Test + fun `updateRoverStartY should filter out letters and special characters`() { + // When + viewModel.updateRoverStartY("ads") + + // Then + val state = viewModel.uiState.value + assertThat(state.roverStartY).isEmpty() + assertThat(state.roverStartYError).isNull() + } + + @Test + fun `updateRoverStartX should handle empty input after filtering`() { + // When + viewModel.updateRoverStartX("abc!@#") + + // Then + val state = viewModel.uiState.value + assertThat(state.roverStartX).isEmpty() + assertThat(state.roverStartXError).isNull() + } + + @Test + fun `validation should show proper error for different input types`() { + // Test validation differentiates between invalid format and negative numbers + + // Update with a valid but negative number (this won't be filtered since we only filter digits) + // But actually, negative numbers would need minus sign which gets filtered + // So let's test with zero which is invalid for plateau dimensions + viewModel.updatePlateauWidth("0") + viewModel.validatePlateauWidth() + + var state = viewModel.uiState.value + assertThat(state.plateauWidthError).isEqualTo("Must be a positive number") + + // Test empty input validation + viewModel.updateRoverStartX("") + viewModel.validateRoverStartX() + + state = viewModel.uiState.value + assertThat(state.roverStartXError).isNull() // Empty is allowed for coordinates + } + @Test fun `validatePlateauWidth with invalid input should set error`() { - // Given + // Given - Input filtering will remove "-" sign, leaving "1" viewModel.updatePlateauWidth("-1") // When viewModel.validatePlateauWidth() - // Then + // Then - Since filtering removes "-", we get "1" which is valid val state = viewModel.uiState.value - assertThat(state.plateauWidthError).isEqualTo("Must be a positive number") + assertThat(state.plateauWidth).isEqualTo("1") + assertThat(state.plateauWidthError).isNull() } @Test @@ -388,28 +465,30 @@ class NewMissionViewModelTest { @Test fun `validateRoverStartX with negative value should set error`() { - // Given + // Given - Input filtering will remove "-" sign, leaving "1" viewModel.updateRoverStartX("-1") // When viewModel.validateRoverStartX() - // Then + // Then - Since filtering removes "-", we get "1" which is valid val state = viewModel.uiState.value - assertThat(state.roverStartXError).isEqualTo("Must be a non-negative number") + assertThat(state.roverStartX).isEqualTo("1") + assertThat(state.roverStartXError).isNull() } @Test fun `validateRoverStartY with non-numeric value should set error`() { - // Given + // Given - Input filtering will remove letters, leaving empty string viewModel.updateRoverStartY("abc") // When viewModel.validateRoverStartY() - // Then + // Then - Empty string doesn't trigger validation error val state = viewModel.uiState.value - assertThat(state.roverStartYError).isEqualTo("Must be a non-negative number") + assertThat(state.roverStartY).isEmpty() + assertThat(state.roverStartYError).isNull() } @Test