From b802b3e16549b63dbaeb587b95a651a3acc12f56 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Wed, 29 Jan 2025 13:39:40 +0500 Subject: [PATCH] web: fix diff session not opening in editor --- apps/web/__e2e__/notehistory.test.ts | 90 +---------- apps/web/__e2e__/tabs.test.ts | 43 +++++ apps/web/__e2e__/utils/index.ts | 41 ++++- apps/web/src/components/diff-viewer/index.tsx | 17 +- apps/web/src/components/editor/index.tsx | 5 +- apps/web/src/stores/editor-store.ts | 147 +++++++++--------- 6 files changed, 186 insertions(+), 157 deletions(-) diff --git a/apps/web/__e2e__/notehistory.test.ts b/apps/web/__e2e__/notehistory.test.ts index 183e86854..ef30340e4 100644 --- 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 . */ -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]); -// }); diff --git a/apps/web/__e2e__/tabs.test.ts b/apps/web/__e2e__/tabs.test.ts index 5ed7d3540..13a605120 100644 --- 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 . 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] + ); +}); diff --git a/apps/web/__e2e__/utils/index.ts b/apps/web/__e2e__/utils/index.ts index e0deb339e..8b65af65b 100644 --- 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, diff --git a/apps/web/src/components/diff-viewer/index.tsx b/apps/web/src/components/diff-viewer/index.tsx index b6c3bbd20..39c3f245e 100644 --- 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 . */ -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(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 ( diff --git a/apps/web/src/components/editor/index.tsx b/apps/web/src/components/editor/index.tsx index fafbdd179..751084a15 100644 --- 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 ( diff --git a/apps/web/src/stores/editor-store.ts b/apps/web/src/stores/editor-store.ts index 8b1931c11..950152ba1 100644 --- 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 { 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 { } ); - 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 = ( @@ -558,12 +566,12 @@ class EditorStore extends BaseStore { }); 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 { 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 { 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 { 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 { 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 { } }; - openNextSession = () => { + focusNextTab = () => { const { tabs, activeTabId } = this.get(); if (tabs.length <= 1) return; @@ -768,7 +781,7 @@ class EditorStore extends BaseStore { 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 { }; 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 { 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 { 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 ? {