diff --git a/packages/lib/models/Folder.sharing.test.ts b/packages/lib/models/Folder.sharing.test.ts index b8854edfcc..c702a5cbed 100644 --- a/packages/lib/models/Folder.sharing.test.ts +++ b/packages/lib/models/Folder.sharing.test.ts @@ -505,7 +505,7 @@ describe('models/Folder.sharing', () => { expect(note4.user_updated_time).toBe(userUpdatedTimes[note4.id]); }); - it('should unshare items that are no longer part of an existing share', async () => { + it('should clear share_ids for items that are no longer part of an existing share', async () => { await createFolderTree('', [ { title: 'folder 1', @@ -591,4 +591,34 @@ describe('models/Folder.sharing', () => { } }); + it('should not change the updated_time when clearing share_ids for no-longer-shared items', async () => { + await createFolderTree('', [ + { + title: 'folder 1', + share_id: '1', + children: [ + { + title: 'note 1', + }, + ], + }, + ]); + + await Folder.updateAllShareIds(resourceService(), []); + + const folder1 = await Folder.loadByTitle('folder 1'); + const note1 = await Note.loadByTitle('note 1'); + await Folder.updateNoLongerSharedItems([]); + + // To avoid conflicts, should not change the updated_time + expect(await Note.loadByTitle('note 1')).toMatchObject({ + updated_time: note1.updated_time, + share_id: '', + }); + expect(await Folder.loadByTitle('folder 1')).toMatchObject({ + updated_time: folder1.updated_time, + share_id: '', + }); + }); + }); diff --git a/packages/lib/models/Folder.ts b/packages/lib/models/Folder.ts index f3c5cf4c35..739b63ba96 100644 --- a/packages/lib/models/Folder.ts +++ b/packages/lib/models/Folder.ts @@ -1,4 +1,4 @@ -import { defaultFolderIcon, FolderEntity, FolderIcon, NoteEntity, ResourceEntity } from '../services/database/types'; +import { BaseItemEntity, defaultFolderIcon, FolderEntity, FolderIcon, NoteEntity, ResourceEntity } from '../services/database/types'; import BaseModel, { DeleteOptions } from '../BaseModel'; import { FolderLoadOptions } from './utils/types'; import time from '../time'; @@ -747,14 +747,18 @@ export default class Folder extends BaseItem { report[tableName] = rows.length; for (const row of rows) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const toSave: any = { + const toSave: BaseItemEntity = { id: row.id, share_id: '', - updated_time: Date.now(), + // Don't change the updated_time. + // This prevents conflicts in the case where the item was unshared remotely. + // See https://github.com/laurent22/joplin/issues/12648 + // updated_time: Date.now(), }; - if (hasParentId) toSave.parent_id = row.parent_id; + if (hasParentId) { + (toSave as FolderEntity|NoteEntity).parent_id = row.parent_id; + } await ItemClass.save(toSave, { autoTimestamp: false }); } diff --git a/readme/dev/spec/server_sharing.md b/readme/dev/spec/server_sharing.md index a5fde73ad6..c4c3a2446f 100644 --- a/readme/dev/spec/server_sharing.md +++ b/readme/dev/spec/server_sharing.md @@ -25,6 +25,18 @@ Technically, the server would only need to know the root shared folder, and from On the other hand, all that information is present on the client. Whenever a notes is moved out a shared folder, or whenever a resources is attached, the changes are tracked, and that can be used to easily assign a `share_id` property. Once this is set, it makes the whole system more simple and reliable. +### When are changes to `share_id` synced? + +In general, changes to `share_id` should be synced only if a Joplin client still has access to the remote item. For example, if `User1` adds an item to a folder shared with `User2`, then `User1` should update and sync the `share_id` for that item. + +However, there are some maintenance tasks that clear `share_id` locally when a user no longer has access to a share. These changes should not be synced to the server because the user no longer has access to the relevant remote items. For example, if, +1. `User1` shares `Folder A` with `User2`. +2. `User2` accepts the share. +3. `User1` unshares `Folder A`. +4. `User2` locally clears the `share_id` property on the local `Folder A`. + +At this point, `User2` no longer has access to `Folder A` on the server. They cannot sync the change to `share_id` because they no longer have access to the share. + ## Publishing a note via a public URL This is done by posting a note ID to `/api/shares`.