The diff is a bit hard to follow. Do you know what was special about revisions which means that they wouldn't be deleted after sync?

The diff is a bit hard to follow. Do you know what was special about revisions which means that they wouldn't be deleted after sync?

From what I understand, there were no deleted_items table entries created when deleting the revisions. But the new call to this.batchDelete will create some on all sync sources. I don't follow what all the other changes are for though

From what I understand, there were no deleted_items table entries created when deleting the revisions. But the new call to this.batchDelete will create some on all sync sources.

That's correct! The other changes include:

• Refactoring to simplify calling batchDelete.
• Removed the SQL query that deleted revisions previously — it has been replaced by batchDelete.
• Added more tests.
• Added logic to prevent revisions for items in a read-only share from being deleted.
  • As commented above, this change needs more testing: It might break if revisions still exist after the original item has been permanently deleted.

Added logic to prevent revisions for items in a read-only share from being deleted.

Currently, this is done by loading the original item (with BaseItem.loadItemById) and checking whether it's part of a read only share.

This could be problematic: If the original item doesn't exist (e.g. not downloaded yet or permanently deleted), then the revisions can be deleted anyway. I'm not sure how to prevent this — would checking if the parent folder for the revisions is part of a read-only share (with RevisionEntity.parent_id) be sufficient? Would it be possible/make sense to add a share_id property to revisions?

Update: Answered here.