fix: address PR #102 review feedback (GRO-145)
- factories.ts: add photoKey/photoUploadedAt null defaults to buildPet (TS regression fix)
- s3.ts: lazy singleton S3Client to avoid re-instantiation per call
- routes/pets.ts: server-side 5MB file size limit, explicit content-type allowlist (drops image/svg+xml etc), validate confirm key ownership against pets/${petId}/ prefix, delete old S3 object on re-upload, fix RBAC comment on DELETE photo
- PetPhotoUpload.tsx: bypass canvas resize for GIFs (preserves animation), pass fileSizeBytes in upload-url request
- Add PetPhotoDisplay.test.tsx: 7 tests covering fetch states, placeholder, refetch on petId change, custom size
- Add PetPhotoUpload.test.tsx: 8 tests covering idle state, type validation, upload flow, progress, GIF bypass
- Update petPhotos.test.ts: add SVG rejection, 5MB limit, key ownership, and old-photo deletion tests (18 total)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -101,7 +101,7 @@ describe("POST /pets/:petId/photo/upload-url", () => {
|
|||||||
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
body: JSON.stringify({ contentType: "image/jpeg" }),
|
body: JSON.stringify({ contentType: "image/jpeg", fileSizeBytes: 1024 }),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
const body = (await res.json()) as { uploadUrl: string; key: string };
|
const body = (await res.json()) as { uploadUrl: string; key: string };
|
||||||
@@ -115,7 +115,27 @@ describe("POST /pets/:petId/photo/upload-url", () => {
|
|||||||
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
body: JSON.stringify({ contentType: "application/pdf" }),
|
body: JSON.stringify({ contentType: "application/pdf", fileSizeBytes: 1024 }),
|
||||||
|
});
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects image/svg+xml with 400 (allowlist enforcement)", async () => {
|
||||||
|
const app = buildApp(MANAGER);
|
||||||
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ contentType: "image/svg+xml", fileSizeBytes: 1024 }),
|
||||||
|
});
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects fileSizeBytes over 5 MB with 400", async () => {
|
||||||
|
const app = buildApp(MANAGER);
|
||||||
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ contentType: "image/jpeg", fileSizeBytes: 6 * 1024 * 1024 }),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
});
|
});
|
||||||
@@ -126,7 +146,7 @@ describe("POST /pets/:petId/photo/upload-url", () => {
|
|||||||
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
body: JSON.stringify({ contentType: "image/jpeg" }),
|
body: JSON.stringify({ contentType: "image/jpeg", fileSizeBytes: 1024 }),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(404);
|
expect(res.status).toBe(404);
|
||||||
});
|
});
|
||||||
@@ -136,7 +156,7 @@ describe("POST /pets/:petId/photo/upload-url", () => {
|
|||||||
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
const res = await app.request(`/pets/${PET_ID}/photo/upload-url`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
body: JSON.stringify({ contentType: "image/png" }),
|
body: JSON.stringify({ contentType: "image/png", fileSizeBytes: 1024 }),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
});
|
});
|
||||||
@@ -177,6 +197,34 @@ describe("POST /pets/:petId/photo/confirm", () => {
|
|||||||
});
|
});
|
||||||
expect(res.status).toBe(404);
|
expect(res.status).toBe(404);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("returns 400 when key does not belong to the pet", async () => {
|
||||||
|
const app = buildApp(MANAGER);
|
||||||
|
const res = await app.request(`/pets/${PET_ID}/photo/confirm`, {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ key: "pets/other-pet-id/1700000000000.jpg" }),
|
||||||
|
});
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
const body = (await res.json()) as { error: string };
|
||||||
|
expect(body.error).toMatch(/invalid key/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deletes old photo from storage when re-uploading", async () => {
|
||||||
|
const { deleteObject } = await import("../lib/s3.js");
|
||||||
|
const oldKey = `pets/${PET_ID}/old.jpg`;
|
||||||
|
dbPetRow = { ...dbPetRow!, photoKey: oldKey };
|
||||||
|
|
||||||
|
const app = buildApp(MANAGER);
|
||||||
|
const res = await app.request(`/pets/${PET_ID}/photo/confirm`, {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ key: PHOTO_KEY }),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(deleteObject).toHaveBeenCalledWith(oldKey);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// ─── DELETE /:petId/photo ────────────────────────────────────────────────────
|
// ─── DELETE /:petId/photo ────────────────────────────────────────────────────
|
||||||
|
|||||||
+16
-9
@@ -6,16 +6,21 @@ import {
|
|||||||
} from "@aws-sdk/client-s3";
|
} from "@aws-sdk/client-s3";
|
||||||
import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
|
import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
|
||||||
|
|
||||||
|
let s3Instance: S3Client | null = null;
|
||||||
|
|
||||||
function getS3Client(): S3Client {
|
function getS3Client(): S3Client {
|
||||||
return new S3Client({
|
if (!s3Instance) {
|
||||||
endpoint: process.env.S3_ENDPOINT,
|
s3Instance = new S3Client({
|
||||||
region: process.env.S3_REGION ?? "us-east-1",
|
endpoint: process.env.S3_ENDPOINT,
|
||||||
credentials: {
|
region: process.env.S3_REGION ?? "us-east-1",
|
||||||
accessKeyId: process.env.S3_ACCESS_KEY_ID ?? "",
|
credentials: {
|
||||||
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY ?? "",
|
accessKeyId: process.env.S3_ACCESS_KEY_ID ?? "",
|
||||||
},
|
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY ?? "",
|
||||||
forcePathStyle: true, // required for Ceph RGW
|
},
|
||||||
});
|
forcePathStyle: true, // required for Ceph RGW
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return s3Instance;
|
||||||
}
|
}
|
||||||
|
|
||||||
function getBucket(): string {
|
function getBucket(): string {
|
||||||
@@ -26,6 +31,7 @@ function getBucket(): string {
|
|||||||
export async function getPresignedUploadUrl(
|
export async function getPresignedUploadUrl(
|
||||||
key: string,
|
key: string,
|
||||||
contentType: string,
|
contentType: string,
|
||||||
|
sizeBytes: number,
|
||||||
expiresIn = 900
|
expiresIn = 900
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
const client = getS3Client();
|
const client = getS3Client();
|
||||||
@@ -33,6 +39,7 @@ export async function getPresignedUploadUrl(
|
|||||||
Bucket: getBucket(),
|
Bucket: getBucket(),
|
||||||
Key: key,
|
Key: key,
|
||||||
ContentType: contentType,
|
ContentType: contentType,
|
||||||
|
ContentLength: sizeBytes,
|
||||||
});
|
});
|
||||||
return getSignedUrl(client, command, { expiresIn });
|
return getSignedUrl(client, command, { expiresIn });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -99,12 +99,22 @@ petsRouter.delete("/:id", async (c) => {
|
|||||||
|
|
||||||
// ─── Photo routes ──────────────────────────────────────────────────────────────
|
// ─── Photo routes ──────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
const ALLOWED_CONTENT_TYPES = new Set([
|
||||||
|
"image/jpeg",
|
||||||
|
"image/png",
|
||||||
|
"image/webp",
|
||||||
|
"image/gif",
|
||||||
|
]);
|
||||||
|
|
||||||
|
const MAX_PHOTO_SIZE = 5 * 1024 * 1024; // 5 MB
|
||||||
|
|
||||||
const uploadUrlSchema = z.object({
|
const uploadUrlSchema = z.object({
|
||||||
contentType: z
|
contentType: z.string().refine((v) => ALLOWED_CONTENT_TYPES.has(v), {
|
||||||
.string()
|
message: "contentType must be one of: image/jpeg, image/png, image/webp, image/gif",
|
||||||
.refine((v) => v.startsWith("image/"), {
|
}),
|
||||||
message: "contentType must be an image/* MIME type",
|
fileSizeBytes: z.number().int().positive().max(MAX_PHOTO_SIZE, {
|
||||||
}),
|
message: "File must not exceed 5 MB",
|
||||||
|
}),
|
||||||
});
|
});
|
||||||
|
|
||||||
const confirmSchema = z.object({
|
const confirmSchema = z.object({
|
||||||
@@ -122,14 +132,14 @@ petsRouter.post(
|
|||||||
async (c) => {
|
async (c) => {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
const petId = c.req.param("petId");
|
const petId = c.req.param("petId");
|
||||||
const { contentType } = c.req.valid("json");
|
const { contentType, fileSizeBytes } = c.req.valid("json");
|
||||||
|
|
||||||
const [pet] = await db.select().from(pets).where(eq(pets.id, petId));
|
const [pet] = await db.select().from(pets).where(eq(pets.id, petId));
|
||||||
if (!pet) return c.json({ error: "Pet not found" }, 404);
|
if (!pet) return c.json({ error: "Pet not found" }, 404);
|
||||||
|
|
||||||
const ext = contentType.split("/")[1] ?? "jpg";
|
const ext = contentType.split("/")[1] ?? "jpg";
|
||||||
const key = `pets/${petId}/${Date.now()}.${ext}`;
|
const key = `pets/${petId}/${Date.now()}.${ext}`;
|
||||||
const uploadUrl = await getPresignedUploadUrl(key, contentType);
|
const uploadUrl = await getPresignedUploadUrl(key, contentType, fileSizeBytes);
|
||||||
|
|
||||||
return c.json({ uploadUrl, key });
|
return c.json({ uploadUrl, key });
|
||||||
}
|
}
|
||||||
@@ -148,6 +158,19 @@ petsRouter.post(
|
|||||||
const petId = c.req.param("petId");
|
const petId = c.req.param("petId");
|
||||||
const { key } = c.req.valid("json");
|
const { key } = c.req.valid("json");
|
||||||
|
|
||||||
|
// Validate that the key belongs to this pet to prevent key hijacking
|
||||||
|
if (!key.startsWith(`pets/${petId}/`)) {
|
||||||
|
return c.json({ error: "Invalid key" }, 400);
|
||||||
|
}
|
||||||
|
|
||||||
|
const [pet] = await db.select().from(pets).where(eq(pets.id, petId));
|
||||||
|
if (!pet) return c.json({ error: "Pet not found" }, 404);
|
||||||
|
|
||||||
|
// Delete the previous photo from storage to avoid orphaned objects
|
||||||
|
if (pet.photoKey) {
|
||||||
|
await deleteObject(pet.photoKey);
|
||||||
|
}
|
||||||
|
|
||||||
const [row] = await db
|
const [row] = await db
|
||||||
.update(pets)
|
.update(pets)
|
||||||
.set({ photoKey: key, photoUploadedAt: new Date(), updatedAt: new Date() })
|
.set({ photoKey: key, photoUploadedAt: new Date(), updatedAt: new Date() })
|
||||||
@@ -162,7 +185,7 @@ petsRouter.post(
|
|||||||
/**
|
/**
|
||||||
* DELETE /:petId/photo
|
* DELETE /:petId/photo
|
||||||
* Removes the photo from object storage and clears the DB record.
|
* Removes the photo from object storage and clears the DB record.
|
||||||
* Manager-only (write-destructive operation).
|
* All staff roles (manager, receptionist, groomer) may call this.
|
||||||
*/
|
*/
|
||||||
petsRouter.delete("/:petId/photo", async (c) => {
|
petsRouter.delete("/:petId/photo", async (c) => {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|||||||
@@ -0,0 +1,100 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
import { render, screen, waitFor } from "@testing-library/react";
|
||||||
|
import { PetPhotoDisplay } from "../components/PetPhotoDisplay.js";
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("PetPhotoDisplay", () => {
|
||||||
|
it("shows loading skeleton while fetching", () => {
|
||||||
|
global.fetch = vi.fn(() => new Promise(() => {})) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
|
||||||
|
expect(screen.getByLabelText("Loading photo…")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders photo img when fetch returns a URL", async () => {
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
json: async () => ({ url: "https://storage.test/pet-1/photo.jpg" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
|
||||||
|
const img = await screen.findByRole("img", { name: "Pet photo" });
|
||||||
|
expect(img).toHaveAttribute("src", "https://storage.test/pet-1/photo.jpg");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows paw placeholder when API returns 404", async () => {
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({ ok: false, status: 404 } as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByLabelText("No photo")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
expect(screen.queryByLabelText("Loading photo…")).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows paw placeholder when fetch rejects (network error)", async () => {
|
||||||
|
global.fetch = vi.fn(() => Promise.reject(new Error("network error"))) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByLabelText("No photo")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows paw placeholder on non-404 error status", async () => {
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({ ok: false, status: 500 } as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByLabelText("No photo")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refetches when petId changes", async () => {
|
||||||
|
const fetchMock = vi.fn((url: string) => {
|
||||||
|
const petId = (url as string).match(/\/api\/pets\/([^/]+)\/photo/)?.[1];
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
status: 200,
|
||||||
|
json: async () => ({ url: `https://storage.test/${petId}/photo.jpg` }),
|
||||||
|
} as Response);
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
global.fetch = fetchMock;
|
||||||
|
|
||||||
|
const { rerender } = render(<PetPhotoDisplay petId="pet-1" />);
|
||||||
|
await screen.findByRole("img");
|
||||||
|
expect(fetchMock).toHaveBeenCalledWith("/api/pets/pet-1/photo");
|
||||||
|
|
||||||
|
rerender(<PetPhotoDisplay petId="pet-2" />);
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(fetchMock).toHaveBeenCalledWith("/api/pets/pet-2/photo");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("applies custom size prop to container", async () => {
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({ ok: false, status: 404 } as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const { container } = render(<PetPhotoDisplay petId="pet-1" size={96} />);
|
||||||
|
|
||||||
|
await screen.findByLabelText("No photo");
|
||||||
|
const div = container.firstChild as HTMLElement;
|
||||||
|
expect(div).toHaveStyle({ width: "96px", height: "96px" });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,311 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||||
|
import { PetPhotoUpload } from "../components/PetPhotoUpload.js";
|
||||||
|
|
||||||
|
// ── XHR mock ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
interface XhrMock {
|
||||||
|
upload: { addEventListener: ReturnType<typeof vi.fn> };
|
||||||
|
addEventListener: ReturnType<typeof vi.fn>;
|
||||||
|
open: ReturnType<typeof vi.fn>;
|
||||||
|
setRequestHeader: ReturnType<typeof vi.fn>;
|
||||||
|
send: ReturnType<typeof vi.fn>;
|
||||||
|
status: number;
|
||||||
|
// Callbacks stored by the mock so tests can trigger them
|
||||||
|
_triggerLoad: () => void;
|
||||||
|
_triggerError: () => void;
|
||||||
|
_triggerProgress: (loaded: number, total: number) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeXhrMock(status = 200): XhrMock {
|
||||||
|
const uploadListeners: Record<string, (ev: ProgressEvent) => void> = {};
|
||||||
|
const listeners: Record<string, () => void> = {};
|
||||||
|
|
||||||
|
const mock: XhrMock = {
|
||||||
|
upload: {
|
||||||
|
addEventListener: vi.fn((event: string, cb: (ev: ProgressEvent) => void) => {
|
||||||
|
uploadListeners[event] = cb;
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
addEventListener: vi.fn((event: string, cb: () => void) => {
|
||||||
|
listeners[event] = cb;
|
||||||
|
}),
|
||||||
|
open: vi.fn(),
|
||||||
|
setRequestHeader: vi.fn(),
|
||||||
|
send: vi.fn(),
|
||||||
|
status,
|
||||||
|
_triggerLoad: () => listeners["load"]?.(),
|
||||||
|
_triggerError: () => listeners["error"]?.(),
|
||||||
|
_triggerProgress: (loaded, total) =>
|
||||||
|
uploadListeners["progress"]?.({ lengthComputable: true, loaded, total } as ProgressEvent),
|
||||||
|
};
|
||||||
|
return mock;
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Canvas mock ───────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
// jsdom doesn't implement canvas — provide a minimal stub
|
||||||
|
function mockCanvas(blob: Blob) {
|
||||||
|
const ctx = { drawImage: vi.fn() };
|
||||||
|
const originalCreateElement = document.createElement.bind(document);
|
||||||
|
vi.spyOn(document, "createElement").mockImplementation((tag: string) => {
|
||||||
|
if (tag === "canvas") {
|
||||||
|
const canvas = {
|
||||||
|
width: 0,
|
||||||
|
height: 0,
|
||||||
|
getContext: () => ctx,
|
||||||
|
toBlob: (cb: (b: Blob | null) => void) => cb(blob),
|
||||||
|
};
|
||||||
|
return canvas as unknown as HTMLCanvasElement;
|
||||||
|
}
|
||||||
|
return originalCreateElement(tag);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Image mock ────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
function mockImage(width = 800, height = 600) {
|
||||||
|
const originalImage = globalThis.Image;
|
||||||
|
const ImageMock = vi.fn().mockImplementation(() => {
|
||||||
|
const img = {
|
||||||
|
width,
|
||||||
|
height,
|
||||||
|
onload: null as (() => void) | null,
|
||||||
|
onerror: null as (() => void) | null,
|
||||||
|
set src(_v: string) {
|
||||||
|
// trigger onload asynchronously
|
||||||
|
setTimeout(() => img.onload?.(), 0);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
return img;
|
||||||
|
});
|
||||||
|
globalThis.Image = ImageMock as unknown as typeof Image;
|
||||||
|
return () => {
|
||||||
|
globalThis.Image = originalImage;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── URL mock ──────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
URL.createObjectURL = vi.fn(() => "blob:mock");
|
||||||
|
URL.revokeObjectURL = vi.fn();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── Helpers ───────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
function makeFile(type = "image/jpeg", name = "photo.jpg", sizeBytes = 1024): File {
|
||||||
|
const buf = new Uint8Array(sizeBytes);
|
||||||
|
return new File([buf], name, { type });
|
||||||
|
}
|
||||||
|
|
||||||
|
function selectFile(file: File) {
|
||||||
|
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
|
||||||
|
Object.defineProperty(input, "files", { value: [file], configurable: true });
|
||||||
|
fireEvent.change(input);
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Tests ─────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
describe("PetPhotoUpload", () => {
|
||||||
|
it("renders the upload button in idle state", () => {
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
expect(screen.getByRole("button", { name: /upload photo/i })).toBeInTheDocument();
|
||||||
|
expect(screen.getByRole("button")).not.toBeDisabled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows an error for an unsupported file type", async () => {
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("text/plain", "doc.txt"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText(/JPEG, PNG, WebP, or GIF/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("disables the button while uploading", async () => {
|
||||||
|
const restoreImage = mockImage();
|
||||||
|
const resizedBlob = new Blob(["x"], { type: "image/jpeg" });
|
||||||
|
mockCanvas(resizedBlob);
|
||||||
|
|
||||||
|
let xhrInstance: XhrMock;
|
||||||
|
const XHRMock = vi.fn().mockImplementation(() => {
|
||||||
|
xhrInstance = makeXhrMock(200);
|
||||||
|
return xhrInstance;
|
||||||
|
});
|
||||||
|
globalThis.XMLHttpRequest = XHRMock as unknown as typeof XMLHttpRequest;
|
||||||
|
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ uploadUrl: "https://storage.test/put", key: "pets/pet-1/123.jpg" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("image/jpeg"));
|
||||||
|
|
||||||
|
// Button should become disabled during upload
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByRole("button")).toBeDisabled();
|
||||||
|
});
|
||||||
|
|
||||||
|
restoreImage();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("calls onUploaded and resets after successful upload", async () => {
|
||||||
|
const restoreImage = mockImage();
|
||||||
|
const resizedBlob = new Blob(["x"], { type: "image/jpeg" });
|
||||||
|
mockCanvas(resizedBlob);
|
||||||
|
|
||||||
|
let xhrInstance!: XhrMock;
|
||||||
|
const XHRMock = vi.fn().mockImplementation(() => {
|
||||||
|
xhrInstance = makeXhrMock(200);
|
||||||
|
return xhrInstance;
|
||||||
|
});
|
||||||
|
globalThis.XMLHttpRequest = XHRMock as unknown as typeof XMLHttpRequest;
|
||||||
|
|
||||||
|
const onUploaded = vi.fn();
|
||||||
|
global.fetch = vi.fn((url: string) => {
|
||||||
|
if ((url as string).includes("upload-url")) {
|
||||||
|
return Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ uploadUrl: "https://storage.test/put", key: "pets/pet-1/123.jpg" }),
|
||||||
|
} as Response);
|
||||||
|
}
|
||||||
|
// confirm
|
||||||
|
return Promise.resolve({ ok: true, json: async () => ({ ok: true }) } as Response);
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={onUploaded} />);
|
||||||
|
selectFile(makeFile("image/jpeg"));
|
||||||
|
|
||||||
|
// Wait for XHR to be set up, then trigger load
|
||||||
|
await waitFor(() => expect(xhrInstance).toBeDefined());
|
||||||
|
xhrInstance._triggerLoad();
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(onUploaded).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
restoreImage();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows error message when upload-url request fails", async () => {
|
||||||
|
const restoreImage = mockImage();
|
||||||
|
const resizedBlob = new Blob(["x"], { type: "image/jpeg" });
|
||||||
|
mockCanvas(resizedBlob);
|
||||||
|
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: false,
|
||||||
|
json: async () => ({ error: "Pet not found" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("image/jpeg"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText(/Pet not found/)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
restoreImage();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows error message when XHR upload fails", async () => {
|
||||||
|
const restoreImage = mockImage();
|
||||||
|
const resizedBlob = new Blob(["x"], { type: "image/jpeg" });
|
||||||
|
mockCanvas(resizedBlob);
|
||||||
|
|
||||||
|
let xhrInstance!: XhrMock;
|
||||||
|
const XHRMock = vi.fn().mockImplementation(() => {
|
||||||
|
xhrInstance = makeXhrMock(0);
|
||||||
|
return xhrInstance;
|
||||||
|
});
|
||||||
|
globalThis.XMLHttpRequest = XHRMock as unknown as typeof XMLHttpRequest;
|
||||||
|
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ uploadUrl: "https://storage.test/put", key: "pets/pet-1/123.jpg" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("image/jpeg"));
|
||||||
|
|
||||||
|
await waitFor(() => expect(xhrInstance).toBeDefined());
|
||||||
|
xhrInstance._triggerError();
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText(/network error/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
restoreImage();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows upload progress percentage", async () => {
|
||||||
|
const restoreImage = mockImage();
|
||||||
|
const resizedBlob = new Blob(["x"], { type: "image/jpeg" });
|
||||||
|
mockCanvas(resizedBlob);
|
||||||
|
|
||||||
|
let xhrInstance!: XhrMock;
|
||||||
|
const XHRMock = vi.fn().mockImplementation(() => {
|
||||||
|
xhrInstance = makeXhrMock(200);
|
||||||
|
return xhrInstance;
|
||||||
|
});
|
||||||
|
globalThis.XMLHttpRequest = XHRMock as unknown as typeof XMLHttpRequest;
|
||||||
|
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ uploadUrl: "https://storage.test/put", key: "pets/pet-1/123.jpg" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("image/jpeg"));
|
||||||
|
|
||||||
|
await waitFor(() => expect(xhrInstance).toBeDefined());
|
||||||
|
xhrInstance._triggerProgress(50, 100);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText(/Uploading 50%/)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
restoreImage();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips canvas resize for GIF files", async () => {
|
||||||
|
const createElementSpy = vi.spyOn(document, "createElement");
|
||||||
|
|
||||||
|
global.fetch = vi.fn(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ uploadUrl: "https://storage.test/put", key: "pets/pet-1/123.gif" }),
|
||||||
|
} as Response)
|
||||||
|
) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
let xhrInstance!: XhrMock;
|
||||||
|
const XHRMock = vi.fn().mockImplementation(() => {
|
||||||
|
xhrInstance = makeXhrMock(200);
|
||||||
|
return xhrInstance;
|
||||||
|
});
|
||||||
|
globalThis.XMLHttpRequest = XHRMock as unknown as typeof XMLHttpRequest;
|
||||||
|
|
||||||
|
render(<PetPhotoUpload petId="pet-1" onUploaded={vi.fn()} />);
|
||||||
|
selectFile(makeFile("image/gif", "anim.gif", 512));
|
||||||
|
|
||||||
|
// Wait for XHR to be invoked
|
||||||
|
await waitFor(() => expect(xhrInstance).toBeDefined());
|
||||||
|
|
||||||
|
// canvas should NOT have been created for GIF
|
||||||
|
const canvasCalls = createElementSpy.mock.calls.filter(([tag]) => tag === "canvas");
|
||||||
|
expect(canvasCalls.length).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -31,6 +31,11 @@ export function PetPhotoUpload({ petId, onUploaded }: Props) {
|
|||||||
>({ status: "idle" });
|
>({ status: "idle" });
|
||||||
|
|
||||||
async function resizeImage(file: File): Promise<{ blob: Blob; contentType: string }> {
|
async function resizeImage(file: File): Promise<{ blob: Blob; contentType: string }> {
|
||||||
|
// GIFs must bypass canvas resize — canvas destroys animation frames
|
||||||
|
if (file.type === "image/gif") {
|
||||||
|
return { blob: file, contentType: "image/gif" };
|
||||||
|
}
|
||||||
|
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const img = new Image();
|
const img = new Image();
|
||||||
const url = URL.createObjectURL(file);
|
const url = URL.createObjectURL(file);
|
||||||
@@ -90,7 +95,7 @@ export function PetPhotoUpload({ petId, onUploaded }: Props) {
|
|||||||
const res = await fetch(`/api/pets/${petId}/photo/upload-url`, {
|
const res = await fetch(`/api/pets/${petId}/photo/upload-url`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
body: JSON.stringify({ contentType }),
|
body: JSON.stringify({ contentType, fileSizeBytes: blob.size }),
|
||||||
});
|
});
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
const err = (await res.json()) as { error?: string };
|
const err = (await res.json()) as { error?: string };
|
||||||
|
|||||||
@@ -92,6 +92,8 @@ export function buildPet(overrides: Partial<PetRow> & { clientId: string }): Pet
|
|||||||
shampooPreference: null,
|
shampooPreference: null,
|
||||||
specialCareNotes: null,
|
specialCareNotes: null,
|
||||||
customFields: {},
|
customFields: {},
|
||||||
|
photoKey: null,
|
||||||
|
photoUploadedAt: null,
|
||||||
createdAt: new Date("2025-01-01T00:00:00Z"),
|
createdAt: new Date("2025-01-01T00:00:00Z"),
|
||||||
updatedAt: new Date("2025-01-01T00:00:00Z"),
|
updatedAt: new Date("2025-01-01T00:00:00Z"),
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user