refactor: fix conflicts view when conflicted copies are already in tr…

…ash (#2339)
standardnotes · Jun 27, 2023 · d3378a7 · d3378a7
1 parent f858f31
commit d3378a7
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 35 deletions.
diff --git a/packages/models/src/Domain/Abstract/Item/index.ts b/packages/models/src/Domain/Abstract/Item/index.ts
@@ -17,6 +17,7 @@ export * from './Interfaces/DeletedItem'
 export * from './Interfaces/EncryptedItem'
 export * from './Interfaces/ItemInterface'
 export * from './Interfaces/TypeCheck'
+export * from './Interfaces/UnionTypes'
 export * from './Mutator/DecryptedItemMutator'
 export * from './Mutator/DeleteMutator'
 export * from './Mutator/ItemMutator'

diff --git a/packages/models/src/Domain/Runtime/Collection/Collection.ts b/packages/models/src/Domain/Runtime/Collection/Collection.ts
@@ -185,18 +185,21 @@ export abstract class Collection<
 
       if (this.isDecryptedElement(element)) {
         const conflictOf = element.content.conflict_of
-        if (conflictOf) {
+
+        if (conflictOf && !element.content.trashed) {
           this.conflictMap.establishRelationship(conflictOf, element.uuid)
         }
 
+        const isConflictOfButTrashed = conflictOf && element.content.trashed
+
         const isInConflictMapButIsNotConflictOf =
           !conflictOf && this.conflictMap.getInverseRelationships(element.uuid).length > 0
 
         const isInConflictMapButDoesNotHaveConflicts =
           this.conflictMap.existsInDirectMap(element.uuid) &&
           this.conflictMap.getDirectRelationships(element.uuid).length === 0
 
-        if (isInConflictMapButIsNotConflictOf || isInConflictMapButDoesNotHaveConflicts) {
+        if (isInConflictMapButIsNotConflictOf || isInConflictMapButDoesNotHaveConflicts || isConflictOfButTrashed) {
           this.conflictMap.removeFromMap(element.uuid)
         }
 

diff --git a/packages/models/src/Domain/Runtime/Collection/Item/ItemCollection.spec.ts b/packages/models/src/Domain/Runtime/Collection/Item/ItemCollection.spec.ts
@@ -6,12 +6,13 @@ import { ItemCollection } from './ItemCollection'
 import { FillItemContent, ItemContent } from '../../../Abstract/Content/ItemContent'
 
 describe('item collection', () => {
-  const createDecryptedPayload = (uuid?: string): DecryptedPayload => {
+  const createDecryptedPayload = (uuid?: string, content?: Partial<NoteContent>): DecryptedPayload => {
     return new DecryptedPayload({
       uuid: uuid || String(Math.random()),
       content_type: ContentType.Note,
       content: FillItemContent<NoteContent>({
         title: 'foo',
+        ...content,
       }),
       ...PayloadTimestampDefaults(),
     })
@@ -33,4 +34,81 @@ describe('item collection', () => {
 
     expect(collection.all()).toHaveLength(1)
   })
+
+  it('should not add conflicted copy to conflictMap if trashed', () => {
+    const collection = new ItemCollection()
+
+    const mainItem = new DecryptedItem(createDecryptedPayload())
+    const nonTrashedConflictedItem = new DecryptedItem(
+      createDecryptedPayload(undefined, {
+        conflict_of: mainItem.uuid,
+      }),
+    )
+    const trashedConflictedItem = new DecryptedItem(
+      createDecryptedPayload(undefined, {
+        conflict_of: mainItem.uuid,
+        trashed: true,
+      }),
+    )
+
+    collection.set([mainItem, nonTrashedConflictedItem, trashedConflictedItem])
+
+    expect(collection.conflictMap.existsInDirectMap(mainItem.uuid)).toBe(true)
+    expect(collection.conflictMap.getDirectRelationships(mainItem.uuid).includes(nonTrashedConflictedItem.uuid)).toBe(
+      true,
+    )
+    expect(collection.conflictMap.getDirectRelationships(mainItem.uuid).includes(trashedConflictedItem.uuid)).toBe(
+      false,
+    )
+  })
+
+  it('should remove conflicted copy from conflictMap if not conflicted anymore', () => {
+    const collection = new ItemCollection()
+
+    const mainItem = new DecryptedItem(createDecryptedPayload())
+    const conflictedItem = new DecryptedItem(
+      createDecryptedPayload(undefined, {
+        conflict_of: mainItem.uuid,
+      }),
+    )
+
+    collection.set([mainItem, conflictedItem])
+
+    expect(collection.conflictMap.existsInInverseMap(conflictedItem.uuid)).toBe(true)
+
+    const updatedConflictedItem = new DecryptedItem(
+      conflictedItem.payload.copy({
+        content: { conflict_of: undefined } as unknown as jest.Mocked<ItemContent>,
+      }),
+    )
+
+    collection.set(updatedConflictedItem)
+
+    expect(collection.conflictMap.existsInInverseMap(conflictedItem.uuid)).toBe(false)
+  })
+
+  it('should remove conflict parent if it has no conflicts anymore', () => {
+    const collection = new ItemCollection()
+
+    const mainItem = new DecryptedItem(createDecryptedPayload())
+    const conflictedItem = new DecryptedItem(
+      createDecryptedPayload(undefined, {
+        conflict_of: mainItem.uuid,
+      }),
+    )
+
+    collection.set([mainItem, conflictedItem])
+
+    expect(collection.conflictMap.existsInDirectMap(mainItem.uuid)).toBe(true)
+
+    const updatedConflictedItem = new DecryptedItem(
+      conflictedItem.payload.copy({
+        content: { conflict_of: undefined } as unknown as jest.Mocked<ItemContent>,
+      }),
+    )
+
+    collection.set([updatedConflictedItem, mainItem])
+
+    expect(collection.conflictMap.existsInDirectMap(mainItem.uuid)).toBe(false)
+  })
 })
diff --git a/packages/models/src/Domain/Runtime/Display/DisplayOptionsToFilters.ts b/packages/models/src/Domain/Runtime/Display/DisplayOptionsToFilters.ts
@@ -6,6 +6,7 @@ import { ItemWithTags } from './Search/ItemWithTags'
 import { itemMatchesQuery, itemPassesFilters } from './Search/SearchUtilities'
 import { ItemFilter, ReferenceLookupCollection, SearchableDecryptedItem } from './Search/Types'
 import { FilterDisplayOptions } from './DisplayOptions'
+import { SystemViewId } from '../../Syncable/SmartView'
 
 export function computeUnifiedFilterForDisplayOptions(
   options: FilterDisplayOptions,
@@ -75,7 +76,10 @@ export function computeFiltersForDisplayOptions(
     filters.push((item) => itemMatchesQuery(item, query, collection))
   }
 
-  if (!viewsPredicate?.keypathIncludesString('conflict_of')) {
+  if (
+    !viewsPredicate?.keypathIncludesString('conflict_of') &&
+    !options.views?.some((v) => v.uuid === SystemViewId.TrashedNotes)
+  ) {
     filters.push((item) => !item.conflictOf)
   }
 

diff --git a/packages/services/src/Domain/Item/ItemsClientInterface.ts b/packages/services/src/Domain/Item/ItemsClientInterface.ts
@@ -18,6 +18,7 @@ import {
   ItemsKeyInterface,
   ItemContent,
   DecryptedPayload,
+  AnyItemInterface,
 } from '@standardnotes/models'
 
 export interface ItemsClientInterface {
@@ -168,5 +169,6 @@ export interface ItemsClientInterface {
     iconString?: string,
   ): Promise<SmartView>
 
+  conflictsOf(uuid: string): AnyItemInterface[]
   numberOfNotesWithConflicts(): number
 }
diff --git a/packages/snjs/lib/Services/Items/ItemManager.ts b/packages/snjs/lib/Services/Items/ItemManager.ts
@@ -1415,7 +1415,11 @@ export class ItemManager
     })
   }
 
-  numberOfNotesWithConflicts(): number {
+  public conflictsOf(uuid: string) {
+    return this.collection.conflictsOf(uuid)
+  }
+
+  public numberOfNotesWithConflicts(): number {
     return this.collection.numberOfItemsWithConflicts()
   }
 }
diff --git a/packages/web/src/javascripts/Components/NoteView/NoteView.tsx b/packages/web/src/javascripts/Components/NoteView/NoteView.tsx
@@ -432,36 +432,11 @@ class NoteView extends AbstractComponent<NoteViewProps, State> {
       this.debounceReloadEditorComponent()
     })
 
-    this.removeNoteStreamObserver = this.application.streamItems<SNNote>(
-      ContentType.Note,
-      async ({ inserted, changed, removed }) => {
-        const insertedOrChanged = inserted.concat(changed)
-
-        for (const note of insertedOrChanged) {
-          if (note.conflictOf === this.note.uuid && !note.trashed) {
-            this.setState((state) => ({
-              conflictedNotes: state.conflictedNotes
-                .filter((conflictedNote) => conflictedNote.uuid !== note.uuid)
-                .concat([note]),
-            }))
-          } else {
-            this.setState((state) => {
-              return {
-                conflictedNotes: state.conflictedNotes.filter((conflictedNote) => conflictedNote.uuid !== note.uuid),
-              }
-            })
-          }
-        }
-
-        for (const note of removed) {
-          this.setState((state) => {
-            return {
-              conflictedNotes: state.conflictedNotes.filter((conflictedNote) => conflictedNote.uuid !== note.uuid),
-            }
-          })
-        }
-      },
-    )
+    this.removeNoteStreamObserver = this.application.streamItems<SNNote>(ContentType.Note, async () => {
+      this.setState({
+        conflictedNotes: this.application.items.conflictsOf(this.note.uuid) as SNNote[],
+      })
+    })
   }
 
   private createComponentViewer(component: SNComponent) {