Add toggle in advanced settings for mobile data #20730
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 (107)
|
cammellos
force-pushed
the
feature/check-for-expensive-network
branch
3 times, most recently
from
a531108
to
74dc20f
Compare
cammellos
force-pushed
the
feature/check-for-expensive-network
branch
4 times, most recently
from
1955d8a
to
252a379
Compare
yevh-berdnyk
force-pushed
the
feature/check-for-expensive-network
branch
from
ffef621
to
f34853a
Compare
43% of end-end tests have passedFailed tests (3)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
|
cammellos
force-pushed
the
feature/check-for-expensive-network
branch
3 times, most recently
from
9e824bb
to
edd547e
Compare
|
Hey @cammellos! Thanks for your PR. Please, look at the issues I am faced during my testing: ISSUE 1: The community is fetched when changing from mobile network to Wi-Fi only after relogin.Steps:
Actual result: The community is fetched when changing from mobile network to Wi-Fi only after relogin (The video is sped up at 2.0x.) video_2024-07-16_15-40-19.mp4Expected result: The community is fetched when changing from mobile network to Wi-Fi without relogin Additional InformationOperating System: Android 13 |
|
@cammellos How it look in app: |
0% of end-end tests have passedFailed tests (51)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Class TestActivityMultipleDevicePR:
Class TestWalletMultipleDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
|
0% of end-end tests have passedFailed tests (7)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
|
ISSUE 2: The popup is shown a few times in the account when switching from Wi-Fi to mobile networks.Steps:
Actual result: The popup is shown a few times video_2024-07-17_12-55-57.mp4Expected result: The popup is shown a one time Additional InformationOperating System: Android 13 |
|
@cammellos can you please rebase it and resolve a conflict? this prevents us from launching the |
|
@cammellos @churik
|
cammellos
force-pushed
the
feature/check-for-expensive-network
branch
from
edd547e
to
aed8997
Compare
|
@cammellos It seems somehow (I don't know exactly how) regardless of that wi-fi is enabled, the app uses mobile data instead. I verified on my device that it shows only Mobile Data in the settings below (not only in Google, but I physically disabled it)
ISSUE 3: Mobile Data usage is growing when "wi-fi only" is enabled and the user switches from a cellular network to wi-fi and tries to fetch data from the communityPay attention to the cellular data of the app. It started increasing when I switched on Wi-FI and tried to fetch community info FILE.2024-07-17.16.02.38.mp4Expected result: mobile data is the same, as I already switched to wi-fi |
|
@cammellos more context about issue 3 (sync and backup: WI-FI only): Test case is the same.
After re-login: And from this step the cellular data stops growing. FILE.2024-07-18.17.33.10.mp4Logs: |
Parveshdhull
force-pushed
the
feature/check-for-expensive-network
branch
from
984aaeb
to
2509ea0
Compare
|
we really need this to fix, as I'm unable to test it properly (only communties is a way to test it more or less reliable) |
|
Hi @Parveshdhull, Please merge #20868 and rebase it so we can start the final re-test. Thank you! |
Parveshdhull
force-pushed
the
feature/check-for-expensive-network
branch
from
2509ea0
to
aa44b4e
Compare
|
It seems sometime bottom sheet are opened twice, looking into it. |
Note: We show the syncing choice bottom sheet to the user when he/she is using an expensive network (more detail in pr description). To avoid opening the bottom sheet multiple times, I have updated the code to only show the sheet once per session (if the user closes the sheet without making a choice). |
67% of end-end tests have passedFailed tests (16)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletMultipleDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (34)Click to expandClass TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
@Parveshdhull last thing - Andrea told that it is possible to disable this setting for e2e builds at all, as it is a bit unstable and might affect e2e. Can you please do this? I'll rerun tests and that will be it, thank you |
|
hi @churik, Syncing toggle should be disabled now for e2e, please let me know if this is not the case. |
51% of end-end tests have passedFailed tests (24)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletMultipleDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (26)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
ilmotta
force-pushed
the
feature/check-for-expensive-network
branch
from
fd48c82
to
0547324
Compare
@Parveshdhull we managed to get the status-go PR status-im/status-go#5511 merged after hours of flaky tests failing in the status-go test suite. I pushed the new tagged version |
88% of end-end tests have passedFailed tests (5)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletMultipleDevice:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (45)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
|
|
@Parveshdhull status-go is already cherry-picked, ready to merge! Thank you!!! |
Parveshdhull
force-pushed
the
feature/check-for-expensive-network
branch
from
0547324
to
f89f86a
Compare
status-im/status-go@4a43b2b...9755b3d Co-authored-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
Parveshdhull
force-pushed
the
feature/check-for-expensive-network
branch
from
f89f86a
to
4586f80
Compare
This was referenced Sep 4, 2024
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: #20748
When a user is on data network it will be presented with a popup to confirm whether they want to continue syncing. If the user answer "yes", it will sync normally, while if the user answer "no", it won't sync any data on cellular networks. The settings can be changed in profile->syncing.
By default the app won't be syncing on mobile networks.
status-go PR status-im/status-go#5511
Testing
Some of the scenarios that should be tested:
Once you answer the popup, you should not see it again unless you go to settings to trigger it again.