Open igor-sirotin opened 3 months ago
Currently if the user is on cellular network, StoreNodeRequestManager will behave like the store node request was made and returned 0 envelopes.
StoreNodeRequestManager
This leads to weird UX. if a user tries to enter a community being on cellular, they will face no fetch issue, but like this community wasn't found.
This issue was described in the original PR: https://github.com/status-im/status-go/pull/5511#discussion_r1674379287
We're starting the request here: https://github.com/status-im/status-go/blob/977a93b0052974ade7f45f2b8f193c9998cc1462/protocol/messenger_store_node_request_manager.go#L554
Which will return nil on expensive networks: https://github.com/status-im/status-go/blob/5d309e2c64f59eb46fbfd40fd38f5f1102f6ff8f/protocol/messenger_mailserver.go#L921-L927
nil
Should be enough to remove the canSyncWithStoreNodes from:
canSyncWithStoreNodes
processMailserverBatchWithOptions
processMailserverBatch
But keep this check on high-level functions, for example in:
syncBackup
syncFiltersFrom
We should always allow manual fetching of communities/contacts/messages from store nodes. But prevent automatic history fetch on expensive networks.
In a perfect world we would implement a special UI popup asking "Are you sure to fetch this much data with cellular network?" or smth like that.
Also, we should return an error on checking canSyncWithStoreNodes. This will prevent such silent behaviour in general.
Problem
Currently if the user is on cellular network,
StoreNodeRequestManager
will behave like the store node request was made and returned 0 envelopes.This leads to weird UX. if a user tries to enter a community being on cellular, they will face no fetch issue, but like this community wasn't found.
This issue was described in the original PR: https://github.com/status-im/status-go/pull/5511#discussion_r1674379287
We're starting the request here: https://github.com/status-im/status-go/blob/977a93b0052974ade7f45f2b8f193c9998cc1462/protocol/messenger_store_node_request_manager.go#L554
Which will return
nil
on expensive networks: https://github.com/status-im/status-go/blob/5d309e2c64f59eb46fbfd40fd38f5f1102f6ff8f/protocol/messenger_mailserver.go#L921-L927Implementation
Should be enough to remove the
canSyncWithStoreNodes
from:processMailserverBatchWithOptions
processMailserverBatch
But keep this check on high-level functions, for example in:
syncBackup
syncFiltersFrom
Acceptance Criteria
We should always allow manual fetching of communities/contacts/messages from store nodes. But prevent automatic history fetch on expensive networks.
Future steps
In a perfect world we would implement a special UI popup asking "Are you sure to fetch this much data with cellular network?" or smth like that.