Skip to content

Comments

[Push Notifications] Remove WPCom login flow from push notifications setup#15408

Open
hichamboushaba wants to merge 6 commits intoissue/WOOMOB-2270-jetpack-statusfrom
issue/WOOMOB-2287-remove-login-flow-push-notifications
Open

[Push Notifications] Remove WPCom login flow from push notifications setup#15408
hichamboushaba wants to merge 6 commits intoissue/WOOMOB-2270-jetpack-statusfrom
issue/WOOMOB-2287-remove-login-flow-push-notifications

Conversation

@hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Feb 20, 2026

Closes: WOOMOB-2287

Note

I will handle code cleanup (removing the login changes) in a separate PR.

Description

Woo Push Notifications only requires a Jetpack site connection (site registration), not a WordPress.com account connection. The current flow unnecessarily routes users through a WPCom login before connecting the store, adding friction and complexity.

This PR removes the login flow and simplifies the connection step to only register the site.

Changes

Introduction screen:

  • Replaced StartWPComLogin event with NavigateToConnectionSteps — tapping "Continue" now goes directly to the connection steps, skipping WPCom login entirely
  • Passes isSiteConnectedToJetpack to the connection steps so they can skip registration when the site is already connected

Connection steps:

  • Replaced connectJetpackAccount() (site registration + account connection) with registerSite() (site registration only)
  • Skips registration entirely when the site is already connected to Jetpack
  • Removed two-stage ConnectStoreStage flow and connectStoreStage StateFlow

Navigation:

  • Removed WPCom login include and action from the push notifications nav graph
  • Removed ShowPushNotificationsConnectionSteps event and handlers from WPCom login fragments (2FA, magic link, password)
  • Made PushNotificationsSetup login mode a no-op in WPComLoginPostLoginViewModel

Repository:

  • Extracted registerSite() as a new public method in JetpackActivationRepository, reused by the existing connectJetpackAccount()

Strings:

  • Removed "account" from WordPress.com references (the store still connects to WP.com, just not to a WPCom account)

Testing instructions

Site not connected to Jetpack

  1. Open a store that does not have Jetpack site connection → should see the introduction screen with "Continue" button
  2. Tap "Continue" → should navigate directly to the connection steps screen (no WPCom login)
  3. Connection steps should register the site

Site connected to Jetpack

  1. Ensure your Woo version is lower than PUSH_NOTIFICATIONS_MIN_WC_VERSION
  2. Connect your store to Jeptack.
  3. Open push notifications setup intro
  4. Tap on Continue → should skip registration step.

Images/gif

Screen_recording_20260220_175613.mp4
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@hichamboushaba hichamboushaba changed the title Remove WPCom login flow from push notifications setup [Push Notifications] Remove WPCom login flow from push notifications setup Feb 20, 2026
@hichamboushaba hichamboushaba changed the base branch from trunk to issue/WOOMOB-2270-jetpack-status February 20, 2026 17:47
@hichamboushaba hichamboushaba force-pushed the issue/WOOMOB-2287-remove-login-flow-push-notifications branch from 3d0b59e to e586d33 Compare February 20, 2026 18:03
@hichamboushaba hichamboushaba added type: enhancement A request for an enhancement. feature: notifications Related to notifications or notifs. status: do not merge Dependent on another PR, ready for review but not ready for merge. labels Feb 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the WordPress.com login flow from the push notifications setup process. Push notifications only require Jetpack site connection (site registration), not a WordPress.com account connection. The changes simplify the flow by allowing users to go directly from the introduction screen to the connection steps, skipping the WPCom login entirely.

Changes:

  • Modified the introduction screen to navigate directly to connection steps instead of routing through WPCom login
  • Updated connection steps to use site registration only (not account connection), and skip registration when the site is already connected to Jetpack
  • Made WPCom login PushNotificationsSetup mode a no-op (with cleanup planned for a separate PR)
  • Extracted registerSite() as a public method in JetpackActivationRepository for reuse
  • Updated strings to remove "account" references, emphasizing site connection rather than account connection

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/pushnotifications/introduction/WooPushNotificationsIntroductionViewModel.kt Replaced StartWPComLogin event with NavigateToConnectionSteps, passing site connection status
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/pushnotifications/introduction/WooPushNotificationsIntroductionDialog.kt Updated navigation handling to go directly to connection steps
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/pushnotifications/connection/WooPushNotificationsConnectionStepsViewModel.kt Simplified connection flow to use registerSite() instead of connectJetpackAccount(), skip registration when already connected
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/wpcom/WPComLoginPostLoginViewModel.kt Made PushNotificationsSetup a no-op with TODO for cleanup
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/wpcom/WPComLoginPasswordFragment.kt Removed push notifications connection steps navigation handler
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/wpcom/WPComLoginMagicLinkHandlerFragment.kt Removed push notifications connection steps navigation handler
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/wpcom/WPComLogin2FAFragment.kt Removed push notifications connection steps navigation handler
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/jetpack/JetpackActivationRepository.kt Extracted registerSite() as public method, refactored connectJetpackAccount() to reuse it
WooCommerce/src/main/res/navigation/nav_graph_push_notifications.xml Removed WPCom login navigation graph inclusion and actions, added isSiteConnectedToJetpack argument
WooCommerce/src/main/res/values/strings.xml Updated push notification strings to remove "account" references
fastlane/resources/values/strings.xml Updated push notification description to remove "account" reference
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/pushnotifications/introduction/WooPushNotificationsIntroductionViewModelTest.kt Updated tests to reflect NavigateToConnectionSteps with site connection status
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/pushnotifications/connection/WooPushNotificationsConnectionStepsViewModelTest.kt Updated tests to use registerSite() instead of connectJetpackAccount()
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/wpcom/WPComLoginPostLoginViewModelTest.kt Removed test for push notifications post-login navigation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

assertThat(viewState).isEqualTo(ViewState.GenericError)
val event = viewModel.event.value
assertThat(event).isEqualTo(
WooPushNotificationsIntroductionViewModel.NavigateToConnectionSteps(isSiteConnectedToJetpack = false)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The test expects isSiteConnectedToJetpack = false, but the test setup uses JetpackConnectionStatus.AccountConnected which means isSiteConnected will be true (based on the logic in JetpackStatus.kt). The expected value should be isSiteConnectedToJetpack = true to match the test setup.

Suggested change
WooPushNotificationsIntroductionViewModel.NavigateToConnectionSteps(isSiteConnectedToJetpack = false)
WooPushNotificationsIntroductionViewModel.NavigateToConnectionSteps(isSiteConnectedToJetpack = true)

Copilot uses AI. Check for mistakes.
Comment on lines 76 to 77
if (wcVersion.isVersionAtLeast(PUSH_NOTIFICATIONS_MIN_WC_VERSION)) {
_viewState.value = ViewState.GenericError
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The test expects that when the site is connected and WC version is compatible, the NavigateToConnectionSteps event should be automatically triggered when the screen opens. However, the current implementation only sets the view state to GenericError and does not trigger the navigation event. The checkWCVersion() method should trigger the NavigateToConnectionSteps event when the version is at least the minimum required version, instead of setting the state to GenericError.

Copilot uses AI. Check for mistakes.
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 20, 2026

App Icon📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Build Number728
Version24.1
Application IDcom.woocommerce.android.prealpha
Commit6c3b3c5
Installation URL102sc2ehtvi18
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@hichamboushaba hichamboushaba force-pushed the issue/WOOMOB-2287-remove-login-flow-push-notifications branch from e586d33 to 6c3b3c5 Compare February 20, 2026 21:23
@dangermattic
Copy link
Collaborator

1 Error
🚫 This PR is tagged with status: do not merge label(s).

Generated by 🚫 Danger

Comment on lines +120 to +124
if (navArgs.isSiteConnectedToJetpack) {
markCurrentStepAsCompleted()
advanceToNextStep()
return
}
Copy link
Member Author

Choose a reason for hiding this comment

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

For now, if the site is connected, we'll skip the task.

We can discuss with iOS team whether removing the task completely is better and update accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: notifications Related to notifications or notifs. status: do not merge Dependent on another PR, ready for review but not ready for merge. type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants