fix(lookup): don't save Lookup's own wording as a block's note

A refuting review found both Block buttons in Lookup saved the
result's description as the new block's note. For a temporary block
with no note that description is the stand-in label "Temporarily
blocked", so making the block permanent wrote that label in as the
note. For a number Lookup had let through it's the reason it was
allowed (a trusted number's note, the India 1600 series name).

lookupBlockNote keeps a match's description and saves nothing for a
user rule or an allowed result; blockNumber then keeps any note the
number already had.

Test changes: LookupBlockNoteTest covers a temporary block made
permanent, a user's own note surviving, and allowed results. Saving
the description unconditionally fails the first and last.

Author: SysAdminDoc

CHANGELOG.md

@@ -196,6 +196,11 @@ All notable changes to CallShield will be documented in this file.
 
 ### Interface
 
+- Blocking a number from Lookup no longer saves Lookup's own wording as the
+  block's note. Making a temporary block permanent wrote "Temporarily
+  blocked" into it, and blocking a number Lookup had let through saved the
+  reason it was allowed, like the name of India's 1600 series. The block
+  keeps whatever note the number already had.
 - Switching filters in the Database tab or the block log no longer leaves
   the old list running in the background. Every chip you tried kept its own
   copy until the app closed, and each one ran its query again whenever the

app/src/main/java/com/sysadmindoc/callshield/ui/screens/lookup/LookupBlockNote.kt

@@ -0,0 +1,13 @@
+package com.sysadmindoc.callshield.ui.screens.lookup
+
+import com.sysadmindoc.callshield.data.BlockReasoning
+import com.sysadmindoc.callshield.domain.model.SpamCheckResult
+
+/**
+ * The note a block saved from Lookup keeps. A database or pattern match
+ * describes the number. A user rule's text is that rule's own note or a
+ * label standing in for it ("Temporarily blocked"), and an allowed result
+ * says why it was let through, so neither is saved: the block keeps any
+ * note the number already had.
+ */
+internal fun lookupBlockNote(result: SpamCheckResult): String = if (!result.isSpam || BlockReasoning.isUserRule(result.reasonCode)) "" else result.description

app/src/main/java/com/sysadmindoc/callshield/ui/screens/lookup/LookupScreen.kt

@@ -477,7 +477,7 @@ fun LookupScreen(viewModel: MainViewModel) {
                                         }
 
                                         else -> {
-                                            viewModel.blockNumber(resultNumber, lookupResult.type.ifEmpty { "spam" }, lookupResult.description)
+                                            viewModel.blockNumber(resultNumber, lookupResult.type.ifEmpty { "spam" }, lookupBlockNote(lookupResult))
                                             hapticConfirm(context)
                                             scope.launch { snackbarHostState.showSnackbar(numberBlockedMessage) }
                                         }
@@ -510,8 +510,8 @@ fun LookupScreen(viewModel: MainViewModel) {
                                                 val blocked =
                                                     withContext(Dispatchers.IO) {
                                                         // The description becomes the block's note, which Lookup and
-                                                        // the block log show; the checker id is no note.
-                                                        repo.blockNumber(resultNumber, lookupResult.type.ifEmpty { "spam" }, lookupResult.description)
+                                                        // the block log show; see lookupBlockNote for what isn't one.
+                                                        repo.blockNumber(resultNumber, lookupResult.type.ifEmpty { "spam" }, lookupBlockNote(lookupResult))
                                                     }
                                                 if (blocked) {
                                                     hapticConfirm(context)

app/src/test/java/com/sysadmindoc/callshield/ui/screens/lookup/LookupBlockNoteTest.kt

@@ -0,0 +1,75 @@
+package com.sysadmindoc.callshield.ui.screens.lookup
+
+import android.content.Context
+import androidx.test.core.app.ApplicationProvider
+import com.sysadmindoc.callshield.R
+import com.sysadmindoc.callshield.data.IsolatedRepositoryFixture
+import com.sysadmindoc.callshield.domain.model.SpamCheckResult
+import kotlinx.coroutines.runBlocking
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34])
+class LookupBlockNoteTest {
+    private val context: Context = ApplicationProvider.getApplicationContext()
+    private lateinit var fixture: IsolatedRepositoryFixture
+    private val number = "+12125550142"
+
+    @Before
+    fun setUp() {
+        fixture = IsolatedRepositoryFixture(context)
+    }
+
+    @After
+    fun tearDown() {
+        fixture.close()
+    }
+
+    /** What Lookup's Block button does with a result. */
+    private suspend fun blockFromLookup(result: SpamCheckResult) {
+        fixture.repository.blockNumber(number, result.type.ifEmpty { "spam" }, lookupBlockNote(result))
+    }
+
+    @Test
+    fun `making a temporary block permanent saves no stand-in note`() =
+        runBlocking {
+            fixture.repository.temporaryBlockNumber(number, System.currentTimeMillis() + 3_600_000L)
+            val result = fixture.repository.isSpam(number)
+            // Lookup shows the label in place of a note the block doesn't have.
+            assertEquals("temporary_block", result.matchSource)
+            assertEquals(context.getString(R.string.block_reason_temporary_block), result.description)
+
+            blockFromLookup(result)
+
+            val row = requireNotNull(fixture.dao.findByNumber(number))
+            assertEquals("", row.description)
+            assertNull(row.expiresAt)
+        }
+
+    @Test
+    fun `a block's own note is kept`() =
+        runBlocking {
+            fixture.repository.blockNumber(number, "spam", "landlord's old line")
+
+            blockFromLookup(fixture.repository.isSpam(number))
+
+            assertEquals("landlord's old line", fixture.dao.findByNumber(number)?.description)
+        }
+
+    @Test
+    fun `a match describes the number, an allowed result does not`() {
+        assertEquals(
+            "IRS impersonation",
+            lookupBlockNote(SpamCheckResult(isSpam = true, matchSource = "database", type = "scam", description = "IRS impersonation")),
+        )
+        assertEquals("", lookupBlockNote(SpamCheckResult(isSpam = false, matchSource = "manual_whitelist", description = "pharmacy")))
+        assertEquals("", lookupBlockNote(SpamCheckResult(isSpam = false, matchSource = "regulatory_allow", description = "India 1600 series")))
+    }
+}