From 2d30ed93ac3aeb26508ec37fd55d0b30301ef89e Mon Sep 17 00:00:00 2001 From: nsfisis Date: Thu, 5 Feb 2026 09:08:14 +0900 Subject: feat(study): add undo support for card reviews Defer API submission of reviews by storing them as pending. The previous pending review is flushed when the next card is rated, and on unmount via fire-and-forget. Undo discards the pending review and returns to the previous card without any API call. Co-Authored-By: Claude Opus 4.5 --- src/client/pages/StudyPage.test.tsx | 383 ++++++++++++++++++++++++++++++------ src/client/pages/StudyPage.tsx | 135 +++++++++++-- 2 files changed, 433 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/client/pages/StudyPage.test.tsx b/src/client/pages/StudyPage.test.tsx index 68a7bb3..6e605c9 100644 --- a/src/client/pages/StudyPage.test.tsx +++ b/src/client/pages/StudyPage.test.tsx @@ -92,25 +92,41 @@ const mockFirstCard: StudyCard = { fieldValuesMap: { Front: "Hello", Back: "こんにちは" }, }; -const mockDueCards: StudyCard[] = [ - mockFirstCard, - { - id: "card-2", - deckId: "deck-1", - noteId: "note-2", - isReversed: false, - front: "Goodbye", - back: "さようなら", - state: 0, - due: "2024-01-01T00:00:00Z", - stability: 0, - difficulty: 0, - reps: 0, - lapses: 0, - noteType: { frontTemplate: "{{Front}}", backTemplate: "{{Back}}" }, - fieldValuesMap: { Front: "Goodbye", Back: "さようなら" }, - }, -]; +const mockSecondCard: StudyCard = { + id: "card-2", + deckId: "deck-1", + noteId: "note-2", + isReversed: false, + front: "Goodbye", + back: "さようなら", + state: 0, + due: "2024-01-01T00:00:00Z", + stability: 0, + difficulty: 0, + reps: 0, + lapses: 0, + noteType: { frontTemplate: "{{Front}}", backTemplate: "{{Back}}" }, + fieldValuesMap: { Front: "Goodbye", Back: "さようなら" }, +}; + +const mockThirdCard: StudyCard = { + id: "card-3", + deckId: "deck-1", + noteId: "note-3", + isReversed: false, + front: "Thank you", + back: "ありがとう", + state: 0, + due: "2024-01-01T00:00:00Z", + stability: 0, + difficulty: 0, + reps: 0, + lapses: 0, + noteType: { frontTemplate: "{{Front}}", backTemplate: "{{Back}}" }, + fieldValuesMap: { Front: "Thank you", Back: "ありがとう" }, +}; + +const mockDueCards: StudyCard[] = [mockFirstCard, mockSecondCard]; function renderWithProviders({ path = "/decks/deck-1/study", @@ -158,6 +174,9 @@ describe("StudyPage", () => { Authorization: "Bearer access-token", }); + // Default: studyPost returns a resolved promise (needed for unmount flush) + mockStudyPost.mockResolvedValue({}); + // handleResponse: just pass through whatever it receives mockHandleResponse.mockImplementation((res) => Promise.resolve(res)); }); @@ -313,13 +332,9 @@ describe("StudyPage", () => { }); describe("Rating Submission", () => { - it("submits review and moves to next card", async () => { + it("moves to next card after rating (API deferred)", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: mockDueCards }, }); @@ -330,12 +345,49 @@ describe("StudyPage", () => { // Rate as Good await user.click(screen.getByTestId("rating-3")); - // Should move to next card + // Should move to next card without API call await waitFor(() => { expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); }); - // Verify API was called with correct params + // API should NOT have been called yet (deferred) + expect(mockStudyPost).not.toHaveBeenCalled(); + }); + + it("flushes previous pending review when rating next card", async () => { + const user = userEvent.setup(); + + mockStudyPost.mockResolvedValue({ + card: { ...mockFirstCard, reps: 1 }, + }); + + renderWithProviders({ + initialStudyData: { + deck: mockDeck, + cards: [mockFirstCard, mockSecondCard, mockThirdCard], + }, + }); + + // Rate first card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + // API not called yet + expect(mockStudyPost).not.toHaveBeenCalled(); + + // Rate second card — should flush the first + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-4")); + + await waitFor(() => { + expect(mockStudyPost).toHaveBeenCalledTimes(1); + }); + + // Verify the flushed review was for card-1 expect(mockStudyPost).toHaveBeenCalledWith( expect.objectContaining({ param: { deckId: "deck-1", cardId: "card-1" }, @@ -347,10 +399,6 @@ describe("StudyPage", () => { it("updates remaining count after review", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: mockDueCards }, }); @@ -369,7 +417,7 @@ describe("StudyPage", () => { }); }); - it("shows error when rating submission fails", async () => { + it("shows error when flush of previous review fails", async () => { const user = userEvent.setup(); mockStudyPost.mockRejectedValue( @@ -377,17 +425,32 @@ describe("StudyPage", () => { ); renderWithProviders({ - initialStudyData: { deck: mockDeck, cards: mockDueCards }, + initialStudyData: { + deck: mockDeck, + cards: [mockFirstCard, mockSecondCard, mockThirdCard], + }, }); + // Rate first card await user.click(screen.getByTestId("card-container")); await user.click(screen.getByTestId("rating-3")); + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + // Rate second card — flush fails + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-4")); + await waitFor(() => { expect(screen.getByRole("alert").textContent).toContain( "Failed to submit review", ); }); + + // Should still move to the third card + expect(screen.getByTestId("card-front").textContent).toBe("Thank you"); }); }); @@ -395,10 +458,6 @@ describe("StudyPage", () => { it("shows session complete screen after all cards reviewed", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: [mockFirstCard] }, }); @@ -430,7 +489,7 @@ describe("StudyPage", () => { await user.click(screen.getByTestId("card-container")); await user.click(screen.getByTestId("rating-3")); - // Review second card + // Review second card (flushes first) await waitFor(() => { expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); }); @@ -447,10 +506,6 @@ describe("StudyPage", () => { it("provides navigation links after session complete", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: [mockFirstCard] }, }); @@ -495,10 +550,6 @@ describe("StudyPage", () => { it("rates card with number keys", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: mockDueCards }, }); @@ -510,21 +561,13 @@ describe("StudyPage", () => { expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); }); - expect(mockStudyPost).toHaveBeenCalledWith( - expect.objectContaining({ - param: { deckId: "deck-1", cardId: "card-1" }, - json: expect.objectContaining({ rating: 3 }), - }), - ); + // API not called yet (deferred) + expect(mockStudyPost).not.toHaveBeenCalled(); }); it("rates card as Good with Space key when card is flipped", async () => { const user = userEvent.setup(); - mockStudyPost.mockResolvedValue({ - card: { ...mockFirstCard, reps: 1 }, - }); - renderWithProviders({ initialStudyData: { deck: mockDeck, cards: mockDueCards }, }); @@ -536,32 +579,244 @@ describe("StudyPage", () => { expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); }); - expect(mockStudyPost).toHaveBeenCalledWith( - expect.objectContaining({ - param: { deckId: "deck-1", cardId: "card-1" }, - json: expect.objectContaining({ rating: 3 }), - }), - ); + // API not called yet (deferred) + expect(mockStudyPost).not.toHaveBeenCalled(); }); it("supports all rating keys (1, 2, 3, 4)", async () => { const user = userEvent.setup(); + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + + await user.keyboard(" "); // Flip + await user.keyboard("1"); // Rate as Again + + // API not called yet (deferred), but should move to next card + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + expect(mockStudyPost).not.toHaveBeenCalled(); + }); + }); + + describe("Undo", () => { + it("does not show undo button before any rating", () => { + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + + expect(screen.queryByTestId("undo-button")).toBeNull(); + }); + + it("shows undo button after rating (when not flipped)", async () => { + const user = userEvent.setup(); + + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + expect(screen.getByTestId("undo-button")).toBeDefined(); + }); + + it("undoes the last rating and returns to previous card", async () => { + const user = userEvent.setup(); + + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + + // Rate first card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + // Click undo + await user.click(screen.getByTestId("undo-button")); + + // Should return to the first card + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Hello"); + }); + + // API should NOT have been called + expect(mockStudyPost).not.toHaveBeenCalled(); + + // Undo button should be gone + expect(screen.queryByTestId("undo-button")).toBeNull(); + }); + + it("decrements completed count on undo", async () => { + const user = userEvent.setup(); + mockStudyPost.mockResolvedValue({ card: { ...mockFirstCard, reps: 1 }, }); + renderWithProviders({ + initialStudyData: { + deck: mockDeck, + cards: [mockFirstCard, mockSecondCard, mockThirdCard], + }, + }); + + // Rate first card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + // Rate second card (flushes first) + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-4")); + + await waitFor(() => { + expect(screen.getByTestId("remaining-count").textContent).toBe( + "1 remaining", + ); + }); + + // Undo + await user.click(screen.getByTestId("undo-button")); + + await waitFor(() => { + expect(screen.getByTestId("remaining-count").textContent).toBe( + "2 remaining", + ); + }); + }); + + it("undoes with z key when card is not flipped", async () => { + const user = userEvent.setup(); + renderWithProviders({ initialStudyData: { deck: mockDeck, cards: mockDueCards }, }); - await user.keyboard(" "); // Flip - await user.keyboard("1"); // Rate as Again + // Rate first card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + // Press z to undo + await user.keyboard("z"); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Hello"); + }); + }); + + it("undoes with Ctrl+Z", async () => { + const user = userEvent.setup(); + + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + // Rate first card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + // Press Ctrl+Z to undo + await user.keyboard("{Control>}z{/Control}"); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Hello"); + }); + }); + + it("shows undo button on session complete screen", async () => { + const user = userEvent.setup(); + + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: [mockFirstCard] }, + }); + + // Review the only card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("session-complete")).toBeDefined(); + }); + + // Undo button should be visible on complete screen + expect(screen.getByTestId("undo-button")).toBeDefined(); + }); + + it("undoes from session complete screen back to last card", async () => { + const user = userEvent.setup(); + + renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: [mockFirstCard] }, + }); + + // Review the only card + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("session-complete")).toBeDefined(); + }); + + // Click undo on session complete screen + await user.click(screen.getByTestId("undo-button")); + + // Should go back to the card + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Hello"); + }); + expect(screen.queryByTestId("session-complete")).toBeNull(); + }); + + it("flushes pending review on unmount", async () => { + const user = userEvent.setup(); + + mockStudyPost.mockResolvedValue({ + card: { ...mockFirstCard, reps: 1 }, + }); + + const { unmount } = renderWithProviders({ + initialStudyData: { deck: mockDeck, cards: mockDueCards }, + }); + + // Rate first card (pending, not sent) + await user.click(screen.getByTestId("card-container")); + await user.click(screen.getByTestId("rating-3")); + + await waitFor(() => { + expect(screen.getByTestId("card-front").textContent).toBe("Goodbye"); + }); + + expect(mockStudyPost).not.toHaveBeenCalled(); + + // Unmount triggers flush + unmount(); + + expect(mockStudyPost).toHaveBeenCalledTimes(1); expect(mockStudyPost).toHaveBeenCalledWith( expect.objectContaining({ param: { deckId: "deck-1", cardId: "card-1" }, - json: expect.objectContaining({ rating: 1 }), + json: expect.objectContaining({ rating: 3 }), }), ); }); diff --git a/src/client/pages/StudyPage.tsx b/src/client/pages/StudyPage.tsx index 423fdd0..4fb7549 100644 --- a/src/client/pages/StudyPage.tsx +++ b/src/client/pages/StudyPage.tsx @@ -2,6 +2,7 @@ import { faCheck, faChevronLeft, faCircleCheck, + faRotateLeft, } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { useAtomValue } from "jotai"; @@ -20,6 +21,7 @@ import { ErrorBoundary } from "../components/ErrorBoundary"; import { renderCard } from "../utils/templateRenderer"; type Rating = 1 | 2 | 3 | 4; +type PendingReview = { cardId: string; rating: Rating; durationMs: number }; const RatingLabels: Record = { 1: "Again", @@ -47,6 +49,15 @@ function StudySession({ deckId }: { deckId: string }) { const [submitError, setSubmitError] = useState(null); const [completedCount, setCompletedCount] = useState(0); const cardStartTimeRef = useRef(Date.now()); + const [pendingReview, setPendingReview] = useState( + null, + ); + const pendingReviewRef = useRef(null); + + // Keep ref in sync with state for cleanup effect + useEffect(() => { + pendingReviewRef.current = pendingReview; + }, [pendingReview]); // biome-ignore lint/correctness/useExhaustiveDependencies: Reset timer when card changes useEffect(() => { @@ -57,6 +68,19 @@ function StudySession({ deckId }: { deckId: string }) { setIsFlipped(true); }, []); + const flushPendingReview = useCallback( + async (review: PendingReview) => { + const res = await apiClient.rpc.api.decks[":deckId"].study[ + ":cardId" + ].$post({ + param: { deckId, cardId: review.cardId }, + json: { rating: review.rating, durationMs: review.durationMs }, + }); + await apiClient.handleResponse(res); + }, + [deckId], + ); + const handleRating = useCallback( async (rating: Rating) => { if (isSubmitting) return; @@ -69,35 +93,67 @@ function StudySession({ deckId }: { deckId: string }) { const durationMs = Date.now() - cardStartTimeRef.current; - try { - const res = await apiClient.rpc.api.decks[":deckId"].study[ - ":cardId" - ].$post({ - param: { deckId, cardId: currentCard.id }, - json: { rating, durationMs }, - }); - await apiClient.handleResponse(res); - - setCompletedCount((prev) => prev + 1); - setIsFlipped(false); - setCurrentIndex((prev) => prev + 1); - } catch (err) { - if (err instanceof ApiClientError) { - setSubmitError(err.message); - } else { - setSubmitError("Failed to submit review. Please try again."); + // Flush previous pending review first + if (pendingReview) { + try { + await flushPendingReview(pendingReview); + } catch (err) { + if (err instanceof ApiClientError) { + setSubmitError(err.message); + } else { + setSubmitError("Failed to submit review. Please try again."); + } } - } finally { - setIsSubmitting(false); } + + // Save current review as pending (don't send yet) + setPendingReview({ cardId: currentCard.id, rating, durationMs }); + setCompletedCount((prev) => prev + 1); + setIsFlipped(false); + setCurrentIndex((prev) => prev + 1); + setIsSubmitting(false); }, - [deckId, isSubmitting, cards, currentIndex], + [isSubmitting, cards, currentIndex, pendingReview, flushPendingReview], ); + const handleUndo = useCallback(() => { + if (!pendingReview) return; + setPendingReview(null); + setCurrentIndex((prev) => prev - 1); + setCompletedCount((prev) => prev - 1); + setIsFlipped(false); + }, [pendingReview]); + + // Flush pending review on unmount (fire-and-forget) + useEffect(() => { + return () => { + const review = pendingReviewRef.current; + if (review) { + apiClient.rpc.api.decks[":deckId"].study[":cardId"] + .$post({ + param: { deckId, cardId: review.cardId }, + json: { rating: review.rating, durationMs: review.durationMs }, + }) + .then((res) => apiClient.handleResponse(res)) + .catch(() => {}); + } + }; + }, [deckId]); + const handleKeyDown = useCallback( (e: KeyboardEvent) => { if (isSubmitting) return; + // Undo: Ctrl+Z / Cmd+Z anytime, or z when card is not flipped + if ( + (e.key === "z" && (e.ctrlKey || e.metaKey)) || + (e.key === "z" && !e.ctrlKey && !e.metaKey && !isFlipped) + ) { + e.preventDefault(); + handleUndo(); + return; + } + if (!isFlipped) { if (e.key === " " || e.key === "Enter") { e.preventDefault(); @@ -119,7 +175,7 @@ function StudySession({ deckId }: { deckId: string }) { } } }, - [isFlipped, isSubmitting, handleFlip, handleRating], + [isFlipped, isSubmitting, handleFlip, handleRating, handleUndo], ); useEffect(() => { @@ -185,6 +241,28 @@ function StudySession({ deckId }: { deckId: string }) { )} + {/* Undo Button */} + {pendingReview && !isFlipped && !isSessionComplete && ( +
+ +
+ )} + {/* No Cards State */} {hasNoCards && (
+ {pendingReview && ( + + )}