Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Jetpack Setup] Use Magic Link for suspicious emails #13409

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from
+322 −74
Open

[Jetpack Setup] Use Magic Link for suspicious emails #13409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ab7ca90
Remove unused argument
hichamboushaba Jan 27, 2025
22170a7
Navigate to magic link screen when email/password is not allowed
hichamboushaba Jan 27, 2025
625a017
Update unit test
hichamboushaba Jan 27, 2025
05b91fe
Update navigation logic to allow for different fallback button types
hichamboushaba Jan 27, 2025
911a2e0
Update fallback button text depending on use case
hichamboushaba Jan 27, 2025
ab371ba
Don't send magic link automatically at start for suspicious emails
hichamboushaba Jan 27, 2025
ede70fe
Handle navigation for fallback magic link actions
hichamboushaba Jan 27, 2025
0a3860d
Update login screen logic to work with username mode
hichamboushaba Jan 27, 2025
3801a1e
Add unit tests
hichamboushaba Jan 27, 2025
9653e9c
Fix detekt issues
hichamboushaba Jan 28, 2025
2f12442
Add a release note
hichamboushaba Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*** For entries which are touching the Android Wear app's, start entry with `[WEAR]` too.
21.6
-----
- [*] [Internal] Improved login flow for accounts using emails marked as suspicious during Jetpack Setup [https://github.com/woocommerce/woocommerce-android/pull/13409]


21.5
Expand Down
22 changes: 22 additions & 0 deletions ...m/woocommerce/android/ui/login/jetpack/wpcom/JetpackActivationMagicLinkRequestFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class JetpackActivationMagicLinkRequestFragment : BaseFragment() {
viewModel.event.observe(viewLifecycleOwner) { event ->
when (event) {
is OpenEmailClient -> openEmailClient()
is JetpackActivationMagicLinkRequestViewModel.ShowPasswordScreen -> openPasswordScreen(event)
is JetpackActivationMagicLinkRequestViewModel.ShowUsernameScreen -> openUsernameScreen(event)
is ShowSnackbar -> uiMessageResolver.showSnack(event.message)
is Exit -> findNavController().navigateUp()
}
Expand All @@ -63,4 +65,24 @@ class JetpackActivationMagicLinkRequestFragment : BaseFragment() {
uiMessageResolver.showSnack(R.string.login_email_client_not_found)
}
}

private fun openPasswordScreen(event: JetpackActivationMagicLinkRequestViewModel.ShowPasswordScreen) {
findNavController().navigate(
JetpackActivationMagicLinkRequestFragmentDirections
.actionJetpackActivationMagicLinkRequestFragmentToJetpackActivationWPComPasswordFragment(
emailOrUsername = event.emailOrUsername,
jetpackStatus = event.jetpackStatus
)
)
}

private fun openUsernameScreen(event: JetpackActivationMagicLinkRequestViewModel.ShowUsernameScreen) {
findNavController().navigate(
JetpackActivationMagicLinkRequestFragmentDirections
.actionJetpackActivationMagicLinkRequestFragmentToJetpackActivationWPComEmailFragment(
usernameOnly = true,
jetpackStatus = event.jetpackStatus
)
)
}
}
24 changes: 15 additions & 9 deletions ...com/woocommerce/android/ui/login/jetpack/wpcom/JetpackActivationMagicLinkRequestScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.woocommerce.android.ui.compose.component.WCOutlinedButton
import com.woocommerce.android.ui.compose.theme.WooThemeWithBackground
import com.woocommerce.android.ui.login.jetpack.components.JetpackToWooHeader
import com.woocommerce.android.ui.login.jetpack.components.UserInfo
import org.wordpress.android.login.MagicLinkFallbackButton

@Composable
fun JetpackActivationMagicLinkRequestScreen(viewModel: JetpackActivationMagicLinkRequestViewModel) {
Expand All @@ -40,7 +41,7 @@ fun JetpackActivationMagicLinkRequestScreen(viewModel: JetpackActivationMagicLin
onCloseClick = viewModel::onCloseClick,
onRequestMagicLinkClick = viewModel::onRequestMagicLinkClick,
onOpenEmailClientClick = viewModel::onOpenEmailClientClick,
onUsePasswordClick = viewModel::onUsePasswordClick
onFallbackButtonClick = viewModel::onFallbackButtonClick
)
}
}
Expand All @@ -51,7 +52,7 @@ fun JetpackActivationMagicLinkRequestScreen(
onCloseClick: () -> Unit = {},
onRequestMagicLinkClick: () -> Unit = {},
onOpenEmailClientClick: () -> Unit = {},
onUsePasswordClick: () -> Unit = {}
onFallbackButtonClick: () -> Unit = {}
) {
Scaffold(
topBar = {
Expand Down Expand Up @@ -91,12 +92,18 @@ fun JetpackActivationMagicLinkRequestScreen(
MagicLinkSentContent(viewState, onOpenEmailClientClick, Modifier.weight(1f))
}
}
if (viewState.allowPasswordLogin) {

val fallbackButtonText = when (viewState.magicLinkFallbackButton) {
MagicLinkFallbackButton.Password -> R.string.enter_your_password_instead
MagicLinkFallbackButton.UsernameAndPassword -> R.string.login_use_wpcom_username_instead
MagicLinkFallbackButton.None -> null
}
Comment on lines +96 to +100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor suggestion: We can move this logic to the view model. So, the ViewState can only have fallbackButtonText.

fallbackButtonText?.let {
WCOutlinedButton(
onClick = onUsePasswordClick,
onClick = onFallbackButtonClick,
modifier = Modifier.fillMaxWidth()
) {
Text(text = stringResource(id = R.string.enter_your_password_instead))
Text(text = stringResource(id = it))
}
}
}
Expand Down Expand Up @@ -214,9 +221,8 @@ private fun MagicLinkRequestPreview() {
emailOrUsername = "test@email.com",
avatarUrl = "avatar",
isJetpackInstalled = false,
allowPasswordLogin = true,
isLoadingDialogShown = false,
isNewWpComAccount = false
magicLinkFallbackButton = MagicLinkFallbackButton.Password,
isLoadingDialogShown = false
)
)
}
Expand All @@ -230,7 +236,7 @@ private fun MagicLinkSentPreview() {
viewState = JetpackActivationMagicLinkRequestViewModel.ViewState.MagicLinkSentState(
email = null,
isJetpackInstalled = false,
allowPasswordLogin = true,
magicLinkFallbackButton = MagicLinkFallbackButton.Password,
)
)
}
Expand Down
47 changes: 32 additions & 15 deletions .../woocommerce/android/ui/login/jetpack/wpcom/JetpackActivationMagicLinkRequestViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.woocommerce.android.R
import com.woocommerce.android.analytics.AnalyticsEvent.JETPACK_SETUP_LOGIN_FLOW
import com.woocommerce.android.analytics.AnalyticsTracker
import com.woocommerce.android.analytics.AnalyticsTrackerWrapper
import com.woocommerce.android.model.JetpackStatus
import com.woocommerce.android.ui.login.MagicLinkFlow
import com.woocommerce.android.ui.login.MagicLinkSource
import com.woocommerce.android.ui.login.WPComLoginRepository
Expand All @@ -27,6 +28,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize
import org.wordpress.android.login.MagicLinkFallbackButton
import javax.inject.Inject

@HiltViewModel
Expand All @@ -45,15 +47,16 @@ class JetpackActivationMagicLinkRequestViewModel @Inject constructor(
emailOrUsername = navArgs.emailOrUsername,
avatarUrl = avatarUrlFromEmail(navArgs.emailOrUsername),
isJetpackInstalled = navArgs.jetpackStatus.isJetpackInstalled,
allowPasswordLogin = !navArgs.isAccountPasswordless,
isLoadingDialogShown = false,
isNewWpComAccount = navArgs.isNewWpComAccount
magicLinkFallbackButton = navArgs.fallbackButton,
isLoadingDialogShown = false
)
)
val viewState = _viewState.asLiveData()

init {
requestMagicLink()
if (navArgs.requestAtStart) {
requestMagicLink()
}
}

fun onRequestMagicLinkClick() = requestMagicLink()
Expand All @@ -62,8 +65,17 @@ class JetpackActivationMagicLinkRequestViewModel @Inject constructor(
triggerEvent(OpenEmailClient)
}

fun onUsePasswordClick() {
triggerEvent(Exit)
fun onFallbackButtonClick() {
when (navArgs.fallbackButton) {
MagicLinkFallbackButton.Password -> triggerEvent(
ShowPasswordScreen(
emailOrUsername = navArgs.emailOrUsername,
jetpackStatus = navArgs.jetpackStatus
)
)
MagicLinkFallbackButton.UsernameAndPassword -> triggerEvent(ShowUsernameScreen(navArgs.jetpackStatus))
MagicLinkFallbackButton.None -> error("No fallback button should be shown")
}
}

fun onCloseClick() {
Expand Down Expand Up @@ -93,9 +105,8 @@ class JetpackActivationMagicLinkRequestViewModel @Inject constructor(
emailOrUsername = navArgs.emailOrUsername,
avatarUrl = avatarUrlFromEmail(navArgs.emailOrUsername),
isJetpackInstalled = navArgs.jetpackStatus.isJetpackInstalled,
allowPasswordLogin = !navArgs.isAccountPasswordless,
isLoadingDialogShown = true,
isNewWpComAccount = navArgs.isNewWpComAccount
magicLinkFallbackButton = navArgs.fallbackButton,
isLoadingDialogShown = true
)
val source = when {
!navArgs.jetpackStatus.isJetpackInstalled -> MagicLinkSource.JetpackInstallation
Expand All @@ -112,7 +123,7 @@ class JetpackActivationMagicLinkRequestViewModel @Inject constructor(
_viewState.value = ViewState.MagicLinkSentState(
email = navArgs.emailOrUsername.takeIf { it.isAnEmail() },
isJetpackInstalled = navArgs.jetpackStatus.isJetpackInstalled,
allowPasswordLogin = !navArgs.isAccountPasswordless,
magicLinkFallbackButton = navArgs.fallbackButton,
)
},
onFailure = {
Expand Down Expand Up @@ -143,25 +154,31 @@ class JetpackActivationMagicLinkRequestViewModel @Inject constructor(

sealed interface ViewState : Parcelable {
val isJetpackInstalled: Boolean
val allowPasswordLogin: Boolean
val magicLinkFallbackButton: MagicLinkFallbackButton

@Parcelize
data class MagicLinkRequestState(
val emailOrUsername: String,
val avatarUrl: String,
override val isJetpackInstalled: Boolean,
override val allowPasswordLogin: Boolean,
val isLoadingDialogShown: Boolean,
val isNewWpComAccount: Boolean
override val magicLinkFallbackButton: MagicLinkFallbackButton,
val isLoadingDialogShown: Boolean
) : ViewState

@Parcelize
data class MagicLinkSentState(
val email: String?,
override val isJetpackInstalled: Boolean,
override val allowPasswordLogin: Boolean
override val magicLinkFallbackButton: MagicLinkFallbackButton
) : ViewState
}

object OpenEmailClient : MultiLiveEvent.Event()

data class ShowPasswordScreen(
val emailOrUsername: String,
val jetpackStatus: JetpackStatus
) : MultiLiveEvent.Event()

data class ShowUsernameScreen(val jetpackStatus: JetpackStatus) : MultiLiveEvent.Event()
}
3 changes: 2 additions & 1 deletion ...lin/com/woocommerce/android/ui/login/jetpack/wpcom/JetpackActivationWPComEmailFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ class JetpackActivationWPComEmailFragment : BaseFragment() {
.actionJetpackActivationWPComEmailFragmentToJetpackActivationMagicLinkRequestFragment(
emailOrUsername = event.emailOrUsername,
jetpackStatus = event.jetpackStatus,
isAccountPasswordless = true,
fallbackButton = event.magicLinkFallbackButton,
requestAtStart = event.requestAtStart,
isNewWpComAccount = event.isNewWpComAccount
)
)
Expand Down
17 changes: 12 additions & 5 deletions ...otlin/com/woocommerce/android/ui/login/jetpack/wpcom/JetpackActivationWPComEmailScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ fun JetpackActivationWPComEmailScreen(
WCOutlinedTextField(
value = viewState.emailOrUsername,
onValueChange = onEmailChanged,
label = stringResource(id = R.string.email_or_username),
label = if (viewState.usernameOnly) {
stringResource(R.string.username)
} else {
stringResource(id = R.string.email_or_username)
},
isError = viewState.errorMessage != null,
helperText = viewState.errorMessage?.let { stringResource(id = it) },
singleLine = true,
Expand All @@ -114,10 +118,12 @@ fun JetpackActivationWPComEmailScreen(
)
)
Spacer(modifier = Modifier.height(dimensionResource(id = R.dimen.major_100)))
Text(
style = MaterialTheme.typography.body2,
text = stringResource(id = R.string.login_jetpack_connection_create_account)
)
if (!viewState.usernameOnly) {
Text(
style = MaterialTheme.typography.body2,
text = stringResource(id = R.string.login_jetpack_connection_create_account)
)
}
}

Spacer(modifier = Modifier.weight(1f))
Expand Down Expand Up @@ -162,6 +168,7 @@ private fun JetpackActivationWPComScreenPreview() {
WooThemeWithBackground {
JetpackActivationWPComEmailScreen(
viewState = JetpackActivationWPComEmailViewModel.ViewState(
usernameOnly = false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct me if I'm mistaken. We were using this screen with a text field labeled "email or username". Now, we've added usernameOnly argument, allowing the same screen to be used with only a "username" text field. Is that correct?

I'm considering whether we should rename the class from JetpackActivationWPComEmailScreen to JetpackActivationEmailOrUsernameScreen. I found it challenging to understand what was happening here, and I believe this change could make things clearer.

emailOrUsername = "",
isJetpackInstalled = false
)
Expand Down
Loading
Loading