fix_: applying removed contacts from backup #5925
Conversation
Jenkins BuildsClick to see older builds (4)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5925 +/- ##
===========================================
- Coverage 61.91% 61.80% -0.11%
===========================================
Files 843 843
Lines 111287 111288 +1
===========================================
- Hits 68903 68783 -120
- Misses 34449 34528 +79
- Partials 7935 7977 +42
Flags with carried forward coverage won't be shown. Click here to find out more.
|
protocol/messenger_handler.go
Outdated
| @@ -583,6 +583,8 @@ func (m *Messenger) handleSyncChats(messageState *ReceivedMessageState, chats [] | |||
| func (m *Messenger) HandleSyncInstallationContactV2(state *ReceivedMessageState, message *protobuf.SyncInstallationContactV2, statusMessage *v1protocol.StatusMessage) error { | |||
| // Ignore own contact installation | |||
There was a problem hiding this comment.
This comment is no longer on the right line
There was a problem hiding this comment.
Sorry, was testing something else and that left. Removed now.
protocol/messenger_handler.go
Outdated
| @@ -583,6 +583,8 @@ func (m *Messenger) handleSyncChats(messageState *ReceivedMessageState, chats [] | |||
| func (m *Messenger) HandleSyncInstallationContactV2(state *ReceivedMessageState, message *protobuf.SyncInstallationContactV2, statusMessage *v1protocol.StatusMessage) error { | |||
| // Ignore own contact installation | |||
|
|
|||
| fromBackup := statusMessage == nil | |||
There was a problem hiding this comment.
It's not clear why having no message means it comes from backups 🤔
There was a problem hiding this comment.
Well, there are 3 places where this function is called from:
handleBackup(used when applying messages from the backup)HandleSyncRawMessages(used when local pairing)dispatchToHandler->handleSyncInstallationContactV2Protobuf(used when a synced message comes from another (paired) device over the network)
In case of point 3 the code is the same as it was, in case of points 1 and 2 we need to store the contact even it has Removed or Blocked flag set to false.
That's why I used this flag fromPairedDevice := statusMessage != nil to filter out the source from which messages are coming.
So the issue was spotted in status-im/status-mobile#18096 but based on the code the same happens for Removed or Blocked contacts for applying messages from backup and local pairing. That should be fixed now.
saledjenic
force-pushed
the
fix/applying-removed-contact
branch
from
8485653 to
cf3f847
Compare
There was a problem hiding this comment.
Thanks. Could you please elaborate on how this resolves the issue of a deleted contact being restored from backup when it shouldn't? From what I understand, the additional condition enforces the creation of a removed contact in all scenarios except for network syncing. Additionally, a regression test would be extremely helpful.
|
@osmaczko have you tested? Are you able to reproduce the issue with these changes?
Actually that's the thing. When we do a backup, we send all contacts from the db, including those marked as removed/blocked. When we restore from waku or from another device, we actually want to replicate the db as it was when the last backup was done. When a removed contact is about to be added, since it's not in db, thus not found, we just "return" instead of adding it to db. What I did, I let the function "return" if the sync message is coming from paired device over a network, but let it be added to db in case it's coming from restoring from waku or another device. |
|
@saledjenic |
|
@churik since this PR is not merged yet, I guess that the issue is still here, you can test the app with and without this change and see. Do you want me to rebase this branch? |
|
@saledjenic yes, please |
saledjenic
force-pushed
the
fix/applying-removed-contact
branch
from
cf3f847 to
0690be2
Compare
|
@saledjenic thanks for the PR. Please take a look at the issues logged in corresponding mobile PR status-im/status-mobile#22011 (comment) |
Hey @saledjenic! Pinging you in case you missed the comment. Thanks. |
Applying removed contact from the backup was removed due to condition.