From a16a66c37b61c22f4198f45504b3aca0a55ce376 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Thu, 6 Feb 2025 09:55:54 -0800 Subject: [PATCH] Desktop,Mobile: Sync: Fix share not marked as readonly if the recipient has an outgoing share (#11770) --- packages/lib/models/utils/readOnly.test.ts | 36 ++++++++++++++- packages/lib/models/utils/readOnly.ts | 3 +- packages/lib/testing/test-utils.ts | 52 +++++++++++++++++----- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/packages/lib/models/utils/readOnly.test.ts b/packages/lib/models/utils/readOnly.test.ts index 60c439921d..62b7ef3eb8 100644 --- a/packages/lib/models/utils/readOnly.test.ts +++ b/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 syncUserId = ''; +const checkReadOnly = (itemType: ModelType, item: ItemSlice, shareData: ShareState = defaultShareState, 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(); + }); }); diff --git a/packages/lib/models/utils/readOnly.ts b/packages/lib/models/utils/readOnly.ts index 0f4f10ee8f..f3b092d452 100644 --- a/packages/lib/models/utils/readOnly.ts +++ b/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); diff --git a/packages/lib/testing/test-utils.ts b/packages/lib/testing/test-utils.ts index 9812a78e65..efa84d3cff 100644 --- a/packages/lib/testing/test-utils.ts +++ b/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: { - id: shareId, - folder_id: '', - master_key_id: '', - note_id: '', - type: 1, - }, + share, 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) {