fix: issue with not being able to unlink a file from a note (#1836)

standardnotes · Oct 19, 2022 · 4030953 · 4030953
1 parent ba9a9f8
commit 4030953
Show file tree

Hide file tree

Showing 10 changed files with 123 additions and 244 deletions.
diff --git a/packages/models/src/Domain/Syncable/File/File.ts b/packages/models/src/Domain/Syncable/File/File.ts
@@ -4,7 +4,8 @@ import { DecryptedPayloadInterface } from '../../Abstract/Payload/Interfaces/Dec
 import { FileMetadata } from './FileMetadata'
 import { FileProtocolV1 } from './FileProtocolV1'
 import { SortableItem } from '../../Runtime/Collection/CollectionSort'
-import { ConflictStrategy } from '../../Abstract/Item'
+import { ConflictStrategy, ItemInterface } from '../../Abstract/Item'
+import { ContentType } from '@standardnotes/common'
 
 type EncryptedBytesLength = number
 type DecryptedBytesLength = number
@@ -31,6 +32,8 @@ export type FileContentSpecialized = FileContentWithoutSize & FileMetadata & Siz
 
 export type FileContent = FileContentSpecialized & ItemContent
 
+export const isFile = (x: ItemInterface): x is FileItem => x.content_type === ContentType.File
+
 export class FileItem
   extends DecryptedItem<FileContent>
   implements FileContentWithoutSize, Sizes, FileProtocolV1, FileMetadata, SortableItem

diff --git a/packages/services/src/Domain/Item/ItemRelationshipDirection.ts b/packages/services/src/Domain/Item/ItemRelationshipDirection.ts
@@ -0,0 +1,7 @@
+/* istanbul ignore file */
+
+export enum ItemRelationshipDirection {
+  AReferencesB,
+  BReferencesA,
+  NoRelationship,
+}
diff --git a/packages/services/src/Domain/Item/ItemsClientInterface.ts b/packages/services/src/Domain/Item/ItemsClientInterface.ts
@@ -1,3 +1,5 @@
+/* istanbul ignore file */
+
 import { ContentType, Uuid } from '@standardnotes/common'
 import {
   SNNote,
@@ -76,9 +78,9 @@ export interface ItemsClientInterface {
   linkNoteToNote(note: SNNote, otherNote: SNNote): Promise<SNNote>
   linkFileToFile(file: FileItem, otherFile: FileItem): Promise<FileItem>
 
-  unlinkItem(
-    item: DecryptedItemInterface<ItemContent>,
-    itemToUnlink: DecryptedItemInterface<ItemContent>,
+  unlinkItems(
+    itemOne: DecryptedItemInterface<ItemContent>,
+    itemTwo: DecryptedItemInterface<ItemContent>,
   ): Promise<DecryptedItemInterface<ItemContent>>
 
   /**
@@ -115,12 +117,6 @@ export interface ItemsClientInterface {
    */
   getSortedTagsForItem(item: DecryptedItemInterface<ItemContent>): SNTag[]
 
-  getSortedLinkedFilesForItem(item: DecryptedItemInterface<ItemContent>): FileItem[]
-  getSortedFilesLinkingToItem(item: DecryptedItemInterface<ItemContent>): FileItem[]
-
-  getSortedLinkedNotesForItem(item: DecryptedItemInterface<ItemContent>): SNNote[]
-  getSortedNotesLinkingToItem(item: DecryptedItemInterface<ItemContent>): SNNote[]
-
   isSmartViewTitle(title: string): boolean
 
   getSmartViews(): SmartView[]
@@ -152,12 +148,4 @@ export interface ItemsClientInterface {
    * @returns Whether the item is a template (unmanaged)
    */
   isTemplateItem(item: DecryptedItemInterface): boolean
-
-  /**
-   * @returns `'direct'` if `itemOne` has the reference to `itemTwo`, `'indirect'` if `itemTwo` has the reference to `itemOne`, `'unlinked'` if neither reference each other
-   */
-  relationshipTypeForItems(
-    itemOne: DecryptedItemInterface,
-    itemTwo: DecryptedItemInterface,
-  ): 'direct' | 'indirect' | 'unlinked'
 }
diff --git a/packages/services/src/Domain/index.ts b/packages/services/src/Domain/index.ts
@@ -54,6 +54,7 @@ export * from './Item/ItemCounterInterface'
 export * from './Item/ItemManagerInterface'
 export * from './Item/ItemsClientInterface'
 export * from './Item/ItemsServerInterface'
+export * from './Item/ItemRelationshipDirection'
 export * from './Mutator/MutatorClientInterface'
 export * from './Payloads/PayloadManagerInterface'
 export * from './Preferences/PreferenceServiceInterface'

diff --git a/packages/snjs/lib/Services/Items/ItemManager.spec.ts b/packages/snjs/lib/Services/Items/ItemManager.spec.ts
@@ -1,5 +1,5 @@
 import { ContentType } from '@standardnotes/common'
-import { InternalEventBusInterface } from '@standardnotes/services'
+import { InternalEventBusInterface, ItemRelationshipDirection } from '@standardnotes/services'
 import { ItemManager } from './ItemManager'
 import { PayloadManager } from '../Payloads/PayloadManager'
 import { UuidGenerator } from '@standardnotes/utils'
@@ -784,21 +784,6 @@ describe('itemManager', () => {
       expect(references).toHaveLength(0)
     })
 
-    it('should get files linked with note', async () => {
-      itemManager = createService()
-      const note = createNoteWithTitle('invoices')
-      const file = createFile('invoice_1.pdf')
-      const secondFile = createFile('unrelated-file.xlsx')
-      await itemManager.insertItems([note, file, secondFile])
-
-      await itemManager.associateFileWithNote(file, note)
-
-      const filesAssociatedWithNote = itemManager.getSortedFilesLinkingToItem(note)
-
-      expect(filesAssociatedWithNote).toHaveLength(1)
-      expect(filesAssociatedWithNote[0].uuid).toBe(file.uuid)
-    })
-
     it('should link note to note', async () => {
       itemManager = createService()
       const note = createNoteWithTitle('research')
@@ -834,101 +819,49 @@ describe('itemManager', () => {
 
       const firstNoteLinkedToSecond = await itemManager.linkNoteToNote(firstNote, secondNote)
 
-      const relationshipOfFirstNoteToSecond = itemManager.relationshipTypeForItems(firstNoteLinkedToSecond, secondNote)
-      const relationshipOfSecondNoteToFirst = itemManager.relationshipTypeForItems(secondNote, firstNoteLinkedToSecond)
-      const relationshipOfFirstNoteToUnlinked = itemManager.relationshipTypeForItems(
+      const relationshipOfFirstNoteToSecond = itemManager.relationshipDirectionBetweenItems(
+        firstNoteLinkedToSecond,
+        secondNote,
+      )
+      const relationshipOfSecondNoteToFirst = itemManager.relationshipDirectionBetweenItems(
+        secondNote,
+        firstNoteLinkedToSecond,
+      )
+      const relationshipOfFirstNoteToUnlinked = itemManager.relationshipDirectionBetweenItems(
         firstNoteLinkedToSecond,
         unlinkedNote,
       )
 
-      expect(relationshipOfFirstNoteToSecond).toBe('direct')
-      expect(relationshipOfSecondNoteToFirst).toBe('indirect')
-      expect(relationshipOfFirstNoteToUnlinked).toBe('unlinked')
+      expect(relationshipOfFirstNoteToSecond).toBe(ItemRelationshipDirection.AReferencesB)
+      expect(relationshipOfSecondNoteToFirst).toBe(ItemRelationshipDirection.BReferencesA)
+      expect(relationshipOfFirstNoteToUnlinked).toBe(ItemRelationshipDirection.NoRelationship)
     })
 
-    it('should unlink itemToUnlink from item', async () => {
+    it('should unlink itemOne from itemTwo if relation is direct', async () => {
       itemManager = createService()
       const note = createNoteWithTitle('Note 1')
       const note2 = createNoteWithTitle('Note 2')
       await itemManager.insertItems([note, note2])
 
       const linkedItem = await itemManager.linkNoteToNote(note, note2)
-      const unlinkedItem = await itemManager.unlinkItem(linkedItem, note2)
+      const unlinkedItem = await itemManager.unlinkItems(linkedItem, note2)
       const references = unlinkedItem.references
 
+      expect(unlinkedItem.uuid).toBe(note.uuid)
       expect(references).toHaveLength(0)
     })
 
-    it('should get all linked files for item', async () => {
-      itemManager = createService()
-      const file = createFile('A1')
-      const file2 = createFile('B2')
-      const file3 = createFile('C3')
-
-      await itemManager.insertItems([file, file2, file3])
-
-      await itemManager.linkFileToFile(file, file3)
-      await itemManager.linkFileToFile(file, file2)
-
-      const sortedFilesForItem = itemManager.getSortedLinkedFilesForItem(file)
-
-      expect(sortedFilesForItem).toHaveLength(2)
-      expect(sortedFilesForItem[0].uuid).toEqual(file2.uuid)
-      expect(sortedFilesForItem[1].uuid).toEqual(file3.uuid)
-    })
-
-    it('should get all files linking to item', async () => {
+    it('should unlink itemTwo from itemOne if relation is indirect', async () => {
       itemManager = createService()
-      const baseFile = createFile('file')
-      const fileToLink1 = createFile('A1')
-      const fileToLink2 = createFile('B2')
-
-      await itemManager.insertItems([baseFile, fileToLink1, fileToLink2])
-
-      await itemManager.linkFileToFile(fileToLink2, baseFile)
-      await itemManager.linkFileToFile(fileToLink1, baseFile)
-
-      const sortedFilesForItem = itemManager.getSortedFilesLinkingToItem(baseFile)
-
-      expect(sortedFilesForItem).toHaveLength(2)
-      expect(sortedFilesForItem[0].uuid).toEqual(fileToLink1.uuid)
-      expect(sortedFilesForItem[1].uuid).toEqual(fileToLink2.uuid)
-    })
-
-    it('should get all linked notes for item', async () => {
-      itemManager = createService()
-      const baseNote = createNoteWithTitle('note')
-      const noteToLink1 = createNoteWithTitle('A1')
-      const noteToLink2 = createNoteWithTitle('B2')
-
-      await itemManager.insertItems([baseNote, noteToLink1, noteToLink2])
-
-      await itemManager.linkNoteToNote(baseNote, noteToLink2)
-      await itemManager.linkNoteToNote(baseNote, noteToLink1)
-
-      const sortedFilesForItem = itemManager.getSortedLinkedNotesForItem(baseNote)
-
-      expect(sortedFilesForItem).toHaveLength(2)
-      expect(sortedFilesForItem[0].uuid).toEqual(noteToLink1.uuid)
-      expect(sortedFilesForItem[1].uuid).toEqual(noteToLink2.uuid)
-    })
-
-    it('should get all notes linking to item', async () => {
-      itemManager = createService()
-      const baseNote = createNoteWithTitle('note')
-      const noteToLink1 = createNoteWithTitle('A1')
-      const noteToLink2 = createNoteWithTitle('B2')
-
-      await itemManager.insertItems([baseNote, noteToLink1, noteToLink2])
-
-      await itemManager.linkNoteToNote(noteToLink2, baseNote)
-      await itemManager.linkNoteToNote(noteToLink1, baseNote)
+      const note = createNoteWithTitle('Note 1')
+      const note2 = createNoteWithTitle('Note 2')
+      await itemManager.insertItems([note, note2])
 
-      const sortedFilesForItem = itemManager.getSortedNotesLinkingToItem(baseNote)
+      const linkedItem = await itemManager.linkNoteToNote(note, note2)
+      const changedItem = await itemManager.unlinkItems(linkedItem, note2)
 
-      expect(sortedFilesForItem).toHaveLength(2)
-      expect(sortedFilesForItem[0].uuid).toEqual(noteToLink1.uuid)
-      expect(sortedFilesForItem[1].uuid).toEqual(noteToLink2.uuid)
+      expect(changedItem.uuid).toBe(note.uuid)
+      expect(changedItem.references).toHaveLength(0)
     })
   })
 })
diff --git a/packages/snjs/lib/Services/Items/ItemManager.ts b/packages/snjs/lib/Services/Items/ItemManager.ts
@@ -7,7 +7,7 @@ import { UuidString } from '../../Types/UuidString'
 import * as Models from '@standardnotes/models'
 import * as Services from '@standardnotes/services'
 import { PayloadManagerChangeData } from '../Payloads'
-import { DiagnosticInfo, ItemsClientInterface } from '@standardnotes/services'
+import { DiagnosticInfo, ItemsClientInterface, ItemRelationshipDirection } from '@standardnotes/services'
 import { ApplicationDisplayOptions } from '@Lib/Application/Options/OptionalOptions'
 import { CollectionSort, DecryptedItemInterface, ItemContent } from '@standardnotes/models'
 
@@ -1169,12 +1169,18 @@ export class ItemManager
     })
   }
 
-  public async unlinkItem(
-    item: DecryptedItemInterface<ItemContent>,
-    itemToUnlink: DecryptedItemInterface<ItemContent>,
-  ) {
-    return this.changeItem(item, (mutator) => {
-      mutator.removeItemAsRelationship(itemToUnlink)
+  public async unlinkItems(itemA: DecryptedItemInterface<ItemContent>, itemB: DecryptedItemInterface<ItemContent>) {
+    const relationshipDirection = this.relationshipDirectionBetweenItems(itemA, itemB)
+
+    if (relationshipDirection === ItemRelationshipDirection.NoRelationship) {
+      throw new Error('Trying to unlink already unlinked items')
+    }
+
+    const itemToChange = relationshipDirection === ItemRelationshipDirection.AReferencesB ? itemA : itemB
+    const itemToRemove = itemToChange === itemA ? itemB : itemA
+
+    return this.changeItem(itemToChange, (mutator) => {
+      mutator.removeItemAsRelationship(itemToRemove)
     })
   }
 
@@ -1192,54 +1198,6 @@ export class ItemManager
     )
   }
 
-  public getSortedLinkedFilesForItem(item: DecryptedItemInterface<ItemContent>): Models.FileItem[] {
-    if (this.isTemplateItem(item)) {
-      return []
-    }
-
-    const filesReferencedByItem = this.referencesForItem(item).filter(
-      (ref) => ref.content_type === ContentType.File,
-    ) as Models.FileItem[]
-
-    return naturalSort(filesReferencedByItem, 'title')
-  }
-
-  public getSortedFilesLinkingToItem(item: DecryptedItemInterface<ItemContent>): Models.FileItem[] {
-    if (this.isTemplateItem(item)) {
-      return []
-    }
-
-    const filesReferencingItem = this.itemsReferencingItem(item).filter(
-      (ref) => ref.content_type === ContentType.File,
-    ) as Models.FileItem[]
-
-    return naturalSort(filesReferencingItem, 'title')
-  }
-
-  public getSortedLinkedNotesForItem(item: DecryptedItemInterface<ItemContent>): Models.SNNote[] {
-    if (this.isTemplateItem(item)) {
-      return []
-    }
-
-    const notesReferencedByItem = this.referencesForItem(item).filter(
-      (ref) => ref.content_type === ContentType.Note,
-    ) as Models.SNNote[]
-
-    return naturalSort(notesReferencedByItem, 'title')
-  }
-
-  public getSortedNotesLinkingToItem(item: Models.DecryptedItemInterface<Models.ItemContent>): Models.SNNote[] {
-    if (this.isTemplateItem(item)) {
-      return []
-    }
-
-    const notesReferencingItem = this.itemsReferencingItem(item).filter(
-      (ref) => ref.content_type === ContentType.Note,
-    ) as Models.SNNote[]
-
-    return naturalSort(notesReferencingItem, 'title')
-  }
-
   public async createTag(title: string, parentItemToLookupUuidFor?: Models.SNTag): Promise<Models.SNTag> {
     const newTag = await this.createItem<Models.SNTag>(
       ContentType.Tag,
@@ -1433,21 +1391,18 @@ export class ItemManager
     return this.findAnyItems(uuids) as (Models.DecryptedItemInterface | Models.DeletedItemInterface)[]
   }
 
-  /**
-   * @returns `'direct'` if `itemOne` has the reference to `itemTwo`, `'indirect'` if `itemTwo` has the reference to `itemOne`, `'unlinked'` if neither reference each other
-   */
-  public relationshipTypeForItems(
-    itemOne: Models.DecryptedItemInterface<Models.ItemContent>,
-    itemTwo: Models.DecryptedItemInterface<Models.ItemContent>,
-  ): 'direct' | 'indirect' | 'unlinked' {
-    const itemOneReferencesItemTwo = this.isTemplateItem(itemOne)
-      ? false
-      : !!this.referencesForItem(itemOne).find((reference) => reference.uuid === itemTwo.uuid)
-    const itemTwoReferencesItemOne = this.isTemplateItem(itemTwo)
-      ? false
-      : !!this.referencesForItem(itemTwo).find((reference) => reference.uuid === itemOne.uuid)
-
-    return itemOneReferencesItemTwo ? 'direct' : itemTwoReferencesItemOne ? 'indirect' : 'unlinked'
+  public relationshipDirectionBetweenItems(
+    itemA: Models.DecryptedItemInterface<Models.ItemContent>,
+    itemB: Models.DecryptedItemInterface<Models.ItemContent>,
+  ): ItemRelationshipDirection {
+    const itemAReferencesItemB = !!itemA.references.find((reference) => reference.uuid === itemB.uuid)
+    const itemBReferencesItemA = !!itemB.references.find((reference) => reference.uuid === itemA.uuid)
+
+    return itemAReferencesItemB
+      ? ItemRelationshipDirection.AReferencesB
+      : itemBReferencesItemA
+      ? ItemRelationshipDirection.BReferencesA
+      : ItemRelationshipDirection.NoRelationship
   }
 
   override getDiagnostics(): Promise<DiagnosticInfo | undefined> {

diff --git a/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx b/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx
@@ -1,5 +1,5 @@
 import { PLAIN_EDITOR_NAME } from '@/Constants/Constants'
-import { sanitizeHtmlString, SNNote } from '@standardnotes/snjs'
+import { isFile, sanitizeHtmlString, SNNote } from '@standardnotes/snjs'
 import { observer } from 'mobx-react-lite'
 import { FunctionComponent, useCallback, useRef } from 'react'
 import Icon from '@/Components/Icon/Icon'
@@ -32,7 +32,7 @@ const NoteListItem: FunctionComponent<DisplayableListItemProps> = ({
   const editorForNote = application.componentManager.editorForNote(item as SNNote)
   const editorName = editorForNote?.name ?? PLAIN_EDITOR_NAME
   const [icon, tint] = application.iconsController.getIconAndTintForNoteType(editorForNote?.package_info.note_type)
-  const hasFiles = application.items.getSortedFilesLinkingToItem(item).length > 0
+  const hasFiles = application.items.itemsReferencingItem(item).filter(isFile).length > 0
 
   const openNoteContextMenu = (posX: number, posY: number) => {
     notesController.setContextMenuOpen(false)

diff --git a/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx b/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx
@@ -103,7 +103,7 @@ const LinkedItemBubble = ({
       <span className="max-w-290px flex items-center overflow-hidden overflow-ellipsis whitespace-nowrap">
         {tagTitle && <span className="text-passive-1">{tagTitle.titlePrefix}</span>}
         <span className="flex items-center gap-1">
-          {link.relationWithSelectedItem === 'indirect' && link.item.content_type !== ContentType.Tag && (
+          {link.type === 'linked-by' && link.item.content_type !== ContentType.Tag && (
             <span className={!isBidirectional ? 'hidden group-focus:block' : ''}>Linked By:</span>
           )}
           {link.item.title}