diff --git a/app/admin/page.tsx b/app/admin/page.tsx index 03a6e4e..61551ca 100644 --- a/app/admin/page.tsx +++ b/app/admin/page.tsx @@ -82,7 +82,7 @@ export default async function AdminDashboardPage() { where: { voiceUrl: { not: null } }, }), db.comment.count({ - where: { imageUrl: { not: null } }, + where: { images: { some: {} } }, }), ]); diff --git a/app/api/admin/feedback/[feedbackId]/route.ts b/app/api/admin/feedback/[feedbackId]/route.ts index 950f3e1..eefe707 100644 --- a/app/api/admin/feedback/[feedbackId]/route.ts +++ b/app/api/admin/feedback/[feedbackId]/route.ts @@ -105,8 +105,8 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { const [commentReferenced, feedbackReferenced, feedbackAttachmentReferenced] = await Promise.all([ - db.comment.findFirst({ - where: { imageUrl: url }, + db.commentImage.findFirst({ + where: { url }, select: { id: true }, }), userFeedbackDelegate.findFirst({ diff --git a/app/api/comments/[commentId]/route.ts b/app/api/comments/[commentId]/route.ts index 2156a86..683a57a 100644 --- a/app/api/comments/[commentId]/route.ts +++ b/app/api/comments/[commentId]/route.ts @@ -10,6 +10,14 @@ import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response import { getGuestIdentityFromRequest } from '@/lib/guest-identity'; import { runWithConcurrency } from '@/lib/async-pool'; import { validateAnnotationStrokes } from '@/lib/validation'; +import { parseCommentImageUrls } from '@/lib/comment-images'; +import { isFreshAttachment } from '@/lib/upload-freshness'; +import { extractImageFileNameFromProxyUrl, sanitizeAssetDisplayName } from '@/lib/video-assets'; +import { + reserveStorageQuota, + releaseStorageReservation, + UPLOAD_RESERVATION_PURPOSES, +} from '@/lib/storage-quota'; import { logError } from '@/lib/logger'; const CLEANUP_DELETE_CONCURRENCY = 5; @@ -36,6 +44,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, parentId: true, authorId: true, tagId: true, @@ -57,6 +66,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, parentId: true, authorId: true, tagId: true, @@ -103,6 +113,10 @@ export async function GET(request: NextRequest, { params }: RouteParams) { // PATCH /api/comments/[commentId] export async function PATCH(request: NextRequest, { params }: RouteParams) { + // Carried out of the try so the catch below can scope the release to the + // account the hold was opened against. + let attachmentReservationId: string | null = null; + let attachmentBilledUserId: string | null = null; try { const limited = await rateLimit(request, 'mutate'); if (limited) return limited; @@ -115,11 +129,12 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) { const comment = await db.comment.findUnique({ where: { id: commentId }, include: { + images: { select: { url: true }, orderBy: { position: 'asc' } }, version: { include: { video: { include: { - project: true, + project: { include: { workspace: { select: { ownerId: true } } } }, }, }, }, @@ -169,14 +184,69 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) { } } - // Only author can edit content or tag + // `imageUrls` (or the legacy `imageUrl`) is the full list the comment should + // end up with, so anything the caller left out is detached. + const wantsImageUpdate = body.imageUrls !== undefined || body.imageUrl !== undefined; + + // Only author can edit content, tag or attachments if ( - (content !== undefined || tagId !== undefined || annotationData !== undefined) && + (content !== undefined || + tagId !== undefined || + annotationData !== undefined || + wantsImageUpdate) && !canEditOwnContent ) { return apiErrors.forbidden('Only the author can edit comment content'); } + let desiredImageUrls: string[] = []; + let removedImageUrls: string[] = []; + let addedImages: { url: string; sizeBytes: bigint }[] = []; + + if (wantsImageUpdate) { + const parsedImageUrls = parseCommentImageUrls(body); + if ('error' in parsedImageUrls) { + return apiErrors.badRequest(parsedImageUrls.error); + } + desiredImageUrls = parsedImageUrls.urls; + + const existingUrls = comment.images.map((image) => image.url); + const addedUrls = desiredImageUrls.filter((url) => !existingUrls.includes(url)); + removedImageUrls = existingUrls.filter((url) => !desiredImageUrls.includes(url)); + + if (addedUrls.length > 0) { + // A file that already hangs off another comment would trip the unique + // index mid-transaction, so refuse it here and answer with a 400. + const alreadyClaimed = await db.commentImage.findFirst({ + where: { url: { in: addedUrls } }, + select: { id: true }, + }); + if (alreadyClaimed) { + return apiErrors.badRequest('Image is already attached to another comment'); + } + + const checks = await Promise.all( + addedUrls.map(async (url) => ({ url, ...(await isFreshAttachment(url, 'image')) })) + ); + if (checks.some((check) => !check.isFresh)) { + return apiErrors.badRequest('Image upload expired. Please upload again.'); + } + addedImages = checks; + } + + const addedBytes = addedImages.reduce((total, image) => total + image.sizeBytes, BigInt(0)); + if (addedBytes > BigInt(0)) { + const reserveResult = await reserveStorageQuota( + project.workspace.ownerId, + addedBytes, + UPLOAD_RESERVATION_PURPOSES.ATTACHMENT + ); + if ('error' in reserveResult) return reserveResult.error; + attachmentReservationId = reserveResult.reservationId; + attachmentBilledUserId = project.workspace.ownerId; + } + } + // Owner, author, members, or workspace members can resolve/unresolve if (isResolved !== undefined && !canResolveComment) { return apiErrors.forbidden('Only admins can resolve comments'); @@ -214,20 +284,79 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) { updateData.isResolved = isResolved; updateData.resolvedAt = isResolved ? new Date() : null; } + if (wantsImageUpdate) { + // The legacy column keeps pointing at the first image. + updateData.imageUrl = desiredImageUrls[0] ?? null; + } - const updatedComment = await db.comment.update({ - where: { id: commentId }, - data: updateData, - include: { - author: { select: { id: true, name: true, image: true } }, - tag: { select: { id: true, name: true, color: true } }, - replies: { - include: { - author: { select: { id: true, name: true, image: true } }, - tag: { select: { id: true, name: true, color: true } }, + const updatedComment = await db.$transaction(async (tx) => { + // Consume the hold inside the transaction so quota is never double-counted. + if (attachmentReservationId) { + await tx.uploadReservation.deleteMany({ + where: { + id: attachmentReservationId, + billedUserId: project.workspace.ownerId, + purpose: UPLOAD_RESERVATION_PURPOSES.ATTACHMENT, + }, + }); + } + + if (wantsImageUpdate) { + if (removedImageUrls.length > 0) { + // Only the link is dropped. The file stays in R2 and in the assets pane, + // which is where a detached upload is deleted from and where its storage + // is already accounted for. + await tx.commentImage.deleteMany({ + where: { commentId, url: { in: removedImageUrls } }, + }); + } + + for (const [index, url] of desiredImageUrls.entries()) { + const added = addedImages.find((image) => image.url === url); + if (!added) { + await tx.commentImage.update({ where: { url }, data: { position: index } }); + continue; + } + + await tx.commentImage.create({ data: { commentId, url, position: index } }); + + const fileName = extractImageFileNameFromProxyUrl(url); + await tx.videoAsset.create({ + data: { + videoId: comment.version.video.id, + kind: 'IMAGE', + provider: 'R2_IMAGE', + displayName: sanitizeAssetDisplayName(null, fileName || 'Comment Image'), + sourceUrl: url, + thumbnailUrl: url, + sizeBytes: added.sizeBytes, + uploadedByUserId: userId, + uploadedByGuestIdentityId: userId ? null : guestIdentityId, + uploadedByGuestName: userId + ? null + : sanitizeAssetDisplayName(comment.guestName, 'Guest').slice(0, 80), + billedUserId: project.workspace.ownerId, + }, + }); + } + } + + return tx.comment.update({ + where: { id: commentId }, + data: updateData, + include: { + author: { select: { id: true, name: true, image: true } }, + tag: { select: { id: true, name: true, color: true } }, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, + replies: { + include: { + author: { select: { id: true, name: true, image: true } }, + tag: { select: { id: true, name: true, color: true } }, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, + }, }, }, - }, + }); }); const updatedCommentData = Object.fromEntries( @@ -256,6 +385,11 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) { }); return withCacheControl(response, 'private, no-store'); } catch (error) { + await releaseStorageReservation( + attachmentReservationId, + attachmentBilledUserId, + UPLOAD_RESERVATION_PURPOSES.ATTACHMENT + ); logError('Error updating comment:', error); return apiErrors.internalError('Failed to update comment'); } @@ -282,7 +416,10 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { }, }, }, - replies: { select: { voiceUrl: true, imageUrl: true } }, + images: { select: { url: true } }, + replies: { + select: { voiceUrl: true, images: { select: { url: true } } }, + }, }, }); @@ -339,10 +476,10 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { // Collect all media URLs to delete from R2 (comment + its replies) const mediaUrls: string[] = []; if (comment.voiceUrl) mediaUrls.push(comment.voiceUrl); - if (comment.imageUrl) mediaUrls.push(comment.imageUrl); + for (const image of comment.images) mediaUrls.push(image.url); for (const reply of comment.replies) { if (reply.voiceUrl) mediaUrls.push(reply.voiceUrl); - if (reply.imageUrl) mediaUrls.push(reply.imageUrl); + for (const image of reply.images) mediaUrls.push(image.url); } await db.comment.delete({ where: { id: commentId } }); diff --git a/app/api/projects/[projectId]/videos/[videoId]/route.ts b/app/api/projects/[projectId]/videos/[videoId]/route.ts index 2ec011c..f1db1f8 100644 --- a/app/api/projects/[projectId]/videos/[videoId]/route.ts +++ b/app/api/projects/[projectId]/videos/[videoId]/route.ts @@ -50,6 +50,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, annotationData: true, parentId: true, authorId: true, @@ -75,6 +76,10 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { + select: { id: true, url: true }, + orderBy: { position: 'asc' }, + }, annotationData: true, parentId: true, authorId: true, diff --git a/app/api/versions/[versionId]/comments/route.ts b/app/api/versions/[versionId]/comments/route.ts index 5e81a92..68c144a 100644 --- a/app/api/versions/[versionId]/comments/route.ts +++ b/app/api/versions/[versionId]/comments/route.ts @@ -6,8 +6,6 @@ import { notifyProjectOwner } from '@/lib/notifications'; import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response'; import { validateShareLinkAccess } from '@/lib/share-links'; import { getShareSessionFromRequest } from '@/lib/share-session'; -import { HeadObjectCommand } from '@aws-sdk/client-s3'; -import { r2Client, R2_BUCKET_NAME } from '@/lib/r2'; import { ensureGuestIdentityFromRequest, getGuestIdentityFromRequest, @@ -20,6 +18,8 @@ import { sanitizeAssetDisplayName, } from '@/lib/video-assets'; import { validateAnnotationStrokes } from '@/lib/validation'; +import { parseCommentImageUrls } from '@/lib/comment-images'; +import { isFreshAttachment } from '@/lib/upload-freshness'; import { logError } from '@/lib/logger'; import { reserveStorageQuota, @@ -29,35 +29,8 @@ import { import { isValidEmailAddress, normalizeEmail } from '@/lib/email-validation'; type RouteParams = { params: Promise<{ versionId: string }> }; -const SAFE_IMAGE_PATH = - /^\/api\/upload\/image\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; const SAFE_AUDIO_PATH = /^\/api\/upload\/audio\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; -const UNATTACHED_UPLOAD_TTL_MS = 15 * 60 * 1000; - -type AttachmentCheck = { isFresh: boolean; sizeBytes: bigint }; - -async function isFreshAttachment(url: string, kind: 'audio' | 'image'): Promise { - const prefix = kind === 'audio' ? '/api/upload/audio/' : '/api/upload/image/'; - if (!url.startsWith(prefix)) return { isFresh: false, sizeBytes: BigInt(0) }; - - const filename = url.slice(prefix.length); - const key = kind === 'audio' ? `voice/${filename}` : `images/${filename}`; - - try { - const head = await r2Client.send( - new HeadObjectCommand({ - Bucket: R2_BUCKET_NAME, - Key: key, - }) - ); - if (!head.LastModified) return { isFresh: false, sizeBytes: BigInt(0) }; - const isFresh = Date.now() - head.LastModified.getTime() <= UNATTACHED_UPLOAD_TTL_MS; - return { isFresh, sizeBytes: BigInt(head.ContentLength ?? 0) }; - } catch { - return { isFresh: false, sizeBytes: BigInt(0) }; - } -} function normalizeEtag(value: string): string { return value.trim().replace(/^W\//, ''); @@ -153,6 +126,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, annotationData: true, parentId: true, authorId: true, @@ -175,6 +149,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, annotationData: true, parentId: true, authorId: true, @@ -275,10 +250,18 @@ export async function POST(request: NextRequest, { params }: RouteParams) { guestName, guestEmail, tagId, - imageUrl, annotationData, } = body; + // A comment carries a list of images now; `imageUrl` is still accepted as a + // one-element list so an older client keeps working. + const imageUrlsResult = parseCommentImageUrls(body); + if ('error' in imageUrlsResult) { + return apiErrors.badRequest(imageUrlsResult.error); + } + const attachedImageUrls = imageUrlsResult.urls; + const primaryImageUrl = attachedImageUrls[0] ?? null; + // Validate required fields if (timestamp === undefined || timestamp === null) { return apiErrors.badRequest('Timestamp is required'); @@ -324,7 +307,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { } } - if (!content && !voiceUrl && !imageUrl && !annotationData) { + if (!content && !voiceUrl && attachedImageUrls.length === 0 && !annotationData) { return apiErrors.badRequest( 'Either content, a voice recording, an image attachment, or an annotation is required' ); @@ -402,17 +385,15 @@ export async function POST(request: NextRequest, { params }: RouteParams) { voiceSizeBytes = voiceCheck.sizeBytes; } - if (imageUrl && !SAFE_IMAGE_PATH.test(imageUrl)) { - return apiErrors.badRequest('Image URL must reference an uploaded image file'); - } - let imageSizeBytes = BigInt(0); - if (imageUrl) { - const imageCheck = await isFreshAttachment(imageUrl, 'image'); - if (!imageCheck.isFresh) { - return apiErrors.badRequest('Image upload expired. Please upload again.'); - } - imageSizeBytes = imageCheck.sizeBytes; + // The uploads happened in parallel, so check them the same way rather than + // paying one R2 round trip per screenshot. + const imageChecks = await Promise.all( + attachedImageUrls.map(async (url) => ({ url, ...(await isFreshAttachment(url, 'image')) })) + ); + if (imageChecks.some((check) => !check.isFresh)) { + return apiErrors.badRequest('Image upload expired. Please upload again.'); } + const imageSizeBytes = imageChecks.reduce((total, check) => total + check.sizeBytes, BigInt(0)); const guestIdentity = isGuest ? ensureGuestIdentityFromRequest(request) : null; @@ -451,7 +432,10 @@ export async function POST(request: NextRequest, { params }: RouteParams) { parentId: parentId || null, voiceUrl: voiceUrl || null, voiceDuration: voiceDuration || null, - imageUrl: imageUrl || null, + imageUrl: primaryImageUrl, + images: { + create: attachedImageUrls.map((url, index) => ({ url, position: index })), + }, annotationData: serializedAnnotationData, authorId: session?.user?.id || null, guestName: isGuest ? guestName : null, @@ -463,18 +447,20 @@ export async function POST(request: NextRequest, { params }: RouteParams) { include: { author: { select: { id: true, name: true, image: true } }, tag: { select: { id: true, name: true, color: true } }, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, replies: { include: { author: { select: { id: true, name: true, image: true } }, tag: { select: { id: true, name: true, color: true } }, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, }, }, }, }); - // If an image was attached to the comment, also add it to the assets pane - if (imageUrl) { - const fileName = extractImageFileNameFromProxyUrl(imageUrl); + // Every attached image also shows up in the assets pane + for (const check of imageChecks) { + const fileName = extractImageFileNameFromProxyUrl(check.url); const displayName = sanitizeAssetDisplayName(null, fileName || 'Comment Image'); const safeGuestName = sanitizeAssetDisplayName(guestName, 'Guest').slice(0, 80); @@ -484,9 +470,9 @@ export async function POST(request: NextRequest, { params }: RouteParams) { kind: 'IMAGE', provider: 'R2_IMAGE', displayName, - sourceUrl: imageUrl, - thumbnailUrl: imageUrl, - sizeBytes: imageSizeBytes, + sourceUrl: check.url, + thumbnailUrl: check.url, + sizeBytes: check.sizeBytes, uploadedByUserId: session?.user?.id || null, uploadedByGuestIdentityId: isGuest ? (guestIdentity?.identityId ?? null) : null, uploadedByGuestName: isGuest ? safeGuestName : null, @@ -543,7 +529,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { projectName: project.name, videoTitle, replyAuthor: commentAuthorName, - replyText: content?.trim() || (imageUrl ? '(image attachment)' : '(voice note)'), + replyText: content?.trim() || (primaryImageUrl ? '(image attachment)' : '(voice note)'), parentAuthor: parentComment?.author?.name || parentComment?.guestName || 'Someone', timestamp: ts, url: `${baseUrl}/watch/${version.video.id}`, @@ -554,7 +540,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { projectName: project.name, videoTitle, commentAuthor: commentAuthorName, - commentText: content?.trim() || (imageUrl ? '(image attachment)' : '(voice note)'), + commentText: content?.trim() || (primaryImageUrl ? '(image attachment)' : '(voice note)'), timestamp: ts, url: `${baseUrl}/watch/${version.video.id}`, }).catch((err) => logError('Notification failed:', err)); diff --git a/app/api/videos/[videoId]/assets/[assetId]/route.ts b/app/api/videos/[videoId]/assets/[assetId]/route.ts index b15c16e..b7d99a1 100644 --- a/app/api/videos/[videoId]/assets/[assetId]/route.ts +++ b/app/api/videos/[videoId]/assets/[assetId]/route.ts @@ -50,7 +50,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { if (asset.provider === VideoAssetProvider.R2_IMAGE) { const [assetReferenceCount, commentReferenceCount] = await Promise.all([ tx.videoAsset.count({ where: { sourceUrl: asset.sourceUrl } }), - tx.comment.count({ where: { imageUrl: asset.sourceUrl } }), + tx.commentImage.count({ where: { url: asset.sourceUrl } }), ]); shouldDeleteImageObject = assetReferenceCount === 0 && commentReferenceCount === 0; } @@ -73,7 +73,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { if (asset.thumbnailUrl) { const [assetThumbnailCount, commentImageCount] = await Promise.all([ tx.videoAsset.count({ where: { thumbnailUrl: asset.thumbnailUrl } }), - tx.comment.count({ where: { imageUrl: asset.thumbnailUrl } }), + tx.commentImage.count({ where: { url: asset.thumbnailUrl } }), ]); shouldDeleteVideoThumbnail = assetThumbnailCount === 0 && commentImageCount === 0; } diff --git a/app/api/watch/[videoId]/route.ts b/app/api/watch/[videoId]/route.ts index 770b7dd..667941e 100644 --- a/app/api/watch/[videoId]/route.ts +++ b/app/api/watch/[videoId]/route.ts @@ -49,6 +49,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, annotationData: true, parentId: true, authorId: true, @@ -72,6 +73,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { voiceUrl: true, voiceDuration: true, imageUrl: true, + images: { select: { id: true, url: true }, orderBy: { position: 'asc' } }, annotationData: true, parentId: true, authorId: true, diff --git a/components/video-page-content.tsx b/components/video-page-content.tsx index bfa52e6..f75d0a9 100644 --- a/components/video-page-content.tsx +++ b/components/video-page-content.tsx @@ -436,14 +436,14 @@ export function VideoPageContent({ recordingTime, audioBlob, isUploadingAudio, - imageBlob, - setImageBlob, + imageFiles, commentRangeStart, commentRangeEnd, toggleCommentRangeSelection, clearCommentRangeSelection, isUploadingImage, imageInputRef, + removeImageFile, handleAddComment, handleImageSelect, handlePaste, @@ -460,8 +460,7 @@ export function VideoPageContent({ isReplyRecording, replyRecordingTime, replyAudioBlob, - replyImageBlob, - setReplyImageBlob, + replyImageFiles, replyRangeStart, replyRangeEnd, toggleReplyRangeSelection, @@ -475,7 +474,6 @@ export function VideoPageContent({ cancelReplyRecording, submitReplyWithMedia, editingCommentId, - setEditingCommentId, editText, setEditText, editTagId, @@ -484,6 +482,13 @@ export function VideoPageContent({ setEditAnnotationData, isEditingAnnotation, setIsEditingAnnotation, + editImageUrls, + editImageFiles, + editImageInputRef, + startEditingComment, + startEditingReply, + cancelEditingComment, + removeEditImageUrl, isSubmittingEdit, handleEditComment, handleDeleteComment, @@ -853,13 +858,17 @@ export function VideoPageContent({ currentUserId={currentUserId} projectOwnerId={video.project.ownerId} editingCommentId={editingCommentId} - setEditingCommentId={setEditingCommentId} + startEditingComment={startEditingComment} + startEditingReply={startEditingReply} + cancelEditingComment={cancelEditingComment} editText={editText} setEditText={setEditText} editTagId={editTagId} setEditTagId={setEditTagId} - setEditAnnotationData={setEditAnnotationData} - setIsEditingAnnotation={setIsEditingAnnotation} + editImageUrls={editImageUrls} + editImageFiles={editImageFiles} + editImageInputRef={editImageInputRef} + removeEditImageUrl={removeEditImageUrl} onStartEditAnnotation={commentsActions.onStartEditAnnotation} isSubmittingEdit={isSubmittingEdit} availableTags={availableTags} @@ -888,9 +897,9 @@ export function VideoPageContent({ stopReplyRecording={stopReplyRecording} cancelReplyRecording={cancelReplyRecording} replyAudioBlob={replyAudioBlob} - replyImageBlob={replyImageBlob} - setReplyImageBlob={setReplyImageBlob} + replyImageFiles={replyImageFiles} replyImageInputRef={replyImageInputRef} + removeImageFile={removeImageFile} handleImageSelect={handleImageSelect} handlePaste={handlePaste} handleDrop={handleDrop} @@ -931,9 +940,9 @@ export function VideoPageContent({ stopRecording={stopRecording} cancelRecording={cancelRecording} audioBlob={audioBlob} - imageBlob={imageBlob} + imageFiles={imageFiles} imageInputRef={imageInputRef} - setImageBlob={setImageBlob} + removeImageFile={(index) => removeImageFile(index, 'comment')} commentText={commentText} setCommentText={setCommentText} commentRangeStart={commentRangeStart} diff --git a/components/video-page/assets-pane.tsx b/components/video-page/assets-pane.tsx index 58545cb..620c95a 100644 --- a/components/video-page/assets-pane.tsx +++ b/components/video-page/assets-pane.tsx @@ -38,7 +38,7 @@ import { AssetListSection } from '@/components/video-page/asset-list-section'; import type { DirectUploadProvider, VideoAsset } from '@/components/video-page/types'; import { uploadAssetVideoToR2 } from '@/lib/client/r2-asset-video-upload'; import { - extractPastedImageFile, + extractPastedImageFiles, validateImageFile, } from '@/components/video-page/image-upload-utils'; import { useCommentMedia } from '@/components/video-page/hooks/use-comment-media'; @@ -472,10 +472,10 @@ export const AssetsPane = memo(function AssetsPane({ const handleImagePaste = async (event: React.ClipboardEvent) => { if (uploadTab !== 'image' || !canUploadAssets || isCreatingAsset) return; - const pastedImage = extractPastedImageFile(event.clipboardData); - if (!pastedImage) return; + const pastedImages = extractPastedImageFiles(event.clipboardData); + if (pastedImages.length === 0) return; event.preventDefault(); - await stageImageFiles([pastedImage]); + await stageImageFiles(pastedImages); }; const handleCreateYoutubeAsset = async () => { diff --git a/components/video-page/comment-composer.tsx b/components/video-page/comment-composer.tsx index ae20336..394a8b2 100644 --- a/components/video-page/comment-composer.tsx +++ b/components/video-page/comment-composer.tsx @@ -2,18 +2,7 @@ import { memo, type RefObject } from 'react'; import Link from 'next/link'; -import { - Image as ImageIcon, - Loader2, - Mic, - Pause, - Pencil, - Play, - Send, - Tag, - Trash2, - X, -} from 'lucide-react'; +import { Image as ImageIcon, Loader2, Mic, Pause, Pencil, Play, Send, Tag, X } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { DropdownMenu, @@ -23,6 +12,8 @@ import { DropdownMenuTrigger, } from '@/components/ui/dropdown-menu'; import type { AnnotationStroke } from '@/components/annotation-canvas'; +import { ImageAttachmentStrip } from '@/components/video-page/image-attachments'; +import { MAX_COMMENT_IMAGES } from '@/lib/comment-images'; import { MentionTextarea } from '@/components/video-page/mention-textarea'; import type { CommentTag, VideoAsset } from '@/components/video-page/types'; @@ -32,9 +23,9 @@ interface CommentComposerProps { stopRecording: () => void; cancelRecording: () => void; audioBlob: Blob | null; - imageBlob: File | null; + imageFiles: File[]; imageInputRef: RefObject; - setImageBlob: (blob: File | null) => void; + removeImageFile: (index: number) => void; commentText: string; setCommentText: (value: string) => void; commentRangeStart: number | null; @@ -58,8 +49,8 @@ interface CommentComposerProps { handleAddComment: () => void; isSubmittingComment: boolean; startRecording: () => void; - handlePaste: (e: React.ClipboardEvent, isReply?: boolean) => void; - handleImageSelect: (e: React.ChangeEvent, isReply?: boolean) => void; + handlePaste: (e: React.ClipboardEvent) => void; + handleImageSelect: (e: React.ChangeEvent) => void; availableTags: CommentTag[]; selectedTagId: string | null; setSelectedTagId: (value: string | null) => void; @@ -75,9 +66,9 @@ export const CommentComposer = memo(function CommentComposer({ stopRecording, cancelRecording, audioBlob, - imageBlob, + imageFiles, imageInputRef, - setImageBlob, + removeImageFile, commentText, setCommentText, commentRangeStart, @@ -179,28 +170,7 @@ export const CommentComposer = memo(function CommentComposer({ - {imageBlob && ( -
- {/* eslint-disable-next-line @next/next/no-img-element */} - Preview -
- -
-
- )} + )} - {imageBlob && ( -
- {/* eslint-disable-next-line @next/next/no-img-element */} - Preview -
- -
-
- )} +
@@ -340,7 +289,7 @@ export const CommentComposer = memo(function CommentComposer({ size="icon" onClick={handleAddComment} disabled={ - (!commentText.trim() && !imageBlob && !annotationStrokes) || + (!commentText.trim() && imageFiles.length === 0 && !annotationStrokes) || isSubmittingComment || isUploadingImage } @@ -363,7 +312,8 @@ export const CommentComposer = memo(function CommentComposer({ size="icon" variant="outline" onClick={() => imageInputRef.current?.click()} - title="Attach Image" + disabled={imageFiles.length >= MAX_COMMENT_IMAGES} + title={`Attach images (up to ${MAX_COMMENT_IMAGES})`} > @@ -387,6 +337,7 @@ export const CommentComposer = memo(function CommentComposer({
-

Cmd+Enter to submit

+

+ Cmd+Enter to submit · paste or drop up to {MAX_COMMENT_IMAGES} images +

)} diff --git a/components/video-page/comments-pane.tsx b/components/video-page/comments-pane.tsx index 36ca8f7..9de5200 100644 --- a/components/video-page/comments-pane.tsx +++ b/components/video-page/comments-pane.tsx @@ -36,7 +36,19 @@ import { import { cn } from '@/lib/utils'; import { MentionTextarea } from '@/components/video-page/mention-textarea'; import { CommentRichText } from '@/components/video-page/comment-rich-text'; -import type { Comment, CommentTag, Version, VideoAsset } from '@/components/video-page/types'; +import { + CommentImageGallery, + ImageAttachmentStrip, +} from '@/components/video-page/image-attachments'; +import type { ImageAttachTarget } from '@/components/video-page/hooks/use-comment-actions'; +import { MAX_COMMENT_IMAGES } from '@/lib/comment-images'; +import type { + Comment, + CommentReply, + CommentTag, + Version, + VideoAsset, +} from '@/components/video-page/types'; interface CommentsPaneProps { isMobileCommentsOpen: boolean; @@ -63,13 +75,17 @@ interface CommentsPaneProps { currentUserId: string | null; projectOwnerId: string; editingCommentId: string | null; - setEditingCommentId: (id: string | null) => void; + startEditingComment: (comment: Comment) => void; + startEditingReply: (reply: CommentReply) => void; + cancelEditingComment: () => void; editText: string; setEditText: (value: string) => void; editTagId: string | null | undefined; setEditTagId: (value: string | null | undefined) => void; - setEditAnnotationData: (value: string | null | undefined) => void; - setIsEditingAnnotation: (value: boolean) => void; + editImageUrls: string[]; + editImageFiles: File[]; + editImageInputRef: RefObject; + removeEditImageUrl: (url: string) => void; onStartEditAnnotation: () => void; isSubmittingEdit: boolean; availableTags: CommentTag[]; @@ -94,7 +110,7 @@ interface CommentsPaneProps { handleReplyComment: ( parentId: string, voiceData?: { url: string; duration: number }, - imageData?: { url: string } + imageUrls?: string[] ) => void; startReplyRecording: () => void; isReplyRecording: boolean; @@ -102,12 +118,12 @@ interface CommentsPaneProps { stopReplyRecording: () => void; cancelReplyRecording: () => void; replyAudioBlob: Blob | null; - replyImageBlob: File | null; - setReplyImageBlob: (file: File | null) => void; + replyImageFiles: File[]; replyImageInputRef: RefObject; - handleImageSelect: (e: React.ChangeEvent, isReply?: boolean) => void; - handlePaste: (e: React.ClipboardEvent, isReply?: boolean) => void; - handleDrop: (e: React.DragEvent, isReply?: boolean) => void; + removeImageFile: (index: number, target: ImageAttachTarget) => void; + handleImageSelect: (e: React.ChangeEvent, target?: ImageAttachTarget) => void; + handlePaste: (e: React.ClipboardEvent, target?: ImageAttachTarget) => void; + handleDrop: (e: React.DragEvent, target?: ImageAttachTarget) => void; submitReplyWithMedia: (parentId: string) => void; isSubmittingReply: boolean; isUploadingReplyAudio: boolean; @@ -141,13 +157,17 @@ export const CommentsPane = memo(function CommentsPane({ currentUserId, projectOwnerId, editingCommentId, - setEditingCommentId, + startEditingComment, + startEditingReply, + cancelEditingComment, editText, setEditText, editTagId, setEditTagId, - setEditAnnotationData, - setIsEditingAnnotation, + editImageUrls, + editImageFiles, + editImageInputRef, + removeEditImageUrl, onStartEditAnnotation, isSubmittingEdit, availableTags, @@ -176,9 +196,9 @@ export const CommentsPane = memo(function CommentsPane({ stopReplyRecording, cancelReplyRecording, replyAudioBlob, - replyImageBlob, - setReplyImageBlob, + replyImageFiles, replyImageInputRef, + removeImageFile, handleImageSelect, handlePaste, handleDrop, @@ -241,12 +261,15 @@ export const CommentsPane = memo(function CommentsPane({ onDrop={(e) => { setIsPaneDraggingOver(false); if (activePane !== 'comments') return; - handleDrop(e, replyingTo !== null); + handleDrop( + e, + editingCommentId !== null ? 'edit' : replyingTo !== null ? 'reply' : 'comment' + ); }} > {isPaneDraggingOver && (
-

Drop image to attach

+

Drop images to attach

)}
@@ -446,13 +469,7 @@ export const CommentsPane = memo(function CommentsPane({ Reply {canEditComment && ( - { - setEditingCommentId(comment.id); - setEditText(comment.content || ''); - setEditTagId(comment.tag?.id || null); - }} - > + startEditingComment(comment)}> Edit @@ -486,19 +503,28 @@ export const CommentsPane = memo(function CommentsPane({ handleEditComment(comment.id); } if (e.key === 'Escape') { - setEditingCommentId(null); - setEditText(''); - setEditTagId(undefined); - setEditAnnotationData(undefined); - setIsEditingAnnotation(false); + cancelEditingComment(); } }} + onPaste={(e) => handlePaste(e, 'edit')} + /> + removeImageFile(index, 'edit')} + compact />
+ + handleImageSelect(e, 'edit')} + />
)} @@ -722,15 +754,7 @@ export const CommentsPane = memo(function CommentsPane({ {canEditReply && ( - { - setEditingCommentId(reply.id); - setEditText(reply.content || ''); - // No tag picker on a reply: undefined keeps - // the PATCH from carrying a tagId at all. - setEditTagId(undefined); - }} - > + startEditingReply(reply)}> Edit @@ -762,16 +786,28 @@ export const CommentsPane = memo(function CommentsPane({ handleEditComment(reply.id); } if (e.key === 'Escape') { - setEditingCommentId(null); - setEditText(''); + cancelEditingComment(); } }} + onPaste={(e) => handlePaste(e, 'edit')} + /> + removeImageFile(index, 'edit')} + compact />
+ + handleImageSelect(e, 'edit')} + />
) : ( @@ -804,19 +858,12 @@ export const CommentsPane = memo(function CommentsPane({ />

)} - {reply.imageUrl && ( -
setPreviewImage(reply.imageUrl)} - > - {/* eslint-disable-next-line @next/next/no-img-element */} - Attachment -
- )} + )} {reply.voiceUrl && ( @@ -943,30 +990,11 @@ export const CommentsPane = memo(function CommentsPane({ - {replyImageBlob && ( -
- {/* eslint-disable-next-line @next/next/no-img-element */} - Preview -
- -
-
- )} + removeImageFile(index, 'reply')} + compact + /> ) : ( <> - {replyImageBlob && ( -
- {/* eslint-disable-next-line @next/next/no-img-element */} - Preview -
- -
-
- )} + removeImageFile(index, 'reply')} + compact + />
handlePaste(e, true)} + onPaste={(e) => handlePaste(e, 'reply')} />
@@ -1127,7 +1138,7 @@ export const CommentsPane = memo(function CommentsPane({ size="sm" onClick={() => handleReplyComment(comment.id)} disabled={ - (!replyText.trim() && !replyImageBlob) || + (!replyText.trim() && replyImageFiles.length === 0) || isSubmittingReply || isUploadingReplyImage } diff --git a/components/video-page/hooks/use-comment-actions.ts b/components/video-page/hooks/use-comment-actions.ts index 097e125..6e9d5c6 100644 --- a/components/video-page/hooks/use-comment-actions.ts +++ b/components/video-page/hooks/use-comment-actions.ts @@ -17,15 +17,17 @@ import type { AnnotationCanvasHandle, AnnotationStroke } from '@/components/anno import type { Comment, CommentActionsConfig, + CommentImage, CommentReply, CommentTag, Version, VideoData, } from '@/components/video-page/types'; import { - extractPastedImageFile, + extractPastedImageFiles, validateImageFile, } from '@/components/video-page/image-upload-utils'; +import { MAX_COMMENT_IMAGES } from '@/lib/comment-images'; import { validateAnnotationStrokes } from '@/lib/validation'; import { withWebmDuration } from '@/lib/webm-duration'; import { ApiRequestError, apiRequestError, toastApiError } from '@/lib/client/api-error'; @@ -53,6 +55,9 @@ interface UseCommentActionsParams extends CommentActionsConfig { fetchAssets: () => Promise; } +/** Which of the three editors an attachment is being staged for. */ +export type ImageAttachTarget = 'comment' | 'reply' | 'edit'; + function getAudioUploadFilename(blob: Blob): string { const mime = blob.type.split(';')[0].trim().toLowerCase(); if (mime === 'audio/mp4') return 'recording.m4a'; @@ -91,7 +96,7 @@ export function useCommentActions({ const [recordingTime, setRecordingTime] = useState(0); const [audioBlob, setAudioBlob] = useState(null); const [isUploadingAudio, setIsUploadingAudio] = useState(false); - const [imageBlob, setImageBlob] = useState(null); + const [imageFiles, setImageFiles] = useState([]); const [isUploadingImage, setIsUploadingImage] = useState(false); const [commentRangeStart, setCommentRangeStart] = useState(null); const [commentRangeEnd, setCommentRangeEnd] = useState(null); @@ -108,7 +113,7 @@ export function useCommentActions({ const [replyRecordingTime, setReplyRecordingTime] = useState(0); const [replyAudioBlob, setReplyAudioBlob] = useState(null); const [isUploadingReplyAudio, setIsUploadingReplyAudio] = useState(false); - const [replyImageBlob, setReplyImageBlob] = useState(null); + const [replyImageFiles, setReplyImageFiles] = useState([]); const [isUploadingReplyImage, setIsUploadingReplyImage] = useState(false); const [replyRangeStart, setReplyRangeStart] = useState(null); const [replyRangeEnd, setReplyRangeEnd] = useState(null); @@ -130,6 +135,10 @@ export function useCommentActions({ undefined ); const [isEditingAnnotation, setIsEditingAnnotation] = useState(false); + // The images the edited comment keeps, and the ones staged to be added to it. + const [editImageUrls, setEditImageUrls] = useState([]); + const [editImageFiles, setEditImageFiles] = useState([]); + const editImageInputRef = useRef(null); const [isSubmittingEdit, setIsSubmittingEdit] = useState(false); const [previewImage, setPreviewImage] = useState(null); @@ -189,9 +198,98 @@ export function useCommentActions({ [isGuest, videoId] ); + /** Upload a batch of staged images and hand back their URLs, in the same order. */ + const uploadImageFiles = useCallback( + async (files: File[]): Promise => { + if (files.length === 0) return []; + + // One grant covers the whole batch: it is bound to the intent and the + // client, not to a single file. + const uploadToken = await getGuestUploadToken('image'); + + return Promise.all( + files.map(async (file) => { + const formData = new FormData(); + formData.append('image', file); + formData.append('videoId', videoId); + if (uploadToken) formData.append('uploadToken', uploadToken); + + const response = await fetch('/api/upload/image', { + method: 'POST', + body: formData, + }); + if (!response.ok) { + // The attachments go up before the comment does, so a full account + // fails here and never reaches the comment at all. Thrown with the + // code attached so the caller can offer the way out. + const payload = (await response.json().catch(() => null)) as { + error?: string; + code?: string; + } | null; + throw apiRequestError(payload, 'Failed to upload image'); + } + const payload = await response.json(); + return payload.data.url as string; + }) + ); + }, + [getGuestUploadToken, videoId] + ); + + /** Stage validated images on one of the editors, up to the per-comment cap. */ + const attachImageFiles = useCallback( + async (files: File[], target: ImageAttachTarget) => { + if (files.length === 0) return; + + for (const file of files) { + const imageError = await validateImageFile(file); + if (imageError) { + toast.error(imageError); + return; + } + } + + const staged = + target === 'reply' ? replyImageFiles : target === 'edit' ? editImageFiles : imageFiles; + // Images the edited comment already has count against the same cap. + const alreadyOnComment = target === 'edit' ? editImageUrls.length : 0; + const room = MAX_COMMENT_IMAGES - alreadyOnComment - staged.length; + if (room <= 0) { + toast.error(`A comment can have at most ${MAX_COMMENT_IMAGES} images`); + return; + } + if (files.length > room) { + toast.error( + room === 1 + ? 'Only 1 more image fits on this comment' + : `Only ${room} more images fit on this comment` + ); + } + + const next = [...staged, ...files.slice(0, room)]; + if (target === 'reply') setReplyImageFiles(next); + else if (target === 'edit') setEditImageFiles(next); + else setImageFiles(next); + }, + [editImageFiles, editImageUrls, imageFiles, replyImageFiles] + ); + + const removeImageFile = useCallback((index: number, target: ImageAttachTarget) => { + const drop = (files: File[]) => files.filter((_, current) => current !== index); + if (target === 'reply') setReplyImageFiles(drop); + else if (target === 'edit') setEditImageFiles(drop); + else setImageFiles(drop); + }, []); + const handleAddComment = useCallback( async (voiceData?: { url: string; duration: number }) => { - if (!voiceData && !imageBlob && !commentText.trim() && !annotationStrokes && !isAnnotating) + if ( + !voiceData && + imageFiles.length === 0 && + !commentText.trim() && + !annotationStrokes && + !isAnnotating + ) return; if (!activeVersion || !activeVersionId) return; @@ -206,14 +304,18 @@ export function useCommentActions({ const tempId = `temp-${Date.now()}`; const commentTimestamp = commentRangeStart ?? currentTime; const serializedAnnotation = effectiveStrokes ? JSON.stringify(effectiveStrokes) : null; + const hasImages = imageFiles.length > 0; const optimisticComment: Comment = { id: tempId, - content: voiceData || imageBlob ? commentText.trim() || null : commentText, + content: voiceData || hasImages ? commentText.trim() || null : commentText, timestamp: commentTimestamp, timestampEnd: commentRangeEnd, voiceUrl: voiceData?.url ?? null, voiceDuration: voiceData?.duration ?? null, - imageUrl: imageBlob ? URL.createObjectURL(imageBlob) : null, + images: imageFiles.map((file, index) => ({ + id: `${tempId}-image-${index}`, + url: URL.createObjectURL(file), + })), annotationData: serializedAnnotation, isResolved: false, createdAt: new Date().toISOString(), @@ -238,7 +340,7 @@ export function useCommentActions({ setCommentText(''); setSelectedTagId(availableTags.length > 0 ? availableTags[0].id : null); setAudioBlob(null); - setImageBlob(null); + setImageFiles([]); setAnnotationStrokes(null); setIsAnnotating(false); clearCommentRangeSelection(); @@ -248,44 +350,22 @@ export function useCommentActions({ isMutatingRef.current = true; try { - let imageData: { url: string } | undefined; + let uploadedImageUrls: string[] = []; - if (imageBlob) { + if (hasImages) { setIsUploadingImage(true); - const imageFormData = new FormData(); - imageFormData.append('image', imageBlob); - imageFormData.append('videoId', videoId); - const uploadToken = await getGuestUploadToken('image'); - if (uploadToken) imageFormData.append('uploadToken', uploadToken); - - const imageRes = await fetch('/api/upload/image', { - method: 'POST', - body: imageFormData, - }); - - if (!imageRes.ok) { - // The attachment goes up before the comment does, so a full account - // fails here and never reaches the comment at all. Thrown with the - // code attached so the catch below can offer the way out. - const imagePayload = (await imageRes.json().catch(() => null)) as { - error?: string; - code?: string; - } | null; - throw apiRequestError(imagePayload, 'Failed to upload image'); - } - const imageDataResponse = await imageRes.json(); - imageData = { url: imageDataResponse.data.url }; + uploadedImageUrls = await uploadImageFiles(imageFiles); } const res = await fetch(`/api/versions/${activeVersion.id}/comments`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - content: voiceData || imageBlob ? commentText.trim() || null : commentText, + content: voiceData || hasImages ? commentText.trim() || null : commentText, timestamp: commentTimestamp, ...(commentRangeEnd !== null && { timestampEnd: commentRangeEnd }), ...(voiceData && { voiceUrl: voiceData.url, voiceDuration: voiceData.duration }), - ...(imageData && { imageUrl: imageData.url }), + ...(uploadedImageUrls.length > 0 && { imageUrls: uploadedImageUrls }), ...(isGuest && normalizedGuestName && { guestName: normalizedGuestName }), ...(selectedTagId && { tagId: selectedTagId }), ...(effectiveStrokes && { annotationData: effectiveStrokes }), @@ -314,8 +394,8 @@ export function useCommentActions({ }; }); - // If an image was attached, refresh the assets list - if (imageData) { + // If images were attached, refresh the assets list + if (uploadedImageUrls.length > 0) { void fetchAssets(); } } else { @@ -369,11 +449,10 @@ export function useCommentActions({ currentUserName, selectedTagId, availableTags, - imageBlob, + imageFiles, + uploadImageFiles, annotationStrokes, isAnnotating, - videoId, - getGuestUploadToken, annotationCanvasRef, setSelectedTagId, setAnnotationStrokes, @@ -386,63 +465,34 @@ export function useCommentActions({ ); const handleImageSelect = useCallback( - async (e: ChangeEvent, isReply: boolean = false) => { - const file = e.target.files?.[0]; - if (!file) return; - - const imageError = await validateImageFile(file); - if (imageError) { - toast.error(imageError); - return; - } - - if (isReply) { - setReplyImageBlob(file); - } else { - setImageBlob(file); - } + async (e: ChangeEvent, target: ImageAttachTarget = 'comment') => { + const files = Array.from(e.target.files ?? []); + // Clearing the input lets the same file be picked again after it is removed. + e.target.value = ''; + await attachImageFiles(files, target); }, - [] + [attachImageFiles] ); const handlePaste = useCallback( - async (e: ClipboardEvent, isReply: boolean = false) => { - const file = extractPastedImageFile(e.clipboardData); - if (!file) return; + async (e: ClipboardEvent, target: ImageAttachTarget = 'comment') => { + const files = extractPastedImageFiles(e.clipboardData); + if (files.length === 0) return; e.preventDefault(); - - const imageError = await validateImageFile(file); - if (imageError) { - toast.error(imageError); - return; - } - - if (isReply) { - setReplyImageBlob(file); - } else { - setImageBlob(file); - } + await attachImageFiles(files, target); }, - [] + [attachImageFiles] ); - const handleDrop = useCallback(async (e: DragEvent, isReply: boolean = false) => { - e.preventDefault(); - const file = extractPastedImageFile(e.dataTransfer); - if (!file) return; - - const imageError = await validateImageFile(file); - if (imageError) { - toast.error(imageError); - return; - } - - if (isReply) { - setReplyImageBlob(file); - } else { - setImageBlob(file); - } - }, []); + const handleDrop = useCallback( + async (e: DragEvent, target: ImageAttachTarget = 'comment') => { + e.preventDefault(); + const files = extractPastedImageFiles(e.dataTransfer); + if (files.length === 0) return; + await attachImageFiles(files, target); + }, + [attachImageFiles] + ); const startRecording = useCallback(async () => { try { @@ -543,13 +593,13 @@ export function useCommentActions({ const submitCommentWithMedia = useCallback(async () => { if (!activeVersion) return; - if (audioBlob && !imageBlob && !commentText.trim()) { + if (audioBlob && imageFiles.length === 0 && !commentText.trim()) { submitVoiceComment(); return; } if (audioBlob) setIsUploadingAudio(true); - if (imageBlob) setIsUploadingImage(true); + if (imageFiles.length > 0) setIsUploadingImage(true); try { let voiceData: { url: string; duration: number } | undefined; @@ -569,7 +619,7 @@ export function useCommentActions({ setAudioBlob(null); setRecordingTime(0); - setImageBlob(null); + setImageFiles([]); if (imageInputRef.current) imageInputRef.current.value = ''; } catch (err) { console.error('Failed to submit comment with media:', err); @@ -580,7 +630,7 @@ export function useCommentActions({ } }, [ audioBlob, - imageBlob, + imageFiles, activeVersion, recordingTime, commentText, @@ -676,21 +726,25 @@ export function useCommentActions({ async ( parentId: string, voiceData?: { url: string; duration: number }, - imageData?: { url: string } + alreadyUploadedImageUrls?: string[] ) => { - if (!voiceData && !replyImageBlob && !replyText.trim()) return; + if (!voiceData && replyImageFiles.length === 0 && !replyText.trim()) return; if (!activeVersion || !activeVersionId) return; + const hasReplyImages = replyImageFiles.length > 0; const tempId = `temp-reply-${Date.now()}`; const replyTimestamp = replyRangeStart ?? currentTime; const optimisticReply: CommentReply = { id: tempId, - content: voiceData || replyImageBlob ? replyText.trim() || null : replyText, + content: voiceData || hasReplyImages ? replyText.trim() || null : replyText, timestamp: replyTimestamp, timestampEnd: replyRangeEnd, voiceUrl: voiceData?.url ?? null, voiceDuration: voiceData?.duration ?? null, - imageUrl: replyImageBlob ? URL.createObjectURL(replyImageBlob) : null, + images: replyImageFiles.map((file, index) => ({ + id: `${tempId}-image-${index}`, + url: URL.createObjectURL(file), + })), annotationData: null, createdAt: new Date().toISOString(), author: isGuest ? null : { id: 'current-user', name: currentUserName, image: null }, @@ -723,43 +777,31 @@ export function useCommentActions({ setReplyingTo(null); setReplyAudioBlob(null); setReplyRecordingTime(0); - setReplyImageBlob(null); + setReplyImageFiles([]); clearReplyRangeSelection(); setIsSubmittingReply(true); isMutatingRef.current = true; try { - let submittedImageData: { url: string } | undefined = imageData; + let submittedImageUrls: string[] = alreadyUploadedImageUrls ?? []; - if (replyImageBlob && !imageData) { + if (hasReplyImages && submittedImageUrls.length === 0) { setIsUploadingReplyImage(true); - const imageFormData = new FormData(); - imageFormData.append('image', replyImageBlob); - imageFormData.append('videoId', videoId); - const uploadToken = await getGuestUploadToken('image'); - if (uploadToken) imageFormData.append('uploadToken', uploadToken); - - const imageRes = await fetch('/api/upload/image', { - method: 'POST', - body: imageFormData, - }); - - if (!imageRes.ok) throw new Error('Failed to upload image reply'); - const imageDataResponse = await imageRes.json(); - submittedImageData = { url: imageDataResponse.data.url }; + submittedImageUrls = await uploadImageFiles(replyImageFiles); } const res = await fetch(`/api/versions/${activeVersion.id}/comments`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - content: voiceData || submittedImageData ? replyText.trim() || null : replyText, + content: + voiceData || submittedImageUrls.length > 0 ? replyText.trim() || null : replyText, timestamp: replyTimestamp, ...(replyRangeEnd !== null && { timestampEnd: replyRangeEnd }), parentId, ...(voiceData && { voiceUrl: voiceData.url, voiceDuration: voiceData.duration }), - ...(submittedImageData && { imageUrl: submittedImageData.url }), + ...(submittedImageUrls.length > 0 && { imageUrls: submittedImageUrls }), ...(isGuest && normalizedGuestName && { guestName: normalizedGuestName }), }), }); @@ -791,8 +833,8 @@ export function useCommentActions({ }; }); - // If an image was attached, refresh the assets list - if (submittedImageData) { + // If images were attached, refresh the assets list + if (submittedImageUrls.length > 0) { void fetchAssets(); } } else { @@ -852,9 +894,8 @@ export function useCommentActions({ isGuest, normalizedGuestName, currentUserName, - replyImageBlob, - videoId, - getGuestUploadToken, + replyImageFiles, + uploadImageFiles, setVideo, fetchAssets, clearReplyRangeSelection, @@ -949,13 +990,13 @@ export function useCommentActions({ async (parentId: string) => { if (!activeVersion) return; - if (replyAudioBlob && !replyImageBlob && !replyText.trim()) { + if (replyAudioBlob && replyImageFiles.length === 0 && !replyText.trim()) { submitVoiceReply(parentId); return; } if (replyAudioBlob) setIsUploadingReplyAudio(true); - if (replyImageBlob) setIsUploadingReplyImage(true); + if (replyImageFiles.length > 0) setIsUploadingReplyImage(true); try { let voiceData: { url: string; duration: number } | undefined; @@ -976,7 +1017,7 @@ export function useCommentActions({ setReplyAudioBlob(null); setReplyRecordingTime(0); - setReplyImageBlob(null); + setReplyImageFiles([]); if (replyImageInputRef.current) replyImageInputRef.current.value = ''; } catch (err) { console.error('Failed to submit reply with media:', err); @@ -988,7 +1029,7 @@ export function useCommentActions({ }, [ replyAudioBlob, - replyImageBlob, + replyImageFiles, activeVersion, replyRecordingTime, replyText, @@ -999,9 +1040,41 @@ export function useCommentActions({ ] ); + const startEditingComment = useCallback((comment: Comment) => { + setEditingCommentId(comment.id); + setEditText(comment.content || ''); + setEditTagId(comment.tag?.id || null); + setEditImageUrls(comment.images.map((image) => image.url)); + setEditImageFiles([]); + }, []); + + const startEditingReply = useCallback((reply: CommentReply) => { + setEditingCommentId(reply.id); + setEditText(reply.content || ''); + // No tag picker on a reply: undefined keeps the PATCH from carrying a tagId at all. + setEditTagId(undefined); + setEditImageUrls(reply.images.map((image) => image.url)); + setEditImageFiles([]); + }, []); + + const cancelEditingComment = useCallback(() => { + setEditingCommentId(null); + setEditText(''); + setEditTagId(undefined); + setEditAnnotationData(undefined); + setIsEditingAnnotation(false); + setEditImageUrls([]); + setEditImageFiles([]); + }, []); + + const removeEditImageUrl = useCallback((url: string) => { + setEditImageUrls((prev) => prev.filter((current) => current !== url)); + }, []); + const handleEditComment = useCallback( async (commentId: string) => { - if (!editText.trim() && !editAnnotationData) return; + const keepsImages = editImageUrls.length > 0 || editImageFiles.length > 0; + if (!editText.trim() && !editAnnotationData && !keepsImages) return; if (!activeVersionId) return; setIsSubmittingEdit(true); @@ -1016,7 +1089,12 @@ export function useCommentActions({ } try { - const body: Record = { content: editText }; + const uploadedImageUrls = await uploadImageFiles(editImageFiles); + // The list the comment should end up with: what the editor kept, then + // whatever was pasted into it while it was open. + const nextImageUrls = [...editImageUrls, ...uploadedImageUrls]; + + const body: Record = { content: editText, imageUrls: nextImageUrls }; if (editTagId !== undefined) body.tagId = editTagId; if (finalAnnotationData !== undefined) { body.annotationData = @@ -1028,10 +1106,21 @@ export function useCommentActions({ headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }); + const payload = (await res.json().catch(() => null)) as { + data?: { images?: CommentImage[] }; + error?: string; + code?: string; + } | null; + if (res.ok) { const editedTag = editTagId ? availableTags.find((t) => t.id === editTagId) || null : null; + // The response carries the saved rows with their real ids; fall back to + // the URLs that were sent if it did not come back as JSON. + const savedImages: CommentImage[] = + payload?.data?.images ?? + nextImageUrls.map((url, index) => ({ id: `${commentId}-image-${index}`, url })); setVideo((prev) => { if (!prev) return prev; return { @@ -1045,6 +1134,7 @@ export function useCommentActions({ return { ...c, content: editText.trim(), + images: savedImages, tag: editTagId !== undefined ? editedTag : c.tag, annotationData: finalAnnotationData !== undefined @@ -1054,7 +1144,9 @@ export function useCommentActions({ return { ...c, replies: (c.replies || []).map((r) => - r.id === commentId ? { ...r, content: editText.trim() } : r + r.id === commentId + ? { ...r, content: editText.trim(), images: savedImages } + : r ), }; }), @@ -1063,11 +1155,10 @@ export function useCommentActions({ ), }; }); - setEditingCommentId(null); - setEditText(''); - setEditTagId(undefined); - setEditAnnotationData(undefined); - setIsEditingAnnotation(false); + cancelEditingComment(); + if (uploadedImageUrls.length > 0) { + void fetchAssets(); + } if (finalAnnotationData !== undefined && finalAnnotationData) { try { const parsed = JSON.parse(finalAnnotationData); @@ -1079,9 +1170,13 @@ export function useCommentActions({ } else if (finalAnnotationData === null) { setViewingAnnotation(null); } + } else { + toastApiError(payload, 'Failed to save changes'); } - } catch (err) { - console.error('Failed to edit comment:', err); + } catch (error) { + // An upload can fail on quota before the comment is ever touched, and + // that message is worth showing; a network fault falls back. + toastApiError(error instanceof ApiRequestError ? error : null, 'Failed to save changes'); } finally { setIsSubmittingEdit(false); isMutatingRef.current = false; @@ -1091,12 +1186,17 @@ export function useCommentActions({ editText, editTagId, editAnnotationData, + editImageFiles, + editImageUrls, + uploadImageFiles, + cancelEditingComment, isEditingAnnotation, activeVersionId, availableTags, isGuest, normalizedGuestName, editAnnotationCanvasRef, + fetchAssets, setVideo, setViewingAnnotation, ] @@ -1203,14 +1303,15 @@ export function useCommentActions({ recordingTime, audioBlob, isUploadingAudio, - imageBlob, - setImageBlob, + imageFiles, + setImageFiles, commentRangeStart, commentRangeEnd, toggleCommentRangeSelection, clearCommentRangeSelection, isUploadingImage, imageInputRef, + removeImageFile, handleAddComment, handleImageSelect, handlePaste, @@ -1228,8 +1329,8 @@ export function useCommentActions({ isReplyRecording, replyRecordingTime, replyAudioBlob, - replyImageBlob, - setReplyImageBlob, + replyImageFiles, + setReplyImageFiles, replyRangeStart, replyRangeEnd, toggleReplyRangeSelection, @@ -1253,6 +1354,13 @@ export function useCommentActions({ setEditAnnotationData, isEditingAnnotation, setIsEditingAnnotation, + editImageUrls, + editImageFiles, + editImageInputRef, + startEditingComment, + startEditingReply, + cancelEditingComment, + removeEditImageUrl, isSubmittingEdit, handleEditComment, handleDeleteComment, diff --git a/components/video-page/hooks/use-object-urls.ts b/components/video-page/hooks/use-object-urls.ts new file mode 100644 index 0000000..1f1ef56 --- /dev/null +++ b/components/video-page/hooks/use-object-urls.ts @@ -0,0 +1,22 @@ +'use client'; + +import { useEffect, useMemo } from 'react'; + +/** + * Blob URLs for a list of staged files, revoked as soon as a file leaves the list. + * + * Calling `URL.createObjectURL` inline in the markup mints a new URL on every + * render and never releases any of them, which a five-screenshot preview grid + * turns into a steady leak. + */ +export function useObjectUrls(files: File[]): string[] { + const urls = useMemo(() => files.map((file) => URL.createObjectURL(file)), [files]); + + useEffect(() => { + return () => { + urls.forEach((url) => URL.revokeObjectURL(url)); + }; + }, [urls]); + + return urls; +} diff --git a/components/video-page/image-attachments.tsx b/components/video-page/image-attachments.tsx new file mode 100644 index 0000000..10cab13 --- /dev/null +++ b/components/video-page/image-attachments.tsx @@ -0,0 +1,128 @@ +'use client'; + +import { memo } from 'react'; +import { Trash2 } from 'lucide-react'; +import { Button } from '@/components/ui/button'; +import { cn } from '@/lib/utils'; +import { useObjectUrls } from '@/components/video-page/hooks/use-object-urls'; +import type { CommentImage } from '@/components/video-page/types'; + +interface ImageAttachmentStripProps { + /** Images already saved on the comment being edited, if any. */ + existingUrls?: string[]; + onRemoveExisting?: (url: string) => void; + /** Files staged in this editor and not uploaded yet. */ + files: File[]; + onRemoveFile: (index: number) => void; + compact?: boolean; + className?: string; +} + +/** + * The row of thumbnails under an editor, showing what will be sent with it. + * Saved images come first, then the ones staged in this session. + */ +export const ImageAttachmentStrip = memo(function ImageAttachmentStrip({ + existingUrls = [], + onRemoveExisting, + files, + onRemoveFile, + compact = false, + className, +}: ImageAttachmentStripProps) { + const previewUrls = useObjectUrls(files); + + if (existingUrls.length === 0 && previewUrls.length === 0) return null; + + const tileSize = compact ? 'h-14 w-14' : 'h-20 w-20'; + const buttonSize = compact ? 'h-5 w-5' : 'h-6 w-6'; + const iconSize = compact ? 'h-2.5 w-2.5' : 'h-3 w-3'; + + const tile = (key: string, src: string, alt: string, onRemove: () => void) => ( +
+ {/* eslint-disable-next-line @next/next/no-img-element */} + {alt} +
+ +
+
+ ); + + return ( +
+ {existingUrls.map((url, index) => + tile(url, url, `Attachment ${index + 1}`, () => onRemoveExisting?.(url)) + )} + {previewUrls.map((url, index) => + tile(`staged-${index}`, url, `Preview ${index + 1}`, () => onRemoveFile(index)) + )} +
+ ); +}); + +interface CommentImageGalleryProps { + images: CommentImage[]; + onOpen: (url: string) => void; + compact?: boolean; + className?: string; +} + +/** The images saved on a comment. One fills the width; several tile into a grid. */ +export const CommentImageGallery = memo(function CommentImageGallery({ + images, + onOpen, + compact = false, + className, +}: CommentImageGalleryProps) { + if (images.length === 0) return null; + + if (images.length === 1) { + return ( +
onOpen(images[0].url)} + > + {/* eslint-disable-next-line @next/next/no-img-element */} + Attachment +
+ ); + } + + return ( +
+ {images.map((image, index) => ( +
onOpen(image.url)} + > + {/* eslint-disable-next-line @next/next/no-img-element */} + {`Attachment +
+ ))} +
+ ); +}); diff --git a/components/video-page/image-upload-utils.ts b/components/video-page/image-upload-utils.ts index 05084bc..b4c8cff 100644 --- a/components/video-page/image-upload-utils.ts +++ b/components/video-page/image-upload-utils.ts @@ -18,16 +18,22 @@ export async function validateImageFile(file: File): Promise { return null; } -export function extractPastedImageFile(data: DataTransfer | null | undefined): File | null { +/** + * Every image on the clipboard or in a drop, in the order the browser lists them. + * A screenshot batch arrives as several items in one paste, so taking only the + * first would silently drop the rest. + */ +export function extractPastedImageFiles(data: DataTransfer | null | undefined): File[] { const items = data?.items; - if (!items) return null; + if (!items) return []; + const files: File[] = []; for (let i = 0; i < items.length; i++) { const item = items[i]; if (!item.type.startsWith('image/')) continue; const file = item.getAsFile(); - if (file) return file; + if (file) files.push(file); } - return null; + return files; } diff --git a/components/video-page/types.ts b/components/video-page/types.ts index 889b986..77ec7bb 100644 --- a/components/video-page/types.ts +++ b/components/video-page/types.ts @@ -75,6 +75,11 @@ export interface ApprovalRequest { decisions: ApprovalDecision[]; } +export interface CommentImage { + id: string; + url: string; +} + export interface CommentReply { id: string; content: string | null; @@ -82,7 +87,7 @@ export interface CommentReply { timestampEnd: number | null; voiceUrl: string | null; voiceDuration: number | null; - imageUrl: string | null; + images: CommentImage[]; annotationData: string | null; createdAt: string; author: { id: string; name: string | null; image: string | null } | null; @@ -99,7 +104,7 @@ export interface Comment { timestampEnd: number | null; voiceUrl: string | null; voiceDuration: number | null; - imageUrl: string | null; + images: CommentImage[]; annotationData: string | null; isResolved: boolean; createdAt: string; @@ -210,7 +215,7 @@ export interface VideoPageCommentsActions { onReplyComment: ( parentId: string, voiceData?: { url: string; duration: number }, - imageData?: { url: string } + imageUrls?: string[] ) => void; onSubmitReplyWithMedia: (parentId: string) => void; onStartEditAnnotation: () => void; diff --git a/lib/admin-stats.ts b/lib/admin-stats.ts index e042b0e..ae066c0 100644 --- a/lib/admin-stats.ts +++ b/lib/admin-stats.ts @@ -390,10 +390,10 @@ export async function getCachedUserMediaStorage(): Promise< const [mediaComments, imageAssets, audioAssets] = await Promise.all([ db.comment.findMany({ - where: { OR: [{ voiceUrl: { not: null } }, { imageUrl: { not: null } }] }, + where: { OR: [{ voiceUrl: { not: null } }, { images: { some: {} } }] }, select: { voiceUrl: true, - imageUrl: true, + images: { select: { url: true } }, version: { select: { video: { @@ -448,8 +448,8 @@ export async function getCachedUserMediaStorage(): Promise< } } - if (comment.imageUrl) { - const keyParts = comment.imageUrl.split('/'); + for (const image of comment.images) { + const keyParts = image.url.split('/'); const filename = keyParts[keyParts.length - 1]; const r2Key = `images/${filename}`; const dedupeKey = `${billedUserId}:${r2Key}`; diff --git a/lib/comment-images.ts b/lib/comment-images.ts new file mode 100644 index 0000000..217f783 --- /dev/null +++ b/lib/comment-images.ts @@ -0,0 +1,53 @@ +import { SAFE_IMAGE_PROXY_PATH } from '@/lib/image-upload-validation'; + +/** + * How many images a single comment (or reply) may carry. Pasting a batch of + * screenshots is the normal case, so the cap is there to bound the upload + * burst and the row width, not to make the feature scarce. + */ +export const MAX_COMMENT_IMAGES = 5; + +export type CommentImageUrlsResult = { urls: string[] } | { error: string }; + +/** + * Normalize whatever a client sent for a comment's images into an ordered, + * de-duplicated list of upload URLs. + * + * Accepts the legacy single `imageUrl` alongside the `imageUrls` list so an + * older client keeps working. Returns a message rather than throwing, because + * every caller turns it straight into a 400. + */ +export function parseCommentImageUrls(input: { + imageUrl?: unknown; + imageUrls?: unknown; +}): CommentImageUrlsResult { + const { imageUrl, imageUrls } = input; + + let raw: unknown[]; + if (imageUrls !== undefined && imageUrls !== null) { + if (!Array.isArray(imageUrls)) { + return { error: 'imageUrls must be an array of uploaded image URLs' }; + } + raw = imageUrls; + } else if (imageUrl !== undefined && imageUrl !== null) { + raw = [imageUrl]; + } else { + raw = []; + } + + const urls: string[] = []; + for (const value of raw) { + if (typeof value !== 'string' || !SAFE_IMAGE_PROXY_PATH.test(value)) { + return { error: 'Image URL must reference an uploaded image file' }; + } + // The same file twice would trip the unique index on comment_images and + // charge the account twice for one object, so collapse it here instead. + if (!urls.includes(value)) urls.push(value); + } + + if (urls.length > MAX_COMMENT_IMAGES) { + return { error: `A comment can have at most ${MAX_COMMENT_IMAGES} images` }; + } + + return { urls }; +} diff --git a/lib/image-upload-validation.ts b/lib/image-upload-validation.ts index 8bc0c13..879ddee 100644 --- a/lib/image-upload-validation.ts +++ b/lib/image-upload-validation.ts @@ -1,3 +1,7 @@ +/** The only shape an image URL may take once it has been through our upload API. */ +export const SAFE_IMAGE_PROXY_PATH = + /^\/api\/upload\/image\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; + export const ALLOWED_IMAGE_MIME_TYPES = [ 'image/jpeg', 'image/png', diff --git a/lib/r2-cleanup.ts b/lib/r2-cleanup.ts index 0acf2be..2cb7df4 100644 --- a/lib/r2-cleanup.ts +++ b/lib/r2-cleanup.ts @@ -85,10 +85,10 @@ export async function collectVideoMediaUrls(videoId: string): Promise const [comments, assets, versions] = await Promise.all([ db.comment.findMany({ where: { - OR: [{ voiceUrl: { not: null } }, { imageUrl: { not: null } }], + OR: [{ voiceUrl: { not: null } }, { images: { some: {} } }], version: { videoParentId: videoId }, }, - select: { voiceUrl: true, imageUrl: true }, + select: { voiceUrl: true, images: { select: { url: true } } }, }), db.videoAsset.findMany({ where: { @@ -105,7 +105,7 @@ export async function collectVideoMediaUrls(videoId: string): Promise const urls: string[] = []; comments.forEach((c) => { if (c.voiceUrl) urls.push(c.voiceUrl); - if (c.imageUrl) urls.push(c.imageUrl); + c.images.forEach((image) => urls.push(image.url)); }); assets.forEach((asset) => { if (asset.sourceUrl) urls.push(asset.sourceUrl); @@ -124,10 +124,10 @@ export async function collectProjectMediaUrls(projectId: string): Promise { if (c.voiceUrl) urls.push(c.voiceUrl); - if (c.imageUrl) urls.push(c.imageUrl); + c.images.forEach((image) => urls.push(image.url)); }); assets.forEach((asset) => { if (asset.sourceUrl) urls.push(asset.sourceUrl); @@ -163,10 +163,10 @@ export async function collectWorkspaceMediaUrls(workspaceId: string): Promise { if (c.voiceUrl) urls.push(c.voiceUrl); - if (c.imageUrl) urls.push(c.imageUrl); + c.images.forEach((image) => urls.push(image.url)); }); assets.forEach((asset) => { if (asset.sourceUrl) urls.push(asset.sourceUrl); diff --git a/lib/upload-freshness.ts b/lib/upload-freshness.ts new file mode 100644 index 0000000..4545e9a --- /dev/null +++ b/lib/upload-freshness.ts @@ -0,0 +1,40 @@ +import { HeadObjectCommand } from '@aws-sdk/client-s3'; +import { r2Client, R2_BUCKET_NAME } from '@/lib/r2'; + +/** + * How long an uploaded file may sit in R2 before the row that would claim it has + * to exist. Anything older is treated as an expired upload, so a URL cannot be + * replayed later to attach a file the caller no longer has a right to. + */ +export const UNATTACHED_UPLOAD_TTL_MS = 15 * 60 * 1000; + +export type AttachmentCheck = { isFresh: boolean; sizeBytes: bigint }; + +/** + * Confirm an upload URL points at an object that was written just now, and + * report its size so the caller can bill it against a quota. + */ +export async function isFreshAttachment( + url: string, + kind: 'audio' | 'image' +): Promise { + const prefix = kind === 'audio' ? '/api/upload/audio/' : '/api/upload/image/'; + if (!url.startsWith(prefix)) return { isFresh: false, sizeBytes: BigInt(0) }; + + const filename = url.slice(prefix.length); + const key = kind === 'audio' ? `voice/${filename}` : `images/${filename}`; + + try { + const head = await r2Client.send( + new HeadObjectCommand({ + Bucket: R2_BUCKET_NAME, + Key: key, + }) + ); + if (!head.LastModified) return { isFresh: false, sizeBytes: BigInt(0) }; + const isFresh = Date.now() - head.LastModified.getTime() <= UNATTACHED_UPLOAD_TTL_MS; + return { isFresh, sizeBytes: BigInt(head.ContentLength ?? 0) }; + } catch { + return { isFresh: false, sizeBytes: BigInt(0) }; + } +} diff --git a/lib/video-assets.ts b/lib/video-assets.ts index 0601ed9..b5f025f 100644 --- a/lib/video-assets.ts +++ b/lib/video-assets.ts @@ -6,13 +6,14 @@ import { getGuestIdentityFromRequest } from '@/lib/guest-identity'; import { getShareSessionFromRequest } from '@/lib/share-session'; import { validateShareLinkAccess } from '@/lib/share-links'; import { canDownloadProjectMedia } from '@/lib/project-download'; +import { SAFE_IMAGE_PROXY_PATH } from '@/lib/image-upload-validation'; + +export { SAFE_IMAGE_PROXY_PATH }; const IMAGE_PROXY_PREFIX = '/api/upload/image/'; const AUDIO_PROXY_PREFIX = '/api/upload/audio/'; const VIDEO_PROXY_PREFIX = '/api/upload/video/'; -export const SAFE_IMAGE_PROXY_PATH = - /^\/api\/upload\/image\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; export const SAFE_AUDIO_PROXY_PATH = /^\/api\/upload\/audio\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; export const SAFE_VIDEO_PROXY_PATH = diff --git a/prisma/migrations/20260820120000_add_comment_images/migration.sql b/prisma/migrations/20260820120000_add_comment_images/migration.sql new file mode 100644 index 0000000..f37808e --- /dev/null +++ b/prisma/migrations/20260820120000_add_comment_images/migration.sql @@ -0,0 +1,28 @@ +-- A comment used to hold at most one image, in "comments"."imageUrl". Screenshots +-- arrive in batches, so the images move into their own table and the old column +-- stays as a pointer to the first one for readers that have not been updated. +CREATE TABLE "comment_images" ( + "id" TEXT NOT NULL, + "url" TEXT NOT NULL, + "position" INTEGER NOT NULL DEFAULT 0, + "commentId" TEXT NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "comment_images_pkey" PRIMARY KEY ("id") +); + +-- An uploaded file belongs to exactly one comment, which is what the old +-- "comments_imageUrl_key" guaranteed. Reference checks before an R2 delete +-- rely on it. +CREATE UNIQUE INDEX "comment_images_url_key" ON "comment_images"("url"); +CREATE INDEX "comment_images_commentId_position_idx" ON "comment_images"("commentId", "position"); + +ALTER TABLE "comment_images" ADD CONSTRAINT "comment_images_commentId_fkey" + FOREIGN KEY ("commentId") REFERENCES "comments"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- Existing single attachments become the first image of their comment, so the +-- new table is the complete list from the first read after this migration. +INSERT INTO "comment_images" ("id", "url", "position", "commentId", "createdAt") +SELECT gen_random_uuid()::text, "imageUrl", 0, "id", "createdAt" +FROM "comments" +WHERE "imageUrl" IS NOT NULL; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 2c4a869..5c6021b 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -432,8 +432,10 @@ model Comment { voiceUrl String? // URL to voice recording file voiceDuration Float? // Duration of voice recording in seconds - // Image attachment (optional) + // Image attachment (optional). `imageUrl` is the first image and stays for + // backwards compatibility; `images` is the full, ordered list. imageUrl String? // URL to uploaded image file + images CommentImage[] // Annotation drawing data (JSON string of strokes) annotationData String? @db.Text @@ -481,6 +483,20 @@ model Comment { @@map("comments") } +model CommentImage { + id String @id @default(cuid()) + url String @unique + position Int @default(0) + + commentId String + comment Comment @relation(fields: [commentId], references: [id], onDelete: Cascade) + + createdAt DateTime @default(now()) + + @@index([commentId, position]) + @@map("comment_images") +} + model CommentTag { id String @id @default(cuid()) name String // e.g., "Feedback", "Technical", "Urgent" diff --git a/scripts/r2-orphan-cleanup.ts b/scripts/r2-orphan-cleanup.ts index e35bc98..a662026 100644 --- a/scripts/r2-orphan-cleanup.ts +++ b/scripts/r2-orphan-cleanup.ts @@ -146,44 +146,51 @@ async function findReferencedUrls(urls: string[]): Promise> { ).userFeedbackScreenshot; for (const group of chunk(urls, CHUNK_SIZE)) { - const [commentRows, feedbackRows, feedbackAttachmentRows, assetRows, versionRows] = - await Promise.all([ - db.comment.findMany({ - where: { - OR: [{ voiceUrl: { in: group } }, { imageUrl: { in: group } }], - }, - select: { - voiceUrl: true, - imageUrl: true, - }, - }), - db.userFeedback.findMany({ - where: { screenshotUrl: { in: group } }, - select: { screenshotUrl: true }, - }), - userFeedbackScreenshotDelegate - ? userFeedbackScreenshotDelegate.findMany({ - where: { url: { in: group } }, - select: { url: true }, - }) - : Promise.resolve([] as Array<{ url: string }>), - db.videoAsset.findMany({ - where: { - OR: [{ sourceUrl: { in: group } }, { thumbnailUrl: { in: group } }], - }, - select: { sourceUrl: true, thumbnailUrl: true }, - }), - db.videoVersion.findMany({ - where: { - OR: [{ originalUrl: { in: group } }, { thumbnailUrl: { in: group } }], - }, - select: { originalUrl: true, thumbnailUrl: true }, - }), - ]); + const [ + commentRows, + commentImageRows, + feedbackRows, + feedbackAttachmentRows, + assetRows, + versionRows, + ] = await Promise.all([ + db.comment.findMany({ + where: { voiceUrl: { in: group } }, + select: { voiceUrl: true }, + }), + db.commentImage.findMany({ + where: { url: { in: group } }, + select: { url: true }, + }), + db.userFeedback.findMany({ + where: { screenshotUrl: { in: group } }, + select: { screenshotUrl: true }, + }), + userFeedbackScreenshotDelegate + ? userFeedbackScreenshotDelegate.findMany({ + where: { url: { in: group } }, + select: { url: true }, + }) + : Promise.resolve([] as Array<{ url: string }>), + db.videoAsset.findMany({ + where: { + OR: [{ sourceUrl: { in: group } }, { thumbnailUrl: { in: group } }], + }, + select: { sourceUrl: true, thumbnailUrl: true }, + }), + db.videoVersion.findMany({ + where: { + OR: [{ originalUrl: { in: group } }, { thumbnailUrl: { in: group } }], + }, + select: { originalUrl: true, thumbnailUrl: true }, + }), + ]); for (const row of commentRows) { if (row.voiceUrl) referenced.add(row.voiceUrl); - if (row.imageUrl) referenced.add(row.imageUrl); + } + for (const row of commentImageRows) { + if (row.url) referenced.add(row.url); } for (const row of feedbackRows) { if (row.screenshotUrl) referenced.add(row.screenshotUrl); diff --git a/tests/api/comments.test.ts b/tests/api/comments.test.ts index 88f853f..b6763c9 100644 --- a/tests/api/comments.test.ts +++ b/tests/api/comments.test.ts @@ -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(); + 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 { + 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(); diff --git a/tests/api/lib-r2-cleanup.test.ts b/tests/api/lib-r2-cleanup.test.ts index 6a392f2..943b404 100644 --- a/tests/api/lib-r2-cleanup.test.ts +++ b/tests/api/lib-r2-cleanup.test.ts @@ -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 () => { diff --git a/tests/component/hooks/use-comment-actions.test.ts b/tests/component/hooks/use-comment-actions.test.ts index c3affab..cb54aef 100644 --- a/tests/component/hooks/use-comment-actions.test.ts +++ b/tests/component/hooks/use-comment-actions.test.ts @@ -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 { 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; + } + + 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(); diff --git a/tests/component/hooks/use-video-page-data.test.ts b/tests/component/hooks/use-video-page-data.test.ts index fe8fc8d..10e633b 100644 --- a/tests/component/hooks/use-video-page-data.test.ts +++ b/tests/component/hooks/use-video-page-data.test.ts @@ -42,7 +42,7 @@ function makeComment(overrides: Partial = {}): 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 }, diff --git a/tests/factories/comment.ts b/tests/factories/comment.ts index 52a9503..29e217a 100644 --- a/tests/factories/comment.ts +++ b/tests/factories/comment.ts @@ -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 { 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 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, diff --git a/tests/setup/db-global.ts b/tests/setup/db-global.ts index f70eff3..cf69ee0 100644 --- a/tests/setup/db-global.ts +++ b/tests/setup/db-global.ts @@ -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. */ diff --git a/tests/unit/lib/comment-images.test.ts b/tests/unit/lib/comment-images.test.ts new file mode 100644 index 0000000..151fb0d --- /dev/null +++ b/tests/unit/lib/comment-images.test.ts @@ -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', + }); + }); +});