From c084dcd11ef9a781c3ee9f1e51d0060f22e87717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Fri, 22 Sep 2023 15:09:45 +0200 Subject: [PATCH] chore: enable vault tests (#2518) --- .../services/src/Domain/Vault/VaultService.ts | 17 ++-- .../src/Domain/Vault/VaultServiceInterface.ts | 5 +- .../snjs/mocha/TestRegistry/VaultTests.js | 2 +- packages/snjs/mocha/lib/Collaboration.js | 9 +- packages/snjs/mocha/vaults/deletion.test.js | 4 +- packages/snjs/mocha/vaults/files.test.js | 25 +++--- packages/snjs/mocha/vaults/revisions.test.js | 84 +++++++++++++++---- .../Vaults/AddToVaultMenuOption.tsx | 5 +- .../Controllers/LinkingController.spec.ts | 3 +- .../Controllers/LinkingController.tsx | 13 ++- 10 files changed, 116 insertions(+), 51 deletions(-) diff --git a/packages/services/src/Domain/Vault/VaultService.ts b/packages/services/src/Domain/Vault/VaultService.ts index 7c27eb859..bcd30b0d0 100644 --- a/packages/services/src/Domain/Vault/VaultService.ts +++ b/packages/services/src/Domain/Vault/VaultService.ts @@ -146,7 +146,7 @@ export class VaultService async moveItemToVault( vault: VaultListingInterface, item: DecryptedItemInterface, - ): Promise { + ): Promise> { if (this.vaultLocks.isVaultLocked(vault)) { throw new Error('Attempting to add item to locked vault') } @@ -167,13 +167,16 @@ export class VaultService text: 'This item is linked to other items that are not in the same vault. Please move those items to this vault first.', }) .catch(console.error) - return undefined + + return Result.fail( + 'This item is linked to other items that are not in the same vault. Please move those items to this vault first.', + ) } } await this._moveItemsToVault.execute({ vault, items: [item] }) - return this.items.findSureItem(item.uuid) + return Result.ok(this.items.findSureItem(item.uuid)) } async removeItemFromVault(item: DecryptedItemInterface): Promise { @@ -248,16 +251,16 @@ export class VaultService } getItemVault(item: DecryptedItemInterface): VaultListingInterface | undefined { - if (this.items.isTemplateItem(item)) { - return undefined - } - const latestItem = this.items.findItem(item.uuid) if (!latestItem) { throw new Error('Cannot find latest version of item to get vault for') } + if (this.items.isTemplateItem(item)) { + return undefined + } + if (!latestItem.key_system_identifier) { return undefined } diff --git a/packages/services/src/Domain/Vault/VaultServiceInterface.ts b/packages/services/src/Domain/Vault/VaultServiceInterface.ts index fcf7d60b8..ef6bb4e57 100644 --- a/packages/services/src/Domain/Vault/VaultServiceInterface.ts +++ b/packages/services/src/Domain/Vault/VaultServiceInterface.ts @@ -32,10 +32,7 @@ export interface VaultServiceInterface authorizeVaultDeletion(vault: VaultListingInterface): Promise> deleteVault(vault: VaultListingInterface): Promise - moveItemToVault( - vault: VaultListingInterface, - item: DecryptedItemInterface, - ): Promise + moveItemToVault(vault: VaultListingInterface, item: DecryptedItemInterface): Promise> removeItemFromVault(item: DecryptedItemInterface): Promise isItemInVault(item: DecryptedItemInterface): boolean getItemVault(item: DecryptedItemInterface): VaultListingInterface | undefined diff --git a/packages/snjs/mocha/TestRegistry/VaultTests.js b/packages/snjs/mocha/TestRegistry/VaultTests.js index d54e840b3..d1df71260 100644 --- a/packages/snjs/mocha/TestRegistry/VaultTests.js +++ b/packages/snjs/mocha/TestRegistry/VaultTests.js @@ -1,5 +1,5 @@ export const VaultTests = { - enabled: false, + enabled: true, exclusive: false, files: [ 'vaults/vaults.test.js', diff --git a/packages/snjs/mocha/lib/Collaboration.js b/packages/snjs/mocha/lib/Collaboration.js index 0af922613..2edb42629 100644 --- a/packages/snjs/mocha/lib/Collaboration.js +++ b/packages/snjs/mocha/lib/Collaboration.js @@ -198,7 +198,12 @@ export const createSharedVaultWithNote = async (context) => { export const moveItemToVault = async (context, sharedVault, item) => { const promise = context.resolveWhenItemCompletesAddingToVault(item) - const updatedItem = await context.vaults.moveItemToVault(sharedVault, item) + const result = await context.vaults.moveItemToVault(sharedVault, item) await promise - return updatedItem + + if (result.isFailed()) { + throw new Error(result.getError()) + } + + return result.getValue() } diff --git a/packages/snjs/mocha/vaults/deletion.test.js b/packages/snjs/mocha/vaults/deletion.test.js index 63fd9b700..d61f0ec84 100644 --- a/packages/snjs/mocha/vaults/deletion.test.js +++ b/packages/snjs/mocha/vaults/deletion.test.js @@ -142,13 +142,13 @@ describe('shared vault deletion', function () { const { sharedVault, contactContext, deinitContactContext } = await Collaboration.createSharedVaultWithAcceptedInvite(context) - const originalSharedVaultUsers = await context.vaultUsers.getSharedVaultUsers(sharedVault) + const originalSharedVaultUsers = await context.vaultUsers.getSharedVaultUsersFromServer(sharedVault) expect(originalSharedVaultUsers.length).to.equal(2) const result = await context.vaultUsers.removeUserFromSharedVault(sharedVault, contactContext.userUuid) expect(result.isFailed()).to.be.false - const updatedSharedVaultUsers = await context.vaultUsers.getSharedVaultUsers(sharedVault) + const updatedSharedVaultUsers = await context.vaultUsers.getSharedVaultUsersFromServer(sharedVault) expect(updatedSharedVaultUsers.length).to.equal(1) await deinitContactContext() diff --git a/packages/snjs/mocha/vaults/files.test.js b/packages/snjs/mocha/vaults/files.test.js index 3e92f1f81..dca236c60 100644 --- a/packages/snjs/mocha/vaults/files.test.js +++ b/packages/snjs/mocha/vaults/files.test.js @@ -67,7 +67,7 @@ describe('shared vault files', function () { const uploadedFile = await Files.uploadFile(context.files, buffer, 'my-file', 'md', 1000) const sharedVault = await Collaboration.createSharedVault(context) - const addedFile = await context.vaults.moveItemToVault(sharedVault, uploadedFile) + const addedFile = await Collaboration.moveItemToVault(context, sharedVault, uploadedFile) const downloadedBytes = await Files.downloadFile(context.files, addedFile) expect(downloadedBytes).to.eql(buffer) @@ -81,7 +81,7 @@ describe('shared vault files', function () { const uploadedFile = await Files.uploadFile(context.files, buffer, 'my-file', 'md', 1000, firstVault) const secondVault = await Collaboration.createSharedVault(context) - const movedFile = await context.vaults.moveItemToVault(secondVault, uploadedFile) + const movedFile = await Collaboration.moveItemToVault(context, secondVault, uploadedFile) const downloadedBytes = await Files.downloadFile(context.files, movedFile) expect(downloadedBytes).to.eql(buffer) @@ -95,13 +95,13 @@ describe('shared vault files', function () { const uploadedFile = await Files.uploadFile(context.files, buffer, 'my-file', 'md', 1000, firstVault) const privateVault = await Collaboration.createPrivateVault(context) - const addedFile = await context.vaults.moveItemToVault(privateVault, uploadedFile) + const addedFile = await Collaboration.moveItemToVault(context, privateVault, uploadedFile) const downloadedBytes = await Files.downloadFile(context.files, addedFile) expect(downloadedBytes).to.eql(buffer) }) - it('moving a note to a vault should also moved linked files', async () => { + it('should not move a note to a vault that is linked with files ', async () => { const note = await context.createSyncedNote() const response = await fetch('/mocha/assets/small_file.md') const buffer = new Uint8Array(await response.arrayBuffer()) @@ -111,17 +111,12 @@ describe('shared vault files', function () { const sharedVault = await Collaboration.createSharedVault(context) - context.vaults.alerts.confirmV2 = () => Promise.resolve(true) + context.vaults.alerts.alertV2 = () => Promise.resolve(true) - await context.vaults.moveItemToVault(sharedVault, note) - - const latestFile = context.items.findItem(updatedFile.uuid) - - expect(context.vaults.getItemVault(latestFile).uuid).to.equal(sharedVault.uuid) - expect(context.vaults.getItemVault(context.items.findItem(note.uuid)).uuid).to.equal(sharedVault.uuid) - - const downloadedBytes = await Files.downloadFile(context.files, latestFile) - expect(downloadedBytes).to.eql(buffer) + await Factory.expectThrowsAsync( + () => Collaboration.moveItemToVault(context, sharedVault, note), + 'This item is linked to other items that are not in the same vault. Please move those items to this vault first.', + ) }) it('should be able to move a file out of its vault', async () => { @@ -225,7 +220,7 @@ describe('shared vault files', function () { const response = await fetch('/mocha/assets/small_file.md') const buffer = new Uint8Array(await response.arrayBuffer()) const uploadedFile = await Files.uploadFile(context.files, buffer, 'my-file', 'md', 1000) - const addedFile = await context.vaults.moveItemToVault(sharedVault, uploadedFile) + const addedFile = await Collaboration.moveItemToVault(context, sharedVault, uploadedFile) await contactContext.sync() diff --git a/packages/snjs/mocha/vaults/revisions.test.js b/packages/snjs/mocha/vaults/revisions.test.js index 667fb91da..78591ab43 100644 --- a/packages/snjs/mocha/vaults/revisions.test.js +++ b/packages/snjs/mocha/vaults/revisions.test.js @@ -46,11 +46,29 @@ describe('shared vault revisions', function () { await Factory.sleep(Factory.ServerRevisionCreationDelay) const contactItemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) - const contactItemHistory = contactItemHistoryOrError.getValue() + expect(contactItemHistoryOrError.isFailed()).to.equal(false) + let contactItemHistory = contactItemHistoryOrError.getValue() + if (contactItemHistory.length < 2) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') + + const contactItemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(contactItemHistoryOrError.isFailed()).to.equal(false) + + contactItemHistory = contactItemHistoryOrError.getValue() + } expect(contactItemHistory.length >= 2).to.be.true const itemHistoryOrError = await context.application.listRevisions.execute({ itemUuid: note.uuid }) - const itemHistory = itemHistoryOrError.getValue() + expect(itemHistoryOrError.isFailed()).to.equal(false) + let itemHistory = itemHistoryOrError.getValue() + if (itemHistory.length < 2) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') + + const itemHistoryOrError = await context.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistory = itemHistoryOrError.getValue() + } expect(itemHistory.length >= 2).to.be.true }) @@ -67,7 +85,16 @@ describe('shared vault revisions', function () { await Factory.sleep(Factory.ServerRevisionCreationDelay) const itemHistoryOrError = await context.application.listRevisions.execute({ itemUuid: note.uuid }) - const itemHistory = itemHistoryOrError.getValue() + expect(itemHistoryOrError.isFailed()).to.equal(false) + let itemHistory = itemHistoryOrError.getValue() + if (itemHistory.length < 2) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') + + const itemHistoryOrError = await context.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistory = itemHistoryOrError.getValue() + } expect(itemHistory.length >= 2).to.be.true }) @@ -82,7 +109,16 @@ describe('shared vault revisions', function () { await Factory.sleep(Factory.ServerRevisionCreationDelay) const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) - const itemHistory = itemHistoryOrError.getValue() + expect(itemHistoryOrError.isFailed()).to.equal(false) + let itemHistory = itemHistoryOrError.getValue() + if (itemHistory.length < 1) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') + + const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistory = itemHistoryOrError.getValue() + } const newestRevision = itemHistory[0] const fetchedOrError = await contactContext.application.getRevision.execute({ @@ -110,9 +146,16 @@ describe('shared vault revisions', function () { await Factory.sleep(Factory.ServerRevisionCreationDelay) const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) - expect(itemHistoryOrError.isFailed()).to.be.false + expect(itemHistoryOrError.isFailed()).to.equal(false) + let itemHistory = itemHistoryOrError.getValue() + if (itemHistory.length != 0) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') - const itemHistory = itemHistoryOrError.getValue() + const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistory = itemHistoryOrError.getValue() + } expect(itemHistory.length).to.equal(0) }) @@ -127,19 +170,32 @@ describe('shared vault revisions', function () { await Factory.sleep(Factory.ServerRevisionCreationDelay) - let itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) - expect(itemHistoryOrError.isFailed()).to.be.false - let itemHistory = itemHistoryOrError.getValue() + const itemHistoryBeforeOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryBeforeOrError.isFailed()).to.equal(false) + let itemHistoryBefore = itemHistoryBeforeOrError.getValue() + if (itemHistoryBefore.length < 1) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') - expect(itemHistory.length >= 1).to.be.true + const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistoryBefore = itemHistoryOrError.getValue() + } + expect(itemHistoryBefore.length >= 1).to.be.true await context.vaultUsers.removeUserFromSharedVault(sharedVault, contactContext.userUuid) await Factory.sleep(Factory.ServerRevisionCreationDelay) - itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) - expect(itemHistoryOrError.isFailed()).to.be.false - itemHistory = itemHistoryOrError.getValue() + const itemHistoryAfterOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + let itemHistoryAfter = itemHistoryAfterOrError.getValue() + if (itemHistoryAfter.length != 0) { + await Factory.sleep(Factory.ServerRevisionCreationDelay, 'Not enough revisions found on the server. This is likely a delay issue. Retrying...') - expect(itemHistory.length).to.equal(0) + const itemHistoryOrError = await contactContext.application.listRevisions.execute({ itemUuid: note.uuid }) + expect(itemHistoryOrError.isFailed()).to.equal(false) + + itemHistoryAfter = itemHistoryOrError.getValue() + } + expect(itemHistoryAfter.length).to.equal(0) }) }) diff --git a/packages/web/src/javascripts/Components/Vaults/AddToVaultMenuOption.tsx b/packages/web/src/javascripts/Components/Vaults/AddToVaultMenuOption.tsx index eaffab1a8..9cf19c9c8 100644 --- a/packages/web/src/javascripts/Components/Vaults/AddToVaultMenuOption.tsx +++ b/packages/web/src/javascripts/Components/Vaults/AddToVaultMenuOption.tsx @@ -22,7 +22,10 @@ const VaultMenu = observer(({ items }: { items: DecryptedItemInterface[] }) => { } for (const item of items) { - await application.vaults.moveItemToVault(vault, item) + const result = await application.vaults.moveItemToVault(vault, item) + if (result.isFailed()) { + console.error(result.getError()) + } } }, [application, items], diff --git a/packages/web/src/javascripts/Controllers/LinkingController.spec.ts b/packages/web/src/javascripts/Controllers/LinkingController.spec.ts index 6d2c5ddbf..b377c4c47 100644 --- a/packages/web/src/javascripts/Controllers/LinkingController.spec.ts +++ b/packages/web/src/javascripts/Controllers/LinkingController.spec.ts @@ -15,6 +15,7 @@ import { InternalFeatureService, InternalFeature, PreferenceServiceInterface, + Result, } from '@standardnotes/snjs' import { FilesController } from './FilesController' import { ItemListController } from './ItemList/ItemListController' @@ -246,7 +247,7 @@ describe('LinkingController', () => { application.mutator.associateFileWithNote = jest.fn().mockReturnValue({}) - const moveToVaultSpy = (application.vaults.moveItemToVault = jest.fn()) + const moveToVaultSpy = (application.vaults.moveItemToVault = jest.fn().mockReturnValue(Result.ok())) const note = createNote('test', { uuid: 'note', diff --git a/packages/web/src/javascripts/Controllers/LinkingController.tsx b/packages/web/src/javascripts/Controllers/LinkingController.tsx index d61358590..43d067567 100644 --- a/packages/web/src/javascripts/Controllers/LinkingController.tsx +++ b/packages/web/src/javascripts/Controllers/LinkingController.tsx @@ -213,7 +213,10 @@ export class LinkingController extends AbstractViewController implements Interna const noteVault = this.vaults.getItemVault(note) const fileVault = this.vaults.getItemVault(updatedFile) if (noteVault && !fileVault) { - await this.vaults.moveItemToVault(noteVault, file) + const result = await this.vaults.moveItemToVault(noteVault, file) + if (result.isFailed()) { + console.error(result.getError()) + } } } } @@ -305,10 +308,12 @@ export class LinkingController extends AbstractViewController implements Interna const itemVault = this.vaults.getItemVault(activeItem) if (itemVault) { - const movedTag = await this.vaults.moveItemToVault(itemVault, newTag) - if (!movedTag) { - throw new Error('Failed to move tag to item vault') + const movedTagOrError = await this.vaults.moveItemToVault(itemVault, newTag) + if (movedTagOrError.isFailed()) { + throw new Error(`Failed to move tag to item vault: ${movedTagOrError.getError()}`) } + const movedTag = movedTagOrError.getValue() + await this.addTagToItem(movedTag as SNTag, activeItem) } else { await this.addTagToItem(newTag, activeItem)