mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 09:36:08 +00:00
feat(comments): carry a batch of screenshots on one comment
A comment held one image, and the paste handler took the first item off the clipboard and dropped the rest. Reviewing a cut usually means several screenshots about the same moment, which meant one comment per screenshot or one screenshot and a paragraph describing the others. Editing a comment could not attach anything at all: the edit box had no paste handler, no file picker and no way to remove what was already there. A comment now carries up to five images, in the composer, in a reply and in the editor. One paste stages every image on the clipboard, the file picker takes a multiple selection, and a drop lands on whichever editor is open. Over the cap the extras are refused out loud rather than dropped quietly. A single image still fills the width; several tile into a grid, and either opens full screen on click. The images move into their own table. `comments.imageUrl` stays and follows the first of them, so a reader that has not been updated keeps working, and the migration copies the existing attachments across so the new table is complete from the first read. Every path that resolves a URL back to a comment now asks the new table: R2 cleanup, the orphan sweep, the storage accounting and the reference checks that decide whether an object can be deleted. Left on the old column they would have treated images two through five as unreferenced and swept them. Detaching an image while editing only breaks the link. The file stays in R2 and in the assets pane, which is where it is deleted from and where its bytes are already billed.
This commit is contained in:
@@ -11,6 +11,7 @@ import {
|
||||
GET as getCommentRoute,
|
||||
PATCH as patchCommentRoute,
|
||||
} from '@/app/api/comments/[commentId]/route';
|
||||
import { isFreshAttachment } from '@/lib/upload-freshness';
|
||||
import { apiRequest, callRoute, readData, readError } from '../helpers/request';
|
||||
import { signedInAs, signedOut } from '../helpers/session';
|
||||
import {
|
||||
@@ -28,6 +29,32 @@ import {
|
||||
seedVersion,
|
||||
} from '../factories';
|
||||
|
||||
// The real check heads the object in R2. Standing in for it lets these tests
|
||||
// drive the attachment paths; a suite that wants a stale upload overrides it.
|
||||
vi.mock('@/lib/upload-freshness', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/lib/upload-freshness')>();
|
||||
return {
|
||||
...actual,
|
||||
isFreshAttachment: vi.fn(async () => ({ isFresh: true, sizeBytes: BigInt(1024) })),
|
||||
};
|
||||
});
|
||||
|
||||
const IMAGE_A = '/api/upload/image/11111111-2222-3333-4444-555555555555.png';
|
||||
const IMAGE_B = '/api/upload/image/66666666-7777-8888-9999-aaaaaaaaaaaa.png';
|
||||
const IMAGE_C = '/api/upload/image/bbbbbbbb-cccc-dddd-eeee-ffffffffffff.png';
|
||||
const IMAGE_D = '/api/upload/image/12121212-3434-5656-7878-909090909090.png';
|
||||
const IMAGE_E = '/api/upload/image/abababab-cdcd-efef-0101-232323232323.png';
|
||||
const IMAGE_F = '/api/upload/image/45454545-6767-8989-0a0a-1b1b1b1b1b1b.png';
|
||||
|
||||
async function imageUrlsOf(commentId: string): Promise<string[]> {
|
||||
const images = await db.commentImage.findMany({
|
||||
where: { commentId },
|
||||
orderBy: { position: 'asc' },
|
||||
select: { url: true },
|
||||
});
|
||||
return images.map((image) => image.url);
|
||||
}
|
||||
|
||||
const VALID_STROKE = {
|
||||
points: [
|
||||
{ x: 0.1, y: 0.2 },
|
||||
@@ -729,6 +756,264 @@ describe('POST /api/versions/[versionId]/comments', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// A comment used to carry a single image. Screenshots arrive in batches, so the
|
||||
// list is the contract now and the old `imageUrl` column follows its first entry.
|
||||
describe('comment image attachments', () => {
|
||||
it('stores every image in order, points imageUrl at the first, and lists them all as assets', async () => {
|
||||
const scenario = await seedVersion();
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
createCommentRoute,
|
||||
apiRequest(commentsUrl(scenario.version.id), {
|
||||
body: { content: 'three shots', timestamp: 1, imageUrls: [IMAGE_A, IMAGE_B, IMAGE_C] },
|
||||
}),
|
||||
{ versionId: scenario.version.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
const created = await readData<{ id: string; images: { url: string }[] }>(response);
|
||||
expect(created.images.map((image) => image.url)).toEqual([IMAGE_A, IMAGE_B, IMAGE_C]);
|
||||
|
||||
const stored = await db.comment.findUniqueOrThrow({ where: { id: created.id } });
|
||||
expect(stored.imageUrl).toBe(IMAGE_A);
|
||||
expect(await imageUrlsOf(created.id)).toEqual([IMAGE_A, IMAGE_B, IMAGE_C]);
|
||||
|
||||
const assets = await db.videoAsset.findMany({
|
||||
where: { videoId: scenario.video.id, provider: 'R2_IMAGE' },
|
||||
select: { sourceUrl: true },
|
||||
});
|
||||
expect(assets.map((asset) => asset.sourceUrl).sort()).toEqual(
|
||||
[IMAGE_A, IMAGE_B, IMAGE_C].sort()
|
||||
);
|
||||
});
|
||||
|
||||
it('still accepts the legacy single imageUrl', async () => {
|
||||
const scenario = await seedVersion();
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
createCommentRoute,
|
||||
apiRequest(commentsUrl(scenario.version.id), {
|
||||
body: { content: 'one shot', timestamp: 1, imageUrl: IMAGE_A },
|
||||
}),
|
||||
{ versionId: scenario.version.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
const created = await readData<{ id: string }>(response);
|
||||
expect(await imageUrlsOf(created.id)).toEqual([IMAGE_A]);
|
||||
});
|
||||
|
||||
it('accepts a comment that is nothing but images', async () => {
|
||||
const scenario = await seedVersion();
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
createCommentRoute,
|
||||
apiRequest(commentsUrl(scenario.version.id), {
|
||||
body: { timestamp: 1, imageUrls: [IMAGE_A] },
|
||||
}),
|
||||
{ versionId: scenario.version.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
});
|
||||
|
||||
it('refuses more images than a comment may hold, and writes nothing', async () => {
|
||||
const scenario = await seedVersion();
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
createCommentRoute,
|
||||
apiRequest(commentsUrl(scenario.version.id), {
|
||||
body: {
|
||||
content: 'too many',
|
||||
timestamp: 1,
|
||||
imageUrls: [IMAGE_A, IMAGE_B, IMAGE_C, IMAGE_D, IMAGE_E, IMAGE_F],
|
||||
},
|
||||
}),
|
||||
{ versionId: scenario.version.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(await readError(response)).toBe('A comment can have at most 5 images');
|
||||
expect(await db.comment.count()).toBe(0);
|
||||
expect(await db.commentImage.count()).toBe(0);
|
||||
});
|
||||
|
||||
it('refuses the whole comment when one of the uploads has expired', async () => {
|
||||
const scenario = await seedVersion();
|
||||
signedInAs(scenario.owner);
|
||||
vi.mocked(isFreshAttachment).mockImplementation(async (url: string) => ({
|
||||
isFresh: url !== IMAGE_B,
|
||||
sizeBytes: BigInt(1024),
|
||||
}));
|
||||
|
||||
const response = await callRoute(
|
||||
createCommentRoute,
|
||||
apiRequest(commentsUrl(scenario.version.id), {
|
||||
body: { content: 'stale', timestamp: 1, imageUrls: [IMAGE_A, IMAGE_B] },
|
||||
}),
|
||||
{ versionId: scenario.version.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(await db.comment.count()).toBe(0);
|
||||
expect(await db.commentImage.count()).toBe(0);
|
||||
vi.mocked(isFreshAttachment).mockResolvedValue({ isFresh: true, sizeBytes: BigInt(1024) });
|
||||
});
|
||||
|
||||
it('replaces the list on edit: keeps one, drops one, adds one', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_A, IMAGE_B],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { content: 'reworded', imageUrls: [IMAGE_B, IMAGE_C] },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([IMAGE_B, IMAGE_C]);
|
||||
// The legacy column follows the new first image.
|
||||
expect((await db.comment.findUniqueOrThrow({ where: { id: comment.id } })).imageUrl).toBe(
|
||||
IMAGE_B
|
||||
);
|
||||
// The image added while editing shows up in the assets pane like any other.
|
||||
expect(await db.videoAsset.count({ where: { sourceUrl: IMAGE_C } })).toBe(1);
|
||||
// The detached file is not deleted here: the assets pane owns its lifetime.
|
||||
expect(await db.commentImage.count({ where: { url: IMAGE_A } })).toBe(0);
|
||||
});
|
||||
|
||||
it('clears every image when the edit sends an empty list', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_A, IMAGE_B],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { content: 'text only now', imageUrls: [] },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([]);
|
||||
expect((await db.comment.findUniqueOrThrow({ where: { id: comment.id } })).imageUrl).toBeNull();
|
||||
});
|
||||
|
||||
it('leaves the images alone when the edit does not mention them', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_A],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { content: 'only the words changed' },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([IMAGE_A]);
|
||||
});
|
||||
|
||||
it('returns 403 when somebody other than the author changes the images', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const author = await createUser();
|
||||
await addProjectMember({ projectId: scenario.project.id, userId: author.id });
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: author.id,
|
||||
imageUrls: [IMAGE_A],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { imageUrls: [IMAGE_A, IMAGE_B] },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([IMAGE_A]);
|
||||
});
|
||||
|
||||
it('refuses to steal an image that already hangs off another comment', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const other = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_A],
|
||||
});
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_B],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { imageUrls: [IMAGE_B, IMAGE_A] },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([IMAGE_B]);
|
||||
expect(await imageUrlsOf(other.id)).toEqual([IMAGE_A]);
|
||||
});
|
||||
|
||||
it('refuses an edit that would carry more images than the cap', async () => {
|
||||
const scenario = await seedVersion();
|
||||
const comment = await createComment({
|
||||
versionId: scenario.version.id,
|
||||
authorId: scenario.owner.id,
|
||||
imageUrls: [IMAGE_A],
|
||||
});
|
||||
signedInAs(scenario.owner);
|
||||
|
||||
const response = await callRoute(
|
||||
patchCommentRoute,
|
||||
apiRequest(`/api/comments/${comment.id}`, {
|
||||
method: 'PATCH',
|
||||
body: { imageUrls: [IMAGE_A, IMAGE_B, IMAGE_C, IMAGE_D, IMAGE_E, IMAGE_F] },
|
||||
}),
|
||||
{ commentId: comment.id }
|
||||
);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(await imageUrlsOf(comment.id)).toEqual([IMAGE_A]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/comments/[commentId]', () => {
|
||||
it('returns 403 to a stranger and never exposes the project row', async () => {
|
||||
const scenario = await seedVersion();
|
||||
|
||||
@@ -270,6 +270,26 @@ describe('collectVideoMediaUrls', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// A comment can carry several screenshots; collecting only the first would
|
||||
// leave the rest behind in R2 after the video is gone.
|
||||
it('collects every image on a comment, not only the first', async () => {
|
||||
const scenario = await seedProject();
|
||||
const video = await createVideo({ projectId: scenario.project.id });
|
||||
const version = await createVersion({
|
||||
videoParentId: video.id,
|
||||
providerId: 'r2',
|
||||
originalUrl: OWN_VERSION_VIDEO,
|
||||
});
|
||||
await createComment({
|
||||
versionId: version.id,
|
||||
imageUrls: [OWN_COMMENT_IMAGE, OWN_ASSET_IMAGE],
|
||||
});
|
||||
|
||||
const urls = await collectVideoMediaUrls(video.id);
|
||||
|
||||
expect(new Set(urls)).toEqual(new Set([OWN_VERSION_VIDEO, OWN_COMMENT_IMAGE, OWN_ASSET_IMAGE]));
|
||||
});
|
||||
|
||||
// A youtube or bunny version's originalUrl is not an object this deployment
|
||||
// owns, and a BUNNY asset is cleaned up through the Bunny API instead.
|
||||
it('ignores versions from other providers and assets that are not R2 images', async () => {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { useState, type ChangeEvent } from 'react';
|
||||
import { useState, type ChangeEvent, type ClipboardEvent } from 'react';
|
||||
import { act, renderHook, type RenderHookResult } from '@testing-library/react';
|
||||
import { useCommentActions } from '@/components/video-page/hooks/use-comment-actions';
|
||||
import type { Comment, CommentTag, VideoData } from '@/components/video-page/types';
|
||||
@@ -31,7 +31,7 @@ function makeComment(overrides: Partial<Comment> = {}): Comment {
|
||||
timestampEnd: null,
|
||||
voiceUrl: null,
|
||||
voiceDuration: null,
|
||||
imageUrl: null,
|
||||
images: [],
|
||||
annotationData: null,
|
||||
isResolved: false,
|
||||
createdAt: '2026-01-01T00:00:00.000Z',
|
||||
@@ -79,7 +79,7 @@ function makeVideo(): VideoData {
|
||||
timestampEnd: null,
|
||||
voiceUrl: null,
|
||||
voiceDuration: null,
|
||||
imageUrl: null,
|
||||
images: [],
|
||||
annotationData: null,
|
||||
createdAt: '2026-01-01T00:01:00.000Z',
|
||||
author: { id: 'user2', name: 'Linus', image: null },
|
||||
@@ -495,7 +495,7 @@ describe('useCommentActions replying', () => {
|
||||
timestampEnd: null,
|
||||
voiceUrl: null,
|
||||
voiceDuration: null,
|
||||
imageUrl: null,
|
||||
images: [],
|
||||
annotationData: null,
|
||||
createdAt: '2026-01-02T00:00:00.000Z',
|
||||
author: { id: 'user1', name: 'Ada', image: null },
|
||||
@@ -815,6 +815,7 @@ describe('useCommentActions editing', () => {
|
||||
|
||||
expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0])).toEqual({
|
||||
content: 'Reworded note',
|
||||
imageUrls: [],
|
||||
});
|
||||
expect(findComment(harness, 'c1')?.tag).toEqual(TAGS[0]);
|
||||
});
|
||||
@@ -832,6 +833,7 @@ describe('useCommentActions editing', () => {
|
||||
|
||||
expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0])).toEqual({
|
||||
content: 'Reworded note',
|
||||
imageUrls: [],
|
||||
tagId: null,
|
||||
});
|
||||
expect(findComment(harness, 'c1')?.tag).toBeNull();
|
||||
@@ -853,6 +855,196 @@ describe('useCommentActions editing', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// A screenshot batch arrives as several clipboard items in one paste, and the
|
||||
// composer used to keep only the first of them.
|
||||
describe('useCommentActions image attachments', () => {
|
||||
// A one-pixel PNG header is enough: the client only sniffs the magic bytes.
|
||||
function pngFile(name: string): File {
|
||||
return new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])], name, {
|
||||
type: 'image/png',
|
||||
});
|
||||
}
|
||||
|
||||
function pasteOf(files: File[]) {
|
||||
return {
|
||||
clipboardData: {
|
||||
items: files.map((file) => ({ type: file.type, getAsFile: () => file })),
|
||||
},
|
||||
preventDefault: vi.fn(),
|
||||
} as unknown as ClipboardEvent<HTMLTextAreaElement>;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
let uploaded = 0;
|
||||
fetchMock.mockImplementation((url: string) => {
|
||||
if (url === '/api/upload/image') {
|
||||
uploaded += 1;
|
||||
return Promise.resolve(ok({ data: { url: `/api/upload/image/shot-${uploaded}.png` } }));
|
||||
}
|
||||
if (url === `/api/versions/${ACTIVE_VERSION}/comments`) {
|
||||
return Promise.resolve(ok({ data: serverComment }));
|
||||
}
|
||||
return Promise.resolve(ok({ data: {} }));
|
||||
});
|
||||
});
|
||||
|
||||
it('stages every image in a single paste', async () => {
|
||||
const harness = renderActions();
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(
|
||||
pasteOf([pngFile('a.png'), pngFile('b.png'), pngFile('c.png')])
|
||||
);
|
||||
});
|
||||
|
||||
expect(harness.result.current.actions.imageFiles.map((file) => file.name)).toEqual([
|
||||
'a.png',
|
||||
'b.png',
|
||||
'c.png',
|
||||
]);
|
||||
});
|
||||
|
||||
it('stops at the cap and says so', async () => {
|
||||
const harness = renderActions();
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(
|
||||
pasteOf(['a', 'b', 'c', 'd', 'e', 'f'].map((name) => pngFile(`${name}.png`)))
|
||||
);
|
||||
});
|
||||
|
||||
expect(harness.result.current.actions.imageFiles).toHaveLength(5);
|
||||
expect(toastError).toHaveBeenCalledWith('Only 5 more images fit on this comment');
|
||||
});
|
||||
|
||||
it('uploads each staged image and posts the whole list', async () => {
|
||||
const harness = renderActions();
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(
|
||||
pasteOf([pngFile('a.png'), pngFile('b.png')])
|
||||
);
|
||||
});
|
||||
act(() => harness.result.current.actions.setCommentText('Two shots'));
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handleAddComment();
|
||||
});
|
||||
|
||||
expect(callsTo('/api/upload/image', 'POST')).toHaveLength(2);
|
||||
expect(bodyOf(callsTo(`/api/versions/${ACTIVE_VERSION}/comments`, 'POST')[0])).toEqual({
|
||||
content: 'Two shots',
|
||||
timestamp: 12,
|
||||
imageUrls: ['/api/upload/image/shot-1.png', '/api/upload/image/shot-2.png'],
|
||||
});
|
||||
expect(harness.result.current.actions.imageFiles).toEqual([]);
|
||||
});
|
||||
|
||||
it('sends the images a reply was pasted into', async () => {
|
||||
const harness = renderActions();
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(
|
||||
pasteOf([pngFile('a.png'), pngFile('b.png')]),
|
||||
'reply'
|
||||
);
|
||||
});
|
||||
act(() => harness.result.current.actions.setReplyText('Same here'));
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handleReplyComment('c1');
|
||||
});
|
||||
|
||||
const body = bodyOf(callsTo(`/api/versions/${ACTIVE_VERSION}/comments`, 'POST')[0]);
|
||||
expect(body.parentId).toBe('c1');
|
||||
expect(body.imageUrls).toEqual([
|
||||
'/api/upload/image/shot-1.png',
|
||||
'/api/upload/image/shot-2.png',
|
||||
]);
|
||||
// The composer's own staging must not have been touched by a reply paste.
|
||||
expect(harness.result.current.actions.imageFiles).toEqual([]);
|
||||
});
|
||||
|
||||
it('seeds the editor from the comment and saves only the images left on it', async () => {
|
||||
const harness = renderActions();
|
||||
const existing = makeComment({
|
||||
id: 'c1',
|
||||
images: [
|
||||
{ id: 'i1', url: '/api/upload/image/kept.png' },
|
||||
{ id: 'i2', url: '/api/upload/image/dropped.png' },
|
||||
],
|
||||
});
|
||||
|
||||
act(() => harness.result.current.actions.startEditingComment(existing));
|
||||
expect(harness.result.current.actions.editImageUrls).toEqual([
|
||||
'/api/upload/image/kept.png',
|
||||
'/api/upload/image/dropped.png',
|
||||
]);
|
||||
|
||||
act(() => harness.result.current.actions.removeEditImageUrl('/api/upload/image/dropped.png'));
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handleEditComment('c1');
|
||||
});
|
||||
|
||||
expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0]).imageUrls).toEqual([
|
||||
'/api/upload/image/kept.png',
|
||||
]);
|
||||
expect(findComment(harness, 'c1')?.images.map((image) => image.url)).toEqual([
|
||||
'/api/upload/image/kept.png',
|
||||
]);
|
||||
});
|
||||
|
||||
it('uploads an image pasted into an open editor and appends it to the comment', async () => {
|
||||
const harness = renderActions();
|
||||
const existing = makeComment({
|
||||
id: 'c1',
|
||||
images: [{ id: 'i1', url: '/api/upload/image/kept.png' }],
|
||||
});
|
||||
|
||||
act(() => harness.result.current.actions.startEditingComment(existing));
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(pasteOf([pngFile('new.png')]), 'edit');
|
||||
});
|
||||
|
||||
expect(harness.result.current.actions.editImageFiles).toHaveLength(1);
|
||||
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handleEditComment('c1');
|
||||
});
|
||||
|
||||
expect(callsTo('/api/upload/image', 'POST')).toHaveLength(1);
|
||||
expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0]).imageUrls).toEqual([
|
||||
'/api/upload/image/kept.png',
|
||||
'/api/upload/image/shot-1.png',
|
||||
]);
|
||||
// The editor closes on a successful save, so its staging has to be empty.
|
||||
expect(harness.result.current.actions.editImageFiles).toEqual([]);
|
||||
expect(harness.result.current.actions.editingCommentId).toBeNull();
|
||||
});
|
||||
|
||||
it('counts the images already on the comment against the cap', async () => {
|
||||
const harness = renderActions();
|
||||
const existing = makeComment({
|
||||
id: 'c1',
|
||||
images: [
|
||||
{ id: 'i1', url: '/api/upload/image/one.png' },
|
||||
{ id: 'i2', url: '/api/upload/image/two.png' },
|
||||
{ id: 'i3', url: '/api/upload/image/three.png' },
|
||||
{ id: 'i4', url: '/api/upload/image/four.png' },
|
||||
],
|
||||
});
|
||||
|
||||
act(() => harness.result.current.actions.startEditingComment(existing));
|
||||
await act(async () => {
|
||||
await harness.result.current.actions.handlePaste(
|
||||
pasteOf([pngFile('a.png'), pngFile('b.png'), pngFile('c.png')]),
|
||||
'edit'
|
||||
);
|
||||
});
|
||||
|
||||
expect(harness.result.current.actions.editImageFiles).toHaveLength(1);
|
||||
expect(toastError).toHaveBeenCalledWith('Only 1 more image fits on this comment');
|
||||
});
|
||||
});
|
||||
|
||||
describe('useCommentActions background refresh', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
@@ -42,7 +42,7 @@ function makeComment(overrides: Partial<Comment> = {}): Comment {
|
||||
timestampEnd: null,
|
||||
voiceUrl: null,
|
||||
voiceDuration: null,
|
||||
imageUrl: null,
|
||||
images: [],
|
||||
annotationData: null,
|
||||
isResolved: false,
|
||||
createdAt: '2026-01-01T00:00:00.000Z',
|
||||
@@ -325,7 +325,7 @@ describe('useVideoPageData loading comments', () => {
|
||||
timestampEnd: null,
|
||||
voiceUrl: null,
|
||||
voiceDuration: null,
|
||||
imageUrl: null,
|
||||
images: [],
|
||||
annotationData: null,
|
||||
createdAt: '2026-01-01T00:01:00.000Z',
|
||||
author: { id: 'user2', name: 'Linus', image: null },
|
||||
|
||||
@@ -15,6 +15,7 @@ export interface CreateCommentInput {
|
||||
tagId?: string | null;
|
||||
annotationData?: string | null;
|
||||
imageUrl?: string | null;
|
||||
imageUrls?: string[];
|
||||
voiceUrl?: string | null;
|
||||
voiceDuration?: number | null;
|
||||
isResolved?: boolean;
|
||||
@@ -23,6 +24,8 @@ export interface CreateCommentInput {
|
||||
|
||||
export async function createComment(input: CreateCommentInput): Promise<Comment> {
|
||||
const seq = nextSeq();
|
||||
// A comment's images live in their own table; `imageUrl` is the first of them.
|
||||
const imageUrls = input.imageUrls ?? (input.imageUrl ? [input.imageUrl] : []);
|
||||
return db.comment.create({
|
||||
data: {
|
||||
versionId: input.versionId,
|
||||
@@ -36,7 +39,8 @@ export async function createComment(input: CreateCommentInput): Promise<Comment>
|
||||
parentId: input.parentId ?? null,
|
||||
tagId: input.tagId ?? null,
|
||||
annotationData: input.annotationData ?? null,
|
||||
imageUrl: input.imageUrl ?? null,
|
||||
imageUrl: imageUrls[0] ?? null,
|
||||
images: { create: imageUrls.map((url, index) => ({ url, position: index })) },
|
||||
voiceUrl: input.voiceUrl ?? null,
|
||||
voiceDuration: input.voiceDuration ?? null,
|
||||
isResolved: input.isResolved ?? false,
|
||||
|
||||
@@ -68,6 +68,7 @@ const REVIEWED_MIGRATIONS = [
|
||||
'20260627140000_add_video_upload_multipart_id',
|
||||
'20260801120000_add_acquisition_analytics',
|
||||
'20260818120000_add_upload_reservation_purpose',
|
||||
'20260820120000_add_comment_images',
|
||||
];
|
||||
|
||||
/** Objects POST_PUSH_SQL must have produced. Verified after it runs. */
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { parseCommentImageUrls } from '@/lib/comment-images';
|
||||
|
||||
const A = '/api/upload/image/11111111-2222-3333-4444-555555555555.png';
|
||||
const B = '/api/upload/image/66666666-7777-8888-9999-aaaaaaaaaaaa.jpg';
|
||||
const C = '/api/upload/image/bbbbbbbb-cccc-dddd-eeee-ffffffffffff.webp';
|
||||
const D = '/api/upload/image/12121212-3434-5656-7878-909090909090.gif';
|
||||
const E = '/api/upload/image/abababab-cdcd-efef-0101-232323232323.png';
|
||||
const F = '/api/upload/image/45454545-6767-8989-0a0a-1b1b1b1b1b1b.png';
|
||||
|
||||
describe('parseCommentImageUrls', () => {
|
||||
it('reads an ordered list', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, B] })).toEqual({ urls: [A, B] });
|
||||
});
|
||||
|
||||
it('treats a comment with no images as an empty list', () => {
|
||||
expect(parseCommentImageUrls({})).toEqual({ urls: [] });
|
||||
expect(parseCommentImageUrls({ imageUrls: [] })).toEqual({ urls: [] });
|
||||
});
|
||||
|
||||
it('accepts the legacy single imageUrl as a one-element list', () => {
|
||||
expect(parseCommentImageUrls({ imageUrl: A })).toEqual({ urls: [A] });
|
||||
});
|
||||
|
||||
it('ignores imageUrl once imageUrls is given, so the list wins', () => {
|
||||
expect(parseCommentImageUrls({ imageUrl: A, imageUrls: [B] })).toEqual({ urls: [B] });
|
||||
});
|
||||
|
||||
it('collapses a URL repeated in one request', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, B, A] })).toEqual({ urls: [A, B] });
|
||||
});
|
||||
|
||||
it('allows exactly five images and refuses a sixth', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, B, C, D, E] })).toEqual({
|
||||
urls: [A, B, C, D, E],
|
||||
});
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, B, C, D, E, F] })).toEqual({
|
||||
error: 'A comment can have at most 5 images',
|
||||
});
|
||||
});
|
||||
|
||||
it('counts the cap after de-duplication', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, A, B, C, D, E] })).toEqual({
|
||||
urls: [A, B, C, D, E],
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
['a URL outside the upload API', 'https://evil.example.com/shot.png'],
|
||||
['a path traversal', '/api/upload/image/../../etc/passwd'],
|
||||
['an audio upload', '/api/upload/audio/11111111-2222-3333-4444-555555555555.webm'],
|
||||
['a filename that is not a uuid', '/api/upload/image/shot.png'],
|
||||
])('refuses %s', (_label, url) => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [url] })).toEqual({
|
||||
error: 'Image URL must reference an uploaded image file',
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a non-string entry', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: [A, 42] })).toEqual({
|
||||
error: 'Image URL must reference an uploaded image file',
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses imageUrls that is not an array', () => {
|
||||
expect(parseCommentImageUrls({ imageUrls: A })).toEqual({
|
||||
error: 'imageUrls must be an array of uploaded image URLs',
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a legacy imageUrl that is not a valid upload URL', () => {
|
||||
expect(parseCommentImageUrls({ imageUrl: 'https://evil.example.com/shot.png' })).toEqual({
|
||||
error: 'Image URL must reference an uploaded image file',
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user