Skip to content

Commit

Permalink
Sync bookmarks - notify user when sync paused (#3913)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/488551667048375/1206017382069028/f

### Description
Notify user when sync is paused due collection limit or payload too
large.

### Steps to test this PR

_Feature 1_
- [ ] Fresh install
- [ ] Go to sync settings and create an account
- [ ] Ensure you don't see any warning or notification about sync paused
- [ ] Add a bookmark
- [ ] Patch should work, no rate limit
- [ ] Go to the code base, `RealSyncEngine:L186` replace Success logic
with:
```
            is Success -> {
                persisterPlugins.getPlugins().forEach {
                    it.onError(SyncErrorResponse(changes.type, COLLECTION_LIMIT_REACHED))
                }
                //persistChanges(result.data, conflictResolution)
            }
```
- [ ] install app with changes
- [ ] run the app
- [ ] Add a bookmark to trigger a Patch
- [ ] You should receive a system notification
- [ ] Go to sync settings, ensure warning is present
- [ ] Clicking on the warning should take you to Bookmarks Screen
- [ ] Undo the change you applied in `RealSyncEngine:L186`
- [ ] install app again
- [ ] on open, will trigger a patch again
- [ ] Notification and sync setting warning should be gone
- [ ] Ensure no warning is present

### UI changes
| Before  | After |
| ------ | ----- |
!(Upload before screenshot)|(Upload after screenshot)|
  • Loading branch information
cmonfortep authored Dec 1, 2023
1 parent a4de895 commit eee77e1
Show file tree
Hide file tree
Showing 48 changed files with 1,802 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import com.duckduckgo.savedsites.store.SavedSitesRelationsDao
import com.duckduckgo.sync.api.SyncCrypto
import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.Moshi
import kotlinx.coroutines.ExperimentalCoroutinesApi
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
Expand All @@ -58,6 +59,7 @@ import org.mockito.kotlin.*
import org.threeten.bp.OffsetDateTime
import org.threeten.bp.ZoneOffset

@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(AndroidJUnit4::class)
class SavedSitesSyncDataProviderTest {

Expand Down Expand Up @@ -115,7 +117,11 @@ class SavedSitesSyncDataProviderTest {
favoritesDelegate,
coroutinesTestRule.testDispatcherProvider,
)
store = RealSavedSitesSyncStore(InstrumentationRegistry.getInstrumentation().context)
store = RealSavedSitesSyncStore(
InstrumentationRegistry.getInstrumentation().context,
coroutinesTestRule.testScope,
coroutinesTestRule.testDispatcherProvider,
)

savedSitesFormFactorSyncMigration = RealSavedSitesFormFactorSyncMigration(
savedSitesEntitiesDao,
Expand Down
5 changes: 1 addition & 4 deletions autofill/autofill-impl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ plugins {

apply from: "$rootProject.projectDir/gradle/android-library.gradle"

android {
namespace 'com.duckduckgo.autofill.impl'
}

dependencies {
implementation project(path: ':app-build-config-api')
implementation project(path: ':privacy-config-api')
Expand Down Expand Up @@ -99,5 +95,6 @@ android {
includeAndroidResources = true
}
}
namespace 'com.duckduckgo.autofill.impl'
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (c) 2023 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.autofill.sync

import android.content.Context
import android.view.View
import com.duckduckgo.sync.api.SyncMessagePlugin
import javax.inject.Inject

class CredentialsRateLimitSyncMessagePlugin @Inject constructor() : SyncMessagePlugin {
override fun getView(context: Context): View {
return CredentialsRateLimitView(context)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (c) 2023 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.autofill.sync

import android.annotation.SuppressLint
import android.content.Context
import android.util.AttributeSet
import android.widget.FrameLayout
import androidx.core.view.isVisible
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.ViewTreeLifecycleOwner
import androidx.lifecycle.findViewTreeViewModelStoreOwner
import com.duckduckgo.anvil.annotations.InjectWith
import com.duckduckgo.autofill.api.AutofillScreens.AutofillSettingsScreenNoParams
import com.duckduckgo.autofill.impl.R
import com.duckduckgo.autofill.impl.databinding.ViewCredentialsRateLimitWarningBinding
import com.duckduckgo.autofill.sync.CredentialsRateLimitViewModel.Command
import com.duckduckgo.autofill.sync.CredentialsRateLimitViewModel.Command.NavigateToCredentials
import com.duckduckgo.autofill.sync.CredentialsRateLimitViewModel.ViewState
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.di.scopes.ViewScope
import com.duckduckgo.navigation.api.GlobalActivityStarter
import dagger.android.support.AndroidSupportInjection
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

@InjectWith(ViewScope::class)
class CredentialsRateLimitView @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null,
defStyle: Int = 0,
) : FrameLayout(context, attrs, defStyle) {

@Inject
lateinit var globalActivityStarter: GlobalActivityStarter

@Inject
lateinit var viewModelFactory: CredentialsRateLimitViewModel.Factory

private var coroutineScope: CoroutineScope? = null

private var job: ConflatedJob = ConflatedJob()

private val binding: ViewCredentialsRateLimitWarningBinding by viewBinding()

private val viewModel: CredentialsRateLimitViewModel by lazy {
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[CredentialsRateLimitViewModel::class.java]
}

override fun onAttachedToWindow() {
AndroidSupportInjection.inject(this)
super.onAttachedToWindow()

ViewTreeLifecycleOwner.get(this)?.lifecycle?.addObserver(viewModel)

configureViewListeners()

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

viewModel.viewState()
.onEach { render(it) }
.launchIn(coroutineScope!!)

job += viewModel.commands()
.onEach { processCommands(it) }
.launchIn(coroutineScope!!)
}

override fun onDetachedFromWindow() {
super.onDetachedFromWindow()
ViewTreeLifecycleOwner.get(this)?.lifecycle?.removeObserver(viewModel)
coroutineScope?.cancel()
job.cancel()
coroutineScope = null
}

private fun processCommands(command: Command) {
when (command) {
NavigateToCredentials -> navigateToCredentials()
}
}

private fun render(viewState: ViewState) {
this.isVisible = viewState.warningVisible
}

private fun configureViewListeners() {
binding.credentialsRateLimitWarning.setClickableLink(
WARNING_ACTION_ANNOTATION,
context.getText(R.string.credentials_limit_warning),
onClick = {
viewModel.onWarningActionClicked()
},
)
}

private fun navigateToCredentials() {
globalActivityStarter.start(this.context, AutofillSettingsScreenNoParams)
}

companion object {
const val WARNING_ACTION_ANNOTATION = "manage_logins"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright (c) 2023 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.autofill.sync

import android.annotation.SuppressLint
import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.viewModelScope
import com.duckduckgo.common.utils.DispatcherProvider
import javax.inject.Inject
import kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch

@SuppressLint("NoLifecycleObserver") // does not subscribe to app lifecycle
class CredentialsRateLimitViewModel(
private val credentialsSyncStore: CredentialsSyncStore,
private val dispatcherProvider: DispatcherProvider,
) : ViewModel(), DefaultLifecycleObserver {

data class ViewState(
val warningVisible: Boolean = false,
)

sealed class Command {
data object NavigateToCredentials : Command()
}

private val command = Channel<Command>(1, DROP_OLDEST)

fun viewState(): Flow<ViewState> = credentialsSyncStore.isSyncPausedFlow()
.map { syncPaused ->
ViewState(
warningVisible = syncPaused,
)
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), ViewState())

fun commands(): Flow<Command> = command.receiveAsFlow()

fun onWarningActionClicked() {
viewModelScope.launch {
command.send(Command.NavigateToCredentials)
}
}

@Suppress("UNCHECKED_CAST")
class Factory @Inject constructor(
private val credentialsSyncStore: CredentialsSyncStore,
private val dispatcherProvider: DispatcherProvider,
) : ViewModelProvider.NewInstanceFactory() {
override fun <T : ViewModel> create(modelClass: Class<T>): T {
return with(modelClass) {
when {
isAssignableFrom(CredentialsRateLimitViewModel::class.java) -> CredentialsRateLimitViewModel(
credentialsSyncStore,
dispatcherProvider,
)
else -> throw IllegalArgumentException("Unknown ViewModel class: ${modelClass.name}")
}
} as T
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.duckduckgo.common.utils.checkMainThread
import com.duckduckgo.di.DaggerMap
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.sync.api.engine.SyncChangesResponse
import com.duckduckgo.sync.api.engine.SyncErrorResponse
import com.duckduckgo.sync.api.engine.SyncMergeResult
import com.duckduckgo.sync.api.engine.SyncMergeResult.Success
import com.duckduckgo.sync.api.engine.SyncableDataPersister
Expand All @@ -43,8 +44,9 @@ class CredentialsSyncDataPersister @Inject constructor(
private val credentialsSyncStore: CredentialsSyncStore,
private val strategies: DaggerMap<SyncConflictResolution, CredentialsMergeStrategy>,
private val appBuildConfig: AppBuildConfig,
private val credentialsSyncFeatureListener: CredentialsSyncFeatureListener,
) : SyncableDataPersister {
override fun persist(
override fun onSuccess(
changes: SyncChangesResponse,
conflictResolution: SyncConflictResolution,
): SyncMergeResult {
Expand All @@ -53,6 +55,7 @@ class CredentialsSyncDataPersister @Inject constructor(
return if (changes.type == CREDENTIALS) {
Timber.d("Sync-autofill-Persist: received remote changes ${changes.jsonString}")
Timber.d("Sync-autofill-Persist: received remote changes, merging with resolution $conflictResolution")
credentialsSyncFeatureListener.onSuccess(changes)
val result = process(changes, conflictResolution)
Timber.d("Sync-autofill-Persist: merging credentials finished with $result")
return result
Expand All @@ -61,6 +64,12 @@ class CredentialsSyncDataPersister @Inject constructor(
}
}

override fun onError(error: SyncErrorResponse) {
if (error.type == CREDENTIALS) {
credentialsSyncFeatureListener.onError(error.featureSyncError)
}
}

private fun process(
changes: SyncChangesResponse,
conflictResolution: SyncConflictResolution,
Expand Down Expand Up @@ -122,6 +131,7 @@ class CredentialsSyncDataPersister @Inject constructor(
credentialsSyncStore.serverModifiedSince = "0"
credentialsSyncStore.startTimeStamp = "0"
credentialsSyncStore.clientModifiedSince = "0"
credentialsSyncFeatureListener.onSyncDisabled()
}

private class Adapters {
Expand Down
Loading

0 comments on commit eee77e1

Please sign in to comment.