diff --git a/packages/core/__tests__/trash.test.ts b/packages/core/__tests__/trash.test.ts index 614db9c31..ab3e91bfe 100644 --- a/packages/core/__tests__/trash.test.ts +++ b/packages/core/__tests__/trash.test.ts @@ -46,10 +46,10 @@ test("permanently delete a note", () => await db.notes.moveToTrash(noteId); expect(await db.trash.all()).toHaveLength(1); - expect(await db.content.get(note.contentId)).toBeDefined(); + expect(await db.content.get(note?.contentId)).toBeDefined(); await db.trash.delete(noteId); expect(await db.trash.all()).toHaveLength(0); - expect(await db.content.get(note.contentId)).toBeUndefined(); + expect(await db.content.get(note?.contentId)).toBeUndefined(); expect(await db.noteHistory.get(noteId).count()).toBe(0); })); @@ -70,10 +70,10 @@ test("restore a deleted note that was in a notebook", () => expect(await db.trash.all()).toHaveLength(0); const note = await db.notes.note(id); - const content = await db.content.get(note.contentId); + const content = await db.content.get(note?.contentId); expect(note).toBeDefined(); - expect(content.data).toBe(TEST_NOTE.content.data); + expect(content?.data).toBe(TEST_NOTE.content.data); expect( await db.relations @@ -89,7 +89,7 @@ test("delete a locked note", () => await db.vault.add(id); await db.notes.moveToTrash(id); expect(await db.trash.all()).toHaveLength(1); - expect(await db.content.get(note.contentId)).toBeDefined(); + expect(await db.content.get(note?.contentId)).toBeDefined(); })); test("restore a deleted locked note", () => @@ -99,7 +99,7 @@ test("restore a deleted locked note", () => await db.vault.add(id); await db.notes.moveToTrash(id); expect(await db.trash.all()).toHaveLength(1); - expect(await db.content.get(note.contentId)).toBeDefined(); + expect(await db.content.get(note?.contentId)).toBeDefined(); await db.trash.restore(id); note = await db.notes.note(id); @@ -136,41 +136,6 @@ test("delete a notebook", () => ).toBe(0); })); -test("restore a deleted notebook", () => - notebookTest().then(async ({ db, id }) => { - const noteId = await db.notes.add(TEST_NOTE); - await db.notes.addToNotebook(id, noteId); - - await db.notebooks.moveToTrash(id); - await db.trash.restore(id); - - const notebook = db.notebooks.notebook(id); - expect(notebook).toBeDefined(); - - expect( - await db.relations.to({ type: "note", id: noteId }, "notebook").count() - ).toBe(1); - expect( - await db.relations.to({ type: "note", id: noteId }, "notebook").has(id) - ).toBe(true); - })); - -test("restore a notebook that has deleted notes", () => - notebookTest().then(async ({ db, id }) => { - const noteId = await db.notes.add(TEST_NOTE); - await db.notes.addToNotebook(id, noteId); - - await db.notebooks.moveToTrash(id); - await db.notes.moveToTrash(noteId); - await db.trash.restore(id); - - const notebook = db.notebooks.notebook(id); - expect(notebook).toBeDefined(); - expect( - await db.relations.from({ type: "notebook", id: id }, "note").has(noteId) - ).toBe(false); - })); - test("permanently delete items older than 7 days", () => databaseTest().then(async (db) => { const sevenDaysEarlier = dayjs().subtract(8, "days").toDate().getTime(); @@ -243,3 +208,155 @@ test("clear trash should delete note content", () => expect(await db.noteHistory.get(noteId).count()).toBe(0); })); + +test("deleting a notebook should delete all its subnotebooks", () => + databaseTest().then(async (db) => { + const parent = await db.notebooks.add({ title: "Parent" }); + const child = await db.notebooks.add({ title: "Child" }); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child, type: "notebook" } + ); + + await db.notebooks.moveToTrash(parent); + + expect(await db.notebooks.notebook(child)).toBeUndefined(); + })); + +test("deleting a notebook should not re-delete already deleted subnotebooks", () => + databaseTest().then(async (db) => { + const parent = await db.notebooks.add({ title: "Parent" }); + const child = await db.notebooks.add({ title: "Child" }); + const child2 = await db.notebooks.add({ title: "Child" }); + const child3 = await db.notebooks.add({ title: "Child" }); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child2, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child3, type: "notebook" } + ); + + await db.notebooks.moveToTrash(child3); + await db.notebooks.moveToTrash(parent); + + expect((await db.trash.all()).some((a) => a.id === child3)).toBe(true); + expect((await db.trash.all()).some((a) => a.id === parent)).toBe(true); + expect((await db.trash.all()).some((a) => a.id === child2)).toBe(false); + expect((await db.trash.all()).some((a) => a.id === child)).toBe(false); + })); + +test("restoring a deleted notebook should also restore all its subnotebooks", () => + databaseTest().then(async (db) => { + const parent = await db.notebooks.add({ title: "Parent" }); + const child = await db.notebooks.add({ title: "Child" }); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child, type: "notebook" } + ); + await db.notebooks.moveToTrash(parent); + + await db.trash.restore(parent); + + expect(await db.notebooks.notebook(child)).toBeDefined(); + expect( + await db.relations + .from({ id: parent, type: "notebook" }, "notebook") + .has(child) + ).toBe(true); + })); + +test("restoring a deleted notebook should link it back to its notes", () => + notebookTest().then(async ({ db, id }) => { + const noteId = await db.notes.add(TEST_NOTE); + await db.notes.addToNotebook(id, noteId); + + await db.notebooks.moveToTrash(id); + await db.trash.restore(id); + + const notebook = db.notebooks.notebook(id); + expect(notebook).toBeDefined(); + + expect( + await db.relations.to({ type: "note", id: noteId }, "notebook").count() + ).toBe(1); + expect( + await db.relations.to({ type: "note", id: noteId }, "notebook").has(id) + ).toBe(true); + })); + +test("restoring a notebook should not restore its deleted notes", () => + notebookTest().then(async ({ db, id }) => { + const noteId = await db.notes.add(TEST_NOTE); + await db.notes.addToNotebook(id, noteId); + + await db.notebooks.moveToTrash(id); + await db.notes.moveToTrash(noteId); + await db.trash.restore(id); + + const notebook = db.notebooks.notebook(id); + expect(notebook).toBeDefined(); + expect( + await db.relations.from({ type: "notebook", id: id }, "note").has(noteId) + ).toBe(false); + })); + +test("restoring a notebook should not restore independently deleted subnotebooks", () => + databaseTest().then(async (db) => { + const parent = await db.notebooks.add({ title: "Parent" }); + const child = await db.notebooks.add({ title: "Child" }); + const child2 = await db.notebooks.add({ title: "Child" }); + const child3 = await db.notebooks.add({ title: "Child" }); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child2, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child3, type: "notebook" } + ); + await db.notebooks.moveToTrash(child3); + await db.notebooks.moveToTrash(parent); + + await db.trash.restore(parent); + + expect(await db.notebooks.notebook(child3)).toBeUndefined(); + })); + +test("permanently deleting a notebook should not delete independently deleted subnotebooks", () => + databaseTest().then(async (db) => { + const parent = await db.notebooks.add({ title: "Parent" }); + const child = await db.notebooks.add({ title: "Child" }); + const child2 = await db.notebooks.add({ title: "Child" }); + const child3 = await db.notebooks.add({ title: "Child" }); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child2, type: "notebook" } + ); + await db.relations.add( + { id: parent, type: "notebook" }, + { id: child3, type: "notebook" } + ); + await db.notebooks.moveToTrash(child3); + await db.notebooks.moveToTrash(parent); + + await db.trash.delete(parent); + + expect((await db.trash.all()).some((a) => a.id === child3)).toBe(true); + expect((await db.trash.all()).some((a) => a.id === parent)).toBe(false); + expect((await db.trash.all()).some((a) => a.id === child2)).toBe(false); + expect((await db.trash.all()).some((a) => a.id === child)).toBe(false); + })); diff --git a/packages/core/src/collections/notebooks.ts b/packages/core/src/collections/notebooks.ts index 6ac64000d..fc5f9851d 100644 --- a/packages/core/src/collections/notebooks.ts +++ b/packages/core/src/collections/notebooks.ts @@ -23,6 +23,7 @@ import { Notebook, TrashOrItem, isTrashItem } from "../types"; import { ICollection } from "./collection"; import { SQLCollection } from "../database/sql-collection"; import { isFalse } from "../database"; +import { sql } from "kysely"; export class Notebooks implements ICollection { name = "notebooks"; @@ -239,7 +240,34 @@ export class Notebooks implements ICollection { } async moveToTrash(...ids: string[]) { - await this.db.trash.add("notebook", ids); + this.db.transaction(async (tr) => { + const query = tr + .withRecursive(`subNotebooks(id)`, (eb) => + eb + .selectFrom((eb) => + sql<{ id: string }>`(VALUES ${sql.join( + ids.map((id) => eb.parens(sql`${id}`)) + )})`.as("notebookIds") + ) + .selectAll() + .unionAll((eb) => + eb + .selectFrom(["relations", "subNotebooks"]) + .select("relations.toId as id") + .where("toType", "==", "notebook") + .where("fromType", "==", "notebook") + .whereRef("fromId", "==", "subNotebooks.id") + .where("toId", "not in", this.db.trash.cache.notebooks) + .$narrowType<{ id: string }>() + ) + ) + .selectFrom("subNotebooks") + .select("id"); + + const subNotebookIds = (await query.execute()).map((ref) => ref.id); + await this.db.trash.add("notebook", subNotebookIds, "app"); + await this.db.trash.add("notebook", ids, "user"); + }); } async remove(...ids: string[]) { diff --git a/packages/core/src/collections/trash.ts b/packages/core/src/collections/trash.ts index 90c053270..9c67653a9 100644 --- a/packages/core/src/collections/trash.ts +++ b/packages/core/src/collections/trash.ts @@ -91,19 +91,25 @@ export default class Trash { await this._delete(noteIds, notebookIds); } - async add(type: "note" | "notebook", ids: string[]) { + async add( + type: "note" | "notebook", + ids: string[], + deletedBy: TrashItem["deletedBy"] = "user" + ) { if (type === "note") { await this.db.notes.collection.update(ids, { type: "trash", itemType: "note", - dateDeleted: Date.now() + dateDeleted: Date.now(), + deletedBy }); this.cache.notes.push(...ids); } else if (type === "notebook") { await this.db.notebooks.collection.update(ids, { type: "trash", itemType: "notebook", - dateDeleted: Date.now() + dateDeleted: Date.now(), + deletedBy }); this.cache.notebooks.push(...ids); } @@ -137,9 +143,10 @@ export default class Trash { } if (notebookIds.length > 0) { - await this.db.relations.unlinkOfType("notebook", notebookIds); - await this.db.notebooks.remove(...notebookIds); - deleteItems(this.cache.notebooks, ...notebookIds); + const ids = [...notebookIds, ...(await this.subNotebooks(notebookIds))]; + await this.db.notebooks.remove(...ids); + await this.db.relations.unlinkOfType("notebook", ids); + deleteItems(this.cache.notebooks, ...ids); } } @@ -152,10 +159,10 @@ export default class Trash { const isNote = this.cache.notes.includes(id); if (isNote) { noteIds.push(id); - this.cache.notes.splice(this.cache.notes.indexOf(id), 1); + // this.cache.notes.splice(this.cache.notes.indexOf(id), 1); } else if (!isNote) { notebookIds.push(id); - this.cache.notebooks.splice(this.cache.notebooks.indexOf(id), 1); + // this.cache.notebooks.splice(this.cache.notebooks.indexOf(id), 1); } } @@ -163,16 +170,21 @@ export default class Trash { await this.db.notes.collection.update(noteIds, { type: "note", dateDeleted: null, - itemType: null + itemType: null, + deletedBy: null }); + deleteItems(this.cache.notes, ...noteIds); } if (notebookIds.length > 0) { - await this.db.notebooks.collection.update(notebookIds, { + const ids = [...notebookIds, ...(await this.subNotebooks(notebookIds))]; + await this.db.notebooks.collection.update(ids, { type: "notebook", dateDeleted: null, - itemType: null + itemType: null, + deletedBy: null }); + deleteItems(this.cache.notebooks, ...ids); } } @@ -202,6 +214,7 @@ export default class Trash { .selectFrom("notes") .where("type", "==", "trash") .where("id", "in", ids) + .where("deletedBy", "==", "user") .selectAll() .execute()) as TrashItem[]; } @@ -212,6 +225,7 @@ export default class Trash { .selectFrom("notebooks") .where("type", "==", "trash") .where("id", "in", ids) + .where("deletedBy", "==", "user") .selectAll() .execute()) as TrashItem[]; } @@ -264,4 +278,42 @@ export default class Trash { exists(id: string) { return this.cache.notebooks.includes(id) || this.cache.notes.includes(id); } + + private async subNotebooks(notebookIds: string[]) { + const ids = await this.db + .sql() + .withRecursive(`subNotebooks(id)`, (eb) => + eb + .selectFrom((eb) => + sql<{ id: string }>`(VALUES ${sql.join( + notebookIds.map((id) => eb.parens(sql`${id}`)) + )})`.as("notebookIds") + ) + .selectAll() + .unionAll((eb) => + eb + .selectFrom(["relations", "subNotebooks", "notebooks"]) + .select("relations.toId as id") + .where("toType", "==", "notebook") + .where("fromType", "==", "notebook") + .whereRef("fromId", "==", "subNotebooks.id") + .where( + (eb) => + eb + .selectFrom("notebooks") + .whereRef("notebooks.id", "==", "relations.toId") + .where("notebooks.type", "==", "trash") + .limit(1) + .select("deletedBy"), + "!=", + "user" + ) + .$narrowType<{ id: string }>() + ) + ) + .selectFrom("subNotebooks") + .select("id") + .execute(); + return ids.map((ref) => ref.id); + } } diff --git a/packages/core/src/database/migrations.ts b/packages/core/src/database/migrations.ts index 133385622..7949816cb 100644 --- a/packages/core/src/database/migrations.ts +++ b/packages/core/src/database/migrations.ts @@ -293,7 +293,8 @@ const addTrashColumns = ( return builder .addColumn("dateDeleted", "integer") - .addColumn("itemType", "text"); + .addColumn("itemType", "text") + .addColumn("deletedBy", "text"); }; type Tokenizer = "porter" | "trigram" | "unicode61" | "ascii"; diff --git a/packages/core/src/migrations.ts b/packages/core/src/migrations.ts index e11c057b2..c74a07546 100644 --- a/packages/core/src/migrations.ts +++ b/packages/core/src/migrations.ts @@ -175,6 +175,10 @@ const migrations: Migration[] = [ { version: 5.9, items: { + trash: (item) => { + if (!item.deletedBy) item.deletedBy = "user"; + return true; + }, tag: async (item, db) => { const oldTagId = makeId(item.title); const alias = db.legacySettings.getAlias(item.id); @@ -311,6 +315,11 @@ const migrations: Migration[] = [ }); if (!subNotebookId) continue; await db.relations.add(item, { id: subNotebookId, type: "notebook" }); + // if the parent notebook is deleted, we should delete the newly + // created notebooks too + if (item.dateDeleted !== null) { + await db.trash.add("notebook", [subNotebookId], "app"); + } } delete item.topics; delete item.totalNotes; diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 82f44044c..5ac177165 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -189,6 +189,7 @@ export interface Note extends BaseItem<"note"> { dateDeleted: null; itemType: null; + deletedBy: null; } export interface Notebook extends BaseItem<"notebook"> { @@ -197,9 +198,6 @@ export interface Notebook extends BaseItem<"notebook"> { dateEdited: number; pinned: boolean; - dateDeleted: null; - itemType: null; - /** * @deprecated only kept here for migration purposes. */ @@ -208,6 +206,10 @@ export interface Notebook extends BaseItem<"notebook"> { * @deprecated only kept here for migration purposes. */ totalNotes?: number; + + dateDeleted: null; + itemType: null; + deletedBy: null; } /** @@ -460,7 +462,11 @@ export type BaseTrashItem> = BaseItem<"trash"> & { itemType: TItem["type"]; dateDeleted: number; - } & Omit; + /** + * deletedBy tells who deleted this specific item. + */ + deletedBy: "user" | "app"; + } & Omit; export type TrashItem = BaseTrashItem | BaseTrashItem;