Skip to content

Comments

All: Fixes #11902: Ensure notebook conflicts do not delete child notes and notebooks when resolved#13167

Merged
laurent22 merged 10 commits intolaurent22:devfrom
mrjo118:ensure-non-note-conflicts-do-not-delete-children
Oct 9, 2025
Merged

All: Fixes #11902: Ensure notebook conflicts do not delete child notes and notebooks when resolved#13167
laurent22 merged 10 commits intolaurent22:devfrom
mrjo118:ensure-non-note-conflicts-do-not-delete-children

Conversation

@mrjo118
Copy link
Contributor

@mrjo118 mrjo118 commented Sep 9, 2025

This PR implements a change to protect against unexpected data loss for a scenario when a notebook conflicts and the remote object no longer exists. Although the cause of the situation is unknown, the situation can be reproduced easily by the following steps:

  1. Take any notebook with some notes in it
  2. Set up file system sync
  3. Find the md file in the synced directory corresponding to the notebook - then delete it
  4. Before the client has chance to sync, rename the notebook, then delete it
  5. Run the sync - the notebook will be disappear
  6. In the log file you will find non-note conflict entries

The existing behaviour happens because when this scenario occurs, Joplin will delete the notebook and all its notes and sub-notebooks without any warning. Although rare and the root cause unknown, this can cause massive data loss if it happens with a top level notebook, so ought to be protected against.

This PR makes a simple change whereby when this scenario occurs, it will only delete the conflicting notebook and not any of the child notes or notebooks. This results in no notes being lost, as they will all still be available in the All Notes notebook, however the notebook hierarchy will be lost. The loss of the notebook hierarchy however will be addressed by PR #13109, but even without this change, it is still possible on Joplin desktop to filter by all notes within a 'misplaced' notebook by clicking the notebook name beneath the note title when one of these notes is opened via All Notes. With this change then, aside from potential difficulties with restoring the notebook hierarchy, the only data which is technically lost is the name of the notebook which was deleted, but even that could be obtained from the log file.

This would be a sufficient fix for #11902

Testing

See video:

electron_fZ7YH2emRQ.mp4

@personalizedrefrigerator
Copy link
Collaborator

CI should be fixed by #13168.

changeSource: ItemChange.SOURCE_SYNC,
sourceDescription: 'sync: handleConflictAction: non-note conflict',
trackDeleted: false,
deleteChildren: false,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that may be unexpected next time someone looks at this code. Could you please add a short comment above it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@laurent22
Copy link
Owner

By the way should a test be added for this in Synchronizer.conflicts.test.ts or would that be too tricky to test?

@mrjo118
Copy link
Contributor Author

mrjo118 commented Sep 30, 2025

By the way should a test be added for this in Synchronizer.conflicts.test.ts or would that be too tricky to test?

There is an existing test for the scenario. I've updated it to verify that children are not deleted

@laurent22 laurent22 merged commit 45a7554 into laurent22:dev Oct 9, 2025
11 checks passed
@laurent22
Copy link
Owner

Looks good now, thank you @mrjo118!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants