Desktop, Mobile: Fixes #12104: Ensure merges to revisions during cleaning are synced to the target #12444
Conversation
mrjo118
changed the title
|
Any chance test units could be added for this? It's scary that no tests failed when the previous change was committed. |
What should it test? Just that the updated_time is updated on the revisions record? |
|
If you look at |
@laurent22 I've now added a test for this and verified that it fails without this change. I've also written the test in a way that it is compatible with my changes in PR #12487 |
| @@ -230,4 +230,57 @@ describe('Synchronizer.revisions', () => { | |||
|
|
|||
| jest.useRealTimers(); | |||
| }); | |||
|
|
|||
| it('should delete old revisions and sync updated revisions remotely, when revision deletion retains some revisions locally', async () => { | |||
There was a problem hiding this comment.
I read this three times and still not sure what it means or why these concepts are part of the same sentence or how they relate to each others.
I don't assume it's possible to split this into several tests?
Or maybe add a comment to explain what it does? Look at the other tests in this file, some have detailed examples in comment to explain the test.
There was a problem hiding this comment.
I've added a detailed comment similar to others in the class. Hopefully that is clear enough
In PR #11035 the revision cleaning process was changed to ensure revision deletions were synced upstream to the sync target. Unfortunately this PR was missing a change to ensure that the update to the surviving revision would be synced upstream as well. This would result in revisions being broken on other Joplin clients, when at least one revision for a note containing revisions has been cleaned. This PR addresses this issue.
This fixes #12104
Testing:
I have tested the change using a backup of the sync target for my own Joplin profile, using a note which has revisions spread across 2 days. Using this backup with file system sync on Android emulator, I forced cleaning of the oldest of the revisions by setting a value of 3 days for the note history expiry. Then I synced the change and pulled the file system sync directory onto my pc, and then synced this directory to a new profile on Joplin desktop. I then confirmed that the merged revision was correctly synced, and the remaining revisions correctly displays a preview of the contents.