From 01780973cba0dd9a452f4390461b70daaa9050cd Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Wed, 19 Nov 2025 11:20:46 +0100 Subject: [PATCH 1/2] Handle removing extracted profile from dashboard --- ...RemoveOptOutFromDashboardMessageHandler.kt | 25 ++++++++--- .../pir/impl/models/scheduling/JobRecord.kt | 3 ++ .../pir/impl/scheduling/JobRecordUpdater.kt | 42 ++++++++++++++++++ .../pir/impl/store/PirRepository.kt | 8 ++++ .../pir/impl/store/db/ExtractedProfileDao.kt | 6 +++ ...veOptOutFromDashboardMessageHandlerTest.kt | 24 +++++++++-- .../scheduling/RealJobRecordUpdaterTest.kt | 43 ++++++++++++++++++- .../pir/impl/store/RealPirRepositoryTest.kt | 13 +++++- 8 files changed, 150 insertions(+), 14 deletions(-) diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandler.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandler.kt index 3eb8f1c960a0..ad7990e627c0 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandler.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandler.kt @@ -16,6 +16,8 @@ package com.duckduckgo.pir.impl.dashboard.messaging.handlers +import com.duckduckgo.app.di.AppCoroutineScope +import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.ActivityScope import com.duckduckgo.js.messaging.api.JsMessage import com.duckduckgo.js.messaging.api.JsMessageCallback @@ -23,7 +25,10 @@ import com.duckduckgo.js.messaging.api.JsMessaging import com.duckduckgo.pir.impl.dashboard.messaging.PirDashboardWebMessages import com.duckduckgo.pir.impl.dashboard.messaging.model.PirWebMessageRequest import com.duckduckgo.pir.impl.dashboard.messaging.model.PirWebMessageResponse +import com.duckduckgo.pir.impl.scheduling.JobRecordUpdater import com.squareup.anvil.annotations.ContributesMultibinding +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import logcat.logcat import javax.inject.Inject @@ -34,7 +39,11 @@ import javax.inject.Inject scope = ActivityScope::class, boundType = PirWebJsMessageHandler::class, ) -class PirRemoveOptOutFromDashboardMessageHandler @Inject constructor() : PirWebJsMessageHandler() { +class PirRemoveOptOutFromDashboardMessageHandler @Inject constructor( + private val jobRecordUpdater: JobRecordUpdater, + private val dispatcherProvider: DispatcherProvider, + @AppCoroutineScope private val appCoroutineScope: CoroutineScope, +) : PirWebJsMessageHandler() { override val message = PirDashboardWebMessages.REMOVE_OPT_OUT_FROM_DASHBOARD @@ -58,12 +67,14 @@ class PirRemoveOptOutFromDashboardMessageHandler @Inject constructor() : PirWebJ return } - logcat { "PIR-WEB: PirRemoveOptOutFromDashboardMessageHandler: removing recordId=$recordId" } + appCoroutineScope.launch(dispatcherProvider.io()) { + jobRecordUpdater.markRecordsAsRemovedByUser(extractedProfileId = recordId) - // TODO: Implement actual removal logic - jsMessaging.sendResponse( - jsMessage = jsMessage, - response = PirWebMessageResponse.DefaultResponse.SUCCESS, - ) + logcat { "PIR-WEB: PirRemoveOptOutFromDashboardMessageHandler: successfully removed recordId=$recordId" } + jsMessaging.sendResponse( + jsMessage = jsMessage, + response = PirWebMessageResponse.DefaultResponse.SUCCESS, + ) + } } } diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/models/scheduling/JobRecord.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/models/scheduling/JobRecord.kt index 965afbf75fe4..5708242e87b1 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/models/scheduling/JobRecord.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/models/scheduling/JobRecord.kt @@ -76,6 +76,9 @@ sealed class JobRecord( /** The job is waiting for email confirmation to complete before we can move it to [REQUESTED]. */ PENDING_EMAIL_CONFIRMATION, + + /** The profile was removed from the dashboard by the user. */ + REMOVED_BY_USER, } } diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt index 9b61388be716..53335cccdb03 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt @@ -234,6 +234,19 @@ interface JobRecordUpdater { * that have an extracted profile associated to it to continue running scan jobs on them. */ suspend fun removeScanJobRecordsWithNoMatchesForProfiles(profileQueryIds: List) + + /** + * Marks the [ExtractedProfile], associated [OptOutJobRecord], and [EmailConfirmationJobRecord] as removed by user. + * + * This method should be called when the user removes an extracted profile from the dashboard. + * It will: + * - Mark the ExtractedProfile as deprecated + * - Update all OptOutJobRecords for that ExtractedProfile (set status to REMOVED_BY_USER, deprecated = true) + * - Update all EmailConfirmationJobRecords for that ExtractedProfile (deprecated = true) + * + * @param extractedProfileId The id of the [ExtractedProfile] to be marked as removed by user + */ + suspend fun markRecordsAsRemovedByUser(extractedProfileId: Long) } @ContributesBinding(AppScope::class) @@ -477,6 +490,35 @@ class RealJobRecordUpdater @Inject constructor( } } + override suspend fun markRecordsAsRemovedByUser(extractedProfileId: Long) { + withContext(dispatcherProvider.io()) { + repository.markExtractedProfileAsDeprecated(extractedProfileId) + + // update the OptOutJobRecord for this ExtractedProfile + schedulingRepository.getValidOptOutJobRecord(extractedProfileId)?.run { + schedulingRepository.saveOptOutJobRecord( + copy( + status = OptOutJobStatus.REMOVED_BY_USER, + deprecated = true, + ).also { + logcat { "PIR-JOB-RECORD: Updated OptOutJobRecord for $extractedProfileId to REMOVED_BY_USER: $it" } + }, + ) + } + + // update the EmailConfirmationJobRecord for this ExtractedProfile (if it exists) + schedulingRepository.getEmailConfirmationJob(extractedProfileId)?.run { + schedulingRepository.saveEmailConfirmationJobRecord( + copy( + deprecated = true, + ).also { + logcat { "PIR-JOB-RECORD: Marked EmailConfirmationJobRecord for $extractedProfileId as deprecated: $it" } + }, + ) + } + } + } + private data class ExtractedProfileComparisonKey( val profileQueryId: Long, val brokerName: String, diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/PirRepository.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/PirRepository.kt index d456686b7e68..cbaf36db8a1e 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/PirRepository.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/PirRepository.kt @@ -143,6 +143,8 @@ interface PirRepository { suspend fun getAllExtractedProfiles(): List + suspend fun markExtractedProfileAsDeprecated(extractedProfileId: Long) + suspend fun getUserProfileQuery(id: Long): ProfileQuery? /** @@ -552,6 +554,12 @@ class RealPirRepository( }.orEmpty() } + override suspend fun markExtractedProfileAsDeprecated(extractedProfileId: Long) { + withContext(dispatcherProvider.io()) { + extractedProfileDao()?.updateExtractedProfileDeprecated(extractedProfileId, deprecated = true) + } + } + override suspend fun getUserProfileQuery(id: Long): ProfileQuery? = withContext(dispatcherProvider.io()) { userProfileDao()?.getUserProfile(id)?.toProfileQuery() diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/db/ExtractedProfileDao.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/db/ExtractedProfileDao.kt index ec3551d5c0ef..1f2b89d82eb2 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/db/ExtractedProfileDao.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/db/ExtractedProfileDao.kt @@ -53,4 +53,10 @@ interface ExtractedProfileDao { @Query("DELETE from pir_extracted_profiles") fun deleteAllExtractedProfiles() + + @Query("UPDATE pir_extracted_profiles SET deprecated = :deprecated WHERE id = :extractedProfileId") + fun updateExtractedProfileDeprecated( + extractedProfileId: Long, + deprecated: Boolean, + ) } diff --git a/pir/pir-impl/src/test/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandlerTest.kt b/pir/pir-impl/src/test/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandlerTest.kt index 116ff87d0940..8e1581313aba 100644 --- a/pir/pir-impl/src/test/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandlerTest.kt +++ b/pir/pir-impl/src/test/java/com/duckduckgo/pir/impl/dashboard/messaging/handlers/PirRemoveOptOutFromDashboardMessageHandlerTest.kt @@ -17,28 +17,41 @@ package com.duckduckgo.pir.impl.dashboard.messaging.handlers import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.js.messaging.api.JsMessageCallback import com.duckduckgo.js.messaging.api.JsMessaging import com.duckduckgo.pir.impl.dashboard.messaging.PirDashboardWebMessages import com.duckduckgo.pir.impl.dashboard.messaging.handlers.PirMessageHandlerUtils.createJsMessage import com.duckduckgo.pir.impl.dashboard.messaging.handlers.PirMessageHandlerUtils.verifyResponse +import com.duckduckgo.pir.impl.scheduling.JobRecordUpdater +import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.mockito.kotlin.verify @RunWith(AndroidJUnit4::class) class PirRemoveOptOutFromDashboardMessageHandlerTest { + @get:Rule + val coroutineRule = CoroutineTestRule() + private lateinit var testee: PirRemoveOptOutFromDashboardMessageHandler + private val mockJobRecordUpdater: JobRecordUpdater = mock() private val mockJsMessaging: JsMessaging = mock() private val mockJsMessageCallback: JsMessageCallback = mock() @Before fun setUp() { - testee = PirRemoveOptOutFromDashboardMessageHandler() + testee = PirRemoveOptOutFromDashboardMessageHandler( + jobRecordUpdater = mockJobRecordUpdater, + dispatcherProvider = coroutineRule.testDispatcherProvider, + appCoroutineScope = coroutineRule.testScope, + ) } @Test @@ -47,10 +60,11 @@ class PirRemoveOptOutFromDashboardMessageHandlerTest { } @Test - fun whenProcessWithValidRecordIdThenSendsSuccessResponse() { + fun whenProcessWithValidRecordIdThenCallsJobRecordUpdaterAndSendsSuccessResponse() = runTest { // Given + val testRecordId = 123L val jsMessage = createJsMessage( - paramsJson = """{"recordId": 2}""", + paramsJson = """{"recordId": $testRecordId}""", method = PirDashboardWebMessages.REMOVE_OPT_OUT_FROM_DASHBOARD, ) @@ -58,11 +72,12 @@ class PirRemoveOptOutFromDashboardMessageHandlerTest { testee.process(jsMessage, mockJsMessaging, mockJsMessageCallback) // Then + verify(mockJobRecordUpdater).markRecordsAsRemovedByUser(testRecordId) verifyResponse(jsMessage, true, mockJsMessaging) } @Test - fun whenProcessWithMissingRecordIdThenSendsErrorResponse() { + fun whenProcessWithMissingRecordIdThenSendsErrorResponseWithoutCallingJobRecordUpdater() = runTest { // Given val jsMessage = createJsMessage( paramsJson = """{}""", @@ -74,5 +89,6 @@ class PirRemoveOptOutFromDashboardMessageHandlerTest { // Then verifyResponse(jsMessage, false, mockJsMessaging) + verify(mockJobRecordUpdater, org.mockito.kotlin.never()).markRecordsAsRemovedByUser(org.mockito.kotlin.any()) } } diff --git a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt index 5b91b6a327c0..c0c031f78e73 100644 --- a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt +++ b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt @@ -795,13 +795,52 @@ class RealJobRecordUpdaterTest { @Test fun whenRemoveJobRecordsForProfileWithExclusionsThenDeletesJobRecordsExceptExcluded() = runTest { - val brokersToExclude = listOf("broker1", "broker2") - toTest.removeScanJobRecordsWithNoMatchesForProfiles(listOf(testProfileQueryId)) verify(mockSchedulingRepository).deleteScanJobRecordsWithoutMatchesForProfiles(listOf(testProfileQueryId)) } + @Test + fun whenMarkRecordsAsRemovedByUserThenMarksExtractedProfileAndOptOutJobRecordAndEmailConfirmationJobRecordAsDeprecated() = + runTest { + whenever(mockSchedulingRepository.getValidOptOutJobRecord(testExtractedProfileId)) + .thenReturn(testOptOutJobRecord) + whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) + .thenReturn(null) + whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) + .thenReturn(testEmailConfirmationJobRecord) + + toTest.markRecordsAsRemovedByUser(testExtractedProfileId) + + verify(mockRepository).markExtractedProfileAsDeprecated(testExtractedProfileId) + verify(mockSchedulingRepository).saveOptOutJobRecord( + testOptOutJobRecord.copy( + status = OptOutJobStatus.REMOVED_BY_USER, + deprecated = true, + ), + ) + verify(mockSchedulingRepository).saveEmailConfirmationJobRecord( + testEmailConfirmationJobRecord.copy( + deprecated = true, + ), + ) + } + + @Test + fun whenMarkRecordsAsRemovedByUserAndOptOutJobRecordAndEmailConfirmationDoesNotExistThenOnlyMarksExtractedProfileAsDeprecated() = + runTest { + whenever(mockSchedulingRepository.getValidOptOutJobRecord(testExtractedProfileId)) + .thenReturn(null) + whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) + .thenReturn(null) + + toTest.markRecordsAsRemovedByUser(testExtractedProfileId) + + verify(mockRepository).markExtractedProfileAsDeprecated(testExtractedProfileId) + verify(mockSchedulingRepository, never()).saveOptOutJobRecord(any()) + verify(mockSchedulingRepository, never()).saveEmailConfirmationJobRecord(any()) + } + companion object { private const val TEST_CURRENT_TIME = 5000L } diff --git a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/store/RealPirRepositoryTest.kt b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/store/RealPirRepositoryTest.kt index cd58a6e1d66a..f1ce96f49fbc 100644 --- a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/store/RealPirRepositoryTest.kt +++ b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/store/RealPirRepositoryTest.kt @@ -18,7 +18,6 @@ package com.duckduckgo.pir.impl.store import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.utils.CurrentTimeProvider -import com.duckduckgo.pir.impl.models.Address import com.duckduckgo.pir.impl.models.AddressCityState import com.duckduckgo.pir.impl.models.ExtractedProfile import com.duckduckgo.pir.impl.models.scheduling.JobRecord.EmailConfirmationJobRecord.EmailData @@ -853,4 +852,16 @@ class RealPirRepositoryTest { verify(mockUserProfileDao).getUserProfile(profileQueryId) verify(mockExtractedProfileDao).insertNewExtractedProfiles(any()) } + + @Test + fun whenMarkExtractedProfileAsDeprecatedThenCallsDaoUpdateMethod() = runTest { + // Given + val extractedProfileId = 123L + + // When + testee.markExtractedProfileAsDeprecated(extractedProfileId) + + // Then + verify(mockExtractedProfileDao).updateExtractedProfileDeprecated(extractedProfileId, true) + } } From 0d8aea035b336b8d7d4ad067546a6aeb9c787483 Mon Sep 17 00:00:00 2001 From: Domen Lanisnik Date: Wed, 19 Nov 2025 11:29:05 +0100 Subject: [PATCH 2/2] Update logic to delete the EmailConfirmationJobRecord --- .../pir/impl/scheduling/JobRecordUpdater.kt | 18 ++++-------------- .../scheduling/RealJobRecordUpdaterTest.kt | 14 ++------------ 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt index 53335cccdb03..b20517b8e8aa 100644 --- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt +++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/JobRecordUpdater.kt @@ -239,10 +239,6 @@ interface JobRecordUpdater { * Marks the [ExtractedProfile], associated [OptOutJobRecord], and [EmailConfirmationJobRecord] as removed by user. * * This method should be called when the user removes an extracted profile from the dashboard. - * It will: - * - Mark the ExtractedProfile as deprecated - * - Update all OptOutJobRecords for that ExtractedProfile (set status to REMOVED_BY_USER, deprecated = true) - * - Update all EmailConfirmationJobRecords for that ExtractedProfile (deprecated = true) * * @param extractedProfileId The id of the [ExtractedProfile] to be marked as removed by user */ @@ -494,7 +490,7 @@ class RealJobRecordUpdater @Inject constructor( withContext(dispatcherProvider.io()) { repository.markExtractedProfileAsDeprecated(extractedProfileId) - // update the OptOutJobRecord for this ExtractedProfile + // update the OptOutJobRecord for this ExtractedProfile (if it exists) schedulingRepository.getValidOptOutJobRecord(extractedProfileId)?.run { schedulingRepository.saveOptOutJobRecord( copy( @@ -506,15 +502,9 @@ class RealJobRecordUpdater @Inject constructor( ) } - // update the EmailConfirmationJobRecord for this ExtractedProfile (if it exists) - schedulingRepository.getEmailConfirmationJob(extractedProfileId)?.run { - schedulingRepository.saveEmailConfirmationJobRecord( - copy( - deprecated = true, - ).also { - logcat { "PIR-JOB-RECORD: Marked EmailConfirmationJobRecord for $extractedProfileId as deprecated: $it" } - }, - ) + // delete the EmailConfirmationJobRecord for this ExtractedProfile (if it exists) + schedulingRepository.deleteEmailConfirmationJobRecord(extractedProfileId).also { + logcat { "PIR-JOB-RECORD: Delete EmailConfirmationJobRecord for $extractedProfileId" } } } } diff --git a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt index c0c031f78e73..dce4a1686dd9 100644 --- a/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt +++ b/pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealJobRecordUpdaterTest.kt @@ -805,10 +805,6 @@ class RealJobRecordUpdaterTest { runTest { whenever(mockSchedulingRepository.getValidOptOutJobRecord(testExtractedProfileId)) .thenReturn(testOptOutJobRecord) - whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) - .thenReturn(null) - whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) - .thenReturn(testEmailConfirmationJobRecord) toTest.markRecordsAsRemovedByUser(testExtractedProfileId) @@ -819,11 +815,7 @@ class RealJobRecordUpdaterTest { deprecated = true, ), ) - verify(mockSchedulingRepository).saveEmailConfirmationJobRecord( - testEmailConfirmationJobRecord.copy( - deprecated = true, - ), - ) + verify(mockSchedulingRepository).deleteEmailConfirmationJobRecord(testExtractedProfileId) } @Test @@ -831,14 +823,12 @@ class RealJobRecordUpdaterTest { runTest { whenever(mockSchedulingRepository.getValidOptOutJobRecord(testExtractedProfileId)) .thenReturn(null) - whenever(mockSchedulingRepository.getEmailConfirmationJob(testExtractedProfileId)) - .thenReturn(null) toTest.markRecordsAsRemovedByUser(testExtractedProfileId) verify(mockRepository).markExtractedProfileAsDeprecated(testExtractedProfileId) verify(mockSchedulingRepository, never()).saveOptOutJobRecord(any()) - verify(mockSchedulingRepository, never()).saveEmailConfirmationJobRecord(any()) + verify(mockSchedulingRepository).deleteEmailConfirmationJobRecord(testExtractedProfileId) } companion object {