-
Notifications
You must be signed in to change notification settings - Fork 76
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
Conversation
Jenkins BuildsClick to see older builds (94)
|
230b7f0
to
9d86474
Compare
dfd6474
to
650e5dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
raribleMainnetApiKey*: string | ||
raribleTestnetApiKey*: string | ||
|
||
# ganacheURL: string // WARNING: is this used? |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this 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.
Will take a look, thank you! |
@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. I'll leave the decision to you to test it before or no 🙂 |
There was a problem hiding this 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.
Thank you, @lukaszso! I'll bring my keycard with me tomorrow and check corresponding flows. |
4ee7be6
to
30947e4
Compare
@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) |
There are some other issues in this PR that have to be addressed (discussed with Igor already) |
There's a problem that was caught by the e2e tests (thanks to @anastasiyaig ❤️). There's a difference in mobile/desktop designs and the Flow examplesHere's a basic onboarding flow on mobile:
Here's a basic onboarding flow on desktop:
Desktop design problems
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looking around 👮 :)
src/app/modules/startup/internal/user_profile_chat_key_state.nim
Outdated
Show resolved
Hide resolved
I've also rebased on latest master, since quite some time has passed already. |
@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 |
The PR is ready to be merged, just waiting for status-im/desktop-qa-automation#645 to be merged to pass e2e tests. |
alright, i launched critical suite now and it is passed https://ci.status.im/job/status-desktop/job/e2e/job/manual/1800/allure/ will launch then full run and update here with results |
@igor-sirotin i ran all the tests we have twice and they are failing in the same places the same tests are passing in master, so we need to investigate what is going on, @Valentina1133 will check on that |
Fixed the issue with online status in status-go. This PR is updated as well. status-im/status-go#5083 |
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
ea2e9b2
to
cbe14c2
Compare
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. |
@igor-sirotin i will be brave and merge our changes into automation repo (to master) and will restart the tests. |
Closes #12977
Closes #14128
Requires status-im/status-go#4980
Requires status-im/nim-status-go#29
Requires status-im/status-go#4994
Description
LoginAccount
,CreateAccountAndLogin
andRestoreAccountAndLogin
.ProfileFetching
state immediately afterBiometricsState
prepareAndInitFetchingData
before switching to this statedisplayName
correctness duringCreateAccount
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
Affected areas
All onboarding flows with and without keycard:
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