Merge main into feat-71-strikethrough
This commit is contained in:
@@ -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", ""),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -332,7 +332,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>
|
||||
|
||||
@@ -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"))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user