Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: fix conflicts view when conflicted copies are already in trash #2339

Merged
merged 4 commits into from Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/models/src/Domain/Abstract/Item/index.ts
Expand Up @@ -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'
Expand Down
7 changes: 5 additions & 2 deletions packages/models/src/Domain/Runtime/Collection/Collection.ts
Expand Up @@ -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)
}

Expand Down
Expand Up @@ -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(),
})
Expand All @@ -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)
})
})
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 2 additions & 0 deletions packages/services/src/Domain/Item/ItemsClientInterface.ts
Expand Up @@ -18,6 +18,7 @@ import {
ItemsKeyInterface,
ItemContent,
DecryptedPayload,
AnyItemInterface,
} from '@standardnotes/models'

export interface ItemsClientInterface {
Expand Down Expand Up @@ -168,5 +169,6 @@ export interface ItemsClientInterface {
iconString?: string,
): Promise<SmartView>

conflictsOf(uuid: string): AnyItemInterface[]
numberOfNotesWithConflicts(): number
}
6 changes: 5 additions & 1 deletion packages/snjs/lib/Services/Items/ItemManager.ts
Expand Up @@ -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()
}
}
35 changes: 5 additions & 30 deletions packages/web/src/javascripts/Components/NoteView/NoteView.tsx
Expand Up @@ -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) {
Expand Down