Skip to content

Fix migration flow error when changing device settings mid-flow - #17659

Merged
antonis merged 2 commits into
trunkfrom
issue/17646-keep-migration-state-on-config-change
Dec 12, 2022
Merged

Fix migration flow error when changing device settings mid-flow#17659
antonis merged 2 commits into
trunkfrom
issue/17646-keep-migration-state-on-config-change

Conversation

@ovitrif

@ovitrif ovitrif commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

Fixes #17646

This PR ensures the migration flow can be continued if device settings are changed mid-flow.

Note: Currently I'm not sure which should be the target branch for this PR, either trunk or release/21.3.
Since I started on top of release/21.3 it's set for the moment to target that branch.
There are merge conflicts that need to be fixed before this PR can be merged in 21.3

Updated to target trunk as discussed

To test:

  1. Prerequisite WordPress app of the same flavor should be installed and logged in.
  2. Uninstall and reinstall Jetpack app but don’t open it.
  3. Tap “try the new Jetpack app” in the WordPress app
  4. See the “Welcome” screen. (alternately: Tap “continue” to go to the “Notifications come from Jetpack screen” or the flow completion screen)
  5. Open Android Settings and change language, or switch light/dark mode.
  6. Swipe back over to the Jetpack app.
  7. Expect To be able to continue (no error screen), from the last shown migration screen
  8. Continue the flow expecting it to work as intended without errors.

🧪 Test "Delete WordPress App Screen" for no regression

After completing the flow in the Jetpack app:

  1. Tap the "Please delete the WordPress app" card on My Site
  2. Expect to see the screen informing you to delete the WordPress app
  3. Open Android Settings and change language, or switch light/dark mode.
  4. Go back to the Jetpack app
  5. Expect to see the screen to delete the WordPress app updated based on the changed settings.
  6. Tap the Got it button
  7. Expect to land on the My Site screen

Preview

change_settings_midflow.mp4

Regression Notes

  1. Potential unintended areas of impact

    • Delete WP app screen (accessible from from My Site after completing the migration flow).
    • Migration flow UI in Jetpack app.
  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing.

  3. What automated tests I added (or what prevented me from doing so)
    Added test to check migration is not retried.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@antonis

antonis commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

Note: Currently I'm not sure which should be the target branch for this PR, either trunk or release/21.3.
Since I started on top of release/21.3 it's set for the moment to target that branch.
There are merge conflicts that need to be fixed before this PR can be merged in 21.3

Thank you for jumping into this so quickly @ovitrif and preparing a PR 🙇
Let's target trunk for this both to avoid the conflict and because it feels like an edge case to me. Wdyt?

@ovitrif

ovitrif commented Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Thank you for jumping into this so quickly @ovitrif and preparing a PR 🙇 Let's target trunk for this both to avoid the conflict and because it feels like an edge case to me. Wdyt?

Sounds good to me @antonis, thank you for your suggestion 🙇

P.S. I'll see what I can do to update this so that it targets trunk 👍

P.P.S. I do agree it's more of an edge case 👍

@ovitrif
ovitrif force-pushed the issue/17646-keep-migration-state-on-config-change branch from ec6be38 to 91cb7b6 Compare December 9, 2022 14:13
@ovitrif
ovitrif changed the base branch from release/21.3 to trunk December 9, 2022 14:13
@ovitrif ovitrif added this to the 21.5 milestone Dec 9, 2022
@wpmobilebot

wpmobilebot commented Dec 9, 2022

Copy link
Copy Markdown
Contributor
WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17659-ae8a38f.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commitae8a38f
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot

wpmobilebot commented Dec 9, 2022

Copy link
Copy Markdown
Contributor
Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17659-ae8a38f.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commitae8a38f
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@ovitrif
ovitrif force-pushed the issue/17646-keep-migration-state-on-config-change branch from 91cb7b6 to ae8a38f Compare December 9, 2022 15:12
@ovitrif ovitrif modified the milestones: 21.5, 21.4 Dec 9, 2022
@antonis antonis modified the milestones: 21.5, 21.4 Dec 12, 2022

@antonis antonis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work @ovitrif 👍
I've tested the PR on a Google Pixel 5 (Android 13) and everything worked as expected for me. The code changes also make sense 🎉

@antonis
antonis merged commit 050bdef into trunk Dec 12, 2022
@antonis
antonis deleted the issue/17646-keep-migration-state-on-config-change branch December 12, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android - The migration flow fails when device settings changes are applied mid-flow

3 participants