Wallet: Activity Items - Sections #19906
Merged
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 (51)
|
J-Son89
reviewed
May 6, 2024
| [test-helpers.unit :as h] | ||
| [utils.re-frame :as rf])) | ||
|
|
||
| (h/deftest-sub :wallet/all-activities |
J-Son89
reviewed
May 6, 2024
ilmotta
reviewed
May 6, 2024
OmarBasem
force-pushed
the
wallet/activity-sections
branch
from
10237b9 to
2a22cf3
Compare
OmarBasem
force-pushed
the
wallet/activity-sections
branch
from
8223986 to
f311af8
Compare
ilmotta
approved these changes
May 8, 2024
There was a problem hiding this comment.
Code-wise looks good! Just one comment about deleting the feature flag. Thank you @OmarBasem
OmarBasem
force-pushed
the
wallet/activity-sections
branch
from
f311af8 to
d84afea
Compare
69% of end-end tests have passedFailed tests (14)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (36)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
|
|
@OmarBasem thanks for the PR. Please, take a look at the issues. ISSUE 1 Only part of transactions are fetched in Activity tabI have checked my old account that has a long history of activity on Mainnet. Expected result: According to Etherscan 37 transactions have been performed on Mainnet https://etherscan.io/txs?a=0x527d22094166ad33e7523d2cbca287798e149c7f Actual result: only 18 transactions have been fetched in Activity tab in Status Mobile app. At the same time, all 37 transactions are fetched on latest Desktop master app. Status-debug-logs - 2024-05-09T173052.604.zip telegram-cloud-document-2-5326063766947777498.mp4 |
ISSUE 2 Some of transactions have wrong sorting by dateExpected result: transactions should be sorted by date in desc order (from latest to oldest) Actual result: on screenshot below you can see Oct 25, 2023 transaction appearing between Oct 6, 2022 and Sep 27, 2022 transactions.
|
yeah I think we need to align with Desktop team on this. Pretty sure @smohamedjavid & I discussed this with @stefandunca and we will need to adjust a bit to get the rest of the activities that we are not seeing here. It's also clear we're missing some address details, i.e right now it just shows the address but it can be an account name or a saved address or a multichain address so we need to eventually handle those cases as well. |
83% of end-end tests have passedFailed tests (7)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (43)Click to expandClass TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
|
|
@OmarBasem thanks for the PR. Issues 2, 5, 7 seem to be fixed. It is hard to test functionality as balances are barely fetched, activities sometimes fetched, sometimes not. I do not want to block the PR anymore, so let's merge it. Will continue more thorough testing once #19864 and #20011 are fixed. |
OmarBasem
force-pushed
the
wallet/activity-sections
branch
from
5069fab to
f6a1a85
Compare
OmarBasem
force-pushed
the
wallet/activity-sections
branch
from
0957655 to
c8f2375
Compare
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: #19852
epic: #19849
Summary
This PR implements sectioning for the activity items.
Note:
The other two issues in this epic will be out of scope from the July release.
Designs
Demo:
Screen_Recording_20240506_125110_Status.mp4