Merge main into feat-155-yahoo-app-password

app/src/androidTest/kotlin/org/libremail/IntentHandledMarkerTest.kt

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package org.libremail
+
+import android.content.Intent
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Locks in the #157 regression fix: [IntentHandledMarker] must tell a redelivered (already-parsed)
+ * intent apart from a genuinely new one using only the [Intent] instance itself, never
+ * `savedInstanceState` — which Android also sets to non-null after a process-death relaunch, where the
+ * delivered intent is a brand new, not-yet-handled notification tap or mailto/share, not a replay.
+ */
+@RunWith(AndroidJUnit4::class)
+class IntentHandledMarkerTest {
+
+    @Test
+    fun first_look_marks_the_intent_and_reports_it_as_unhandled() {
+        val intent = Intent(Intent.ACTION_MAIN)
+        assertTrue(IntentHandledMarker.markIfUnhandled(intent))
+    }
+
+    @Test
+    fun redelivering_the_same_instance_is_recognized_as_already_handled() {
+        // Simulates a config-change recreation: Android redelivers the very same Intent object to the
+        // new Activity instance's onCreate, so a second look must be recognized as a replay.
+        val intent = Intent(Intent.ACTION_MAIN)
+        IntentHandledMarker.markIfUnhandled(intent)
+        assertFalse(IntentHandledMarker.markIfUnhandled(intent))
+    }
+
+    @Test
+    fun a_freshly_constructed_equivalent_intent_is_still_unhandled() {
+        // Simulates a process-death relaunch (e.g. tapping a notification after the app was killed in
+        // the background): a brand new Intent instance arrives — content may equal one already marked
+        // in a prior (now-dead) process, but it was never marked in THIS one, so it must be parsed.
+        val first = Intent(Intent.ACTION_VIEW)
+        IntentHandledMarker.markIfUnhandled(first)
+
+        val second = Intent(Intent.ACTION_VIEW)
+        assertTrue(IntentHandledMarker.markIfUnhandled(second))
+    }
+}

app/src/main/kotlin/org/libremail/MainActivity.kt

@@ -66,12 +66,7 @@ class MainActivity : FragmentActivity() {
                 }
             }
         }
-        // Only on a fresh launch — on a config-change recreation the NavHost restores the compose /
-        // reader destination itself, so re-parsing the (unchanged) intent would open a duplicate.
-        if (savedInstanceState == null) {
-            pendingCompose.value = IntentComposeParser.parse(intent)
-            pendingOpenMessageId.value = NotificationIntents.messageId(intent)
-        }
+        handleIntent(intent)
         setContent {
             val dynamicColor by settingsRepository.dynamicColor.collectAsStateWithLifecycle(initialValue = true)
             LibreMailTheme(dynamicColor = dynamicColor) {
@@ -92,7 +87,54 @@ class MainActivity : FragmentActivity() {
     override fun onNewIntent(intent: Intent) {
         super.onNewIntent(intent)
         setIntent(intent)
+        handleIntent(intent)
+    }
+
+    /**
+     * Parses [intent] for a pending compose ([IntentComposeParser]) or open-message
+     * ([NotificationIntents]) request — unless [IntentHandledMarker] says this exact intent instance
+     * was already parsed.
+     *
+     * This used to be gated on `savedInstanceState == null` in [onCreate]: a non-null value was
+     * assumed to mean "config-change recreation" — where Android redelivers the very same,
+     * already-parsed intent and the NavHost restores its own compose/reader destination itself, so
+     * re-parsing would only navigate to a duplicate. But Android *also* passes a restored, non-null
+     * savedInstanceState when it recreates this activity after the process was killed in the
+     * background and is then relaunched — e.g. by tapping a notification. There, `intent` is the new
+     * tap, not a replay, but the old guard swallowed it exactly like a rotation: `pendingOpenMessageId`
+     * was never set, so the tap silently landed wherever the restored back stack was, never the
+     * message. That regression is #157.
+     *
+     * Marking the [Intent] instance itself — rather than branching on savedInstanceState, which can't
+     * tell a config change and a process-death relaunch apart — is correct for both: a config-change
+     * recreation redelivers the very same intent this activity already marked, so it's recognized and
+     * skipped; a genuinely new intent — a fresh notification tap or mailto/share, whether delivered
+     * warm via [onNewIntent] or cold via [onCreate] after a process-death relaunch — is never marked
+     * yet, so it's always (re)parsed.
+     */
+    private fun handleIntent(intent: Intent) {
+        if (!IntentHandledMarker.markIfUnhandled(intent)) return
         IntentComposeParser.parse(intent)?.let { pendingCompose.value = it }
         NotificationIntents.messageId(intent)?.let { pendingOpenMessageId.value = it }
     }
 }
+
+/**
+ * Marks an [Intent] as already parsed by [MainActivity.handleIntent], so a redelivery of the very same
+ * instance — which is what Android does when it recreates an Activity for a configuration change — is
+ * recognized and skipped instead of re-triggering a duplicate navigation. A freshly constructed intent
+ * (a new notification tap or mailto/share, however it arrives) is never marked yet, so it is always
+ * treated as unhandled — including right after a process-death relaunch, where `savedInstanceState` is
+ * restored (non-null) but the intent itself is new. Internal (not private) so androidTest can verify
+ * the marking contract directly. See #157.
+ */
+internal object IntentHandledMarker {
+    private const val EXTRA_HANDLED = "org.libremail.extra.INTENT_HANDLED"
+
+    /** Marks [intent] handled and returns `true` — but only the first time this instance is seen. */
+    fun markIfUnhandled(intent: Intent): Boolean {
+        if (intent.getBooleanExtra(EXTRA_HANDLED, false)) return false
+        intent.putExtra(EXTRA_HANDLED, true)
+        return true
+    }
+}

app/src/main/kotlin/org/libremail/reporting/DebugReport.kt

@@ -29,6 +29,8 @@ data class DebugReport(
     val settings: Map<String, String>,
     val logs: List<String>,
     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", ""),
             )
         }
     }

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))

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<ReportReviewState> =
-        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

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

@@ -331,7 +331,13 @@
     <string name="report_review_title">Review report</string>
     <string name="report_pii_disclaimer_title">May contain personal information</string>
     <string name="report_pii_disclaimer">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.</string>
-    <string name="report_comment_label">What went wrong? (optional)</string>
+    <string name="report_comment_label">What went wrong?</string>
+    <!-- Live "x/200" counter below the comment field; %1$d = characters typed, %2$d = minimum. -->
+    <string name="report_comment_counter">%1$d/%2$d</string>
+    <string name="report_email_label">Your email address</string>
+    <string name="report_email_invalid">Enter a valid email address</string>
+    <!-- Consent copy shown near the email field / submit button (#159) — required verbatim. -->
+    <string name="report_email_consent">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.</string>
     <string name="report_payload_label">Exactly what will be sent</string>
     <string name="report_submit">Submit</string>
     <string name="report_discard">Discard report</string>

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"))
+    }
 }

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"
+    }
 }