From 57daa96be640b1511656043df485779db6cfc5c1 Mon Sep 17 00:00:00 2001 From: Christopher Sanden Date: Thu, 19 Mar 2026 16:15:37 +0100 Subject: [PATCH] hardened camera mounting handling --- FastNotes/__tests__/detail-screen.test.tsx | 194 +++++++++++++++++- FastNotes/__tests__/new-note.test.tsx | 162 ++++++++++++++- FastNotes/app/detail.tsx | 26 ++- FastNotes/app/newNote.tsx | 26 ++- .../src/notes/use-picker-lifecycle-guard.ts | 38 ++++ 5 files changed, 434 insertions(+), 12 deletions(-) create mode 100644 FastNotes/src/notes/use-picker-lifecycle-guard.ts diff --git a/FastNotes/__tests__/detail-screen.test.tsx b/FastNotes/__tests__/detail-screen.test.tsx index be06b1c..3a39fcf 100644 --- a/FastNotes/__tests__/detail-screen.test.tsx +++ b/FastNotes/__tests__/detail-screen.test.tsx @@ -1,12 +1,14 @@ -import { act, render, screen, waitFor } from "@testing-library/react-native" +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react-native" import React, { PropsWithChildren } from "react" import DetailScreen from "@/app/detail" import { AuthContext, AuthData } from "@/hooks/use-auth-context" import { supabase } from "@/libs/supabase" +import { pickImageFromCamera, pickImageFromLibrary } from "@/src/notes/native-image-picker" import { NotesProvider } from "@/src/notes/NotesContext" import { AppThemeProvider } from "@/src/theme/AppThemeProvider" import { useLocalSearchParams } from "expo-router" +import { useIsFocused } from "@react-navigation/native" type Deferred = { promise: Promise @@ -43,9 +45,42 @@ jest.mock("@/libs/supabase", () => ({ supabaseAnonKey: "test-anon-key", })) +jest.mock("@react-navigation/native", () => { + const actual = jest.requireActual("@react-navigation/native") + + return { + ...actual, + useIsFocused: jest.fn(() => true), + } +}) + +jest.mock("@/src/notes/native-image-picker", () => ({ + pickImageFromCamera: jest.fn(), + pickImageFromLibrary: jest.fn(), +})) + jest.mock("@/components/note-image-panel", () => ({ __esModule: true, - default: () => null, + default: (props: { + onChooseFromLibrary?: () => void + onTakePhoto?: () => void + stagedImage?: { fileName?: string | null } | null + }) => { + const React = require("react") + const { Pressable, Text, View } = require("react-native") + + return ( + + + Take photo + + + Choose from gallery + + {props.stagedImage?.fileName ? {props.stagedImage.fileName} : null} + + ) + }, })) describe("DetailScreen", () => { @@ -53,6 +88,9 @@ describe("DetailScreen", () => { const mockSupabase = supabase as unknown as { from: jest.Mock } + const mockPickImageFromCamera = pickImageFromCamera as jest.MockedFunction + const mockPickImageFromLibrary = pickImageFromLibrary as jest.MockedFunction + const mockUseIsFocused = useIsFocused as jest.MockedFunction function TestWrapper({ children }: PropsWithChildren) { const authValue: AuthData = { @@ -81,8 +119,80 @@ describe("DetailScreen", () => { beforeEach(() => { mockUseLocalSearchParams.mockReturnValue({ id: "42" }) + mockPickImageFromCamera.mockResolvedValue(null) + mockPickImageFromLibrary.mockResolvedValue(null) + mockUseIsFocused.mockReturnValue(true) }) + async function renderLoadedDetailScreen() { + const notesQuery = { + order: jest.fn(), + eq: jest.fn(), + maybeSingle: jest.fn(() => + Promise.resolve({ + data: { + id: 42, + created_by: "user-1", + title: "Fetched note", + content: "Loaded from Supabase for the integration test", + created_at: "2026-03-18T10:00:00.000Z", + updated_at: "2026-03-18T10:05:00.000Z", + image_url: null, + image_path: null, + image_mime_type: null, + image_size_bytes: null, + }, + error: null, + }) + ), + } + + notesQuery.order + .mockImplementationOnce(() => notesQuery) + .mockImplementationOnce(() => Promise.resolve({ data: [], error: null })) + notesQuery.eq.mockReturnValue(notesQuery) + + mockSupabase.from.mockImplementation((table: string) => { + if (table === "Notes") { + return { + select: jest.fn(() => notesQuery), + } + } + + if (table === "profiles") { + return { + select: jest.fn(() => ({ + in: jest.fn(() => + Promise.resolve({ + data: [ + { + id: "user-1", + email: "user-1@example.com", + username: null, + full_name: "Exam User", + }, + ], + error: null, + }) + ), + })), + } + } + + throw new Error(`Unexpected table requested in test: ${table}`) + }) + + const rendered = render(, { + wrapper: TestWrapper, + }) + + await waitFor(() => { + expect(screen.getByDisplayValue("Fetched note")).toBeTruthy() + }) + + return rendered + } + it("shows a loader while fetching a note, then renders the loaded content", async () => { const deferredNote = createDeferred<{ data: { @@ -175,4 +285,84 @@ describe("DetailScreen", () => { expect(screen.getByDisplayValue("Loaded from Supabase for the integration test")).toBeTruthy() }) }) + + it("only launches the camera once while a picker request is active", async () => { + const deferredCamera = createDeferred>>() + mockPickImageFromCamera.mockReturnValue(deferredCamera.promise) + + await renderLoadedDetailScreen() + + fireEvent.press(screen.getByText("Take photo")) + fireEvent.press(screen.getByText("Take photo")) + + expect(mockPickImageFromCamera).toHaveBeenCalledTimes(1) + + await act(async () => { + deferredCamera.resolve({ + fileName: "detail-camera.jpg", + fileSize: 1024, + height: 200, + mimeType: "image/jpeg", + uri: "file:///detail-camera.jpg", + width: 100, + }) + + await deferredCamera.promise + }) + + expect(screen.getByText("detail-camera.jpg")).toBeTruthy() + }) + + it("keeps the detail screen unchanged when the gallery picker is canceled", async () => { + mockPickImageFromLibrary.mockResolvedValue(null) + + await renderLoadedDetailScreen() + + fireEvent.press(screen.getByText("Choose from gallery")) + + await waitFor(() => { + expect(mockPickImageFromLibrary).toHaveBeenCalledTimes(1) + }) + + expect(screen.queryByText(/detail-camera\.jpg$/)).toBeNull() + }) + + it("shows the camera error when the picker fails on the detail screen", async () => { + mockPickImageFromCamera.mockRejectedValue(new Error("Camera access is required to take a photo.")) + + await renderLoadedDetailScreen() + + fireEvent.press(screen.getByText("Take photo")) + + await waitFor(() => { + expect(screen.getByText("Camera access is required to take a photo.")).toBeTruthy() + }) + }) + + it("ignores a late camera result after leaving the detail screen", async () => { + const deferredCamera = createDeferred>>() + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}) + mockPickImageFromCamera.mockReturnValue(deferredCamera.promise) + + const rendered = await renderLoadedDetailScreen() + + fireEvent.press(screen.getByText("Take photo")) + rendered.unmount() + + await act(async () => { + deferredCamera.resolve({ + fileName: "late-detail-camera.jpg", + fileSize: 1024, + height: 200, + mimeType: "image/jpeg", + uri: "file:///late-detail-camera.jpg", + width: 100, + }) + + await deferredCamera.promise + }) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + consoleErrorSpy.mockRestore() + }) }) diff --git a/FastNotes/__tests__/new-note.test.tsx b/FastNotes/__tests__/new-note.test.tsx index 058482f..4c964da 100644 --- a/FastNotes/__tests__/new-note.test.tsx +++ b/FastNotes/__tests__/new-note.test.tsx @@ -1,10 +1,30 @@ -import { fireEvent, screen, waitFor } from "@testing-library/react-native" +import { act, fireEvent, screen, waitFor } from "@testing-library/react-native" import React from "react" import NewNoteScreen from "@/app/newNote" import { useNotes } from "@/src/notes/NotesContext" +import { pickImageFromCamera, pickImageFromLibrary } from "@/src/notes/native-image-picker" import { renderWithTheme } from "@/test-utils/renderWithTheme" import { router } from "expo-router" +import { useIsFocused } from "@react-navigation/native" + +type Deferred = { + promise: Promise + resolve: (value: T) => void + reject: (reason?: unknown) => void +} + +function createDeferred(): Deferred { + let resolve!: (value: T) => void + let reject!: (reason?: unknown) => void + + const promise = new Promise((innerResolve, innerReject) => { + resolve = innerResolve + reject = innerReject + }) + + return { promise, resolve, reject } +} const mockAddNote = jest.fn() @@ -20,9 +40,42 @@ jest.mock("@/src/notes/NotesContext", () => ({ useNotes: jest.fn(), })) +jest.mock("@react-navigation/native", () => { + const actual = jest.requireActual("@react-navigation/native") + + return { + ...actual, + useIsFocused: jest.fn(() => true), + } +}) + +jest.mock("@/src/notes/native-image-picker", () => ({ + pickImageFromCamera: jest.fn(), + pickImageFromLibrary: jest.fn(), +})) + jest.mock("@/components/note-image-panel", () => ({ __esModule: true, - default: () => null, + default: (props: { + onChooseFromLibrary?: () => void + onTakePhoto?: () => void + stagedImage?: { fileName?: string | null } | null + }) => { + const React = require("react") + const { Pressable, Text, View } = require("react-native") + + return ( + + + Take photo + + + Choose from gallery + + {props.stagedImage?.fileName ? {props.stagedImage.fileName} : null} + + ) + }, })) describe("NewNoteScreen", () => { @@ -32,9 +85,15 @@ describe("NewNoteScreen", () => { back: jest.Mock replace: jest.Mock } + const mockPickImageFromCamera = pickImageFromCamera as jest.MockedFunction + const mockPickImageFromLibrary = pickImageFromLibrary as jest.MockedFunction + const mockUseIsFocused = useIsFocused as jest.MockedFunction beforeEach(() => { mockAddNote.mockResolvedValue(true) + mockPickImageFromCamera.mockResolvedValue(null) + mockPickImageFromLibrary.mockResolvedValue(null) + mockUseIsFocused.mockReturnValue(true) mockUseNotes.mockReturnValue({ notes: [], @@ -74,4 +133,103 @@ describe("NewNoteScreen", () => { expect(mockRouter.replace).not.toHaveBeenCalled() }) + + it("only launches the camera once while a picker request is active", async () => { + const deferredCamera = createDeferred>>() + mockPickImageFromCamera.mockReturnValue(deferredCamera.promise) + + renderWithTheme() + + fireEvent.press(screen.getByText("Take photo")) + fireEvent.press(screen.getByText("Take photo")) + + expect(mockPickImageFromCamera).toHaveBeenCalledTimes(1) + + await act(async () => { + deferredCamera.resolve({ + fileName: "captured-note.jpg", + fileSize: 1024, + height: 200, + mimeType: "image/jpeg", + uri: "file:///captured-note.jpg", + width: 100, + }) + + await deferredCamera.promise + }) + + expect(screen.getByText("captured-note.jpg")).toBeTruthy() + }) + + it("applies the selected camera image while the screen is still active", async () => { + mockPickImageFromCamera.mockResolvedValue({ + fileName: "camera-note.jpg", + fileSize: 512, + height: 200, + mimeType: "image/jpeg", + uri: "file:///camera-note.jpg", + width: 100, + }) + + renderWithTheme() + + fireEvent.press(screen.getByText("Take photo")) + + await waitFor(() => { + expect(screen.getByText("camera-note.jpg")).toBeTruthy() + }) + }) + + it("keeps the staged image unchanged when the gallery picker is canceled", async () => { + mockPickImageFromLibrary.mockResolvedValue(null) + + renderWithTheme() + + fireEvent.press(screen.getByText("Choose from gallery")) + + await waitFor(() => { + expect(mockPickImageFromLibrary).toHaveBeenCalledTimes(1) + }) + + expect(screen.queryByText(/\.jpg$/)).toBeNull() + }) + + it("shows the camera error when the native picker fails", async () => { + mockPickImageFromCamera.mockRejectedValue(new Error("Camera access is required to take a photo.")) + + renderWithTheme() + + fireEvent.press(screen.getByText("Take photo")) + + await waitFor(() => { + expect(screen.getByText("Camera access is required to take a photo.")).toBeTruthy() + }) + }) + + it("ignores a late camera result after the screen unmounts", async () => { + const deferredCamera = createDeferred>>() + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}) + mockPickImageFromCamera.mockReturnValue(deferredCamera.promise) + + const screenRender = renderWithTheme() + + fireEvent.press(screen.getByText("Take photo")) + screenRender.unmount() + + await act(async () => { + deferredCamera.resolve({ + fileName: "late-camera.jpg", + fileSize: 1024, + height: 200, + mimeType: "image/jpeg", + uri: "file:///late-camera.jpg", + width: 100, + }) + + await deferredCamera.promise + }) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + consoleErrorSpy.mockRestore() + }) }) diff --git a/FastNotes/app/detail.tsx b/FastNotes/app/detail.tsx index d092552..2fc5716 100644 --- a/FastNotes/app/detail.tsx +++ b/FastNotes/app/detail.tsx @@ -21,6 +21,7 @@ import { useAuthContext } from "@/hooks/use-auth-context" import { NoteImageChange, useNotes } from "@/src/notes/NotesContext" import { StagedNoteImage, validateStagedNoteImage } from "@/src/notes/image-utils" import { pickImageFromCamera, pickImageFromLibrary } from "@/src/notes/native-image-picker" +import { usePickerLifecycleGuard } from "@/src/notes/use-picker-lifecycle-guard" import { detailScreenStyles as styles } from "@/src/styles/app-styles" import { useAppTheme } from "@/src/theme/AppThemeProvider" @@ -45,6 +46,7 @@ export default function DetailScreen() { const insets = useSafeAreaInsets() const headerHeight = useHeaderHeight() const { colorScheme, palette } = useAppTheme() + const { endPicker, isScreenActive, tryBeginPicker } = usePickerLifecycleGuard() const formatTimestamp = (value: string) => { const parsed = new Date(value) @@ -85,10 +87,14 @@ export default function DetailScreen() { }, [fetchNoteById, id, note]) const attachFromCamera = async () => { + if (!tryBeginPicker()) { + return + } + try { const image = await pickImageFromCamera() - if (image) { + if (image && isScreenActive()) { validateStagedNoteImage(image) setStagedImage(image) setImageChange({ type: "replace", image }) @@ -96,15 +102,23 @@ export default function DetailScreen() { setStatusMessage(null) } } catch (error) { - setLocalErrorMessage(error instanceof Error ? error.message : "The camera could not be opened.") + if (isScreenActive()) { + setLocalErrorMessage(error instanceof Error ? error.message : "The camera could not be opened.") + } + } finally { + endPicker() } } const attachFromGallery = async () => { + if (!tryBeginPicker()) { + return + } + try { const image = await pickImageFromLibrary() - if (image) { + if (image && isScreenActive()) { validateStagedNoteImage(image) setStagedImage(image) setImageChange({ type: "replace", image }) @@ -112,7 +126,11 @@ export default function DetailScreen() { setStatusMessage(null) } } catch (error) { - setLocalErrorMessage(error instanceof Error ? error.message : "The gallery could not be opened.") + if (isScreenActive()) { + setLocalErrorMessage(error instanceof Error ? error.message : "The gallery could not be opened.") + } + } finally { + endPicker() } } diff --git a/FastNotes/app/newNote.tsx b/FastNotes/app/newNote.tsx index 362d27d..4105af7 100644 --- a/FastNotes/app/newNote.tsx +++ b/FastNotes/app/newNote.tsx @@ -18,6 +18,7 @@ import UploadProgressBar from "@/components/upload-progress-bar" import { useNotes } from "@/src/notes/NotesContext" import { StagedNoteImage, validateStagedNoteImage } from "@/src/notes/image-utils" import { pickImageFromCamera, pickImageFromLibrary } from "@/src/notes/native-image-picker" +import { usePickerLifecycleGuard } from "@/src/notes/use-picker-lifecycle-guard" import { newNoteScreenStyles as styles } from "@/src/styles/app-styles" import { useAppTheme } from "@/src/theme/AppThemeProvider" @@ -34,32 +35,49 @@ export default function NewNoteScreen() { const { colorScheme, palette } = useAppTheme() const [contentHeight, setContentHeight] = useState(160) const scrollRef = useRef(null) + const { endPicker, isScreenActive, tryBeginPicker } = usePickerLifecycleGuard() const attachFromCamera = async () => { + if (!tryBeginPicker()) { + return + } + try { const image = await pickImageFromCamera() - if (image) { + if (image && isScreenActive()) { validateStagedNoteImage(image) setStagedImage(image) setLocalErrorMessage(null) } } catch (error) { - setLocalErrorMessage(error instanceof Error ? error.message : "The camera could not be opened.") + if (isScreenActive()) { + setLocalErrorMessage(error instanceof Error ? error.message : "The camera could not be opened.") + } + } finally { + endPicker() } } const attachFromGallery = async () => { + if (!tryBeginPicker()) { + return + } + try { const image = await pickImageFromLibrary() - if (image) { + if (image && isScreenActive()) { validateStagedNoteImage(image) setStagedImage(image) setLocalErrorMessage(null) } } catch (error) { - setLocalErrorMessage(error instanceof Error ? error.message : "The gallery could not be opened.") + if (isScreenActive()) { + setLocalErrorMessage(error instanceof Error ? error.message : "The gallery could not be opened.") + } + } finally { + endPicker() } } diff --git a/FastNotes/src/notes/use-picker-lifecycle-guard.ts b/FastNotes/src/notes/use-picker-lifecycle-guard.ts new file mode 100644 index 0000000..41879a3 --- /dev/null +++ b/FastNotes/src/notes/use-picker-lifecycle-guard.ts @@ -0,0 +1,38 @@ +import { useIsFocused } from "@react-navigation/native" +import { useCallback, useEffect, useRef } from "react" + +export function usePickerLifecycleGuard() { + const isFocused = useIsFocused() + const isMountedRef = useRef(true) + const isPickerActiveRef = useRef(false) + + useEffect(() => { + return () => { + isMountedRef.current = false + isPickerActiveRef.current = false + } + }, []) + + const isScreenActive = useCallback(() => { + return isMountedRef.current && isFocused + }, [isFocused]) + + const tryBeginPicker = useCallback(() => { + if (isPickerActiveRef.current) { + return false + } + + isPickerActiveRef.current = true + return true + }, []) + + const endPicker = useCallback(() => { + isPickerActiveRef.current = false + }, []) + + return { + endPicker, + isScreenActive, + tryBeginPicker, + } +}