Skip to content

Commit 8a1e333

Browse files
jmartinespjmartinesp
committed
Fix most review comments
1 parent d5eb71a commit 8a1e333

File tree

14 files changed

+136
-78
lines changed

14 files changed

+136
-78
lines changed

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/ResetIdentityFlowManager.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import io.element.android.libraries.matrix.api.encryption.IdentityResetHandle
2323
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
2424
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
2525
import kotlinx.coroutines.CoroutineScope
26+
import kotlinx.coroutines.Job
2627
import kotlinx.coroutines.flow.MutableStateFlow
2728
import kotlinx.coroutines.flow.StateFlow
2829
import kotlinx.coroutines.flow.filterIsInstance
@@ -37,9 +38,10 @@ class ResetIdentityFlowManager @Inject constructor(
3738
) {
3839
private val resetHandleFlow: MutableStateFlow<AsyncData<IdentityResetHandle>> = MutableStateFlow(AsyncData.Uninitialized)
3940
val currentHandleFlow: StateFlow<AsyncData<IdentityResetHandle>> = resetHandleFlow
41+
private var whenResetIsDoneWaitingJob: Job? = null
4042

4143
fun whenResetIsDone(block: () -> Unit) {
42-
sessionCoroutineScope.launch {
44+
whenResetIsDoneWaitingJob = sessionCoroutineScope.launch {
4345
sessionVerificationService.sessionVerifiedStatus.filterIsInstance<SessionVerifiedStatus.Verified>().first()
4446
block()
4547
}
@@ -70,5 +72,8 @@ class ResetIdentityFlowManager @Inject constructor(
7072
suspend fun cancel() {
7173
currentHandleFlow.value.dataOrNull()?.cancel()
7274
resetHandleFlow.value = AsyncData.Uninitialized
75+
76+
whenResetIsDoneWaitingJob?.cancel()
77+
whenResetIsDoneWaitingJob = null
7378
}
7479
}

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/ResetIdentityFlowNode.kt‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import androidx.compose.runtime.collectAsState
2323
import androidx.compose.runtime.getValue
2424
import androidx.compose.ui.Modifier
2525
import androidx.compose.ui.platform.LocalContext
26+
import androidx.compose.ui.window.DialogProperties
2627
import androidx.lifecycle.DefaultLifecycleObserver
2728
import androidx.lifecycle.LifecycleOwner
2829
import com.bumble.appyx.core.modality.BuildContext
@@ -45,14 +46,13 @@ import io.element.android.libraries.designsystem.components.ProgressDialog
4546
import io.element.android.libraries.di.SessionScope
4647
import io.element.android.libraries.matrix.api.encryption.IdentityOidcResetHandle
4748
import io.element.android.libraries.matrix.api.encryption.IdentityPasswordResetHandle
48-
import io.element.android.libraries.matrix.api.encryption.IdentityResetHandle
4949
import io.element.android.libraries.oidc.api.OidcEntryPoint
5050
import kotlinx.coroutines.CoroutineScope
5151
import kotlinx.coroutines.Job
52-
import kotlinx.coroutines.flow.filterIsInstance
53-
import kotlinx.coroutines.flow.first
52+
import kotlinx.coroutines.flow.collectLatest
5453
import kotlinx.coroutines.launch
5554
import kotlinx.parcelize.Parcelize
55+
import timber.log.Timber
5656

5757
@ContributesNode(SessionScope::class)
5858
class ResetIdentityFlowNode @AssistedInject constructor(
@@ -94,7 +94,7 @@ class ResetIdentityFlowNode @AssistedInject constructor(
9494
lifecycle.addObserver(object : DefaultLifecycleObserver {
9595
override fun onStart(owner: LifecycleOwner) {
9696
// If the custom tab was opened, we need to cancel the reset job
97-
// when we come back to the node if it the reset wasn't successful
97+
// when we come back to the node if the reset wasn't successful
9898
cancelResetJob()
9999
}
100100

@@ -129,22 +129,29 @@ class ResetIdentityFlowNode @AssistedInject constructor(
129129
}
130130

131131
private fun CoroutineScope.startReset() = launch {
132-
val handle = resetIdentityFlowManager.getResetHandle()
133-
.filterIsInstance<AsyncData.Success<IdentityResetHandle>>()
134-
.first()
135-
.data
136-
137-
when (handle) {
138-
is IdentityOidcResetHandle -> {
139-
if (oidcEntryPoint.canUseCustomTab()) {
140-
activity.openUrlInChromeCustomTab(null, false, handle.url)
141-
} else {
142-
backstack.push(NavTarget.ResetOidc(handle.url))
132+
resetIdentityFlowManager.getResetHandle()
133+
.collectLatest { state ->
134+
when (state) {
135+
is AsyncData.Failure -> {
136+
cancelResetJob()
137+
Timber.e(state.error, "Could not load the reset identity handle.")
138+
}
139+
is AsyncData.Success -> {
140+
when (val handle = state.data) {
141+
is IdentityOidcResetHandle -> {
142+
if (oidcEntryPoint.canUseCustomTab()) {
143+
activity.openUrlInChromeCustomTab(null, false, handle.url)
144+
} else {
145+
backstack.push(NavTarget.ResetOidc(handle.url))
146+
}
147+
resetJob = launch { handle.resetOidc() }
148+
}
149+
is IdentityPasswordResetHandle -> backstack.push(NavTarget.ResetPassword)
150+
}
151+
}
152+
else -> Unit
143153
}
144-
resetJob = launch { handle.resetOidc() }
145154
}
146-
is IdentityPasswordResetHandle -> backstack.push(NavTarget.ResetPassword)
147-
}
148155
}
149156

150157
private fun cancelResetJob() {
@@ -162,7 +169,10 @@ class ResetIdentityFlowNode @AssistedInject constructor(
162169

163170
val startResetState by resetIdentityFlowManager.currentHandleFlow.collectAsState()
164171
if (startResetState.isLoading()) {
165-
ProgressDialog()
172+
ProgressDialog(
173+
properties = DialogProperties(dismissOnBackPress = true, dismissOnClickOutside = true),
174+
onDismissRequest = { cancelResetJob() }
175+
)
166176
}
167177

168178
BackstackView(modifier)

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/password/ResetIdentityPasswordNode.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ import io.element.android.libraries.matrix.api.encryption.IdentityPasswordResetH
3434
class ResetIdentityPasswordNode @AssistedInject constructor(
3535
@Assisted buildContext: BuildContext,
3636
@Assisted plugins: List<Plugin>,
37-
private val coroutineDispatchers: CoroutineDispatchers,
37+
coroutineDispatchers: CoroutineDispatchers,
3838
) : Node(buildContext, plugins = plugins) {
3939
data class Inputs(val handle: IdentityPasswordResetHandle) : NodeInputs
4040

41-
private val presenter by lazy {
42-
val inputs = inputs<Inputs>()
43-
ResetIdentityPasswordPresenter(inputs.handle, dispatchers = coroutineDispatchers)
44-
}
41+
private val presenter = ResetIdentityPasswordPresenter(
42+
identityPasswordResetHandle = inputs<Inputs>().handle,
43+
dispatchers = coroutineDispatchers
44+
)
4545

4646
@Composable
4747
override fun View(modifier: Modifier) {

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/password/ResetIdentityPasswordStateProvider.kt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (c) 2024 New Vector Ltd
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.element.android.features.securebackup.impl.reset.password
18+
19+
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
20+
import io.element.android.libraries.architecture.AsyncAction
21+
22+
class ResetIdentityPasswordStateProvider : PreviewParameterProvider<ResetIdentityPasswordState> {
23+
override val values: Sequence<ResetIdentityPasswordState>
24+
get() = sequenceOf(
25+
aResetIdentityPasswordState(),
26+
aResetIdentityPasswordState(resetAction = AsyncAction.Loading),
27+
aResetIdentityPasswordState(resetAction = AsyncAction.Success(Unit)),
28+
aResetIdentityPasswordState(resetAction = AsyncAction.Failure(IllegalStateException("Failed"))),
29+
)
30+
}
31+
32+
private fun aResetIdentityPasswordState(
33+
resetAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
34+
eventSink: (ResetIdentityPasswordEvent) -> Unit = {},
35+
) = ResetIdentityPasswordState(
36+
resetAction = resetAction,
37+
eventSink = eventSink,
38+
)

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/password/ResetIdentityPasswordView.kt‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ import androidx.compose.ui.platform.LocalFocusManager
2727
import androidx.compose.ui.res.stringResource
2828
import androidx.compose.ui.text.input.PasswordVisualTransformation
2929
import androidx.compose.ui.text.input.VisualTransformation
30+
import androidx.compose.ui.tooling.preview.PreviewParameter
3031
import io.element.android.compound.tokens.generated.CompoundIcons
31-
import io.element.android.libraries.architecture.AsyncAction
32+
import io.element.android.features.securebackup.impl.R
3233
import io.element.android.libraries.designsystem.atomic.pages.FlowStepPage
3334
import io.element.android.libraries.designsystem.components.BigIcon
3435
import io.element.android.libraries.designsystem.components.ProgressDialog
35-
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
3636
import io.element.android.libraries.designsystem.components.form.textFieldState
3737
import io.element.android.libraries.designsystem.preview.ElementPreview
3838
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
@@ -54,13 +54,19 @@ fun ResetIdentityPasswordView(
5454
FlowStepPage(
5555
modifier = modifier,
5656
iconStyle = BigIcon.Style.Default(CompoundIcons.LockSolid()),
57-
title = stringResource(CommonStrings.screen_reset_encryption_password_title),
58-
subTitle = stringResource(CommonStrings.screen_reset_encryption_password_subtitle),
57+
title = stringResource(R.string.screen_reset_encryption_password_title),
58+
subTitle = stringResource(R.string.screen_reset_encryption_password_subtitle),
5959
onBackClick = onBack,
6060
content = {
6161
Content(
6262
text = passwordState.value,
63-
onTextChange = { passwordState.value = it }
63+
onTextChange = { newText ->
64+
if (state.resetAction.isFailure()) {
65+
state.eventSink(ResetIdentityPasswordEvent.DismissError)
66+
}
67+
passwordState.value = newText
68+
},
69+
hasError = state.resetAction.isFailure(),
6470
)
6571
},
6672
buttons = {
@@ -69,22 +75,19 @@ fun ResetIdentityPasswordView(
6975
text = stringResource(CommonStrings.action_reset_identity),
7076
onClick = { state.eventSink(ResetIdentityPasswordEvent.Reset(passwordState.value)) },
7177
destructive = true,
78+
enabled = passwordState.value.isNotEmpty(),
7279
)
7380
}
7481
)
7582

83+
// On success we need to wait until the screen is automatically dismissed, so we keep the progress dialog
7684
if (state.resetAction.isLoading() || state.resetAction.isSuccess()) {
7785
ProgressDialog()
78-
} else if (state.resetAction.isFailure()) {
79-
ErrorDialog(
80-
content = stringResource(CommonStrings.error_unknown),
81-
onDismiss = { state.eventSink(ResetIdentityPasswordEvent.DismissError) }
82-
)
8386
}
8487
}
8588

8689
@Composable
87-
private fun Content(text: String, onTextChange: (String) -> Unit) {
90+
private fun Content(text: String, onTextChange: (String) -> Unit, hasError: Boolean) {
8891
var showPassword by remember { mutableStateOf(false) }
8992
OutlinedTextField(
9093
modifier = Modifier
@@ -93,7 +96,7 @@ private fun Content(text: String, onTextChange: (String) -> Unit) {
9396
value = text,
9497
onValueChange = onTextChange,
9598
label = { Text(stringResource(CommonStrings.common_password)) },
96-
placeholder = { Text(stringResource(CommonStrings.screen_reset_encryption_password_placeholder)) },
99+
placeholder = { Text(stringResource(R.string.screen_reset_encryption_password_placeholder)) },
97100
singleLine = true,
98101
visualTransformation = if (showPassword) VisualTransformation.None else PasswordVisualTransformation(),
99102
trailingIcon = {
@@ -105,19 +108,22 @@ private fun Content(text: String, onTextChange: (String) -> Unit) {
105108
IconButton(onClick = { showPassword = !showPassword }) {
106109
Icon(imageVector = image, description)
107110
}
111+
},
112+
isError = hasError,
113+
supportingText = if (hasError) {
114+
{ Text(stringResource(R.string.screen_reset_encryption_password_error)) }
115+
} else {
116+
null
108117
}
109118
)
110119
}
111120

112121
@PreviewsDayNight
113122
@Composable
114-
internal fun ResetIdentityPasswordViewPreview() {
123+
internal fun ResetIdentityPasswordViewPreview(@PreviewParameter(ResetIdentityPasswordStateProvider::class) state: ResetIdentityPasswordState) {
115124
ElementPreview {
116125
ResetIdentityPasswordView(
117-
state = ResetIdentityPasswordState(
118-
resetAction = AsyncAction.Uninitialized,
119-
eventSink = {}
120-
),
126+
state = state,
121127
onBack = {}
122128
)
123129
}

‎features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/reset/root/ResetIdentityRootView.kt‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
2929
import androidx.compose.ui.unit.dp
3030
import io.element.android.compound.theme.ElementTheme
3131
import io.element.android.compound.tokens.generated.CompoundIcons
32+
import io.element.android.features.securebackup.impl.R
3233
import io.element.android.libraries.designsystem.atomic.organisms.InfoListItem
3334
import io.element.android.libraries.designsystem.atomic.organisms.InfoListOrganism
3435
import io.element.android.libraries.designsystem.atomic.pages.FlowStepPage
@@ -52,8 +53,8 @@ fun ResetIdentityRootView(
5253
FlowStepPage(
5354
modifier = modifier,
5455
iconStyle = BigIcon.Style.AlertSolid,
55-
title = stringResource(io.element.android.libraries.ui.strings.R.string.screen_encryption_reset_title),
56-
subTitle = stringResource(io.element.android.libraries.ui.strings.R.string.screen_encryption_reset_subtitle),
56+
title = stringResource(R.string.screen_encryption_reset_title),
57+
subTitle = stringResource(R.string.screen_encryption_reset_subtitle),
5758
isScrollable = true,
5859
content = { Content() },
5960
buttons = {
@@ -69,9 +70,9 @@ fun ResetIdentityRootView(
6970

7071
if (state.displayConfirmationDialog) {
7172
ConfirmationDialog(
72-
title = stringResource(CommonStrings.screen_reset_encryption_confirmation_alert_title),
73-
content = stringResource(CommonStrings.screen_reset_encryption_confirmation_alert_subtitle),
74-
submitText = stringResource(CommonStrings.screen_reset_encryption_confirmation_alert_action),
73+
title = stringResource(R.string.screen_reset_encryption_confirmation_alert_title),
74+
content = stringResource(R.string.screen_reset_encryption_confirmation_alert_subtitle),
75+
submitText = stringResource(R.string.screen_reset_encryption_confirmation_alert_action),
7576
onSubmitClick = {
7677
state.eventSink(ResetIdentityRootEvent.DismissDialog)
7778
onContinue()
@@ -92,7 +93,7 @@ private fun Content() {
9293
modifier = Modifier.fillMaxWidth(),
9394
items = persistentListOf(
9495
InfoListItem(
95-
message = stringResource(CommonStrings.screen_encryption_reset_bullet_1),
96+
message = stringResource(R.string.screen_encryption_reset_bullet_1),
9697
iconComposable = {
9798
Icon(
9899
modifier = Modifier.size(20.dp),
@@ -103,7 +104,7 @@ private fun Content() {
103104
},
104105
),
105106
InfoListItem(
106-
message = stringResource(CommonStrings.screen_encryption_reset_bullet_2),
107+
message = stringResource(R.string.screen_encryption_reset_bullet_2),
107108
iconComposable = {
108109
Icon(
109110
modifier = Modifier.size(20.dp),
@@ -114,7 +115,7 @@ private fun Content() {
114115
},
115116
),
116117
InfoListItem(
117-
message = stringResource(CommonStrings.screen_encryption_reset_bullet_3),
118+
message = stringResource(R.string.screen_encryption_reset_bullet_3),
118119
iconComposable = {
119120
Icon(
120121
modifier = Modifier.size(20.dp),
@@ -130,7 +131,7 @@ private fun Content() {
130131

131132
Text(
132133
modifier = Modifier.fillMaxWidth(),
133-
text = stringResource(CommonStrings.screen_encryption_reset_footer),
134+
text = stringResource(R.string.screen_encryption_reset_footer),
134135
style = ElementTheme.typography.fontBodyMdMedium,
135136
color = ElementTheme.colors.textActionPrimary,
136137
textAlign = TextAlign.Center,

‎features/securebackup/impl/src/main/res/values/localazy.xml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
<string name="screen_create_new_recovery_key_list_item_4">"Follow the instructions to create a new recovery key"</string>
1717
<string name="screen_create_new_recovery_key_list_item_5">"Save your new recovery key in a password manager or encrypted note"</string>
1818
<string name="screen_create_new_recovery_key_title">"Reset the encryption for your account using another device"</string>
19+
<string name="screen_encryption_reset_bullet_1">"Your account details, contacts, preferences, and chat list will be kept"</string>
20+
<string name="screen_encryption_reset_bullet_2">"You will lose your existing message history"</string>
21+
<string name="screen_encryption_reset_bullet_3">"You will need to verify all your existing devices and contacts again"</string>
22+
<string name="screen_encryption_reset_footer">"Only reset your identity if you don’t have access to another signed-in device and you’ve lost your recovery key."</string>
23+
<string name="screen_encryption_reset_subtitle">"If you’re not signed in to any other devices and you’ve lost your recovery key, then you’ll need to reset your identity to continue using the app. "</string>
24+
<string name="screen_encryption_reset_title">"Reset your identity in case you can’t confirm another way"</string>
1925
<string name="screen_key_backup_disable_confirmation_action_turn_off">"Turn off"</string>
2026
<string name="screen_key_backup_disable_confirmation_description">"You will lose your encrypted messages if you are signed out of all devices."</string>
2127
<string name="screen_key_backup_disable_confirmation_title">"Are you sure you want to turn off backup?"</string>
@@ -51,4 +57,11 @@
5157
<string name="screen_recovery_key_setup_generate_key_description">"Make sure you can store your recovery key somewhere safe"</string>
5258
<string name="screen_recovery_key_setup_success">"Recovery setup successful"</string>
5359
<string name="screen_recovery_key_setup_title">"Set up recovery"</string>
60+
<string name="screen_reset_encryption_confirmation_alert_action">"Yes, reset now"</string>
61+
<string name="screen_reset_encryption_confirmation_alert_subtitle">"This process is irreversible."</string>
62+
<string name="screen_reset_encryption_confirmation_alert_title">"Are you sure you want to reset your encryption?"</string>
63+
<string name="screen_reset_encryption_password_error">"An unknown error happened. Please check your account password is correct and try again."</string>
64+
<string name="screen_reset_encryption_password_placeholder">"Enter…"</string>
65+
<string name="screen_reset_encryption_password_subtitle">"Confirm that you want to reset your encryption."</string>
66+
<string name="screen_reset_encryption_password_title">"Enter your account password to continue"</string>
5467
</resources>

‎features/securebackup/impl/src/test/kotlin/io/element/android/features/securebackup/impl/reset/password/ResetIdentityPasswordViewTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,12 @@ class ResetIdentityPasswordViewTest {
7676
}
7777

7878
@Test
79-
fun `clicking OK dismisses the error dialog`() {
79+
fun `modifying the password dismisses the error state`() {
8080
val eventsRecorder = EventsRecorder<ResetIdentityPasswordEvent>()
8181
rule.setResetPasswordView(
8282
ResetIdentityPasswordState(resetAction = AsyncAction.Failure(IllegalStateException("A failure")), eventSink = eventsRecorder),
8383
)
84-
rule.clickOn(CommonStrings.action_ok)
84+
rule.onNodeWithText("Password").performTextInput("A password")
8585

8686
eventsRecorder.assertSingle(ResetIdentityPasswordEvent.DismissError)
8787
}

0 commit comments

Comments
 (0)