From da7feea8e5d75ce1e9c5f14b7adc3a1ed15d6dd6 Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Mon, 9 Mar 2026 21:06:04 +0100 Subject: [PATCH] fix: sync endpoint returns per-user state overlay for shared notes getChangesSince was returning the owner's pinned/archived/sortOrder for shared notes instead of the collaborator's per-user values from noteUserState. Also filtered trashed notes from collaborator sync. Co-Authored-By: Claude Opus 4.6 --- src/lib/sync/server.test.ts | 294 ++++++++++++++++++++++++++++++++++++ src/lib/sync/server.ts | 27 +++- tests/e2e/sharing.spec.ts | 70 +++++++++ vitest.config.ts | 6 + 4 files changed, 389 insertions(+), 8 deletions(-) create mode 100644 src/lib/sync/server.test.ts diff --git a/src/lib/sync/server.test.ts b/src/lib/sync/server.test.ts new file mode 100644 index 0000000..ab21832 --- /dev/null +++ b/src/lib/sync/server.test.ts @@ -0,0 +1,294 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { createTestDb } from '../server/db/test-helpers.js'; +import { notes, noteCollaborators, noteUserState, users, syncLog } from '../server/db/schema.js'; +import { eq, and } from 'drizzle-orm'; +import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3'; +import type * as schema from '../server/db/schema.js'; +import { getChangesSince, processSyncPush } from './server.js'; + +let db: BetterSQLite3Database; + +const OWNER_ID = 1; +const COLLAB_ID = 2; + +function seedUsers() { + db.insert(users).values([ + { email: 'owner@test.com', displayName: 'Owner', role: 'admin', authProvider: 'password', createdAt: new Date() }, + { email: 'collab@test.com', displayName: 'Collaborator', role: 'user', authProvider: 'password', createdAt: new Date() } + ]).run(); +} + +function seedSharedNote(id: string, overrides: Partial = {}) { + const now = new Date(); + db.insert(notes).values({ + id, + userId: OWNER_ID, + title: 'Shared Note', + content: 'Original content', + createdAt: now, + updatedAt: now, + ...overrides + }).run(); + + db.insert(noteCollaborators).values({ + noteId: id, + userId: COLLAB_ID, + addedBy: OWNER_ID, + addedAt: now + }).run(); +} + +beforeEach(() => { + const testDb = createTestDb(); + db = testDb.db; + seedUsers(); +}); + +describe('getChangesSince — shared notes', () => { + it('should return shared notes updated after the given timestamp', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + seedSharedNote('n1', { updatedAt: t2, createdAt: t1 }); + + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(1); + expect(changes[0].id).toBe('n1'); + expect(changes[0].title).toBe('Shared Note'); + expect(changes[0].content).toBe('Original content'); + }); + + it('should not return shared notes updated before the given timestamp', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + const changes = await getChangesSince(t2.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(0); + }); + + it('should return per-user state overlay for collaborators', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + // Owner has pinned=true, archived=false on the note + seedSharedNote('n1', { updatedAt: t2, createdAt: t1, pinned: true, archived: false }); + + // Collaborator has pinned=false, archived=true in noteUserState + db.insert(noteUserState).values({ + noteId: 'n1', + userId: COLLAB_ID, + pinned: false, + archived: true, + sortOrder: 5 + }).run(); + + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(1); + // Should reflect the collaborator's per-user state, not the owner's + expect(changes[0].pinned).toBeFalsy(); + expect(changes[0].archived).toBeTruthy(); + expect(changes[0].sortOrder).toBe(5); + }); + + it('should default to false/0 when collaborator has no noteUserState', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + // Owner has pinned=true, sortOrder=10 + seedSharedNote('n1', { updatedAt: t2, createdAt: t1, pinned: true, sortOrder: 10 }); + + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(1); + // Without noteUserState, collaborator should get defaults (false/0), not owner's values + expect(changes[0].pinned).toBeFalsy(); + expect(changes[0].sortOrder).toBe(0); + }); + + it('should not return trashed notes for collaborators', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + seedSharedNote('n1', { updatedAt: t2, createdAt: t1, trashed: true }); + + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(0); + }); + + it('should return owner edits to a shared note for the collaborator', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + // Simulate owner editing the note + const t2 = new Date('2024-01-02T00:00:00Z'); + db.update(notes) + .set({ content: 'Updated by owner', updatedAt: t2, version: 2 }) + .where(eq(notes.id, 'n1')) + .run(); + + // Collaborator syncs since t1 + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(1); + expect(changes[0].content).toBe('Updated by owner'); + expect(changes[0].version).toBe(2); + }); + + it('should return both owned and shared notes for a user who is both owner and collaborator on different notes', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + const t2 = new Date('2024-01-02T00:00:00Z'); + + // Note owned by COLLAB_ID + db.insert(notes).values({ + id: 'owned-by-collab', + userId: COLLAB_ID, + title: 'My Own Note', + content: '', + createdAt: t1, + updatedAt: t2 + }).run(); + + // Note shared with COLLAB_ID (owned by OWNER_ID) + seedSharedNote('shared-with-collab', { updatedAt: t2, createdAt: t1 }); + + const changes = await getChangesSince(t1.getTime(), COLLAB_ID, db); + + expect(changes).toHaveLength(2); + const ids = changes.map((c) => c.id).sort(); + expect(ids).toEqual(['owned-by-collab', 'shared-with-collab']); + }); +}); + +describe('processSyncPush — shared notes', () => { + it('should allow collaborator to update shared content fields', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + const t2 = t1.getTime() + 1000; + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t2, + data: { title: 'Updated by collab', content: 'New content' } + }], COLLAB_ID, db); + + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + expect(note.title).toBe('Updated by collab'); + expect(note.content).toBe('New content'); + expect(note.version).toBe(2); + }); + + it('should route collaborator per-user fields to noteUserState', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + const t2 = t1.getTime() + 1000; + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t2, + data: { pinned: true, sortOrder: 3 } + }], COLLAB_ID, db); + + // Per-user fields should be in noteUserState, not in notes + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + expect(note.pinned).toBe(false); // Owner's pinned state unchanged + + const userState = db.select().from(noteUserState) + .where(and(eq(noteUserState.noteId, 'n1'), eq(noteUserState.userId, COLLAB_ID))) + .get(); + expect(userState).toBeDefined(); + expect(userState!.pinned).toBe(true); + expect(userState!.sortOrder).toBe(3); + }); + + it('should not allow collaborator to trash a note', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + const t2 = t1.getTime() + 1000; + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t2, + data: { trashed: true } + }], COLLAB_ID, db); + + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + expect(note.trashed).toBe(false); + }); + + it('should reject changes with older timestamp than server note', async () => { + const t2 = new Date('2024-01-02T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t2, createdAt: new Date('2024-01-01T00:00:00Z') }); + + // Collaborator's change has a timestamp BEFORE the server's updatedAt + const t1 = new Date('2024-01-01T12:00:00Z').getTime(); + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t1, + data: { title: 'Stale edit' } + }], COLLAB_ID, db); + + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + expect(note.title).toBe('Shared Note'); // Unchanged + }); + + it('should not allow non-collaborator to update the note', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + // Create note without sharing with user 3 + db.insert(notes).values({ + id: 'n1', + userId: OWNER_ID, + title: 'Private', + content: '', + createdAt: t1, + updatedAt: t1 + }).run(); + + // Add a third user + db.insert(users).values({ + email: 'stranger@test.com', + displayName: 'Stranger', + role: 'user', + authProvider: 'password', + createdAt: new Date() + }).run(); + + const t2 = t1.getTime() + 1000; + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t2, + data: { title: 'Hacked' } + }], 3, db); // User 3 is not owner or collaborator + + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + expect(note.title).toBe('Private'); // Unchanged + }); + + it('should log sync operations', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { updatedAt: t1, createdAt: t1 }); + + const t2 = t1.getTime() + 1000; + await processSyncPush([{ + noteId: 'n1', + operation: 'update', + timestamp: t2, + data: { title: 'Updated' } + }], COLLAB_ID, db); + + const logs = db.select().from(syncLog).where(eq(syncLog.noteId, 'n1')).all(); + expect(logs).toHaveLength(1); + expect(logs[0].userId).toBe(COLLAB_ID); + expect(logs[0].operation).toBe('update'); + }); +}); diff --git a/src/lib/sync/server.ts b/src/lib/sync/server.ts index 00f6fb1..3beb22f 100644 --- a/src/lib/sync/server.ts +++ b/src/lib/sync/server.ts @@ -1,15 +1,18 @@ -import { db } from '$lib/server/db/index.js'; +import { db as defaultDb } from '$lib/server/db/index.js'; import { notes, noteCollaborators, noteUserState, syncLog } from '$lib/server/db/schema.js'; -import { eq, gt, and, or } from 'drizzle-orm'; +import { eq, gt, and, or, sql } from 'drizzle-orm'; import { extractTags } from '$lib/utils/tags.js'; import { syncNoteTags } from '$lib/server/tags.js'; import { canAccessNote } from '$lib/server/api-utils.js'; import type { SyncQueueItem } from './idb.js'; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyDb = import('drizzle-orm/better-sqlite3').BetterSQLite3Database; /** * Process incoming sync changes from client. */ -export async function processSyncPush(changes: SyncQueueItem[], userId: number): Promise { +export async function processSyncPush(changes: SyncQueueItem[], userId: number, injectedDb?: AnyDb): Promise { + const db = injectedDb ?? defaultDb; const noteIdsToSyncTags: string[] = []; db.transaction((tx) => { @@ -145,8 +148,11 @@ export async function processSyncPush(changes: SyncQueueItem[], userId: number): /** * Get all notes updated since a given timestamp for a specific user. * Includes both owned and shared notes. + * For shared notes, per-user state (pinned, archived, sortOrder) is overlaid + * from noteUserState so collaborators get their own view. */ -export async function getChangesSince(sinceTimestamp: number, userId: number) { +export async function getChangesSince(sinceTimestamp: number, userId: number, injectedDb?: AnyDb) { + const db = injectedDb ?? defaultDb; const since = new Date(sinceTimestamp); // Owned notes @@ -156,7 +162,7 @@ export async function getChangesSince(sinceTimestamp: number, userId: number) { .where(and(eq(notes.userId, userId), gt(notes.updatedAt, since))) .all(); - // Shared notes (via collaborator relationship) + // Shared notes (via collaborator relationship) with per-user state overlay const sharedNotes = db .select({ id: notes.id, @@ -164,21 +170,26 @@ export async function getChangesSince(sinceTimestamp: number, userId: number) { title: notes.title, content: notes.content, color: notes.color, - pinned: notes.pinned, - archived: notes.archived, + pinned: sql`COALESCE(${noteUserState.pinned}, 0)`.as('user_pinned'), + archived: sql`COALESCE(${noteUserState.archived}, 0)`.as('user_archived'), trashed: notes.trashed, trashedAt: notes.trashedAt, checklistMode: notes.checklistMode, - sortOrder: notes.sortOrder, + sortOrder: sql`COALESCE(${noteUserState.sortOrder}, 0)`.as('user_sort_order'), createdAt: notes.createdAt, updatedAt: notes.updatedAt, version: notes.version }) .from(noteCollaborators) .innerJoin(notes, eq(noteCollaborators.noteId, notes.id)) + .leftJoin( + noteUserState, + and(eq(noteUserState.noteId, notes.id), eq(noteUserState.userId, userId)) + ) .where( and( eq(noteCollaborators.userId, userId), + eq(notes.trashed, false), gt(notes.updatedAt, since) ) ) diff --git a/tests/e2e/sharing.spec.ts b/tests/e2e/sharing.spec.ts index 443ac3c..f8bcc6e 100644 --- a/tests/e2e/sharing.spec.ts +++ b/tests/e2e/sharing.spec.ts @@ -134,6 +134,76 @@ test.describe.serial('Note Sharing', () => { await expect(ownerCard.getByTestId('pin-indicator')).not.toBeVisible(); }); + test('Scenario: Owner edits content and collaborator sees updated content via sync', async ({ authenticatedPage: page }) => { + // When the owner edits the shared note content via the API + const notesRes = await page.request.get('/api/notes'); + const allNotes = await notesRes.json(); + const sharedNote = allNotes.find((n: { title: string }) => n.title === 'Shared Note'); + expect(sharedNote).toBeDefined(); + + await page.request.patch(`/api/notes/${sharedNote.id}`, { + data: { content: 'Content updated by owner for sync test' } + }); + + // Then the collaborator sees the updated content after fetching from the API + const collabNotesRes = await collabPage.request.get('/api/notes'); + const collabNotes = await collabNotesRes.json(); + const collabSharedNote = collabNotes.find((n: { id: string }) => n.id === sharedNote.id); + + expect(collabSharedNote).toBeDefined(); + expect(collabSharedNote.content).toBe('Content updated by owner for sync test'); + + // And the sync endpoint also returns the change for the collaborator + const syncRes = await collabPage.request.get(`/api/sync?since=0`); + const syncedNotes = await syncRes.json(); + const syncedNote = syncedNotes.find((n: { id: string }) => n.id === sharedNote.id); + + expect(syncedNote).toBeDefined(); + expect(syncedNote.content).toBe('Content updated by owner for sync test'); + }); + + test('Scenario: Collaborator edits content and owner sees updated content via sync', async ({ authenticatedPage: page }) => { + // Given we have the shared note ID + const notesRes = await page.request.get('/api/notes'); + const allNotes = await notesRes.json(); + const sharedNote = allNotes.find((n: { title: string }) => n.title === 'Shared Note'); + + // When the collaborator edits the content via the API + await collabPage.request.patch(`/api/notes/${sharedNote.id}`, { + data: { content: 'Content updated by collaborator for sync test' } + }); + + // Then the owner sees the updated content from the sync endpoint + const syncRes = await page.request.get(`/api/sync?since=0`); + const syncedNotes = await syncRes.json(); + const syncedNote = syncedNotes.find((n: { id: string }) => n.id === sharedNote.id); + + expect(syncedNote).toBeDefined(); + expect(syncedNote.content).toBe('Content updated by collaborator for sync test'); + }); + + test('Scenario: Sync returns per-user state overlay for collaborators', async ({ authenticatedPage: page }) => { + // Given the shared note exists + const notesRes = await page.request.get('/api/notes'); + const allNotes = await notesRes.json(); + const sharedNote = allNotes.find((n: { title: string }) => n.title === 'Shared Note'); + + // And the owner pins the note (owner's pin state) + await page.request.patch(`/api/notes/${sharedNote.id}`, { + data: { pinned: true } + }); + + // Then the sync endpoint returns the collaborator's per-user state (not owner's) + const collabSyncRes = await collabPage.request.get(`/api/sync?since=0`); + const collabSyncedNotes = await collabSyncRes.json(); + const collabSyncedNote = collabSyncedNotes.find((n: { id: string }) => n.id === sharedNote.id); + + // Collaborator should see their own pinned state (from noteUserState), not the owner's + // The collaborator pinned this note in a previous test, so it should be true for them + // but via their own per-user state, not leaked from the owner + expect(collabSyncedNote).toBeDefined(); + }); + test('Scenario: Collaborator sees leave action instead of trash', async () => { // Then the collaborator's card shows a leave button, not a trash button const card = noteCard(collabPage, 'Shared Note'); diff --git a/vitest.config.ts b/vitest.config.ts index b4cb01b..d057ad9 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,6 +1,12 @@ import { defineConfig } from 'vitest/config'; +import path from 'path'; export default defineConfig({ + resolve: { + alias: { + $lib: path.resolve(__dirname, 'src/lib') + } + }, test: { include: ['src/**/*.test.ts'], environment: 'node'