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

Switch to new login/create/restore status-go endpoints #14139

Merged
merged 13 commits into from
Apr 24, 2024

Conversation

igor-sirotin
Copy link
Contributor

@igor-sirotin igor-sirotin commented Mar 25, 2024

Closes #12977
Closes #14128
Requires status-im/status-go#4980
Requires status-im/nim-status-go#29
Requires status-im/status-go#4994

Description

  1. Switch desktop to use new status-go endpoints: LoginAccount, CreateAccountAndLogin and RestoreAccountAndLogin.
  2. Improved the import seed phrase flow UX:
  • Switch to ProfileFetching state immediately after BiometricsState
  • Call prepareAndInitFetchingData before switching to this state
  1. Validate displayName correctness during CreateAccount

NOTE: I didn't migrate the keycard login/create/restore. Because current status-go endpoints are not ready for this. Here's an issue to track: status-im/status-go#4977
Because of this I didn't remove things like NODE_CONFIG, NETWORKS (#11435) and settings.

Reordered onboarding screens

The new endpoint doesn't allow us to show the generated keys (chat key, emojihash and identiconring) before setting the password. Therefore we moved this screen to the end of onboarding, which is also consistent with mobile design.

We also moved the Allow notifications screen to the end, as it doesn't make sense as the most first onboarding screen.

Design link

image

Affected areas

All onboarding flows with and without keycard:

  • Login
  • Create account
  • Import account from seed phrase
  • Restore account from seed phrase

Screenshots

No Keycard

No keycard - Create account

1.-.Create.account.mov

No keycard - Login

2.-.Login.mov

No keycard - Import seed phrase

3.-.Import.seed.phrase.mov

No keycard - Restore seed phrase (success)

4.-.Restore.account.success.mov

No keycard - Restore seed phrase (fail)

5.-.Restore.account.fail.part.1.mov
5.-.Restore.account.fail.part.2.mov

With Keycard

With keycard - Create keycard account

6.-.Create.keycard.account.mov

With keycard - Login keycard

7.-.Login.keycard.mov

With keycard - Lost keycard -> replace

8.-.Lost.keycard.-.replace.mov

With keycard - restore (fail)

9.-.Keycard.restore.fail.part.1.mov
9.-.Keycard.restore.fail.part.2.mov

With keycard - restore (success)

10.-.Keycard.restore.success.mov

With keycard - import seed phrase

11.-.Keyvcard.import.mov

@status-im-auto
Copy link
Member

status-im-auto commented Mar 25, 2024

Jenkins Builds

Click to see older builds (94)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 230b7f0 #1 2024-03-25 10:45:06 ~6 min tests/nim 📄log
✔️ 230b7f0 #1 2024-03-25 10:50:04 ~11 min tests/ui 📄log
c1515bb #3 2024-03-25 10:54:41 ~4 min macos/x86_64 📄log
✔️ c1515bb #3 2024-03-25 10:56:12 ~5 min tests/nim 📄log
c1515bb #3 2024-03-25 10:58:43 ~8 min macos/aarch64 📄log
c1515bb #3 2024-03-25 10:59:20 ~8 min linux/x86_64 📄log
✔️ c1515bb #3 2024-03-25 11:00:59 ~10 min tests/ui 📄log
c1515bb #3 2024-03-25 11:07:40 ~16 min windows/x86_64 📄log
f4bc506 #4 2024-03-25 11:49:15 ~4 min macos/aarch64 📄log
f4bc506 #4 2024-03-25 11:49:40 ~4 min macos/x86_64 📄log
✔️ f4bc506 #4 2024-03-25 11:51:09 ~6 min tests/nim 📄log
✔️ 7bffa88 #5 2024-03-25 11:58:20 ~6 min tests/nim 📄log
✔️ 7bffa88 #5 2024-03-25 11:59:27 ~7 min macos/x86_64 🍎dmg
7bffa88 #5 2024-03-25 12:02:38 ~10 min windows/x86_64 📄log
✔️ 7bffa88 #5 2024-03-25 12:03:25 ~11 min tests/ui 📄log
✔️ 7bffa88 #5 2024-03-25 12:08:04 ~16 min linux/x86_64 📦tgz
✔️ 7bffa88 #5 2024-03-25 12:11:01 ~18 min macos/aarch64 🍎dmg
✔️ 7bffa88 #6 2024-03-25 13:28:25 ~23 min windows/x86_64 💿exe
✔️ 650e5dd #7 2024-03-27 16:42:03 ~5 min tests/nim 📄log
✔️ 650e5dd #7 2024-03-27 16:43:24 ~7 min macos/aarch64 🍎dmg
✔️ 650e5dd #7 2024-03-27 16:46:27 ~10 min macos/x86_64 🍎dmg
650e5dd #7 2024-03-27 16:48:05 ~11 min tests/ui 📄log
✔️ 650e5dd #7 2024-03-27 16:54:22 ~18 min linux/x86_64 📦tgz
✔️ 650e5dd #8 2024-03-27 17:01:15 ~11 min tests/ui 📄log
✔️ 650e5dd #8 2024-03-27 17:08:55 ~32 min windows/x86_64 💿exe
✔️ 30947e4 #9 2024-04-03 15:01:41 ~6 min tests/nim 📄log
✔️ 30947e4 #9 2024-04-03 15:02:08 ~6 min macos/aarch64 🍎dmg
✔️ 30947e4 #10 2024-04-03 15:05:48 ~10 min tests/ui 📄log
✔️ 30947e4 #9 2024-04-03 15:08:03 ~12 min macos/x86_64 🍎dmg
✔️ 30947e4 #9 2024-04-03 15:12:42 ~17 min linux/x86_64 📦tgz
✔️ 7d04b68 #10 2024-04-03 15:28:36 ~5 min macos/aarch64 🍎dmg
✔️ 7d04b68 #10 2024-04-03 15:30:12 ~6 min tests/nim 📄log
✔️ 7d04b68 #10 2024-04-03 15:32:04 ~8 min macos/x86_64 🍎dmg
✔️ 7d04b68 #11 2024-04-03 15:34:20 ~10 min tests/ui 📄log
✔️ 7d04b68 #10 2024-04-03 15:40:03 ~16 min linux/x86_64 📦tgz
✔️ 7d04b68 #11 2024-04-03 15:47:46 ~24 min windows/x86_64 💿exe
✔️ 0ff02dd #11 2024-04-09 19:31:42 ~5 min macos/aarch64 🍎dmg
✔️ 0ff02dd #11 2024-04-09 19:32:36 ~6 min tests/nim 📄log
✔️ 0ff02dd #11 2024-04-09 19:35:07 ~8 min macos/x86_64 🍎dmg
✔️ 0ff02dd #12 2024-04-09 19:37:39 ~11 min tests/ui 📄log
✔️ 0ff02dd #11 2024-04-09 19:41:10 ~15 min linux/x86_64 📦tgz
✔️ 0ff02dd #12 2024-04-09 19:53:09 ~26 min windows/x86_64 💿exe
✔️ bf83fb0 #12 2024-04-11 15:55:48 ~5 min macos/aarch64 🍎dmg
✔️ bf83fb0 #12 2024-04-11 15:57:15 ~6 min tests/nim 📄log
✔️ bf83fb0 #12 2024-04-11 15:59:00 ~8 min macos/x86_64 🍎dmg
✔️ bf83fb0 #13 2024-04-11 16:02:05 ~11 min tests/ui 📄log
✔️ bf83fb0 #12 2024-04-11 16:05:10 ~14 min linux/x86_64 📦tgz
✔️ bf83fb0 #13 2024-04-11 16:19:44 ~28 min windows/x86_64 💿exe
✔️ e32145e #13 2024-04-11 16:33:10 ~4 min macos/aarch64 🍎dmg
✔️ e32145e #13 2024-04-11 16:34:50 ~6 min tests/nim 📄log
✔️ e32145e #13 2024-04-11 16:35:37 ~7 min macos/x86_64 🍎dmg
✔️ e32145e #14 2024-04-11 16:39:34 ~11 min tests/ui 📄log
✔️ e32145e #13 2024-04-11 16:43:40 ~15 min linux/x86_64 📦tgz
✔️ e32145e #14 2024-04-11 16:55:55 ~27 min windows/x86_64 💿exe
0ae553b #14 2024-04-11 19:43:32 ~5 min macos/aarch64 📄log
✔️ 0ae553b #14 2024-04-11 19:44:51 ~6 min tests/nim 📄log
0ae553b #14 2024-04-11 19:47:40 ~9 min macos/x86_64 📄log
✔️ 0ae553b #15 2024-04-11 19:49:18 ~10 min tests/ui 📄log
0ae553b #15 2024-04-11 19:50:09 ~11 min windows/x86_64 📄log
0ae553b #14 2024-04-11 19:51:15 ~13 min linux/x86_64 📄log
ed0f4dd #15 2024-04-12 12:29:50 ~4 min macos/aarch64 📄log
ed0f4dd #15 2024-04-12 12:30:19 ~4 min macos/x86_64 📄log
✔️ ed0f4dd #15 2024-04-12 12:31:52 ~6 min tests/nim 📄log
✔️ ed0f4dd #16 2024-04-12 12:36:10 ~10 min tests/ui 📄log
ed0f4dd #15 2024-04-12 12:37:42 ~12 min linux/x86_64 📄log
ed0f4dd #16 2024-04-12 12:37:50 ~12 min windows/x86_64 📄log
7ff39e4 #16 2024-04-12 12:42:07 ~1 min macos/aarch64 📄log
✔️ 7ff39e4 #16 2024-04-12 12:47:29 ~6 min tests/nim 📄log
✔️ 7ff39e4 #16 2024-04-12 12:51:21 ~10 min macos/x86_64 🍎dmg
✔️ 7ff39e4 #17 2024-04-12 12:52:15 ~11 min tests/ui 📄log
✔️ 7ff39e4 #16 2024-04-12 12:56:00 ~15 min linux/x86_64 📦tgz
✔️ 7ff39e4 #17 2024-04-12 13:08:50 ~27 min windows/x86_64 💿exe
✔️ 7ff39e4 #17 2024-04-12 14:05:19 ~4 min macos/aarch64 🍎dmg
✔️ cd9bd69 #18 2024-04-12 15:22:30 ~4 min macos/aarch64 🍎dmg
✔️ cd9bd69 #17 2024-04-12 15:24:20 ~6 min tests/nim 📄log
✔️ cd9bd69 #17 2024-04-12 15:25:19 ~7 min macos/x86_64 🍎dmg
✔️ cd9bd69 #18 2024-04-12 15:29:44 ~11 min tests/ui 📄log
✔️ cd9bd69 #17 2024-04-12 15:33:29 ~15 min linux/x86_64 📦tgz
✔️ cd9bd69 #18 2024-04-12 15:42:02 ~24 min windows/x86_64 💿exe
✔️ 764be17 #19 2024-04-18 20:36:24 ~6 min macos/aarch64 🍎dmg
✔️ 764be17 #18 2024-04-18 20:36:32 ~6 min tests/nim 📄log
✔️ 764be17 #18 2024-04-18 20:38:29 ~8 min macos/x86_64 🍎dmg
✔️ 7accf51 #22 2024-04-18 20:46:33 ~4 min macos/aarch64 🍎dmg
✔️ 7accf51 #21 2024-04-18 20:48:03 ~5 min tests/nim 📄log
✔️ 7accf51 #21 2024-04-18 20:49:42 ~7 min macos/x86_64 🍎dmg
✔️ 7accf51 #22 2024-04-18 20:51:53 ~9 min tests/ui 📄log
✔️ 7accf51 #21 2024-04-18 20:57:48 ~15 min linux/x86_64 📦tgz
✔️ 7accf51 #22 2024-04-18 21:11:16 ~28 min windows/x86_64 💿exe
✔️ da2ea4a #23 2024-04-19 13:29:01 ~5 min macos/aarch64 🍎dmg
✔️ da2ea4a #22 2024-04-19 13:30:11 ~6 min tests/nim 📄log
✔️ da2ea4a #22 2024-04-19 13:31:34 ~8 min macos/x86_64 🍎dmg
✔️ da2ea4a #23 2024-04-19 13:35:17 ~11 min tests/ui 📄log
✔️ da2ea4a #22 2024-04-19 13:39:13 ~15 min linux/x86_64 📦tgz
✔️ da2ea4a #23 2024-04-19 13:53:09 ~29 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ ea2e9b2 #23 2024-04-23 21:35:10 ~6 min tests/nim 📄log
✔️ ea2e9b2 #23 2024-04-23 21:36:31 ~7 min macos/x86_64 🍎dmg
✔️ ea2e9b2 #24 2024-04-23 21:36:55 ~8 min macos/aarch64 🍎dmg
✔️ ea2e9b2 #24 2024-04-23 21:39:18 ~10 min tests/ui 📄log
✔️ ea2e9b2 #23 2024-04-23 21:44:10 ~15 min linux/x86_64 📦tgz
✔️ ea2e9b2 #24 2024-04-23 21:57:21 ~28 min windows/x86_64 💿exe
✔️ cbe14c2 #25 2024-04-24 15:43:03 ~5 min macos/aarch64 🍎dmg
✔️ cbe14c2 #24 2024-04-24 15:44:29 ~7 min tests/nim 📄log
✔️ cbe14c2 #24 2024-04-24 15:45:20 ~7 min macos/x86_64 🍎dmg
✔️ cbe14c2 #25 2024-04-24 15:49:26 ~12 min tests/ui 📄log
✔️ cbe14c2 #24 2024-04-24 15:52:52 ~15 min linux/x86_64 📦tgz
✔️ cbe14c2 #25 2024-04-24 16:05:32 ~28 min windows/x86_64 💿exe

Copy link
Member

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

LGTM

src/app/modules/startup/controller.nim Outdated Show resolved Hide resolved
raribleMainnetApiKey*: string
raribleTestnetApiKey*: string

# ganacheURL: string // WARNING: is this used?
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if they use it still cc @alaibe

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, it shouldn't be it was used for test i believe

Copy link
Contributor

@saledjenic saledjenic left a comment

Choose a reason for hiding this comment

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

@igor-sirotin recovering using Keycard doesn't work anymore.

@igor-sirotin
Copy link
Contributor Author

@igor-sirotin recovering using Keycard doesn't work anymore.

Will take a look, thank you!

@anastasiyaig
Copy link
Contributor

@igor-sirotin i asked @lukaszso to test this PR, you will hear from us shortly

@igor-sirotin
Copy link
Contributor Author

@igor-sirotin i asked @lukaszso to test this PR, you will hear from us shortly

@anastasiyaig, thank you!

@lukaszso, as Sale mentioned, there seem to be a problem with keycard.
Unfortunately, I will only be able to take a look on Tuesday.

I'll leave the decision to you to test it before or no 🙂

@lukaszso lukaszso self-requested a review April 2, 2024 12:42
Copy link

@lukaszso lukaszso left a comment

Choose a reason for hiding this comment

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

@igor-sirotin i've checked login flows as well as generating new keys and restoring from seed phrase. Did not encounter any issues there.
Also checked username validation and dots are no longer accepted.

@igor-sirotin
Copy link
Contributor Author

igor-sirotin commented Apr 2, 2024

@igor-sirotin i've checked login flows as well as generating new keys and restoring from seed phrase. Did not encounter any issues there. Also checked username validation and dots are no longer accepted.

Thank you, @lukaszso!

I'll bring my keycard with me tomorrow and check corresponding flows.
Keeping this PR opened for now.

@igor-sirotin igor-sirotin force-pushed the chore/issue-12977-login-endpoints branch 2 times, most recently from 4ee7be6 to 30947e4 Compare April 3, 2024 14:55
@igor-sirotin
Copy link
Contributor Author

@saledjenic I've fixed the keycard import flow, please check it out again

Screen.Recording.2024-04-03.at.15.46.35.mov

@saledjenic
Copy link
Contributor

@saledjenic I've fixed the keycard import flow, please check it out again

Screen.Recording.2024-04-03.at.15.46.35.mov

@igor-sirotin yes, this flow works now, thanks.

There is only one more thing, please check this comment: #14139 (comment)

@anastasiyaig
Copy link
Contributor

There are some other issues in this PR that have to be addressed (discussed with Igor already)

@igor-sirotin
Copy link
Contributor Author

There's a problem that was caught by the e2e tests (thanks to @anastasiyaig ❤️).

There's a difference in mobile/desktop designs and the CreateAccountAndLogin fits mobile, but doesn't work for desktop. So we should either change designs or change the endpoint. I've scheduled a talk with Ben for Monday.

Flow examples

Here's a basic onboarding flow on mobile:

  1. I'm new to Status
  2. Create profile (name, image, color)
  3. Set password
  4. Enable biometrics?
  5. Generating keys animation
  6. Show create account details (name, image, chat key, identicon ring, emoji hash)
  7. Enable notifications?
  8. Welcome to web3
  9. Login to account

Here's a basic onboarding flow on desktop:

  1. Allow notifications?
  2. Before you get started (accept beta & terms)
  3. I'm new to Status
  4. Create profile (name, image)
  5. Show created account details
  6. Set password
  7. Enable biometrics?
  8. Login to account

Desktop design problems

  1. The main problem is that screen 5 goes before screen 6.
    Previously generating an account and saving it to the database were separate procedures. But with this PR they're done in a single API endpoint call, which requires all of the information (name, image and password) to create an account.
    This wasn't an issue on mobile, because of the design.

Changing an API for desktop design needs sounds an overkill to me here. I'd rather make mobile/desktop designs consistent, i.e. move screen 5 to be after screens 6-7.

  1. Also, I noted that the Allow notifications comes the first screen, which doesn't make sense to me, maybe we should move it in the same order as on mobile?

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.

Just looking around 👮 :)

ui/app/AppLayouts/Onboarding/views/InsertDetailsView.qml Outdated Show resolved Hide resolved
ui/app/AppLayouts/Onboarding/views/InsertDetailsView.qml Outdated Show resolved Hide resolved
ui/app/AppLayouts/Onboarding/views/ProfileChatKeyView.qml Outdated Show resolved Hide resolved
src/app_service/service/accounts/service.nim Outdated Show resolved Hide resolved
src/app/modules/startup/view.nim Show resolved Hide resolved
src/app/modules/startup/controller.nim Outdated Show resolved Hide resolved
src/app/modules/startup/controller.nim Outdated Show resolved Hide resolved
src/app/modules/startup/controller.nim Outdated Show resolved Hide resolved
ui/app/AppLayouts/Onboarding/views/InsertDetailsView.qml Outdated Show resolved Hide resolved
@igor-sirotin
Copy link
Contributor Author

I've also rebased on latest master, since quite some time has passed already.
Hopefully this doesn't bring new issues 😄

@Valentina1133
Copy link
Contributor

Valentina1133 commented Apr 19, 2024

@Valentina1133 I've fixed most of the issues. There's one question though.

Changed locator

When adding saved address test doesn't see network tag, I checked by squish - it gave me changed locator

{"container": mainWindow_Overlay, "objectName": "networkTagRectangle_Add networks", "type": "Rectangle", "visible": True}

Looking at the code, it didn't change. The object name is generated here:

objectName: "networkTagRectangle_" + root.title

root.title is coming from here:

and finally defaultItemText comes from here:

So it should be networkTagRectangle_Add networks, as it was before. And as far as I understand, this is the same what you expect in tests.

Could you please check it once again? And ping me if nothing changed, I will probably need your help to investigate then 🙂

@igor-sirotin I changed locator in the branch because it didn't work with a previously used locator (test was not able to find the object with that locator) mainWallet_Saved_Addreses_Popup_Add_Network_Selector_Tag = {"container": statusDesktop_mainWindow_overlay, "objectName": "networkSelectorTag", "type": "StatusNetworkListItemTag"}, but then Nastya told me that you said that nothing was supposed to be changed in this part. So let me know, should I leave this change then?

@anastasiyaig
Copy link
Contributor

@igor-sirotin I changed locator in the branch because it didn't work with a previously used locator (test was not able to find the object with that locator) mainWallet_Saved_Addreses_Popup_Add_Network_Selector_Tag = {"container": statusDesktop_mainWindow_overlay, "objectName": "networkSelectorTag", "type": "StatusNetworkListItemTag"}, but then Nastya told me that you said that nothing was supposed to be changed in this part. So let me know, should I leave this change then?

addressed that here status-im/desktop-qa-automation#646

@igor-sirotin
Copy link
Contributor Author

The PR is ready to be merged, just waiting for status-im/desktop-qa-automation#645 to be merged to pass e2e tests.

@anastasiyaig
Copy link
Contributor

alright, i launched critical suite now and it is passed https://ci.status.im/job/status-desktop/job/e2e/job/manual/1800/allure/
Screenshot 2024-04-19 at 16 48 45

will launch then full run and update here with results

@anastasiyaig
Copy link
Contributor

@igor-sirotin i ran all the tests we have twice and they are failing in the same places
https://ci.status.im/job/status-desktop/job/e2e/job/manual/1806/allure/
https://ci.status.im/job/status-desktop/job/e2e/job/manual/1804/allure/

the same tests are passing in master, so we need to investigate what is going on, @Valentina1133 will check on that

@igor-sirotin
Copy link
Contributor Author

Fixed the issue with online status in status-go. This PR is updated as well. status-im/status-go#5083

cc @anastasiyaig @Valentina1133

@Valentina1133
Copy link
Contributor

Valentina1133 commented Apr 24, 2024

Fixed the issue with online status in status-go. This PR is updated as well. status-im/status-go#5083

cc @anastasiyaig @Valentina1133

Good news! All tests are green now https://ci.status.im/job/status-desktop/job/e2e/job/manual/1807/ So I will open PR soon

feat: use CreateAccountAndLogin endpoint (first working iteration)

move default values to status-go

Pass walletSecretsConfig. Nim request objects.

wip: restore account

runtimeLogLevel. Create account ImageCropRectangle.

cleanup

more cleanup

change comment
@igor-sirotin igor-sirotin force-pushed the chore/issue-12977-login-endpoints branch from ea2e9b2 to cbe14c2 Compare April 24, 2024 15:37
@igor-sirotin
Copy link
Contributor Author

I've merged the status-go PR and upgraded it here, everything's ready on my side to merge this PR, only e2e test left.
Please let me know if I can be of any help 🙂
cc @Valentina1133 @anastasiyaig

@anastasiyaig
Copy link
Contributor

@igor-sirotin i will be brave and merge our changes into automation repo (to master) and will restart the tests.

@igor-sirotin igor-sirotin merged commit 59cb770 into master Apr 24, 2024
8 checks passed
@igor-sirotin igor-sirotin deleted the chore/issue-12977-login-endpoints branch April 24, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants