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.

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

feat(onboarding): add AC notif for importing old accounts #17078

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrainville
Copy link
Member

What does the PR do

Fixes #17028

When an old user imports an account, we now fetch the backups in the background and show an AC notification.

When the fetch is successful, the AC notif switches to a success message.

If after a timeout we detect that we didn't fetch anything or just part, we show an error and the possibility to try again.

Based on top of #17058

Status-go PR: status-im/status-go#6255

Affected areas

Onboarding and AC. In the status-go side, backup handling

Architecture compliance

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it (kindof lol)
backup-fetching.webm

@micieslak @caybro if I could request your help please. I managed to make all the states (pending, failure, success), but the badge looks ugly in all but the pending state.
If you could give me a hand in making sure the badge icon looks ok, I'd appreciate. See in the video how the checkmark is way to big for no reason.

Impact on end user

Instead of waiting on a screen for the fetching to work only at the end they could move to the main screen, now they can see the app straight away and data is fetched in the background and shows up when it's ready.

How to test

  • Have an account that is backed up and import it using the new onboading with the recovery phrase
  • To test the failure state, either let me know and I can tell you how to hack the code to simulate a failure or use the recovery phrase for an account that hasn't been used in 30 days. Or just use a random mnemonic

Risk

Described potential risks and worst case scenarios.

Tick one:

  • Low risk: 2 devs MUST perform testing as specified above and attach their results as comments to this PR before merging.
  • High risk: QA team MUST perform additional testing in the specified affected areas before merging.

Worst case, the bakcup fetching doesn't work

@jrainville jrainville requested review from micieslak, caybro, alexjba and a team as code owners January 15, 2025 19:03
@status-im-auto
Copy link
Member

status-im-auto commented Jan 15, 2025

Jenkins Builds

Click to see older builds (42)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ ee35755 #1 2025-01-15 19:11:26 ~8 min macos/aarch64 🍎dmg
✔️ ee35755 #1 2025-01-15 19:12:55 ~9 min tests/nim 📄log
✔️ ee35755 #1 2025-01-15 19:15:09 ~11 min tests/ui 📄log
✔️ ee35755 #1 2025-01-15 19:22:08 ~18 min macos/x86_64 🍎dmg
✔️ ee35755 #1 2025-01-15 19:23:18 ~20 min linux-nix/x86_64 📦tgz
✔️ ee35755 #1 2025-01-15 19:24:13 ~20 min linux/x86_64 📦tgz
✔️ ee35755 #1 2025-01-15 19:34:29 ~31 min windows/x86_64 💿exe
✔️ a61d968 #2 2025-01-15 21:05:10 ~5 min macos/aarch64 🍎dmg
✔️ a61d968 #2 2025-01-15 21:09:11 ~9 min tests/nim 📄log
✔️ a61d968 #2 2025-01-15 21:10:58 ~11 min tests/ui 📄log
✔️ a61d968 #2 2025-01-15 21:11:24 ~11 min macos/x86_64 🍎dmg
✔️ a61d968 #2 2025-01-15 21:17:18 ~17 min linux-nix/x86_64 📦tgz
✔️ a61d968 #2 2025-01-15 21:19:25 ~19 min linux/x86_64 📦tgz
✔️ a61d968 #2 2025-01-15 21:25:47 ~25 min windows/x86_64 💿exe
✔️ 6958aa5 #3 2025-01-16 16:36:47 ~6 min macos/aarch64 🍎dmg
✔️ 6958aa5 #3 2025-01-16 16:39:56 ~9 min tests/nim 📄log
✔️ 6958aa5 #3 2025-01-16 16:42:17 ~11 min tests/ui 📄log
✔️ 6958aa5 #3 2025-01-16 16:43:24 ~12 min macos/x86_64 🍎dmg
✔️ 6958aa5 #3 2025-01-16 16:48:58 ~18 min linux-nix/x86_64 📦tgz
✔️ 6958aa5 #3 2025-01-16 16:51:26 ~21 min linux/x86_64 📦tgz
✔️ 6958aa5 #3 2025-01-16 17:00:43 ~30 min windows/x86_64 💿exe
✔️ a3fcb11 #4 2025-01-16 17:19:09 ~7 min macos/aarch64 🍎dmg
✔️ a3fcb11 #4 2025-01-16 17:21:09 ~9 min tests/nim 📄log
a3fcb11 #4 2025-01-16 17:23:00 ~11 min tests/ui 📄log
✔️ a3fcb11 #4 2025-01-16 17:23:08 ~11 min macos/x86_64 🍎dmg
✔️ a3fcb11 #4 2025-01-16 17:28:34 ~17 min linux-nix/x86_64 📦tgz
✔️ a3fcb11 #4 2025-01-16 17:32:57 ~21 min linux/x86_64 📦tgz
✔️ a3fcb11 #4 2025-01-16 17:37:53 ~26 min windows/x86_64 💿exe
4d04bb3 #5 2025-01-23 17:09:12 ~2 min macos/aarch64 📄log
4d04bb3 #5 2025-01-23 17:11:14 ~4 min macos/x86_64 📄log
✔️ 4d04bb3 #5 2025-01-23 17:18:18 ~11 min tests/nim 📄log
4d04bb3 #5 2025-01-23 17:18:34 ~12 min tests/ui 📄log
✔️ 4d04bb3 #5 2025-01-23 17:25:15 ~18 min linux-nix/x86_64 📦tgz
✔️ 4d04bb3 #5 2025-01-23 17:25:47 ~19 min linux/x86_64 📦tgz
✔️ 4d04bb3 #5 2025-01-23 17:40:56 ~34 min windows/x86_64 💿exe
✔️ 2316db0 #6 2025-01-24 15:53:46 ~7 min macos/aarch64 🍎dmg
✔️ 2316db0 #6 2025-01-24 15:57:58 ~12 min tests/nim 📄log
✔️ 2316db0 #6 2025-01-24 16:00:13 ~14 min macos/x86_64 🍎dmg
2316db0 #6 2025-01-24 16:00:50 ~14 min tests/ui 📄log
✔️ 2316db0 #6 2025-01-24 16:07:52 ~22 min linux-nix/x86_64 📦tgz
✔️ 2316db0 #6 2025-01-24 16:09:39 ~23 min linux/x86_64 📦tgz
✔️ 2316db0 #6 2025-01-24 16:15:00 ~29 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a542a6e #7 2025-01-24 18:34:23 ~6 min macos/aarch64 🍎dmg
✔️ a542a6e #7 2025-01-24 18:37:12 ~9 min tests/nim 📄log
a542a6e #7 2025-01-24 18:39:25 ~11 min tests/ui 📄log
✔️ a542a6e #7 2025-01-24 18:41:06 ~12 min macos/x86_64 🍎dmg
✔️ a542a6e #7 2025-01-24 18:44:43 ~16 min linux-nix/x86_64 📦tgz
✔️ a542a6e #7 2025-01-24 18:48:44 ~20 min linux/x86_64 📦tgz
✔️ a542a6e #7 2025-01-24 18:55:03 ~26 min windows/x86_64 💿exe
✔️ a542a6e #8 2025-01-27 10:50:13 ~12 min tests/ui 📄log
7cca187 #9 2025-01-27 16:18:30 ~9 min macos/x86_64 📄log
✔️ 7cca187 #9 2025-01-27 16:18:52 ~9 min macos/aarch64 🍎dmg
✔️ 7cca187 #9 2025-01-27 16:24:04 ~14 min tests/nim 📄log
7cca187 #10 2025-01-27 16:27:38 ~18 min tests/ui 📄log
✔️ 7cca187 #9 2025-01-27 16:31:31 ~22 min linux/x86_64 📦tgz
✔️ 7cca187 #9 2025-01-27 16:32:00 ~22 min linux-nix/x86_64 📦tgz
✔️ 7cca187 #9 2025-01-27 16:39:53 ~30 min windows/x86_64 💿exe

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

Looks good in general!

@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from ee35755 to a61d968 Compare January 15, 2025 20:59
@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch from d2d7fe9 to 0e412d4 Compare January 16, 2025 16:29
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from a61d968 to 6958aa5 Compare January 16, 2025 16:30
@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch from 0e412d4 to aa5321c Compare January 16, 2025 17:11
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from 6958aa5 to a3fcb11 Compare January 16, 2025 17:11
Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

👍

@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch 2 times, most recently from 50391c9 to 476e752 Compare January 23, 2025 16:44
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from a3fcb11 to 4d04bb3 Compare January 23, 2025 17:06
@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch from 35bea58 to 668777f Compare January 23, 2025 22:00
else
stack.push(loginScreenComponent)
onboardingFlow.init()
// if (loginAccountsModel.ModelCount.empty)
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftovers?

Copy link
Member

Choose a reason for hiding this comment

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

yeah indeed, it shouldn't be here; probably a bad rebase

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, sorry I'll rebase in a few

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch from 668777f to b14dcf3 Compare January 24, 2025 15:28
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from 4d04bb3 to 2316db0 Compare January 24, 2025 15:45
@jrainville jrainville force-pushed the feat/new-onboarding-metrics branch from b14dcf3 to 344d28f Compare January 24, 2025 18:26
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from 2316db0 to a542a6e Compare January 24, 2025 18:27
Base automatically changed from feat/new-onboarding-metrics to master January 27, 2025 16:06
Fixes #17028

When an old user imports an account, we now fetch the backups in the background and show an AC notification.

When the fetch is successful, the AC notif switches to a success message.

If after a timeout we detect that we didn't fetch anything or just part, we show an error and the possibility to try again.
@jrainville jrainville force-pushed the feat/new-onboarding-profile-fetching branch from a542a6e to 7cca187 Compare January 27, 2025 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Post Onboarding] Fetching profile data
5 participants