From d3378a704a7a673611c8d6688b8b7529bb652f91 Mon Sep 17 00:00:00 2001 From: Aman Harwara Date: Tue, 27 Jun 2023 22:08:46 +0530 Subject: [PATCH] refactor: fix conflicts view when conflicted copies are already in trash (#2339) --- .../models/src/Domain/Abstract/Item/index.ts | 1 + .../Domain/Runtime/Collection/Collection.ts | 7 +- .../Collection/Item/ItemCollection.spec.ts | 80 ++++++++++++++++++- .../Display/DisplayOptionsToFilters.ts | 6 +- .../src/Domain/Item/ItemsClientInterface.ts | 2 + .../snjs/lib/Services/Items/ItemManager.ts | 6 +- .../Components/NoteView/NoteView.tsx | 35 ++------ 7 files changed, 102 insertions(+), 35 deletions(-) diff --git a/packages/models/src/Domain/Abstract/Item/index.ts b/packages/models/src/Domain/Abstract/Item/index.ts index 23c09f03c..5aa7086c5 100644 --- 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 index 9f3664af5..8073fce12 100644 --- a/packages/models/src/Domain/Runtime/Collection/Collection.ts +++ b/packages/models/src/Domain/Runtime/Collection/Collection.ts @@ -185,10 +185,13 @@ 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 @@ -196,7 +199,7 @@ export abstract class Collection< 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 index ae84e33d5..2dbb1d391 100644 --- 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): DecryptedPayload => { return new DecryptedPayload({ uuid: uuid || String(Math.random()), content_type: ContentType.Note, content: FillItemContent({ 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, + }), + ) + + 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, + }), + ) + + 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 index fee35643a..90a029d45 100644 --- 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 index 8c544c4b8..38add2623 100644 --- 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 + conflictsOf(uuid: string): AnyItemInterface[] numberOfNotesWithConflicts(): number } diff --git a/packages/snjs/lib/Services/Items/ItemManager.ts b/packages/snjs/lib/Services/Items/ItemManager.ts index 745fbe5c4..dc5182158 100644 --- 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 index 4cefd0ec7..8ade849a5 100644 --- 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 { this.debounceReloadEditorComponent() }) - this.removeNoteStreamObserver = this.application.streamItems( - 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(ContentType.Note, async () => { + this.setState({ + conflictedNotes: this.application.items.conflictsOf(this.note.uuid) as SNNote[], + }) + }) } private createComponentViewer(component: SNComponent) {