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 0554cde..24cd6ae 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -332,7 +332,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" + } }