core: fix moving nested notebooks to trash

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);
+  }));

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[]) {

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);
+  }
 }

packages/core/src/database/migrations.ts

@@ -293,7 +293,8 @@ const addTrashColumns = <T extends string, C extends string = never>(
   return builder
 
     .addColumn("dateDeleted", "integer")
-    .addColumn("itemType", "text");
+    .addColumn("itemType", "text")
+    .addColumn("deletedBy", "text");
 };
 
 type Tokenizer = "porter" | "trigram" | "unicode61" | "ascii";

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;

packages/core/src/types.ts

@@ -188,6 +188,7 @@ export interface Note extends BaseItem<"note"> {
 
   dateDeleted: null;
   itemType: null;
+  deletedBy: null;
 }
 
 export interface Notebook extends BaseItem<"notebook"> {
@@ -196,9 +197,6 @@ export interface Notebook extends BaseItem<"notebook"> {
   dateEdited: number;
   pinned: boolean;
 
-  dateDeleted: null;
-  itemType: null;
-
   /**
    * @deprecated only kept here for migration purposes.
    */
@@ -207,6 +205,10 @@ export interface Notebook extends BaseItem<"notebook"> {
    * @deprecated only kept here for migration purposes.
    */
   totalNotes?: number;
+
+  dateDeleted: null;
+  itemType: null;
+  deletedBy: null;
 }
 
 /**
@@ -459,7 +461,11 @@ export type BaseTrashItem<TItem extends BaseItem<"note" | "notebook">> =
   BaseItem<"trash"> & {
     itemType: TItem["type"];
     dateDeleted: number;
-  } & Omit<TItem, "id" | "type" | "dateDeleted" | "itemType">;
+    /**
+     * deletedBy tells who deleted this specific item.
+     */
+    deletedBy: "user" | "app";
+  } & Omit<TItem, "id" | "type" | "dateDeleted" | "itemType" | "deletedBy">;
 
 export type TrashItem = BaseTrashItem<Note> | BaseTrashItem<Notebook>;