(SP:2) [SHOP] harden admin photo submission and PDP review fixes

Author: liudmylasovetovs

frontend/app/[locale]/admin/shop/products/_components/ProductForm.tsx

@@ -171,10 +171,36 @@ function isSerializableUiPhoto(photo: UiPhoto): photo is SerializableUiPhoto {
   return false;
 }
 
+export const LEGACY_PHOTO_MIGRATION_REQUIRED_MESSAGE =
+  'Legacy product photos must be migrated before adding or reordering gallery photos.';
+
+class PhotoPlanSubmissionError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'PhotoPlanSubmissionError';
+  }
+}
+
+export function getPhotoPlanSubmissionError(photos: UiPhoto[]): string | null {
+  const hasLegacyPhotos = photos.some(photo => photo.source === 'legacy');
+  const hasNonLegacyPhotos = photos.some(photo => photo.source !== 'legacy');
+
+  if (hasLegacyPhotos && hasNonLegacyPhotos) {
+    return LEGACY_PHOTO_MIGRATION_REQUIRED_MESSAGE;
+  }
+
+  return null;
+}
+
 export function buildPhotoPlanSubmission(photos: UiPhoto[]): {
   photoPlan?: AdminProductPhotoPlan;
   newPhotos: Array<UiPhoto & { source: 'new'; uploadId: string; file: File }>;
 } {
+  const submissionError = getPhotoPlanSubmissionError(photos);
+  if (submissionError) {
+    throw new PhotoPlanSubmissionError(submissionError);
+  }
+
   const serializablePhotos = photos.filter(isSerializableUiPhoto);
 
   if (serializablePhotos.length === 0) {
@@ -200,6 +226,35 @@ export function buildPhotoPlanSubmission(photos: UiPhoto[]): {
   return { photoPlan, newPhotos };
 }
 
+function getBlobPreviewUrls(photos: UiPhoto[]): Set<string> {
+  return new Set(
+    photos
+      .filter(
+        photo => photo.source === 'new' && photo.previewUrl.startsWith('blob:')
+      )
+      .map(photo => photo.previewUrl)
+  );
+}
+
+export function revokeSupersededPhotoPreviewUrls(
+  previousPhotos: UiPhoto[],
+  nextPhotos: UiPhoto[]
+) {
+  const nextBlobPreviewUrls = getBlobPreviewUrls(nextPhotos);
+
+  previousPhotos.forEach(photo => {
+    if (photo.source !== 'new' || !photo.previewUrl.startsWith('blob:')) {
+      return;
+    }
+
+    if (nextBlobPreviewUrls.has(photo.previewUrl)) {
+      return;
+    }
+
+    URL.revokeObjectURL(photo.previewUrl);
+  });
+}
+
 export function ensureUiPhotos(fromInitial: {
   images?: ProductImage[];
   imageUrl?: string;
@@ -310,6 +365,20 @@ export function ProductForm({
     Partial<Record<CurrencyCode, string>>
   >({});
 
+  function replacePhotos(
+    nextOrUpdater: UiPhoto[] | ((prev: UiPhoto[]) => UiPhoto[])
+  ) {
+    setPhotos(prev => {
+      const next =
+        typeof nextOrUpdater === 'function'
+          ? nextOrUpdater(prev)
+          : nextOrUpdater;
+      revokeSupersededPhotoPreviewUrls(prev, next);
+      photosRef.current = next;
+      return next;
+    });
+  }
+
   useEffect(() => {
     if (mode !== 'edit') {
       hydratedKeyRef.current = null;
@@ -357,7 +426,7 @@ export function ProductForm({
     setDescription(initialValues.description ?? '');
     setIsActive(initialValues.isActive ?? true);
     setIsFeatured(initialValues.isFeatured ?? false);
-    setPhotos(
+    replacePhotos(
       ensureUiPhotos({
         images: initialValues.images,
         imageUrl: initialValues.imageUrl,
@@ -435,13 +504,13 @@ export function ProductForm({
         file,
       }));
 
-    setPhotos(prev => normalizeUiPhotos([...prev, ...nextPhotos]));
+    replacePhotos(prev => normalizeUiPhotos([...prev, ...nextPhotos]));
     setImageError(null);
     event.target.value = '';
   };
 
   const setPrimaryPhoto = (key: string) => {
-    setPhotos(prev =>
+    replacePhotos(prev =>
       prev.map(photo => ({
         ...photo,
         isPrimary: photo.key === key,
@@ -451,7 +520,7 @@ export function ProductForm({
   };
 
   const movePhoto = (key: string, direction: -1 | 1) => {
-    setPhotos(prev => {
+    replacePhotos(prev => {
       const index = prev.findIndex(photo => photo.key === key);
       if (index < 0) return prev;
 
@@ -466,14 +535,9 @@ export function ProductForm({
   };
 
   const removePhoto = (key: string) => {
-    setPhotos(prev => {
-      const removed = prev.find(photo => photo.key === key);
-      if (removed?.source === 'new' && removed.previewUrl.startsWith('blob:')) {
-        URL.revokeObjectURL(removed.previewUrl);
-      }
-
-      return normalizeUiPhotos(prev.filter(photo => photo.key !== key));
-    });
+    replacePhotos(prev =>
+      normalizeUiPhotos(prev.filter(photo => photo.key !== key))
+    );
   };
 
   const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
@@ -554,7 +618,24 @@ export function ProductForm({
       formData.append('isActive', isActive ? 'true' : 'false');
       formData.append('isFeatured', isFeatured ? 'true' : 'false');
 
-      const { photoPlan, newPhotos } = buildPhotoPlanSubmission(photos);
+      const photoSubmission = (() => {
+        try {
+          return buildPhotoPlanSubmission(photos);
+        } catch (photoPlanError) {
+          if (photoPlanError instanceof PhotoPlanSubmissionError) {
+            setImageError(photoPlanError.message);
+            return null;
+          }
+
+          throw photoPlanError;
+        }
+      })();
+
+      if (!photoSubmission) {
+        return;
+      }
+
+      const { photoPlan, newPhotos } = photoSubmission;
 
       if (photoPlan?.length) {
         formData.append('photoPlan', JSON.stringify(photoPlan));

frontend/lib/shop/data.ts

@@ -61,17 +61,27 @@ export interface ProductPageDisplayProduct {
   badge: ProductBadge;
 }
 
+type AvailableProductPageViewModelInput = {
+  kind: 'available';
+  commerceProduct: ShopProduct;
+};
+
+type UnavailableProductPageViewModelInput = {
+  kind: 'unavailable';
+  product: ProductPageDisplayProduct;
+  commerceProduct: null;
+};
+
 export type ProductPageData =
-  | {
-      kind: 'available';
-      product: ProductPageDisplayProduct;
-      commerceProduct: ShopProduct;
-    }
-  | {
-      kind: 'unavailable';
+  | (AvailableProductPageViewModelInput & {
       product: ProductPageDisplayProduct;
-      commerceProduct: null;
-    }
+    })
+  | UnavailableProductPageViewModelInput
+  | { kind: 'not_found' };
+
+type ProductPageViewModelInput =
+  | AvailableProductPageViewModelInput
+  | UnavailableProductPageViewModelInput
   | { kind: 'not_found' };
 
 export async function getProductPageData(
@@ -83,23 +93,16 @@ export async function getProductPageData(
   const dbProduct = await getPublicProductBySlug(slug, currency);
   if (dbProduct) {
     const mapped = mapToShopProduct(dbProduct);
-    if (mapped) {
-      return toProductPageViewModel({
-        kind: 'available',
-        product: toProductPageDisplayProduct({
-          id: mapped.id,
-          slug: mapped.slug,
-          name: mapped.name,
-          image: mapped.image,
-          images: mapped.images,
-          primaryImage: mapped.primaryImage,
-          description: mapped.description,
-          badge: mapped.badge ?? 'NONE',
-        }),
-        commerceProduct: mapped,
-      });
+    if (!mapped) {
+      throw new Error(
+        `Invalid shop product data for PDP: slug=${slug} productId=${dbProduct.id}`
+      );
     }
-    return { kind: 'not_found' };
+
+    return toProductPageViewModel({
+      kind: 'available',
+      commerceProduct: mapped,
+    });
   }
 
   const base = await getPublicProductBaseBySlug(slug);
@@ -186,7 +189,9 @@ function toProductPageDisplayProduct(input: {
   };
 }
 
-export function toProductPageViewModel(data: ProductPageData): ProductPageData {
+export function toProductPageViewModel(
+  data: ProductPageViewModelInput
+): ProductPageData {
   if (data.kind === 'not_found') return data;
 
   if (data.kind === 'available') {

frontend/lib/tests/shop/admin-product-photo-management.test.ts

@@ -247,7 +247,9 @@ describe.sequential('admin product photo management', () => {
       imageUrl: 'https://example.com/p2.png',
       imagePublicId: 'products/p2',
     });
-    expect(cloudinaryMocks.destroyProductImage).toHaveBeenCalledWith(
+    expect(cloudinaryMocks.destroyProductImage).toHaveBeenCalledTimes(1);
+    expect(cloudinaryMocks.destroyProductImage).toHaveBeenNthCalledWith(
+      1,
       primaryImage.imagePublicId
     );
   });

frontend/lib/tests/shop/product-gallery-view-model.test.ts

@@ -1,7 +1,17 @@
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
+
+const shopQueryMocks = vi.hoisted(() => ({
+  getPublicProductBySlug: vi.fn(),
+  getPublicProductBaseBySlug: vi.fn(),
+  getActiveProductsPage: vi.fn(),
+  getFeaturedProducts: vi.fn(),
+}));
+
+vi.mock('@/db/queries/shop/products', () => shopQueryMocks);
 
 import {
   getProductGalleryImages,
+  getProductPageData,
   toProductPageViewModel,
 } from '@/lib/shop/data';
 
@@ -87,30 +97,6 @@ describe('product gallery view model', () => {
   it('normalizes PDP display and commerce data into separate concrete branches', () => {
     const viewModel = toProductPageViewModel({
       kind: 'available',
-      product: {
-        id: 'product-1',
-        slug: 'product-1',
-        name: 'Product 1',
-        image: 'https://example.com/primary.png',
-        images: [
-          {
-            id: 'img-primary',
-            url: 'https://example.com/primary.png',
-            publicId: 'products/primary',
-            sortOrder: 0,
-            isPrimary: true,
-          },
-        ],
-        primaryImage: {
-          id: 'img-primary',
-          url: 'https://example.com/primary.png',
-          publicId: 'products/primary',
-          sortOrder: 0,
-          isPrimary: true,
-        },
-        description: 'desc',
-        badge: 'SALE',
-      },
       commerceProduct: {
         id: 'product-1',
         slug: 'product-1',
@@ -152,4 +138,35 @@ describe('product gallery view model', () => {
     expect(viewModel.commerceProduct.price).toBe(5000);
     expect(viewModel.commerceProduct.currency).toBe('USD');
   });
+
+  it('throws a descriptive error when a found PDP product cannot be mapped safely', async () => {
+    shopQueryMocks.getPublicProductBySlug.mockResolvedValueOnce({
+      id: 'db-product-1',
+      slug: 'broken-product',
+      title: 'Broken product',
+      description: null,
+      imageUrl: '',
+      imagePublicId: undefined,
+      price: 'not-a-number',
+      originalPrice: null,
+      currency: 'USD',
+      isActive: true,
+      isFeatured: false,
+      stock: 2,
+      sku: null,
+      category: null,
+      type: null,
+      colors: [],
+      sizes: [],
+      badge: 'NONE',
+      images: [],
+      primaryImage: undefined,
+      createdAt: new Date('2026-01-01T00:00:00.000Z'),
+      updatedAt: new Date('2026-01-01T00:00:00.000Z'),
+    });
+
+    await expect(getProductPageData('broken-product', 'en')).rejects.toThrow(
+      'Invalid shop product data for PDP: slug=broken-product productId=db-product-1'
+    );
+  });
 });