web: fix diff session not opening in editor

2026-02-24 04:00:59 +01:00 · 2025-01-29 13:39:40 +05:00
parent 46583e12d9
commit b802b3e165
6 changed files with 186 additions and 157 deletions
--- a/apps/web/e2e/notehistory.test.ts
+++ b/apps/web/e2e/notehistory.test.ts
@@ -17,47 +17,11 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

-import { test, expect, Page } from "@playwright/test";
-import { AppModel } from "./models/app.model";
-import { NOTE, PASSWORD } from "./utils";
+import { test, expect } from "@playwright/test";
+import { createHistorySession, PASSWORD } from "./utils";

 test.setTimeout(60 * 1000);

-async function createSession(page: Page, locked = false) {
-  const app = new AppModel(page);
-  await app.goto();
-  const notes = await app.goToNotes();
-  let note = await notes.createNote(NOTE);
-
-  if (locked) {
-    await note?.contextMenu.lock(PASSWORD);
-    await note?.openLockedNote(PASSWORD);
-  }
-
-  const edits = ["Some edited text.", "Some more edited text."];
-  for (const edit of edits) {
-    await notes.editor.setContent(edit);
-
-    await page.waitForTimeout(600);
-
-    await page.reload().catch(console.error);
-    await notes.waitForItem(NOTE.title);
-    note = await notes.findNote(NOTE);
-    locked ? await note?.openLockedNote(PASSWORD) : await note?.openNote();
-  }
-  const contents = [
-    `${NOTE.content}${edits[0]}${edits[1]}`,
-    `${NOTE.content}${edits[0]}`
-  ];
-
-  return {
-    note,
-    notes,
-    app,
-    contents
-  };
-}
-
 const sessionTypes = ["locked", "unlocked"] as const;

 for (const type of sessionTypes) {
@@ -66,7 +30,7 @@ for (const type of sessionTypes) {
  test(`editing a note should create a new ${type} session in its session history`, async ({
    page
  }) => {
-    const { note } = await createSession(page, isLocked);
+    const { note } = await createHistorySession(page, isLocked);

    const history = await note?.properties.getSessionHistory();
    expect(history?.length).toBeGreaterThan(1);
@@ -81,7 +45,7 @@ for (const type of sessionTypes) {
  test(`switching ${type} sessions should change editor content`, async ({
    page
  }) => {
-    const { note, contents } = await createSession(page, isLocked);
+    const { note, contents } = await createHistorySession(page, isLocked);

    const history = await note?.properties.getSessionHistory();
    let preview = await history?.at(1)?.open();
@@ -103,7 +67,10 @@ for (const type of sessionTypes) {
  test(`restoring a ${type} session should change note's content`, async ({
    page
  }) => {
-    const { note, notes, contents } = await createSession(page, isLocked);
+    const { note, notes, contents } = await createHistorySession(
+      page,
+      isLocked
+    );
    const history = await note?.properties.getSessionHistory();
    const preview = await history?.at(1)?.open();
    if (type === "locked") await preview?.unlock(PASSWORD);
@@ -115,44 +82,3 @@ for (const type of sessionTypes) {
    expect(await notes.editor.getContent("text")).toBe(contents[1]);
  });
 }
-// test("editing locked note should create locked history sessions", async ({
-//   page
-// }) => {
-//   const { note } = await createLockedSession(page);
-
-//   const history = await note?.properties.getSessionHistory();
-//   expect(history).toHaveLength(2);
-// for (const item of history || []) {
-//   expect(await item.isLocked()).toBeTruthy();
-// }
-// });
-
-// test("switching locked sessions should change editor content", async ({
-//   page
-// }) => {
-//   const { note, notes, contents } = await createLockedSession(page);
-
-//   const history = await note?.properties.getSessionHistory();
-//   await history?.at(1)?.previewLocked(PASSWORD);
-//   await notes.editor.waitForLoading(NOTE.title, contents[1]);
-//   const content1 = await notes.editor.getContent("text");
-
-//   await history?.at(0)?.previewLocked(PASSWORD);
-//   await notes.editor.waitForLoading(NOTE.title, contents[0]);
-//   const content0 = await notes.editor.getContent("text");
-
-//   expect(content1).toBe(contents[1]);
-//   expect(content0).toBe(contents[0]);
-// });
-
-// test("restore a locked session", async ({ page }) => {
-//   const { note, notes, contents } = await createLockedSession(page);
-//   const history = await note?.properties.getSessionHistory();
-//   await history?.at(1)?.previewLocked(PASSWORD);
-//   await notes.editor.waitForLoading(NOTE.title, contents[1]);
-
-//   await notes.editor.restoreSession();
-
-//   const content = await notes.editor.getContent("text");
-//   expect(content).toBe(contents[1]);
-// });
--- a/apps/web/e2e/tabs.test.ts
+++ b/apps/web/e2e/tabs.test.ts
@@ -19,6 +19,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.

 import { test, expect } from "@playwright/test";
 import { AppModel } from "./models/app.model";
+import { createHistorySession } from "./utils";

 test("notes should open in the same tab", async ({ page }) => {
  const app = new AppModel(page);
@@ -58,6 +59,7 @@ test("open note in new tab (using context menu)", async ({ page }) => {

  const note = await notes.findNote({ title: "Note 2" });
  await note?.contextMenu.openInNewTab();
+  await notes.editor.waitForLoading();

  const tabs = await notes.editor.getTabs();
  expect(tabs.length).toBe(2);
@@ -74,6 +76,7 @@ test("open note in new tab (using middle click)", async ({ page }) => {

  const note = await notes.findNote({ title: "Note 2" });
  await note?.click({ middleClick: true });
+  await notes.editor.waitForLoading();

  const tabs = await notes.editor.getTabs();
  expect(tabs.length).toBe(2);
@@ -164,3 +167,43 @@ test("open same note in 2 tabs and refresh page", async ({ page }) => {
  await notes.editor.waitForLoading();
  expect(await notes.editor.getContent("text")).toBe("Some edits.");
 });
+
+test("reloading with a note diff open in a tab", async ({ page }) => {
+  const { note, contents } = await createHistorySession(page);
+  const history = await note?.properties.getSessionHistory();
+  const preview = await history?.[0].open();
+  await preview!.firstEditor.waitFor({ state: "visible" });
+
+  await page.reload();
+
+  await expect(preview!.firstEditor.locator(".ProseMirror")).toHaveText(
+    contents[0]
+  );
+  await expect(preview!.secondEditor.locator(".ProseMirror")).toHaveText(
+    contents[0]
+  );
+});
+
+test("navigate back and forth between normal and diff session", async ({
+  page
+}) => {
+  const { note, contents, notes } = await createHistorySession(page);
+  const history = await note?.properties.getSessionHistory();
+  const preview = await history?.[0].open();
+  await preview!.firstEditor.waitFor({ state: "visible" });
+
+  await notes.editor.goBack();
+  await preview!.firstEditor.waitFor({ state: "hidden" });
+
+  expect(await notes.editor.getContent("text")).toBe(contents[0]);
+
+  await notes.editor.goForward();
+  await preview!.firstEditor.waitFor({ state: "visible" });
+
+  await expect(preview!.firstEditor.locator(".ProseMirror")).toHaveText(
+    contents[0]
+  );
+  await expect(preview!.secondEditor.locator(".ProseMirror")).toHaveText(
+    contents[0]
+  );
+});
--- a/apps/web/e2e/utils/index.ts
+++ b/apps/web/e2e/utils/index.ts
@@ -28,6 +28,7 @@ import {
  SortByOptions
 } from "../models/types";
 import { tmpdir } from "os";
+import { AppModel } from "../models/app.model";

 type Note = {
  title: string;
@@ -74,7 +75,10 @@ const PASSWORD = "123abc123abc";

 const APP_LOCK_PASSWORD = "lockapporelse🔪";

-function getTestId(id: string, variant: "data-test-id" | "data-testid" = "data-test-id") {
+function getTestId(
+  id: string,
+  variant: "data-test-id" | "data-testid" = "data-test-id"
+) {
  return `[${variant}="${id}"]`;
 }

@@ -153,6 +157,41 @@ const groupByOptions: GroupByOptions[] = [
  "week"
 ];

+export async function createHistorySession(page: Page, locked = false) {
+  const app = new AppModel(page);
+  await app.goto();
+  const notes = await app.goToNotes();
+  let note = await notes.createNote(NOTE);
+
+  if (locked) {
+    await note?.contextMenu.lock(PASSWORD);
+    await note?.openLockedNote(PASSWORD);
+  }
+
+  const edits = ["Some edited text.", "Some more edited text."];
+  for (const edit of edits) {
+    await notes.editor.setContent(edit);
+
+    await page.waitForTimeout(600);
+
+    await page.reload().catch(console.error);
+    await notes.waitForItem(NOTE.title);
+    note = await notes.findNote(NOTE);
+    locked ? await note?.openLockedNote(PASSWORD) : await note?.openNote();
+  }
+  const contents = [
+    `${NOTE.content}${edits[0]}${edits[1]}`,
+    `${NOTE.content}${edits[0]}`
+  ];
+
+  return {
+    note,
+    notes,
+    app,
+    contents
+  };
+}
+
 export {
  USER,
  NOTE,
--- a/apps/web/src/components/diff-viewer/index.tsx
+++ b/apps/web/src/components/diff-viewer/index.tsx
@@ -17,7 +17,13 @@ You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

-import { useCallback, useLayoutEffect, useRef, useState } from "react";
+import {
+  useCallback,
+  useEffect,
+  useLayoutEffect,
+  useRef,
+  useState
+} from "react";
 import { Flex, Text, Button } from "@theme-ui/components";
 import { Copy, Restore } from "../icons";
 import ContentToggle from "./content-toggle";
@@ -25,6 +31,7 @@ import { store as notesStore } from "../../stores/note-store";
 import { db } from "../../common/db";
 import {
  ConflictedEditorSession,
+  DiffEditorSession,
  useEditorStore
 } from "../../stores/editor-store";
 import { ScrollSync, ScrollSyncPane } from "react-scroll-sync";
@@ -35,7 +42,7 @@ import { getFormattedDate } from "@notesnook/common";
 import { diff } from "diffblazer";
 import { strings } from "@notesnook/intl";

-type DiffViewerProps = { session: ConflictedEditorSession };
+type DiffViewerProps = { session: ConflictedEditorSession | DiffEditorSession };
 function DiffViewer(props: DiffViewerProps) {
  const { session } = props;

@@ -46,6 +53,11 @@ function DiffViewer(props: DiffViewerProps) {
  );
  const root = useRef<HTMLDivElement>(null);

+  useEffect(() => {
+    setContent(session.content);
+    setConflictedContent(session.content?.conflicted);
+  }, [session.content]);
+
  const onResolveContent = useCallback(
    (saveCopy: boolean) => {
      const toKeep =
@@ -79,6 +91,7 @@ function DiffViewer(props: DiffViewerProps) {
    };
  }, []);

+  console.log({ conflictedContent, content, session });
  if (!conflictedContent || !content) return null;

  return (
--- a/apps/web/src/components/editor/index.tsx
+++ b/apps/web/src/components/editor/index.tsx
@@ -133,7 +133,7 @@ export default function TabsView() {
        event.key === "ArrowRight"
      ) {
        event.preventDefault();
-        useEditorStore.getState().openNextSession();
+        useEditorStore.getState().focusNextTab();
      }
      if (
        (event.ctrlKey || event.metaKey) &&
@@ -141,7 +141,7 @@ export default function TabsView() {
        event.key === "ArrowLeft"
      ) {
        event.preventDefault();
-        useEditorStore.getState().openPreviousSession();
+        useEditorStore.getState().focusPreviousTab();
      }
    };
    document.body.addEventListener("keydown", onKeyDown);
@@ -180,6 +180,7 @@ export default function TabsView() {
              const session = useEditorStore
                .getState()
                .getSession(tab.sessionId);
+              console.log("rendering tab", tab, session);
              if (!session) return null;
              return (
                <Freeze key={session.id} freeze={tab.id !== activeTab?.id}>
--- a/apps/web/src/stores/editor-store.ts
+++ b/apps/web/src/stores/editor-store.ts
@@ -127,16 +127,24 @@ export type NewEditorSession = BaseEditorSession & {
 };

 export type ConflictedEditorSession = BaseEditorSession & {
-  type: "conflicted" | "diff";
+  type: "conflicted";
  note: Note;
  content?: ContentItem;
 };

+export type DiffEditorSession = BaseEditorSession & {
+  type: "diff";
+  note: Note;
+  content: ContentItem;
+  historySessionId: string;
+};
+
 export type EditorSession =
  | DefaultEditorSession
  | LockedEditorSession
  | NewEditorSession
  | ConflictedEditorSession
+  | DiffEditorSession
  | ReadonlyEditorSession
  | DeletedEditorSession;

@@ -146,7 +154,7 @@ type SessionTypeMap = {
  locked: LockedEditorSession;
  new: NewEditorSession;
  conflicted: ConflictedEditorSession;
-  diff: ConflictedEditorSession;
+  diff: DiffEditorSession;
  readonly: ReadonlyEditorSession;
  deleted: DeletedEditorSession;
 };
@@ -470,7 +478,6 @@ class EditorStore extends BaseStore<EditorStore> {
                session.tags?.every((t) => !event.ids.includes(t.id))
              )
                continue;
-              console.log("UDPATE");
              updateSession(session.id, undefined, {
                tags: await db.notes.tags(session.note.id)
              });
@@ -481,25 +488,26 @@ class EditorStore extends BaseStore<EditorStore> {
      }
    );

-    const { rehydrateTab, activeTabId, newSession } = this.get();
+    const { rehydrateSession, activeTabId, newSession } = this.get();
    if (activeTabId) {
-      rehydrateTab(activeTabId);
+      const tab = this.get().tabs.find((t) => t.id === activeTabId);
+      if (!tab) return;
+      rehydrateSession(tab.sessionId);
    } else newSession();
  };

-  private rehydrateTab = (tabId: string) => {
-    const { openSession, openDiffSession, activateSession, getSession } =
-      this.get();
+  private rehydrateSession = (sessionId: string) => {
+    const { openSession, openDiffSession, getSession } = this.get();

-    const tab = this.get().tabs.find((t) => t.id === tabId);
-    if (!tab) return;
-    const session = getSession(tab.sessionId);
+    const session = getSession(sessionId);
    if (!session || !session.needsHydration) return;

-    if (session.type === "diff" || session.type === "conflicted")
-      openDiffSession(session.note.id, session.id);
-    else if (session.type === "new") activateSession(session.id);
-    else openSession(session.note);
+    if (session.type === "diff")
+      openDiffSession(session.note.id, session.historySessionId);
+    else if (session.type !== "new")
+      openSession(session.note.id, {
+        force: true
+      });
  };

  updateSession = <T extends SessionType[] = SessionType[]>(
@@ -558,12 +566,12 @@ class EditorStore extends BaseStore<EditorStore> {
      });

    if (session?.tabId) {
-      this.focusTab(session.tabId);
      this.set((state) => {
        const index = state.tabs.findIndex((t) => t.id === session.tabId);
        if (index === -1) return;
        state.tabs[index].sessionId = session.id;
      });
+      this.focusTab(session.tabId, session.id);
    }
  };

@@ -577,15 +585,23 @@ class EditorStore extends BaseStore<EditorStore> {

    if (!oldContent || !currentContent) return;

+    const { getSession, addSession } = this.get();
+
    const label = getFormattedHistorySessionDate(session);
    const tabId = this.get().activeTabId ?? this.addTab();
-    const tabSessionId = tabSessionHistory.add(tabId);
-    this.get().addSession({
+    const tab = this.get().tabs.find((t) => t.id === tabId);
+    const tabSession = tab && getSession(tab.sessionId);
+    const tabSessionId =
+      tabSession?.needsHydration || tabSession?.type === "new"
+        ? session.id
+        : tabSessionHistory.add(tabId);
+    addSession({
      type: "diff",
      id: tabSessionId,
      note,
      tabId,
      title: label,
+      historySessionId: session.id,
      content: {
        type: oldContent.type,
        dateCreated: session.dateCreated,
@@ -614,7 +630,7 @@ class EditorStore extends BaseStore<EditorStore> {
    const tabId = options.openInNewTab
      ? this.addTab()
      : this.get().activeTabId ?? this.addTab();
-    const { getSession, openDiffSession } = this.get();
+    const { getSession, activateSession, rehydrateSession } = this.get();
    const noteId = typeof noteOrId === "string" ? noteOrId : noteOrId.id;

    const tab = this.get().tabs.find((t) => t.id === tabId);
@@ -625,11 +641,9 @@ class EditorStore extends BaseStore<EditorStore> {
      session.note.id === noteId &&
      !options.force
    ) {
-      if (!session.needsHydration)
-        return this.activateSession(noteId, options.activeBlockId);
-      if (session.type === "diff" || session.type === "conflicted") {
-        return openDiffSession(session.note.id, session.id);
-      }
+      return session.needsHydration
+        ? rehydrateSession(session.id)
+        : activateSession(noteId, options.activeBlockId);
    }

    if (session && session.id) await db.fs().cancel(session.id);
@@ -644,7 +658,6 @@ class EditorStore extends BaseStore<EditorStore> {
      session?.needsHydration || session?.type === "new"
        ? session.id
        : tabSessionHistory.add(tabId);
-    console.log("opening session", session);
    const isLocked = await db.vaults.itemExists(note);

    if (note.conflicted) {
@@ -755,7 +768,7 @@ class EditorStore extends BaseStore<EditorStore> {
    }
  };

-  openNextSession = () => {
+  focusNextTab = () => {
    const { tabs, activeTabId } = this.get();
    if (tabs.length <= 1) return;

@@ -768,7 +781,7 @@ class EditorStore extends BaseStore<EditorStore> {
    return this.focusTab(tabs[index + 1].id);
  };

-  openPreviousSession = () => {
+  focusPreviousTab = () => {
    const { tabs, activeTabId } = this.get();
    if (tabs.length <= 1) return;

@@ -782,34 +795,12 @@ class EditorStore extends BaseStore<EditorStore> {
  };

  goBack = async () => {
-    console.log("GO BACK!");
    const activeTabId = this.get().activeTabId;
    if (!activeTabId || !tabSessionHistory.canGoBack(activeTabId)) return;
    const sessionId = tabSessionHistory.back(activeTabId);
    if (!sessionId) return;
-    const session = this.get().getSession(sessionId);
-    if (!session) {
-      tabSessionHistory.remove(activeTabId, sessionId);
+    if (!(await this.goToSession(activeTabId, sessionId))) {
      await this.goBack();
-      return;
-    }
-    // we must rehydrate the session as the note's content can be stale
-    this.updateSession(sessionId, undefined, {
-      needsHydration: true
-    });
-    this.activateSession(sessionId);
-    if ("note" in session) {
-      const note = await db.notes.note(session.note.id);
-      if (!note) {
-        tabSessionHistory.remove(activeTabId, sessionId);
-        this.set((state) => {
-          const index = state.sessions.findIndex((s) => s.id === session.id);
-          state.sessions.splice(index, 1);
-        });
-        await this.goBack();
-        return;
-      }
-      await this.openSession(note);
    }
  };

@@ -818,33 +809,44 @@ class EditorStore extends BaseStore<EditorStore> {
    if (!activeTabId || !tabSessionHistory.canGoForward(activeTabId)) return;
    const sessionId = tabSessionHistory.forward(activeTabId);
    if (!sessionId) return;
+    if (!(await this.goToSession(activeTabId, sessionId))) {
+      await this.goForward();
+    }
+  };
+
+  goToSession = async (tabId: string, sessionId: string) => {
    const session = this.get().getSession(sessionId);
    if (!session) {
-      tabSessionHistory.remove(activeTabId, sessionId);
-      await this.goForward();
-      return;
+      tabSessionHistory.remove(tabId, sessionId);
+      return false;
    }
-    this.activateSession(sessionId);
+
    if ("note" in session) {
-      const note = await db.notes.note(session.note.id);
-      if (!note) {
-        tabSessionHistory.remove(activeTabId, sessionId);
+      if (!(await db.notes.exists(session.note.id))) {
+        tabSessionHistory.remove(tabId, session.id);
        this.set((state) => {
          const index = state.sessions.findIndex((s) => s.id === session.id);
          state.sessions.splice(index, 1);
        });
-        await this.goForward();
-        return;
+        return false;
      }
-      await this.openSession(note);
+
+      // we must rehydrate the session as the note's content can be stale
+      this.updateSession(session.id, undefined, {
+        needsHydration: true
+      });
+      this.activateSession(session.id);
+      return true;
    }
+    return false;
  };

  addSession = (session: EditorSession, activate = true) => {
    this.set((state) => {
      const index = state.sessions.findIndex((s) => s.id === session.id);
-      if (index > -1) state.sessions[index] = session;
-      else state.sessions.push(session);
+      if (index > -1) {
+        state.sessions[index] = session;
+      } else state.sessions.push(session);
    });

    if (activate) this.activateSession(session.id);
@@ -1144,20 +1146,22 @@ class EditorStore extends BaseStore<EditorStore> {
    return id;
  };

-  focusTab = (id: string | undefined) => {
-    if (id === undefined) return;
+  focusTab = (tabId: string | undefined, sessionId?: string) => {
+    if (!tabId) return;

    const { history } = this.get();
-    if (history.includes(id)) history.splice(history.indexOf(id), 1);
-    history.push(id);
+    if (history.includes(tabId)) history.splice(history.indexOf(tabId), 1);
+    history.push(tabId);

    this.set({
-      activeTabId: id,
-      canGoBack: tabSessionHistory.canGoBack(id),
-      canGoForward: tabSessionHistory.canGoForward(id)
+      activeTabId: tabId,
+      canGoBack: tabSessionHistory.canGoBack(tabId),
+      canGoForward: tabSessionHistory.canGoForward(tabId)
    });

-    this.rehydrateTab(id);
+    sessionId =
+      sessionId || this.get().tabs.find((t) => t.id === tabId)?.sessionId;
+    if (sessionId) this.rehydrateSession(sessionId);
  };
 }

@@ -1178,6 +1182,9 @@ const useEditorStore = createPersistedStore(EditorStore, {
        type: isLockedSession(session) ? "locked" : session.type,
        needsHydration: session.type === "new" ? false : true,
        title: session.title,
+        historySessionId:
+          session.type === "diff" ? session.historySessionId : undefined,
+        tabId: session.tabId,
        note:
          "note" in session
            ? {