Desktop,Mobile: Sync: Fix share not marked as readonly if the recipient has an outgoing share (#11770)

packages/lib/models/utils/readOnly.test.ts

@@ -10,8 +10,7 @@ import { setupDatabaseAndSynchronizer, simulateReadOnlyShareEnv, switchClient, t
 import BaseItem from '../BaseItem';
 
 
-const checkReadOnly = (itemType: ModelType, item: ItemSlice, shareData: ShareState = defaultShareState) => {
+const checkReadOnly = (itemType: ModelType, item: ItemSlice, shareData: ShareState = defaultShareState, syncUserId = '') => {
-	const syncUserId = '';
 	return itemIsReadOnlySync(itemType, ItemChange.SOURCE_UNSPECIFIED, item, syncUserId, shareData);
 };
 
@@ -22,6 +21,21 @@ const createTestResource = async () => {
 	await shim.attachFileToNote(note1, tempFile);
 };
 
+const makeOwnerOfShare = (shareId: string, syncUserId: string) => {
+	BaseItem.syncShareCache.shares = BaseItem.syncShareCache.shares.map(share => {
+		if (share.id === shareId) {
+			return {
+				...share,
+				user: {
+					id: syncUserId,
+					email: 'test@example.com',
+				},
+			};
+		}
+		return share;
+	});
+};
+
 describe('readOnly', () => {
 	beforeEach(async () => {
 		await setupDatabaseAndSynchronizer(1);
@@ -64,4 +78,22 @@ describe('readOnly', () => {
 		expect(checkReadOnly(ModelType.Resource, resource, BaseItem.syncShareCache)).toBe(true);
 		cleanup();
 	});
+
+	test('should support checking that items are read-only when there are multiple shares', async () => {
+		const shareId1 = '123456';
+		const shareId2 = '234567';
+		const note = await Note.save({ body: 'test', share_id: shareId1 });
+
+		const syncUserId = 'test-user-id';
+		const cleanup = simulateReadOnlyShareEnv([shareId1, shareId2]);
+
+		// If the owner of a different share, should be read-only
+		makeOwnerOfShare(shareId2, syncUserId);
+		expect(checkReadOnly(ModelType.Note, note as ItemSlice, BaseItem.syncShareCache, syncUserId)).toBe(true);
+
+		// If also the owner of the same share, it should not be read-only
+		makeOwnerOfShare(shareId1, syncUserId);
+		expect(checkReadOnly(ModelType.Note, note as ItemSlice, BaseItem.syncShareCache, syncUserId)).toBe(false);
+		cleanup();
+	});
 });

packages/lib/models/utils/readOnly.ts

@@ -92,7 +92,8 @@ export const itemIsReadOnlySync = (itemType: ModelType, changeSource: number, it
 	if (!item.share_id) return false;
 
 	// Item belongs to the user
-	if (shareState.shares.find(s => s.user.id === userId)) return false;
+	const parentShare = shareState.shares.find(s => s.id === item.share_id);
+	if (parentShare && parentShare.user?.id === userId) return false;
 
 	const shareUser = shareState.shareInvitations.find(si => si.share.id === item.share_id);
 

packages/lib/testing/test-utils.ts

@@ -1023,21 +1023,23 @@ class TestApp extends BaseApplication {
 }
 
 const createTestShareData = (shareId: string): ShareState => {
+	const share = {
+		id: shareId,
+		folder_id: '',
+		master_key_id: '',
+		note_id: '',
+		type: 1,
+	};
+
 	return {
 		processingShareInvitationResponse: false,
-		shares: [],
+		shares: [share],
 		shareInvitations: [
 			{
 				id: '',
 				master_key: {},
 				status: 0,
-				share: {
+				share,
-					id: shareId,
-					folder_id: '',
-					master_key_id: '',
-					note_id: '',
-					type: 1,
-				},
 				can_read: 1,
 				can_write: 0,
 			},
@@ -1046,10 +1048,40 @@ const createTestShareData = (shareId: string): ShareState => {
 	};
 };
 
-const simulateReadOnlyShareEnv = (shareId: string, store?: Store) => {
+const mergeShareData = (state1: ShareState, state2: ShareState) => {
+	return {
+		...state1,
+		shares: [...state1.shares, ...state2.shares],
+		shareInvitations: [
+			...state1.shareInvitations,
+			...state2.shareInvitations,
+		],
+		shareUsers: {
+			...state1.shareUsers,
+			...state2.shareUsers,
+		},
+	};
+};
+
+const simulateReadOnlyShareEnv = (shareIds: string[]|string, store?: Store) => {
+	if (!Array.isArray(shareIds)) {
+		shareIds = [shareIds];
+	}
+
 	Setting.setValue('sync.target', 10);
 	Setting.setValue('sync.userId', 'abcd');
-	const shareData = createTestShareData(shareId);
+
+	// Create all shares
+	let shareData: ShareState|null = null;
+	for (const shareId of shareIds) {
+		const newShareData = createTestShareData(shareId);
+		if (!shareData) {
+			shareData = newShareData;
+		} else {
+			shareData = mergeShareData(shareData, newShareData);
+		}
+	}
+
 	BaseItem.syncShareCache = shareData;
 
 	if (store) {