Skip to content

Commit 8fbc9ca

Browse files
authored
Ensure bulk saves also trigger the password saved listeners (#6526)
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1210974473587922?focus=true ### Description - We have a `PasswordStoreEventListener` plugin which allows other code to be notified when a password has been saved - This is called for individual password saves, but bulk insertions doesn't trigger it ### Steps to test this PR Suggested logcat filter: `m_autofill_onboardeduser` ### Required testing #### Importing passwords from Google Passwords - [ ] Fresh install on a device with a modern `WebView` (e.g.., not an old emulator) - [ ] Visit Password management screen (`Overflow -> Passwords`) and choose to `Import Passwords From Google` - [ ] Go through the flow until success - [ ] Verify `m_autofill_onboardeduser` is in the logs ### Optional extra testing (functionality unchanged) #### Manually adding a credential - [ ] Fresh install, visit Password management screen and manually add a password - [ ] Verify `m_autofill_onboardeduser` is in the logs. #### Saving a credential from website autofill - [ ] Fresh install, visit https://fill.dev/form/login-simple, add username and password (>= 4 characters) and login - [ ] Save when prompted - [ ] Verify `m_autofill_onboardeduser` is in the logs. #### Manually adding a credential - [ ] Fresh install, visit Password management screen and manually add a password - [ ] Verify `m_autofill_onboardeduser` is in the logs. Co-authored-by: Craig Russell <[email protected]>
1 parent d4c3c16 commit 8fbc9ca

File tree

5 files changed

+15
-12
lines changed

5 files changed

+15
-12
lines changed

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/PasswordStoreEventListener.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ import com.duckduckgo.di.scopes.AppScope
2222
@ContributesPluginPoint(AppScope::class)
2323
interface PasswordStoreEventListener {
2424
/**
25-
* Called when a credential is added to the password store
25+
* Called when credentials are added to the password store
2626
* This includes one being automatically saved, the user manually saving one, or one being imported via sync.
2727
*
28-
* @param id the id of the credential that was added
28+
* @param newCredentialIds the list of IDs for added credentials
2929
*/
30-
fun onCredentialAdded(id: Long) {}
30+
fun onCredentialAdded(newCredentialIds: List<Long>) {}
3131
}

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/SecureStoreBackedAutofillStore.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ class SecureStoreBackedAutofillStore @Inject constructor(
183183
secureStorage.addWebsiteLoginDetailsWithCredentials(webSiteLoginCredentials)?.toLoginCredentials().also {
184184
syncCredentialsListener.onCredentialAdded(it?.id!!)
185185
it.id?.let { newCredentialId ->
186-
passwordStoreEventListeners.forEach { listener -> listener.onCredentialAdded(newCredentialId) }
186+
val credentialList = listOf(newCredentialId)
187+
passwordStoreEventListeners.forEach { listener -> listener.onCredentialAdded(credentialList) }
187188
}
188189
}
189190
}
@@ -386,8 +387,9 @@ class SecureStoreBackedAutofillStore @Inject constructor(
386387
override suspend fun bulkInsert(credentials: List<LoginCredentials>): List<Long> {
387388
return withContext(dispatcherProvider.io()) {
388389
val mappedCredentials = credentials.map { it.prepareForBulkInsertion() }
389-
return@withContext secureStorage.addWebsiteLoginDetailsWithCredentials(mappedCredentials).also {
390-
syncCredentialsListener.onCredentialsAdded(it)
390+
return@withContext secureStorage.addWebsiteLoginDetailsWithCredentials(mappedCredentials).also { credentialsAdded ->
391+
syncCredentialsListener.onCredentialsAdded(credentialsAdded)
392+
passwordStoreEventListeners.forEach { it.onCredentialAdded(credentialsAdded) }
391393
}
392394
}
393395
}

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/engagement/EngagementPasswordAddedListener.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class EngagementPasswordAddedListener @Inject constructor(
4343
private var credentialAdded = false
4444

4545
@Synchronized
46-
override fun onCredentialAdded(id: Long) {
46+
override fun onCredentialAdded(newCredentialIds: List<Long>) {
4747
if (credentialAdded) return
4848

4949
credentialAdded = true

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/sync/CredentialsSync.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ class CredentialsSync @Inject constructor(
115115
credentialsSyncMetadata.addOrUpdate(
116116
CredentialsSyncMetadataEntity(syncId = remoteId, localId = autofillId, deleted_at = null, modified_at = null),
117117
)
118-
passwordStoreEventPlugins.getPlugins().forEach { it.onCredentialAdded(autofillId) }
118+
val credentialList = listOf(autofillId)
119+
passwordStoreEventPlugins.getPlugins().forEach { it.onCredentialAdded(credentialList) }
119120
}
120121
}
121122

autofill/autofill-impl/src/test/java/com/duckduckgo/autofill/impl/engagement/EngagementPasswordAddedListenerTest.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,29 @@ class EngagementPasswordAddedListenerTest {
3030
@Test
3131
fun whenDaysInstalledLessThan7ThenPixelSent() {
3232
whenever(userBrowserProperties.daysSinceInstalled()).thenReturn(0)
33-
testee.onCredentialAdded(0)
33+
testee.onCredentialAdded(listOf(0))
3434
verifyPixelSentOnce()
3535
}
3636

3737
@Test
3838
fun whenDaysInstalledExactly7ThenPixelNotSent() {
3939
whenever(userBrowserProperties.daysSinceInstalled()).thenReturn(7)
40-
testee.onCredentialAdded(0)
40+
testee.onCredentialAdded(listOf(0))
4141
verifyPixelNotSent()
4242
}
4343

4444
@Test
4545
fun whenDaysInstalledAbove7ThenPixelNotSent() {
4646
whenever(userBrowserProperties.daysSinceInstalled()).thenReturn(10)
47-
testee.onCredentialAdded(0)
47+
testee.onCredentialAdded(listOf(0))
4848
verifyPixelNotSent()
4949
}
5050

5151
@Test
5252
fun whenCalledMultipleTimesThenOnlySendsPixelOnce() {
5353
whenever(userBrowserProperties.daysSinceInstalled()).thenReturn(0)
5454
repeat(10) {
55-
testee.onCredentialAdded(0)
55+
testee.onCredentialAdded(listOf(0))
5656
}
5757
verifyPixelSentOnce()
5858
}

0 commit comments

Comments
 (0)