Feature flag sync #4019
Feature flag sync #4019
Conversation
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
cmonfortep
force-pushed
the
feature/cristian/sync/feature_flag
branch
4 times, most recently
from
53c5f73 to
0937e7d
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/feature_flag
branch
from
0937e7d to
0a136c4
Compare
There was a problem hiding this comment.
besides what we’ve seen in the review task, this is great. Good job @cmonfortep !
| fun self(): Toggle | ||
|
|
||
| @Toggle.DefaultValue(true) | ||
| fun level0ShowSync(): Toggle |
There was a problem hiding this comment.
nit: remove level prefix, the toggle itself doesn’t need it.
There was a problem hiding this comment.
needs to follow same name as in privacy config, so we can't change that.
| @@ -61,7 +61,7 @@ class RealSyncEngine @Inject constructor( | |||
|
|
|||
| override fun triggerSync(trigger: SyncTrigger) { | |||
| Timber.i("Sync-Engine: petition to sync now trigger: $trigger") | |||
| if (syncStore.isSignedIn()) { | |||
| if (syncStore.isSignedIn() && syncStore.syncingDataEnabled) { | |||
There was a problem hiding this comment.
why not have both checks inside the method in the store?
syncingDataEnabled can check internally for the flags and the signedin state.
There was a problem hiding this comment.
We could do that, but we still need to keep isSignedIn and syncingDataEnabled for other usages, so we are not having any extra benefit. Will leave it like this for now.
sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/engine/SyncNotificationBuilder.kt
Show resolved
Hide resolved
| ) : ViewModel(), DefaultLifecycleObserver { | ||
|
|
||
| data class ViewState( | ||
| val warningMessage: Int? = null, |
There was a problem hiding this comment.
nit
| val warningMessage: Int? = null, | |
| val message: Int? = null, |
cmonfortep
force-pushed
the
feature/cristian/sync/feature_flag
branch
from
0a136c4 to
fa5929a
Compare
Task/Issue URL: https://app.asana.com/0/488551667048375/1206046777189412/f
Description
Add Remote flags around sync.
Steps to test this PR
https://app.asana.com/0/0/1206190385973972/f
UI changes