Secure backup: create a feature flag (disabled) #1716
Conversation
bmarty
force-pushed
the
feature/bma/secureBackupFeatureFlag
branch
from
e0c3546
to
c52a50d
Compare
| @@ -47,8 +52,15 @@ class LogoutPresenter @Inject constructor( | |||
| mutableStateOf(Async.Uninitialized) | |||
| } | |||
|
|
|||
| val backupUploadState: BackupUploadState by remember { | |||
| encryptionService.waitForBackupUploadSteadyState() | |||
| val secureStorageFlag by featureFlagService.isFeatureEnabledFlow(FeatureFlags.SecureStorage) | |||
There was a problem hiding this comment.
Is setting to false for initial value not enough?
Otherwise we might want to use an Async so we are sure we don't do anything while it's not Success.
There was a problem hiding this comment.
If we set to false, it's false then true, which is not ideal, both for test (more intermediate states) and for prod.
I am looking how to improve this with Async.
There was a problem hiding this comment.
Actually, I start with a null value, and it seems to be OK (still testing it)
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
| if (secureStorageFlag) { | ||
| encryptionService.waitForBackupUploadSteadyState() | ||
| } else { | ||
| flowOf(BackupUploadState.Done) |
There was a problem hiding this comment.
Actually, I'd let the EncryptionService handle this, so we don't have to check that in Presenters.
There was a problem hiding this comment.
I thought about doing like this, but I prefer to avoid this, since we had to give the FeatureFlagService to the SDK which is a bit strange. We may iterate on this later.
There was a problem hiding this comment.
Our matrix module is not really a sdk. I see what you mean, but it's not really the responsability of the Presenter neither :/
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.runBlocking | ||
| import javax.inject.Inject | ||
|
|
||
| class LogoutPresenter @Inject constructor( | ||
| private val matrixClient: MatrixClient, | ||
| private val encryptionService: EncryptionService, |
There was a problem hiding this comment.
Not related to this PR, but could we change the EncryptionService to something else? Maybe MatrixEncryptionService?
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #1716 +/- ##
===========================================
- Coverage 63.43% 63.43% -0.01%
===========================================
Files 1282 1279 -3
Lines 33247 33184 -63
Branches 6883 6865 -18
===========================================
- Hits 21090 21050 -40
+ Misses 8975 8959 -16
+ Partials 3182 3175 -7
☔ View full report in Codecov by Sentry. |
bmarty
force-pushed
the
feature/bma/secureBackupFeatureFlag
branch
from
c52a50d
to
d09d95b
Compare
bmarty
added
the
Z-NextRelease
There was a problem hiding this comment.
LGTM, since I think @ganfra isn't around at the moment and I think everything important is fixed. We can iterate later.
|
Kudos, SonarCloud Quality Gate passed!
|
Type of change
Content
Add a feature flag for chat backup feature, disabled by default.
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist