fix handling result

packages/services/src/Domain/Vault/VaultService.ts

@@ -146,7 +146,7 @@ export class VaultService
   async moveItemToVault(
     vault: VaultListingInterface,
     item: DecryptedItemInterface,
-  ): Promise<DecryptedItemInterface | undefined> {
+  ): Promise<Result<DecryptedItemInterface>> {
     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<DecryptedItemInterface> {
@@ -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
     }

packages/services/src/Domain/Vault/VaultServiceInterface.ts

@@ -32,10 +32,7 @@ export interface VaultServiceInterface
   authorizeVaultDeletion(vault: VaultListingInterface): Promise<Result<boolean>>
   deleteVault(vault: VaultListingInterface): Promise<boolean>
 
-  moveItemToVault(
-    vault: VaultListingInterface,
-    item: DecryptedItemInterface,
-  ): Promise<DecryptedItemInterface | undefined>
+  moveItemToVault(vault: VaultListingInterface, item: DecryptedItemInterface): Promise<Result<DecryptedItemInterface>>
   removeItemFromVault(item: DecryptedItemInterface): Promise<DecryptedItemInterface>
   isItemInVault(item: DecryptedItemInterface): boolean
   getItemVault(item: DecryptedItemInterface): VaultListingInterface | undefined

packages/snjs/mocha/TestRegistry/VaultTests.js

@@ -1,5 +1,5 @@
 export const VaultTests = {
-  enabled: false,
+  enabled: true,
   exclusive: false,
   files: [
     'vaults/vaults.test.js',

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()
 }

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()

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()
 

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)
   })
 })

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],

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',

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)