feat(backup)_: implement remaining backups and tests #6671
Closed
Conversation
This file contains hidden or 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
jrainville
commented
Jun 12, 2025
| @@ -35,6 +35,8 @@ message Backup { | |||
| FetchingBackedUpDataDetails watchOnlyAccountDetails = 14; | |||
| repeated SyncChat chats = 15; | |||
| FetchingBackedUpDataDetails chatsDetails = 16; | |||
| repeated SyncSetting settings = 17; | |||
| repeated SyncAccount watchOnlyAccounts = 18; | |||
There was a problem hiding this comment.
I think when this feature is all done, we can remove the old "single" properties or at least deprecate them
There was a problem hiding this comment.
Yeah keeping both doesn't make sense 💯
But I think it might better to do it right away, because it's not really related to the local backup feature 🤔
There was a problem hiding this comment.
I have this task to clean up all Waku backup stuff when the feature is done and the feature flag enabled by default: status-im/status-desktop#18123
I planned on doing the removal of those old protos then, what do you think?
There was a problem hiding this comment.
I guess it's fine yes
|
It's a big diff, but most of it is tests 😄 |
Jenkins BuildsClick to see older builds (36)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/poc-local-backup #6671 +/- ##
==========================================================
+ Coverage 28.10% 60.22% +32.11%
==========================================================
Files 794 831 +37
Lines 100520 103667 +3147
==========================================================
+ Hits 28253 62433 +34180
+ Misses 67914 33671 -34243
- Partials 4353 7563 +3210
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jrainville
force-pushed
the
feat/local-backup-remaining-settings
branch
from
88a5531 to
26f9fbd
Compare
jrainville
force-pushed
the
feat/poc-local-backup
branch
from
b347fc1 to
449c41c
Compare
jrainville
force-pushed
the
feat/local-backup-remaining-settings
branch
from
26f9fbd to
315cfe0
Compare
jrainville
force-pushed
the
feat/poc-local-backup
branch
from
449c41c to
810241f
Compare
jrainville
force-pushed
the
feat/local-backup-remaining-settings
branch
from
315cfe0 to
76af0e3
Compare
| @@ -35,6 +35,8 @@ message Backup { | |||
| FetchingBackedUpDataDetails watchOnlyAccountDetails = 14; | |||
| repeated SyncChat chats = 15; | |||
| FetchingBackedUpDataDetails chatsDetails = 16; | |||
| repeated SyncSetting settings = 17; | |||
| repeated SyncAccount watchOnlyAccounts = 18; | |||
There was a problem hiding this comment.
Yeah keeping both doesn't make sense 💯
But I think it might better to do it right away, because it's not really related to the local backup feature 🤔
protocol/messenger_local_backup.go
Outdated
Comment on lines
74
to
77
| _, settings, errors := m.prepareSyncSettingsMessages(clock, true) | ||
| if len(errors) != 0 { | ||
| // return just the first error, the others have been logged | ||
| return errors[0] |
There was a problem hiding this comment.
Suggested change
| _, settings, errors := m.prepareSyncSettingsMessages(clock, true) | |
| if len(errors) != 0 { | |
| // return just the first error, the others have been logged | |
| return errors[0] | |
| _, settings, errs := m.prepareSyncSettingsMessages(clock, true) | |
| if len(errors) != 0 { | |
| return errors.Join(errs) |
or even better to join the errors inside prepareSyncSettingsMessages
jrainville
force-pushed
the
feat/poc-local-backup
branch
from
810241f to
a9d669c
Compare
jrainville
force-pushed
the
feat/local-backup-remaining-settings
branch
from
76af0e3 to
2941baa
Compare
jrainville
force-pushed
the
feat/poc-local-backup
branch
from
a9d669c to
917104b
Compare
Fixes status-im/status-desktop#18120 Implements the missing user data like watch only accounts and settings. Implements tests to validate.
jrainville
force-pushed
the
feat/local-backup-remaining-settings
branch
from
2941baa to
4505816
Compare
|
Replaced by #6722 |
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 status-im/status-desktop#18120
Implements the missing user data like watch only accounts and settings.
Implements tests to validate.
I skipped the keypairs since I'm waiting on confirmation whether we want to just get rid of it.