diff --git a/packages/core/__tests__/notebooks.test.js b/packages/core/__tests__/notebooks.test.js index 8ba717e94..b31c2ecbc 100644 --- a/packages/core/__tests__/notebooks.test.js +++ b/packages/core/__tests__/notebooks.test.js @@ -4,6 +4,7 @@ import { TEST_NOTEBOOK, TEST_NOTE, } from "./utils"; +import { makeTopic } from "../collections/topics"; beforeEach(async () => { StorageInterface.clear(); @@ -43,6 +44,92 @@ test("unpin a notebook", () => test("updating notebook with empty title should throw", () => notebookTest().then(async ({ db, id }) => { - expect(id).toBeDefined(); await expect(db.notebooks.add({ id, title: "" })).rejects.toThrow(); })); + +test("merge notebook with new topics", () => + notebookTest().then(async ({ db, id }) => { + let notebook = db.notebooks.notebook(id); + + const newNotebook = { ...notebook.data, remote: true }; + newNotebook.topics.push(makeTopic("Home", id)); + + await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow(); + + expect(notebook.topics.has("Home")).toBe(true); + expect(notebook.topics.has("General")).toBe(true); + expect(notebook.topics.has("hello")).toBe(true); + })); + +test("merge notebook with topics removed", () => + notebookTest().then(async ({ db, id }) => { + let notebook = db.notebooks.notebook(id); + + const newNotebook = { ...notebook.data, remote: true }; + newNotebook.topics.splice(1, 1); // remove hello topic + newNotebook.topics.push(makeTopic("Home", id)); + + await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow(); + + expect(notebook.topics.has("Home")).toBe(true); + expect(notebook.topics.has("General")).toBe(true); + expect(notebook.topics.has("hello")).toBe(false); + })); + +test("merge notebook with topic edited", () => + notebookTest().then(async ({ db, id }) => { + let notebook = db.notebooks.notebook(id); + + const newNotebook = { ...notebook.data, remote: true }; + newNotebook.topics[1].title = "hello (edited)"; + + await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow(); + + expect(notebook.topics.has("hello (edited)")).toBe(true); + expect(notebook.topics.has("hello")).toBe(false); + })); + +test("merge notebook when local notebook is also edited", () => + notebookTest().then(async ({ db, id }) => { + let notebook = db.notebooks.notebook(id); + + const newNotebook = { ...notebook.data, remote: true }; + newNotebook.topics[1].title = "hello (edited)"; + + await delay(500); + + await notebook.topics.add({ + ...notebook.topics.all[1], + title: "hello (edited too)", + }); + + await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow(); + + expect(notebook.topics.has("hello (edited too)")).toBe(true); + expect(notebook.topics.has("hello (edited)")).toBe(false); + expect(notebook.topics.has("hello")).toBe(false); + })); + +test("merge notebook with topic removed that is edited in the local notebook", () => + notebookTest().then(async ({ db, id }) => { + let notebook = db.notebooks.notebook(id); + + const newNotebook = { ...notebook.data, remote: true }; + newNotebook.topics.splice(1, 1); // remove hello topic + + await delay(500); + + await notebook.topics.add({ + ...notebook.topics.all[1], + title: "hello (i exist)", + }); + + await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow(); + + expect(notebook.topics.has("hello (i exist)")).toBe(true); + expect(notebook.topics.has("hello")).toBe(false); + })); + +function delay(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/packages/core/api/sync/merger.js b/packages/core/api/sync/merger.js index 0ab77e2d6..3d3dcb3d0 100644 --- a/packages/core/api/sync/merger.js +++ b/packages/core/api/sync/merger.js @@ -106,7 +106,7 @@ class Merger { await this._mergeArray( notebooks, (id) => this._db.notebooks.notebook(id), - (item) => this._db.notebooks.add(item) + (item) => this._db.notebooks.merge(item) ); await this._mergeArrayWithConflicts( diff --git a/packages/core/collections/notebooks.js b/packages/core/collections/notebooks.js index 4aaa5cbb0..a5100784c 100644 --- a/packages/core/collections/notebooks.js +++ b/packages/core/collections/notebooks.js @@ -6,12 +6,46 @@ import { CHECK_IDS, sendCheckUserStatusEvent } from "../common"; import { qclone } from "qclone"; export default class Notebooks extends Collection { + async merge(remoteNotebook) { + const id = remoteNotebook.id || getId(); + let localNotebook = this._collection.getItem(id); + + if (localNotebook && localNotebook.topics?.length) { + // merge new and old topics + + // We need to handle 3 cases: + for (let oldTopic of localNotebook.topics) { + const newTopicIndex = remoteNotebook.topics.findIndex( + (t) => t.id === oldTopic.id + ); + const newTopic = remoteNotebook.topics[newTopicIndex]; + + // CASE 1: if topic exists in old notebook but not in new notebook, it's deleted. + // However, if the dateEdited of topic in the old notebook is > the dateEdited of newNotebook + // it was newly added or edited so add it to the new notebook. + if (!newTopic && oldTopic.dateEdited > remoteNotebook.dateEdited) { + remoteNotebook.topics.push(oldTopic); + } + + // CASE 2: if topic exists in new notebook but not in old notebook, it's new. + // This case will be automatically handled as the new notebook is our source of truth. + + // CASE 3: if topic exists in both notebooks: + // if oldTopic.dateEdited > newTopic.dateEdited: we keep oldTopic. + if (newTopic && oldTopic.dateEdited > newTopic.dateEdited) { + remoteNotebook.topics[newTopicIndex] = oldTopic; + } + } + } + return await this._collection.addItem(remoteNotebook); + } + async add(notebookArg) { if (!notebookArg) throw new Error("Notebook cannot be undefined or null."); - - if (notebookArg.remote) { - return await this._collection.addItem(notebookArg); - } + if (notebookArg.remote) + throw new Error( + "Please use db.notebooks.merge to merge remote notebooks" + ); //TODO reliably and efficiently check for duplicates. const id = notebookArg.id || getId(); diff --git a/packages/core/collections/topics.js b/packages/core/collections/topics.js index e8eeaf4a0..defd03d93 100644 --- a/packages/core/collections/topics.js +++ b/packages/core/collections/topics.js @@ -111,7 +111,8 @@ export default class Topics { } } -function makeTopic(topic, notebookId) { +// we export this for testing. +export function makeTopic(topic, notebookId) { if (typeof topic !== "string") return topic; return { type: "topic",