From 2151bc6d7ca2e7380dfbd9c1cb5e7494f2c8e839 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 18:52:21 -0500 Subject: [PATCH] feat(reporting): require a reply-to email and a 200-char minimum on problem reports Adds friction to the "Report a Problem" form: a required email field (basic local-part@domain.tld validation), a required consent notice about being contacted at that address, and a 200-character minimum on the comment field with a live "x/200" counter that turns red (with the field outline) until the threshold is met. Submit stays disabled until both the comment and email are valid, mirroring and extending the existing SUBMITTING gate. The email rides along on DebugReport (userEmail) so it round-trips through the storage JSON and the exact payload that's previewed, copied, saved, and POSTed. The new ViewModel-level guard on submit() also fully integrates with the #161 success-confirmation dialog: invalid attempts never reach SUBMITTING/SUCCEEDED, so the dialog flow is unaffected. Closes #159 Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/reporting/DebugReport.kt | 4 + .../ui/reporting/ReportReviewScreen.kt | 34 ++++- .../ui/reporting/ReportReviewViewModel.kt | 60 ++++++++- app/src/main/res/values/strings.xml | 8 +- .../libremail/reporting/DebugReportTest.kt | 24 ++++ .../ui/reporting/ReportReviewViewModelTest.kt | 121 ++++++++++++++++-- 6 files changed, 230 insertions(+), 21 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt b/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt index 3b6d929..2c782f7 100644 --- a/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt +++ b/app/src/main/kotlin/org/libremail/reporting/DebugReport.kt @@ -29,6 +29,8 @@ data class DebugReport( val settings: Map, val logs: List, val userComment: String = "", + /** Reply-to address the user supplied when submitting (see #159); required for online submit. */ + val userEmail: String = "", ) { /** The exact text shown for review, copied, saved to a file, and POSTed on submit. */ fun toSubmissionPayload(): String = toJson().toString(JSON_INDENT) @@ -55,6 +57,7 @@ data class DebugReport( .put("app", app) .put("device", device) .put("userComment", userComment) + .put("userEmail", userEmail) .put("settings", settingsJson) .put("logs", JSONArray(logs)) if (stackTrace != null) json.put("stackTrace", stackTrace) @@ -88,6 +91,7 @@ data class DebugReport( settings = settings, logs = logs, userComment = json.optString("userComment", ""), + userEmail = json.optString("userEmail", ""), ) } } diff --git a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewScreen.kt b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewScreen.kt index d14c76d..a106a2f 100644 --- a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewScreen.kt @@ -128,6 +128,36 @@ fun ReportReviewScreen(onDone: () -> Unit, viewModel: ReportReviewViewModel = hi label = { Text(stringResource(R.string.report_comment_label)) }, modifier = Modifier.fillMaxWidth(), minLines = 2, + isError = !state.isCommentLongEnough, + supportingText = { + Text( + stringResource( + R.string.report_comment_counter, + state.comment.length, + ReportSubmissionRules.MIN_COMMENT_LENGTH, + ), + ) + }, + ) + Spacer(Modifier.height(16.dp)) + OutlinedTextField( + value = state.email, + onValueChange = viewModel::updateEmail, + label = { Text(stringResource(R.string.report_email_label)) }, + modifier = Modifier.fillMaxWidth(), + singleLine = true, + isError = !state.isEmailValid, + supportingText = { + if (!state.isEmailValid) { + Text(stringResource(R.string.report_email_invalid)) + } + }, + ) + Spacer(Modifier.height(8.dp)) + Text( + stringResource(R.string.report_email_consent), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, ) Spacer(Modifier.height(16.dp)) Text( @@ -141,7 +171,9 @@ fun ReportReviewScreen(onDone: () -> Unit, viewModel: ReportReviewViewModel = hi Spacer(Modifier.height(16.dp)) Button( onClick = viewModel::submit, - enabled = state.submit != SubmitUiState.SUBMITTING, + enabled = state.submit != SubmitUiState.SUBMITTING && + state.isCommentLongEnough && + state.isEmailValid, modifier = Modifier.fillMaxWidth(), ) { Text(stringResource(R.string.report_submit)) diff --git a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt index e42a8b7..a374a26 100644 --- a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt @@ -20,14 +20,45 @@ import javax.inject.Inject /** UI-facing status of a submission attempt. [UNAVAILABLE] means no endpoint is configured. */ enum class SubmitUiState { IDLE, SUBMITTING, SUCCEEDED, FAILED, UNAVAILABLE } +/** + * Pure validation rules for problem-report submission (#159). Kept dependency-free — in particular, + * no `android.util.Patterns`, which is a no-op stub under plain JVM unit tests — so both the + * min-length and email-shape checks are directly unit-testable and shareable between + * [ReportReviewState] and [ReportReviewViewModel]. + */ +object ReportSubmissionRules { + /** Minimum comment length (characters) required before Submit is enabled. */ + const val MIN_COMMENT_LENGTH = 200 + + // Basic shape only: a non-blank local part, an `@`, and a domain with at least one `.` and + // non-blank labels either side of it. Deliberately not a full RFC 5322 validator. + private val EMAIL_REGEX = Regex("^[^\\s@]+@[^\\s@]+\\.[^\\s@]+$") + + /** Whether [comment] reaches [MIN_COMMENT_LENGTH]. */ + fun isCommentLongEnough(comment: String): Boolean = comment.length >= MIN_COMMENT_LENGTH + + /** Whether [email] has a plausible local-part@domain.tld shape. */ + fun isValidEmail(email: String): Boolean = EMAIL_REGEX.matches(email.trim()) +} + data class ReportReviewState( val loaded: Boolean = false, val exists: Boolean = false, val payload: String = "", val comment: String = "", + val email: String = "", val canSubmitOnline: Boolean = false, val submit: SubmitUiState = SubmitUiState.IDLE, -) +) { + /** True once [comment] reaches [ReportSubmissionRules.MIN_COMMENT_LENGTH]. */ + val isCommentLongEnough: Boolean get() = ReportSubmissionRules.isCommentLongEnough(comment) + + /** True once [email] is a plausible reply-to address. */ + val isEmailValid: Boolean get() = ReportSubmissionRules.isValidEmail(email) + + /** Submit gate: the original SUBMITTING check, plus the new length/email requirements. */ + val canSubmit: Boolean get() = submit != SubmitUiState.SUBMITTING && isCommentLongEnough && isEmailValid +} @HiltViewModel class ReportReviewViewModel @Inject constructor( @@ -38,17 +69,20 @@ class ReportReviewViewModel @Inject constructor( private val reportId: String = checkNotNull(savedStateHandle[Routes.REPORT_REVIEW_ARG_ID]) private val comment = MutableStateFlow(store.find(reportId)?.userComment.orEmpty()) + private val email = MutableStateFlow(store.find(reportId)?.userEmail.orEmpty()) private val submitState = MutableStateFlow(SubmitUiState.IDLE) val state: StateFlow = - combine(store.reports, comment, submitState) { reports, currentComment, submit -> + combine(store.reports, comment, email, submitState) { reports, currentComment, currentEmail, submit -> val report = reports.firstOrNull { it.id == reportId } ReportReviewState( loaded = true, exists = report != null, - // The comment is folded in so the preview is byte-for-byte what a submit would send. - payload = report?.copy(userComment = currentComment)?.toSubmissionPayload().orEmpty(), + // The comment/email are folded in so the preview is byte-for-byte what a submit would send. + payload = report?.copy(userComment = currentComment, userEmail = currentEmail) + ?.toSubmissionPayload().orEmpty(), comment = currentComment, + email = currentEmail, canSubmitOnline = submitter.isEnabled, submit = submit, ) @@ -58,6 +92,10 @@ class ReportReviewViewModel @Inject constructor( comment.value = value } + fun updateEmail(value: String) { + email.value = value + } + fun discard() { viewModelScope.launch { store.delete(reportId) } } @@ -66,11 +104,20 @@ class ReportReviewViewModel @Inject constructor( * The only path that can send a report off-device, and only from an explicit Submit tap. Persists * the reviewed comment first so the upload matches exactly what was shown, then enqueues the * worker (unless no endpoint is configured, in which case it steers the user to Copy/Save). + * + * Guarded by [ReportSubmissionRules] directly (rather than trusting the caller) so this can't + * succeed with an under-length comment or an invalid email even if invoked outside the + * Compose button's `enabled` gate — e.g. from an accessibility service activating a control it + * still considers actionable. */ fun submit() { viewModelScope.launch { + val currentSubmit = submitState.value + if (currentSubmit == SubmitUiState.SUBMITTING) return@launch + if (!ReportSubmissionRules.isCommentLongEnough(comment.value)) return@launch + if (!ReportSubmissionRules.isValidEmail(email.value)) return@launch val report = store.find(reportId) ?: return@launch - store.save(report.copy(userComment = comment.value)) + store.save(report.copy(userComment = comment.value, userEmail = email.value)) if (!submitter.isEnabled) { submitState.value = SubmitUiState.UNAVAILABLE return@launch @@ -82,7 +129,8 @@ class ReportReviewViewModel @Inject constructor( } /** The exact text shown for review — used for Copy and Save-to-file. */ - fun payload(): String = store.find(reportId)?.copy(userComment = comment.value)?.toSubmissionPayload().orEmpty() + fun payload(): String = store.find(reportId)?.copy(userComment = comment.value, userEmail = email.value) + ?.toSubmissionPayload().orEmpty() private fun SubmitStatus.toUi(): SubmitUiState = when (this) { SubmitStatus.IDLE, SubmitStatus.SUBMITTING -> SubmitUiState.SUBMITTING diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 976335b..b0cd3ac 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -330,7 +330,13 @@ Review report May contain personal information This report can include email addresses, server names, and other details from your device. Read the whole thing below before sending. Nothing is sent unless you tap Submit. - What went wrong? (optional) + What went wrong? + + %1$d/%2$d + Your email address + Enter a valid email address + + By submitting this report and supplying your email, you agree that the maintainers of LibreMail may contact you at the supplied email. Supplying an email and submitting a report does not guarantee reply or resolution to your concern. Exactly what will be sent Submit Discard report diff --git a/app/src/test/kotlin/org/libremail/reporting/DebugReportTest.kt b/app/src/test/kotlin/org/libremail/reporting/DebugReportTest.kt index b582078..4cf670b 100644 --- a/app/src/test/kotlin/org/libremail/reporting/DebugReportTest.kt +++ b/app/src/test/kotlin/org/libremail/reporting/DebugReportTest.kt @@ -69,4 +69,28 @@ class DebugReportTest { assertTrue(edited.contains("edited note")) } + + @Test + fun `round-trips the reply-to email`() { + val original = sample().copy(userEmail = "reporter@example.com") + + val restored = DebugReport.fromStorageJson(original.toStorageJson()) + + assertEquals("reporter@example.com", restored.userEmail) + assertEquals(original, restored) + } + + @Test + fun `a report with no email round-trips to a blank one`() { + val restored = DebugReport.fromStorageJson(sample().toStorageJson()) + + assertEquals("", restored.userEmail) + } + + @Test + fun `submission payload contains the reply-to email`() { + val payload = sample().copy(userEmail = "reporter@example.com").toSubmissionPayload() + + assertTrue(payload.contains("reporter@example.com")) + } } diff --git a/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt index 79eb032..f592c1b 100644 --- a/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt @@ -24,6 +24,8 @@ import org.libremail.reporting.ReportStore import org.libremail.reporting.ReportSubmitter import org.libremail.ui.navigation.Routes import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue @OptIn(ExperimentalCoroutinesApi::class) class ReportReviewViewModelTest { @@ -83,31 +85,82 @@ class ReportReviewViewModelTest { } @Test - fun `submit enqueues the upload exactly once and persists the reviewed comment`() = runTest(testDispatcher) { - every { submitter.isEnabled } returns true - every { submitter.submit("rid") } just Runs - every { submitter.status("rid") } returns emptyFlow() - every { store.save(any()) } just Runs + fun `payload shown reflects the entered email`() = runTest(testDispatcher) { val vm = viewModel() - vm.updateComment("edited before submit") - vm.submit() + vm.updateEmail("me@example.com") - verify(exactly = 1) { store.save(match { it.userComment == "edited before submit" }) } - verify(exactly = 1) { submitter.submit("rid") } + assertEquals(report.copy(userEmail = "me@example.com").toSubmissionPayload(), vm.payload()) } @Test - fun `submit with no endpoint configured never transmits`() = runTest(testDispatcher) { + fun `submit enqueues the upload exactly once and persists the reviewed comment and email`() = + runTest(testDispatcher) { + every { submitter.isEnabled } returns true + every { submitter.submit("rid") } just Runs + every { submitter.status("rid") } returns emptyFlow() + every { store.save(any()) } just Runs + val vm = viewModel() + vm.updateComment(VALID_COMMENT) + vm.updateEmail(VALID_EMAIL) + + vm.submit() + + verify(exactly = 1) { + store.save(match { it.userComment == VALID_COMMENT && it.userEmail == VALID_EMAIL }) + } + verify(exactly = 1) { submitter.submit("rid") } + } + + @Test + fun `submit with no endpoint configured never transmits but still persists`() = runTest(testDispatcher) { every { submitter.isEnabled } returns false every { store.save(any()) } just Runs val vm = viewModel() - vm.updateComment("please send") + vm.updateComment(VALID_COMMENT) + vm.updateEmail(VALID_EMAIL) vm.submit() - // The comment is still persisted for Copy/Save, but nothing is enqueued for upload. - verify(exactly = 1) { store.save(match { it.userComment == "please send" }) } + // The comment/email are still persisted for Copy/Save, but nothing is enqueued for upload. + verify(exactly = 1) { + store.save(match { it.userComment == VALID_COMMENT && it.userEmail == VALID_EMAIL }) + } + verify(exactly = 0) { submitter.submit(any()) } + } + + @Test + fun `submit does nothing when the comment is under the minimum length`() = runTest(testDispatcher) { + val vm = viewModel() + vm.updateEmail(VALID_EMAIL) + vm.updateComment("way too short") + + vm.submit() + + verify(exactly = 0) { store.save(any()) } + verify(exactly = 0) { submitter.submit(any()) } + } + + @Test + fun `submit does nothing when the email is blank`() = runTest(testDispatcher) { + val vm = viewModel() + vm.updateComment(VALID_COMMENT) + + vm.submit() + + verify(exactly = 0) { store.save(any()) } + verify(exactly = 0) { submitter.submit(any()) } + } + + @Test + fun `submit does nothing when the email is malformed`() = runTest(testDispatcher) { + val vm = viewModel() + vm.updateComment(VALID_COMMENT) + vm.updateEmail("not-an-email") + + vm.submit() + + verify(exactly = 0) { store.save(any()) } verify(exactly = 0) { submitter.submit(any()) } } @@ -121,4 +174,46 @@ class ReportReviewViewModelTest { verify(exactly = 1) { store.delete("rid") } verify(exactly = 0) { submitter.submit(any()) } } + + @Test + fun `isCommentLongEnough is false below the threshold and true at or above it`() { + val short = ReportReviewState(comment = "a".repeat(ReportSubmissionRules.MIN_COMMENT_LENGTH - 1)) + val exact = ReportReviewState(comment = "a".repeat(ReportSubmissionRules.MIN_COMMENT_LENGTH)) + + assertFalse(short.isCommentLongEnough) + assertTrue(exact.isCommentLongEnough) + } + + @Test + fun `isEmailValid accepts plausible addresses and rejects malformed or blank input`() { + assertTrue(ReportReviewState(email = "user@example.com").isEmailValid) + assertTrue(ReportReviewState(email = "first.last+tag@sub.example.co.uk").isEmailValid) + assertFalse(ReportReviewState(email = "").isEmailValid) + assertFalse(ReportReviewState(email = "no-at-sign.com").isEmailValid) + assertFalse(ReportReviewState(email = "user@").isEmailValid) + assertFalse(ReportReviewState(email = "user@nodot").isEmailValid) + assertFalse(ReportReviewState(email = "@example.com").isEmailValid) + assertFalse(ReportReviewState(email = "has space@example.com").isEmailValid) + } + + @Test + fun `canSubmit requires both a long-enough comment and a valid email`() { + val validComment = "a".repeat(ReportSubmissionRules.MIN_COMMENT_LENGTH) + + assertTrue(ReportReviewState(comment = validComment, email = "user@example.com").canSubmit) + assertFalse(ReportReviewState(comment = "short", email = "user@example.com").canSubmit) + assertFalse(ReportReviewState(comment = validComment, email = "not-an-email").canSubmit) + assertFalse( + ReportReviewState( + comment = validComment, + email = "user@example.com", + submit = SubmitUiState.SUBMITTING, + ).canSubmit, + ) + } + + private companion object { + val VALID_COMMENT = "a".repeat(ReportSubmissionRules.MIN_COMMENT_LENGTH) + const val VALID_EMAIL = "reporter@example.com" + } }