feat: add support for updating key-pairs and accounts from app signal #20550
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
seanstrom
requested review from
ilmotta,
smohamedjavid,
mohsen-ghafouri and
ulisesmac
Jenkins BuildsClick to see older builds (61)
|
seanstrom
force-pushed
the
seanstrom/process-keypair-data-signal
branch
4 times, most recently
from
1140625 to
11bdfb4
Compare
seanstrom
commented
Jun 25, 2024
src/status_im/contexts/chat/messenger/messages/transport/events.cljs
Outdated
Show resolved
Hide resolved
J-Son89
approved these changes
Jun 25, 2024
ilmotta
approved these changes
Jun 25, 2024
There was a problem hiding this comment.
Thanks for adding me as a reviewer @seanstrom. As usual in your PRs, there's interesting work to learn from. I left a few comments, but nothing is a blocker to me, except for using double dashes in var names :P
seanstrom
force-pushed
the
seanstrom/process-keypair-data-signal
branch
from
11bdfb4 to
c18c731
Compare
mohsen-ghafouri
approved these changes
Jun 27, 2024
smohamedjavid
approved these changes
Jun 28, 2024
seanstrom
force-pushed
the
seanstrom/process-keypair-data-signal
branch
from
1651608 to
131a65a
Compare
49% of end-end tests have passedFailed tests (24)Click to expandClass TestWalletMultipleDevice:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (2)Click to expandClass TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (25)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
VolodLytvynenko
force-pushed
the
seanstrom/process-keypair-data-signal
branch
from
ac84283 to
d4fa585
Compare
71% of end-end tests have passedFailed tests (14)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (36)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
|
|
Hi @seanstrom thank you for PR. Take a look found issues: ISSUE 1: The network preferences are not synced for accountSteps:
Actual result:The network preferences are not synced prefferences.mp4Expected result:The network preferences are not synced |
ISSUE 2: Assets are disappeared for derived accounts of synced user until the wallet is refreshedSteps:
Actual result:Assets disappeared, empty space is shown empty.assets.mp4Expected result:Assets are shown logs: |
76% of end-end tests have passedFailed tests (10)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Expected to fail tests (2)Click to expandClass TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (39)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Class TestActivityMultipleDevicePR:
|
|
@seanstrom issues are fixed. thank you for your work. PR can be merged |
seanstrom
force-pushed
the
seanstrom/process-keypair-data-signal
branch
2 times, most recently
from
fede8d9 to
2deae99
Compare
|
@VolodLytvynenko thanks for testing this PR 🙌 |
78% of end-end tests have passedFailed tests (9)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (40)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
|
56% of end-end tests have passedFailed tests (4)Click to expandClass TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Passed tests (5)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
|
@seanstrom thanx for the update. Just rechecked. PR can be merged |
seanstrom
force-pushed
the
seanstrom/process-keypair-data-signal
branch
from
2deae99 to
411b444
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Jul 14, 2024
…#20550) This change adds support for processing app signals for syncing changes when adding, editing, or removing keypairs, accounts, and watch-only accounts.
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.
fixes #20416
Summary
Testing notes
In order to test these changes the testing environment will need to have:
Platforms
Areas that maybe impacted
Functional
Steps to test
Reproducing the intended behaviour can be seen in the screen captures provided in this PR.
Though here are some brief steps to reproduce each case:
chataccount.Before and after screenshots comparison
Renaming.account.mov
Editing.Default.Status.Account.mov
Adding.new.account.to.existing.key.pair.mov
Removing.an.account.from.key.pair.with.multiple.accounts.mov
Adding.new.account.and.key.pair.mov
Removing.account.and.missing.key.pair.mov
Removing.account.and.existing.key.pair.mov
status: ready
Risk
Described potential risks and worst case scenarios.
Tick one: