mirror of
https://github.com/bretzel-app/crumbs.git
synced 2026-10-13 04:06:57 +02:00
fix: preserve search results when background sync runs
Keep canonical notes separate from active search results so periodic reloads no longer replace the filtered view. Cancel superseded requests and ignore stale responses to prevent older queries from overwriting newer results. Reconcile results with canonical notes so updates propagate and removed or trashed notes disappear. Add unit and end-to-end coverage for reconciliation, background sync, and out-of-order search responses. Co-authored-by: Google Gemini <gemini@google.com>
This commit is contained in:
committed by
Fred Cabassut
co-authored by
Google Gemini
parent
4269de2a95
commit
27eaf86845
@@ -1,9 +1,10 @@
|
||||
<script lang="ts">
|
||||
import { notes, loadNotes, currentFilter } from '$lib/stores/notes.js';
|
||||
import { searchQuery, searchResults } from '$lib/stores/notes.js';
|
||||
import type { Note } from '$lib/types/index.js';
|
||||
import Search from 'lucide-svelte/icons/search';
|
||||
import X from 'lucide-svelte/icons/x';
|
||||
import ArrowLeft from 'lucide-svelte/icons/arrow-left';
|
||||
import { onDestroy } from 'svelte';
|
||||
|
||||
interface Props {
|
||||
onClose?: () => void;
|
||||
@@ -12,41 +13,58 @@
|
||||
let { onClose }: Props = $props();
|
||||
|
||||
let query = $state('');
|
||||
let originalNotes: Note[] = [];
|
||||
let isSearching = $state(false);
|
||||
let inputEl: HTMLInputElement | undefined = $state();
|
||||
let searchController: AbortController | undefined;
|
||||
|
||||
$effect(() => {
|
||||
if (onClose) inputEl?.focus();
|
||||
});
|
||||
|
||||
onDestroy(() => {
|
||||
searchController?.abort();
|
||||
searchQuery.set('');
|
||||
searchResults.set([]);
|
||||
});
|
||||
|
||||
async function handleSearch() {
|
||||
if (!query.trim()) {
|
||||
if (isSearching) {
|
||||
notes.set(originalNotes);
|
||||
isSearching = false;
|
||||
}
|
||||
searchController?.abort();
|
||||
searchController = undefined;
|
||||
|
||||
const trimmedQuery = query.trim();
|
||||
searchQuery.set(trimmedQuery);
|
||||
|
||||
if (!trimmedQuery) {
|
||||
searchResults.set([]);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!isSearching) {
|
||||
notes.subscribe((n) => (originalNotes = n))();
|
||||
isSearching = true;
|
||||
}
|
||||
const controller = new AbortController();
|
||||
searchController = controller;
|
||||
|
||||
const res = await fetch(`/api/search?q=${encodeURIComponent(query)}`);
|
||||
if (res.ok) {
|
||||
const results = await res.json();
|
||||
notes.set(results);
|
||||
try {
|
||||
const res = await fetch(`/api/search?q=${encodeURIComponent(trimmedQuery)}`, {
|
||||
signal: controller.signal
|
||||
});
|
||||
if (res.ok) {
|
||||
const results: Note[] = await res.json();
|
||||
if (searchController !== controller || query.trim() !== trimmedQuery) return;
|
||||
searchResults.set(results);
|
||||
}
|
||||
} catch (error) {
|
||||
if (!(error instanceof DOMException && error.name === 'AbortError')) {
|
||||
console.error('Search failed:', error);
|
||||
}
|
||||
} finally {
|
||||
if (searchController === controller) searchController = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function clearSearch() {
|
||||
searchController?.abort();
|
||||
searchController = undefined;
|
||||
query = '';
|
||||
if (isSearching) {
|
||||
notes.set(originalNotes);
|
||||
isSearching = false;
|
||||
}
|
||||
searchQuery.set('');
|
||||
searchResults.set([]);
|
||||
}
|
||||
|
||||
function close() {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { mergeNotesByVersion } from './notes.js';
|
||||
import { mergeNotesByVersion, reconcileSearchResults } from './notes.js';
|
||||
import type { Note } from '$lib/types/index.js';
|
||||
|
||||
function makeNote(overrides: Partial<Note> = {}): Note {
|
||||
@@ -97,3 +97,17 @@ describe('mergeNotesByVersion', () => {
|
||||
expect(result[1].content).toBe('updated on server');
|
||||
});
|
||||
});
|
||||
|
||||
describe('reconcileSearchResults', () => {
|
||||
it('updates existing results and removes notes absent from canonical state', () => {
|
||||
const updated = makeNote({ id: 'n1', title: 'Updated', version: 2 });
|
||||
const removed = makeNote({ id: 'n2', title: 'Removed' });
|
||||
|
||||
const result = reconcileSearchResults([updated], [
|
||||
makeNote({ id: 'n1', title: 'Stale', version: 1 }),
|
||||
removed
|
||||
]);
|
||||
|
||||
expect(result).toEqual([updated]);
|
||||
});
|
||||
});
|
||||
|
||||
+24
-3
@@ -8,11 +8,29 @@ export const notes = writable<Note[]>([]);
|
||||
export const currentFilter = writable<NoteFilter>('all');
|
||||
export const selectedTag = writable<string | null>(null);
|
||||
export const notesLoaded = writable(false);
|
||||
export const searchQuery = writable<string>('');
|
||||
export const searchResults = writable<Note[]>([]);
|
||||
|
||||
export function reconcileSearchResults(canonicalNotes: Note[], results: Note[]): Note[] {
|
||||
const notesMap = new Map(canonicalNotes.map((note) => [note.id, note]));
|
||||
return results.flatMap((result) => {
|
||||
const updated = notesMap.get(result.id);
|
||||
return updated && !updated.trashed ? [updated] : [];
|
||||
});
|
||||
}
|
||||
|
||||
// Keep searchResults in sync with note updates/deletions in notes store
|
||||
notes.subscribe(($notes) => {
|
||||
searchResults.update(($results) => {
|
||||
if ($results.length === 0) return $results;
|
||||
return reconcileSearchResults($notes, $results);
|
||||
});
|
||||
});
|
||||
|
||||
export const filteredNotes = derived(
|
||||
[notes, selectedTag, currentFilter],
|
||||
([$notes, $selectedTag, $filter]) => {
|
||||
let result = $notes;
|
||||
[notes, selectedTag, currentFilter, searchQuery, searchResults],
|
||||
([$notes, $selectedTag, $filter, $searchQuery, $searchResults]) => {
|
||||
let result = $searchQuery.trim() ? $searchResults : $notes;
|
||||
if ($filter === 'all') {
|
||||
result = result.filter((n) => !n.trashed && !n.archived);
|
||||
} else if ($filter === 'archived') {
|
||||
@@ -218,6 +236,7 @@ export async function deleteNote(id: string): Promise<boolean> {
|
||||
const res = await fetch(`/api/notes/${id}`, { method: 'DELETE' });
|
||||
if (res.ok) {
|
||||
notes.update((list) => list.filter((n) => n.id !== id));
|
||||
searchResults.update((list) => list.filter((n) => n.id !== id));
|
||||
await deleteNoteFromIdb(id);
|
||||
return true;
|
||||
}
|
||||
@@ -225,6 +244,7 @@ export async function deleteNote(id: string): Promise<boolean> {
|
||||
} catch (err) {
|
||||
if (isNetworkError(err)) {
|
||||
notes.update((list) => list.filter((n) => n.id !== id));
|
||||
searchResults.update((list) => list.filter((n) => n.id !== id));
|
||||
await deleteNoteFromIdb(id);
|
||||
await addToSyncQueue({ noteId: id, operation: 'delete', timestamp: Date.now() });
|
||||
showToast('Saved offline — will sync when reconnected', 'info');
|
||||
@@ -263,6 +283,7 @@ export async function leaveNote(id: string): Promise<boolean> {
|
||||
const res = await fetch(`/api/notes/${id}/collaborators?userId=self`, { method: 'DELETE' });
|
||||
if (res.ok) {
|
||||
notes.update((list) => list.filter((n) => n.id !== id));
|
||||
searchResults.update((list) => list.filter((n) => n.id !== id));
|
||||
await deleteNoteFromIdb(id);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -31,4 +31,56 @@ test.describe('Search', () => {
|
||||
// Then no notes are displayed
|
||||
await expect(page.getByTestId('note-card')).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('Scenario: Background sync preserves active search results', async ({ authenticatedPage: page }) => {
|
||||
await createNote(page, 'Sync Search Target 3812925');
|
||||
await createNote(page, 'Sync Search Other 3812925');
|
||||
|
||||
await page.getByTestId('search-input').fill('Sync Search Target 3812925');
|
||||
await expect(page.getByText('Sync Search Target 3812925')).toBeVisible();
|
||||
await expect(page.getByText('Sync Search Other 3812925')).not.toBeVisible();
|
||||
|
||||
const notesReloaded = page.waitForResponse(
|
||||
(response) => response.url().includes('/api/notes?filter=all') && response.ok()
|
||||
);
|
||||
await page.getByTestId('sync-indicator').click();
|
||||
await notesReloaded;
|
||||
|
||||
await expect(page.getByText('Sync Search Target 3812925')).toBeVisible();
|
||||
await expect(page.getByText('Sync Search Other 3812925')).not.toBeVisible();
|
||||
});
|
||||
|
||||
test('Scenario: A stale response cannot replace newer search results', async ({ authenticatedPage: page }) => {
|
||||
await createNote(page, 'Race Slow 3812925');
|
||||
await createNote(page, 'Race Fast 3812925');
|
||||
|
||||
const slowResponse = await page.request.get('/api/search?q=Race%20Slow%203812925');
|
||||
const fastResponse = await page.request.get('/api/search?q=Race%20Fast%203812925');
|
||||
const slowResults = await slowResponse.json();
|
||||
const fastResults = await fastResponse.json();
|
||||
|
||||
await page.route('**/api/search?*', async (route) => {
|
||||
const requestQuery = new URL(route.request().url()).searchParams.get('q');
|
||||
if (requestQuery === 'Race Slow 3812925') {
|
||||
await new Promise((resolve) => setTimeout(resolve, 250));
|
||||
await route.fulfill({ json: slowResults });
|
||||
return;
|
||||
}
|
||||
await route.fulfill({ json: fastResults });
|
||||
});
|
||||
|
||||
const searchInput = page.getByTestId('search-input');
|
||||
const slowRequestStarted = page.waitForRequest(
|
||||
(request) => new URL(request.url()).searchParams.get('q') === 'Race Slow 3812925'
|
||||
);
|
||||
await searchInput.fill('Race Slow 3812925');
|
||||
await slowRequestStarted;
|
||||
await searchInput.fill('Race Fast 3812925');
|
||||
|
||||
await expect(page.getByText('Race Fast 3812925')).toBeVisible();
|
||||
await expect(page.getByText('Race Slow 3812925')).not.toBeVisible();
|
||||
await page.waitForTimeout(300);
|
||||
await expect(page.getByText('Race Fast 3812925')).toBeVisible();
|
||||
await expect(page.getByText('Race Slow 3812925')).not.toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user