Server: Fixed issue when a notebook is shared, then unshared, then shared again

packages/server/src/models/ChangeModel.ts

@@ -230,6 +230,7 @@ export default class ChangeModel extends BaseModel<Change> {
 	//     create - delete => NOOP
 	//     update - update => update
 	//     update - delete => delete
+	//     delete - create => create
 	//
 	// There's one exception for changes that include a "previous_item". This is
 	// used to save specific properties about the previous state of the item,
@@ -237,6 +238,13 @@ export default class ChangeModel extends BaseModel<Change> {
 	// to know if an item has been moved from one folder to another. In that
 	// case, we need to know about each individual change, so they are not
 	// compressed.
+	//
+	// The latest change, when an item goes from DELETE to CREATE seems odd but
+	// can happen because we are not checking for "item" changes but for
+	// "user_item" changes. When sharing is involved, an item can be shared
+	// (CREATED), then unshared (DELETED), then shared again (CREATED). When it
+	// happens, we want the user to get the item, thus we generate a CREATE
+	// event.
 	private compressChanges(changes: Change[]): Change[] {
 		const itemChanges: Record<Uuid, Change> = {};
 
@@ -268,6 +276,10 @@ export default class ChangeModel extends BaseModel<Change> {
 				if (previous.type === ChangeType.Update && change.type === ChangeType.Delete) {
 					itemChanges[itemId] = change;
 				}
+
+				if (previous.type === ChangeType.Delete && change.type === ChangeType.Create) {
+					itemChanges[itemId] = change;
+				}
 			} else {
 				itemChanges[itemId] = change;
 			}

packages/server/src/routes/api/shares.folder.test.ts

@@ -2,7 +2,7 @@ import { ChangeType, Share, ShareType, ShareUser, ShareUserStatus } from '../../
 import { beforeAllDb, afterAllTests, beforeEachDb, createUserAndSession, models, createNote, createFolder, updateItem, createItemTree, makeNoteSerializedBody, updateNote, expectHttpError, createResource } from '../../utils/testing/testUtils';
 import { postApi, patchApi, getApi, deleteApi } from '../../utils/testing/apiUtils';
 import { PaginatedDeltaChanges } from '../../models/ChangeModel';
-import { shareFolderWithUser } from '../../utils/testing/shareApiUtils';
+import { inviteUserToShare, shareFolderWithUser } from '../../utils/testing/shareApiUtils';
 import { msleep } from '../../utils/time';
 import { ErrorForbidden } from '../../utils/errors';
 import { resourceBlobPath, serializeJoplinItem, unserializeJoplinItem } from '../../utils/joplinUtils';
@@ -860,4 +860,45 @@ describe('shares.folder', function() {
 		);
 	});
 
+	test('should allow sharing, unsharing and sharing again', async function() {
+		// - U1 share a folder that contains a note
+		// - U2 accept
+		// - U2 syncs
+		// - U1 remove U2
+		// - U1 adds back U2
+		// - U2 accept
+		//
+		// => Previously, the notebook would be deleted fro U2 due to a quirk in
+		// delta sync, that doesn't handle user_items being deleted, then
+		// created again. Instead U2 should end up with both the folder and the
+		// note.
+		//
+		// Ref: https://discourse.joplinapp.org/t/20977
+
+		const { session: session1 } = await createUserAndSession(1);
+		const { user: user2, session: session2 } = await createUserAndSession(2);
+
+		const { shareUser: shareUserA, share } = await shareFolderWithUser(session1.id, session2.id, '000000000000000000000000000000F1', {
+			'000000000000000000000000000000F1': {
+				'00000000000000000000000000000001': null,
+			},
+		});
+
+		await models().share().updateSharedItems3();
+
+		await deleteApi(session1.id, `share_users/${shareUserA.id}`);
+
+		await models().share().updateSharedItems3();
+
+		await inviteUserToShare(share, session1.id, user2.email, true);
+
+		await models().share().updateSharedItems3();
+
+		const page = await getApi<PaginatedDeltaChanges>(session2.id, 'items/root/delta', { query: { cursor: '' } });
+
+		expect(page.items.length).toBe(2);
+		expect(page.items.find(it => it.item_name === '000000000000000000000000000000F1.md').type).toBe(ChangeType.Create);
+		expect(page.items.find(it => it.item_name === '00000000000000000000000000000001.md').type).toBe(ChangeType.Create);
+	});
+
 });

packages/server/src/utils/testing/shareApiUtils.ts

@@ -150,13 +150,17 @@ export async function shareWithUserAndAccept(sharerSessionId: string, shareeSess
 
 	shareUser = await models().shareUser().load(shareUser.id);
 
-	await patchApi(shareeSessionId, `share_users/${shareUser.id}`, { status: ShareUserStatus.Accepted });
+	await respondInvitation(shareeSessionId, shareUser.id, ShareUserStatus.Accepted);
 
 	await models().share().updateSharedItems3();
 
 	return { share, item, shareUser };
 }
 
+export async function respondInvitation(recipientSessionId: Uuid, shareUserId: Uuid, status: ShareUserStatus) {
+	await patchApi(recipientSessionId, `share_users/${shareUserId}`, { status });
+}
+
 export async function postShareContext(sessionId: string, shareType: ShareType, itemId: Uuid): Promise<AppContext> {
 	const context = await koaAppContext({
 		sessionId: sessionId,