mirror of
https://github.com/zadam/trilium.git
synced 2026-05-07 12:47:10 +02:00
Editing quirks (#9362)
This commit is contained in:
87
apps/client/src/services/spaced_update.spec.ts
Normal file
87
apps/client/src/services/spaced_update.spec.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import SpacedUpdate from "./spaced_update";
|
||||
|
||||
// Mock logError which is a global in Trilium
|
||||
vi.stubGlobal("logError", vi.fn());
|
||||
|
||||
describe("SpacedUpdate", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("should only call updater once per interval even with multiple pending callbacks", async () => {
|
||||
const updater = vi.fn(async () => {
|
||||
// Simulate a slow network request - this is where the race condition occurs
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
});
|
||||
|
||||
const spacedUpdate = new SpacedUpdate(updater, 50);
|
||||
|
||||
// Simulate rapid typing - each keystroke calls scheduleUpdate()
|
||||
// This queues multiple setTimeout callbacks due to recursive scheduleUpdate() calls
|
||||
for (let i = 0; i < 10; i++) {
|
||||
spacedUpdate.scheduleUpdate();
|
||||
// Small delay between keystrokes
|
||||
await vi.advanceTimersByTimeAsync(5);
|
||||
}
|
||||
|
||||
// Advance time past the update interval to trigger the update
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
// Let the "network request" complete and any pending callbacks run
|
||||
await vi.advanceTimersByTimeAsync(200);
|
||||
|
||||
// The updater should have been called only ONCE, not multiple times
|
||||
// With the bug, multiple pending setTimeout callbacks would all pass the time check
|
||||
// during the async updater call and trigger multiple concurrent requests
|
||||
expect(updater).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should call updater again if changes occur during the update", async () => {
|
||||
const updater = vi.fn(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
});
|
||||
|
||||
const spacedUpdate = new SpacedUpdate(updater, 30);
|
||||
|
||||
// First update
|
||||
spacedUpdate.scheduleUpdate();
|
||||
await vi.advanceTimersByTimeAsync(40);
|
||||
|
||||
// Schedule another update while the first one is in progress
|
||||
spacedUpdate.scheduleUpdate();
|
||||
|
||||
// Let first update complete
|
||||
await vi.advanceTimersByTimeAsync(60);
|
||||
|
||||
// Advance past the interval again for the second update
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
// Should have been called twice - once for each distinct change period
|
||||
expect(updater).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("should restore changed flag on error so retry can happen", async () => {
|
||||
const updater = vi.fn()
|
||||
.mockRejectedValueOnce(new Error("Network error"))
|
||||
.mockResolvedValue(undefined);
|
||||
|
||||
const spacedUpdate = new SpacedUpdate(updater, 50);
|
||||
|
||||
spacedUpdate.scheduleUpdate();
|
||||
|
||||
// Advance to trigger first update (which will fail)
|
||||
await vi.advanceTimersByTimeAsync(60);
|
||||
|
||||
// The error should have restored the changed flag, so scheduling again should work
|
||||
spacedUpdate.scheduleUpdate();
|
||||
await vi.advanceTimersByTimeAsync(60);
|
||||
|
||||
expect(updater).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
@@ -77,16 +77,22 @@ export default class SpacedUpdate {
|
||||
}
|
||||
|
||||
if (Date.now() - this.lastUpdated > this.updateInterval) {
|
||||
// Update these BEFORE the async call to prevent race conditions.
|
||||
// Multiple setTimeout callbacks may be pending from recursive scheduleUpdate() calls.
|
||||
// Without this, they would all pass the time check during the await and trigger multiple requests.
|
||||
this.lastUpdated = Date.now();
|
||||
this.changed = false;
|
||||
|
||||
this.onStateChanged("saving");
|
||||
try {
|
||||
await this.updater();
|
||||
this.onStateChanged("saved");
|
||||
this.changed = false;
|
||||
} catch (e) {
|
||||
// Restore changed flag on error so a retry can happen
|
||||
this.changed = true;
|
||||
this.onStateChanged("error");
|
||||
logError(getErrorMessage(e));
|
||||
}
|
||||
this.lastUpdated = Date.now();
|
||||
} else {
|
||||
// update isn't triggered but changes are still pending, so we need to schedule another check
|
||||
this.scheduleUpdate();
|
||||
|
||||
@@ -144,7 +144,12 @@ export default function CalendarView({ note, noteIds }: ViewModeProps<CalendarVi
|
||||
const event = api.getEventById(noteId);
|
||||
const note = froca.getNoteFromCache(noteId);
|
||||
if (!event || !note) continue;
|
||||
event.setProp("title", note.title);
|
||||
// Only update the title if it has actually changed.
|
||||
// setProp() triggers FullCalendar's eventChange callback, which would
|
||||
// re-save the event's dates and cause unwanted side effects.
|
||||
if (event.title !== note.title) {
|
||||
event.setProp("title", note.title);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@@ -299,6 +304,12 @@ function useEditing(note: FNote, isEditable: boolean, isCalendarRoot: boolean, c
|
||||
}, [ note, componentId ]);
|
||||
|
||||
const onEventChange = useCallback(async (e: EventChangeArg) => {
|
||||
// Only process actual date/time changes, not other property changes (e.g., title via setProp).
|
||||
const datesChanged = e.oldEvent.start?.getTime() !== e.event.start?.getTime()
|
||||
|| e.oldEvent.end?.getTime() !== e.event.end?.getTime()
|
||||
|| e.oldEvent.allDay !== e.event.allDay;
|
||||
if (!datesChanged) return;
|
||||
|
||||
const { startDate, endDate } = parseStartEndDateFromEvent(e.event);
|
||||
if (!startDate) return;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user