chore_: use status.go v2 endpoint #21450
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (44)
|
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
2 times, most recently
from
d7f4c51
to
0011971
Compare
|
For objective-c code review I'd also tag @briansztamfater I'll also review later in the day, Thanks for tagging @qfrank |
briansztamfater
approved these changes
Oct 21, 2024
There was a problem hiding this comment.
Thanks for tagging / requesting review @siddarthkay @qfrank!
From the Objective-C side looks good to me, I only added two minor comments. Seems like the new endpoint requires the parameters to be sent as a json string, and although I haven't written Objective-C code for some time, the approach looks correct. Let me know if you need help debugging it.
modules/react-native-status/android/src/main/java/im/status/ethereum/module/DatabaseManager.kt
Show resolved
Hide resolved
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
e8d5870
to
e0b5d15
Compare
siddarthkay
reviewed
Oct 22, 2024
| utils.executeRunnableStatusGoMethod( | ||
| { Statusgo.verifyAccountPassword(newKeystoreDir, address, password) }, | ||
| { Statusgo.verifyAccountPasswordV2(jsonParams.toString()) }, |
There was a problem hiding this comment.
I couldn't find verifyAccountPasswordV2 in status-go changes
88% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
|
|
siddarthkay
approved these changes
Oct 22, 2024
my bad, when I searched your PR, initially diffs for large file were not rendered by Github UI, hence search returned no results |
ilmotta
approved these changes
Oct 22, 2024
|
@qfrank thank you for the PR. Could you please summarise what exactly should be QAed in this PR? I have read comments/description of previous PRs and still not sure I understand correctly. Thanks. |
Thank you for taking this, updated the PR description :) @pavloburykh |
62% of end-end tests have passedFailed tests (21)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (34)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestFallbackMultipleDevice:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
Hey @qfrank thanks for the PR. Please take a look at the issue. ISSUE 1
|
|
Hey @qfrank @pavloburykh even if this PR is approved for merging, let's wait until the QA team has tested the release build. Once we get the preliminary results we can re-check and see if it's okay to merge. This is a simple strategy to reduce the risk and to avoid rebase hell in release branches. Large and/or important refactors that can affect the whole app should ideally wait until the release testing process is approaching completion/stability. I hope that's a sensible thing to ask 🙏🏼 Thank you |
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
e0b5d15
to
4712dac
Compare
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
4712dac
to
815b249
Compare
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
815b249
to
f3068cf
Compare
|
FYI @qfrank, just a few more days of waiting and if nothing outstanding appears from the release testing, this PR can be merged with peace of mind 🕊️ Maybe this Friday, Nov 01 we will consider mobile 2.31 tested and ready to be shipped (we won't release yet due to other processes, that will be for next week). |
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
f3068cf
to
7e23b16
Compare
issue 1 should be fixed |
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
7e23b16
to
73db641
Compare
status-im/status-go@3951129...399c3d5 * feat: use status backend server * chore: add env variable STATUS_BACKEND_SERVER_IMAGE_SERVER_URI_PREFIX * update doc * fix_: image_server lint issue
qfrank
force-pushed
the
chore/use_v2_endpoints
branch
from
0a21ecc
to
0374cb4
Compare
|
Hi @pavloburykh , the PR is ready to do manual QA now, we need to have a full regression test since this PR touch all of the function in |
60% of end-end tests have passedFailed tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (20)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Passed tests (33)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestFallbackMultipleDevice:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
|
50% of end-end tests have passedFailed tests (1)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
88% of end-end tests have passedExpected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (7)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
100% of end-end tests have passedPassed tests (1)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this is a followup PR for PR 21312
related status-go PR
new endpoints updated in this PR:
For QA testing, pls ignore no usage endpoints like
verifyDatabasePasswordV2, we should ensure the relate feature which mentioned in brackets works as before. Also we should ensure there's no sensitive information likepassword/mnemoniclogged inrequests.logstatus: ready.
NOTE: Hope someone know OC could take a deep review as the OC code was generated by AI cc @siddarthkay