refactor(overlays): tighten phase 1 review fixes

enoch85 · enoch85 · commit 44d5cbca61bb · 2026-05-07T09:23:56.000Z
- Promote process result + request schema to @maintainerr/contracts so
  server and UI share one wire shape; drop the duplicate UI interface
  (skipped was incorrectly optional) and the inline zod schema in the
  controller.
- Remove the appliedMediaItems recursion sentinel from the public
  processCollection signature so the lock check can't be bypassed by a
  truthy empty array; processAllCollections now calls the internal
  helper directly.
- Gate DELETE /overlays/reset against concurrent processing and add
  spec coverage for both reset-while-disabled and reset-while-running.
- Surface the skipped count in the inline overlay summary alert.
- Replace the TriggerRuleButton re-export shim with a direct import of
  TriggerRuleActionButton at the one usage site, and revert the no-op
  canTestMedia change on CollectionDetailPage.
diff --git a/apps/server/src/modules/overlays/overlay-processor.service.spec.ts b/apps/server/src/modules/overlays/overlay-processor.service.spec.ts
@@ -247,11 +247,7 @@ describe('OverlayProcessorService', () => {
 
     jest.spyOn(service, 'applyTemplateOverlay').mockResolvedValue(true);
 
-    const result = await service.processCollection(
-      collection as any,
-      undefined,
-      true,
-    );
+    const result = await service.processCollection(collection as any, true);
 
     expect(service.applyTemplateOverlay).toHaveBeenCalledWith(
       'media-1',
diff --git a/apps/server/src/modules/overlays/overlay-processor.service.ts b/apps/server/src/modules/overlays/overlay-processor.service.ts
@@ -1,5 +1,6 @@
 import {
   MaintainerrEvent,
+  OverlayProcessorRunResult,
   OverlayResult,
   OverlayTemplate,
   OverlayTemplateMode,
@@ -26,12 +27,7 @@ import { IOverlayProvider } from './providers/overlay-provider.interface';
 
 export type ProcessorStatus = 'idle' | 'running' | 'error';
 
-export interface ProcessorRunResult {
-  processed: number;
-  reverted: number;
-  skipped: number;
-  errors: number;
-}
+export type ProcessorRunResult = OverlayProcessorRunResult;
 
 type RevertItemResult = 'restored' | 'failed' | 'no-backup' | 'item-gone';
 
@@ -368,17 +364,8 @@ export class OverlayProcessorService {
 
   async processCollection(
     collection: Collection & { collectionMedia: CollectionMedia[] },
-    appliedMediaItems?: { mediaServerId: string }[],
     force = false,
   ): Promise<ProcessorRunResult> {
-    if (appliedMediaItems) {
-      return this.processCollectionInternal(
-        collection,
-        appliedMediaItems,
-        force,
-      );
-    }
-
     if (this.status === 'running') {
       this.logger.warn('Overlay processor is already running, skipping');
       return this.createEmptyResult();
@@ -487,7 +474,7 @@ export class OverlayProcessorService {
         this.logger.log(
           `--- Processing: "${coll.title}" (${coll.collectionMedia.length} items) ---`,
         );
-        const collResult = await this.processCollection(
+        const collResult = await this.processCollectionInternal(
           coll,
           appliedMediaItems,
           force,
diff --git a/apps/server/src/modules/overlays/overlays.controller.spec.ts b/apps/server/src/modules/overlays/overlays.controller.spec.ts
@@ -46,6 +46,7 @@ describe('OverlaysController', () => {
     status: 'idle' | 'running' | 'error';
     processAllCollections: jest.Mock;
     processCollection: jest.Mock;
+    resetAllOverlays: jest.Mock;
   };
   let collectionsService: {
     getCollection: jest.Mock;
@@ -65,6 +66,7 @@ describe('OverlaysController', () => {
       status: 'idle',
       processAllCollections: jest.fn(),
       processCollection: jest.fn(),
+      resetAllOverlays: jest.fn(),
     };
     collectionsService = {
       getCollection: jest.fn(),
@@ -336,4 +338,23 @@ describe('OverlaysController', () => {
 
     expect(processorService.processCollection).toHaveBeenCalledWith(collection);
   });
+
+  it('allows reset while overlays are globally disabled', async () => {
+    processorService.resetAllOverlays.mockResolvedValue(undefined);
+
+    await expect(controller.resetAll()).resolves.toEqual({ success: true });
+
+    expect(processorService.resetAllOverlays).toHaveBeenCalled();
+  });
+
+  it('rejects reset with 409 while a processor run is in progress', async () => {
+    processorService.status = 'running';
+
+    await expect(controller.resetAll()).rejects.toMatchObject({
+      status: 409,
+      response: 'Overlay processing is already running',
+    });
+
+    expect(processorService.resetAllOverlays).not.toHaveBeenCalled();
+  });
 });
diff --git a/apps/server/src/modules/overlays/overlays.controller.ts b/apps/server/src/modules/overlays/overlays.controller.ts
@@ -41,7 +41,10 @@ import * as fs from 'fs';
 import { ZodValidationPipe } from 'nestjs-zod';
 import * as path from 'path';
 import sharp from 'sharp';
-import { z } from 'zod';
+import {
+  type OverlayProcessRequest,
+  overlayProcessRequestSchema,
+} from '@maintainerr/contracts';
 import { dataDir as configDataDir } from '../../app/config/dataDir';
 import { MediaServerSetupGuard } from '../api/media-server/guards/media-server-setup.guard';
 import { CollectionsService } from '../collections/collections.service';
@@ -53,14 +56,6 @@ import { OverlayTemplateService } from './overlay-template.service';
 import { OverlayProviderFactory } from './providers/overlay-provider.factory';
 import { IOverlayProvider } from './providers/overlay-provider.interface';
 
-const overlayProcessBodySchema = z.object({
-  force: z.boolean().optional(),
-});
-
-const overlayProcessRequestSchema = overlayProcessBodySchema.default({});
-
-type OverlayProcessBody = z.infer<typeof overlayProcessBodySchema>;
-
 @Controller('api/overlays')
 @UseGuards(MediaServerSetupGuard)
 export class OverlaysController {
@@ -194,7 +189,7 @@ export class OverlaysController {
   @Post('process')
   async processAll(
     @Body(new ZodValidationPipe(overlayProcessRequestSchema))
-    request: OverlayProcessBody,
+    request: OverlayProcessRequest,
   ) {
     if (this.processorService.status === 'running') {
       throw new HttpException(
@@ -246,6 +241,12 @@ export class OverlaysController {
 
   @Delete('reset')
   async resetAll() {
+    if (this.processorService.status === 'running') {
+      throw new HttpException(
+        'Overlay processing is already running',
+        HttpStatus.CONFLICT,
+      );
+    }
     await this.processorService.resetAllOverlays();
     return { success: true };
   }
diff --git a/apps/ui/src/api/overlays.ts b/apps/ui/src/api/overlays.ts
@@ -1,6 +1,7 @@
 import type {
   OverlayLibrarySection,
   OverlayPreviewItem,
+  OverlayProcessorRunResult,
   OverlaySettings,
   OverlaySettingsUpdate,
   OverlayTemplate,
@@ -22,13 +23,6 @@ import GetApiHandler, {
   PutApiHandler,
 } from '../utils/ApiHandler'
 
-export interface OverlayProcessResult {
-  processed: number
-  reverted: number
-  skipped?: number
-  errors: number
-}
-
 export const getOverlaySettings = () =>
   GetApiHandler<OverlaySettings>('/overlays/settings')
 
@@ -144,7 +138,7 @@ export const deleteOverlayImage = (imageName: string) =>
   )
 
 export const processAllOverlays = (options?: { force?: boolean }) =>
-  PostApiHandler<OverlayProcessResult>(
+  PostApiHandler<OverlayProcessorRunResult>(
     '/overlays/process',
     options?.force ? { force: true } : {},
   )
diff --git a/apps/ui/src/components/Collection/CollectionDetail/TriggerRuleButton/index.spec.tsx b/apps/ui/src/components/Collection/CollectionDetail/TriggerRuleButton/index.spec.tsx
diff --git a/apps/ui/src/components/Collection/CollectionDetail/TriggerRuleButton/index.tsx b/apps/ui/src/components/Collection/CollectionDetail/TriggerRuleButton/index.tsx
diff --git a/apps/ui/src/components/Common/MediaCard/MediaModal/index.spec.tsx b/apps/ui/src/components/Common/MediaCard/MediaModal/index.spec.tsx
@@ -13,7 +13,7 @@ import GetApiHandler from '../../../../utils/ApiHandler'
 import { clearMaintainerrStatusDetailsCache } from '../maintainerrStatus'
 import MediaModal from './index'
 
-vi.mock('../../../Collection/CollectionDetail/TriggerRuleButton', () => ({
+vi.mock('../../../Collection/CollectionDetail/TriggerRuleActionButton', () => ({
   default: () => <div>trigger-rule-action</div>,
 }))
 
diff --git a/apps/ui/src/components/Common/MediaCard/MediaModal/index.tsx b/apps/ui/src/components/Common/MediaCard/MediaModal/index.tsx
@@ -23,7 +23,7 @@ import {
   rememberMaintainerrStatusDetails,
 } from '../maintainerrStatus'
 import type { ICollection } from '../../../Collection'
-import TriggerRuleButton from '../../../Collection/CollectionDetail/TriggerRuleButton'
+import TriggerRuleButton from '../../../Collection/CollectionDetail/TriggerRuleActionButton'
 
 interface ModalContentProps {
   onClose: () => void
diff --git a/apps/ui/src/components/Settings/Overlays/index.spec.tsx b/apps/ui/src/components/Settings/Overlays/index.spec.tsx
@@ -49,6 +49,7 @@ describe('OverlaySettings', () => {
     processAllOverlays.mockResolvedValue({
       processed: 3,
       reverted: 1,
+      skipped: 2,
       errors: 0,
     })
     resetAllOverlays.mockResolvedValue({ success: true })
@@ -70,7 +71,9 @@ describe('OverlaySettings', () => {
     })
 
     expect(
-      await screen.findByText('Processed: 3, Reverted: 1, Errors: 0'),
+      await screen.findByText(
+        'Processed: 3, Reverted: 1, Skipped: 2, Errors: 0',
+      ),
     ).toBeTruthy()
   })
 
diff --git a/apps/ui/src/pages/CollectionDetailPage.tsx b/apps/ui/src/pages/CollectionDetailPage.tsx
@@ -132,7 +132,7 @@ const CollectionDetailPage = () => {
           </div>
           {currentTab === 'info' && ruleGroup?.useRules ? (
             <CollectionDetailControlRow
-              canTestMedia={Boolean(ruleGroup?.useRules)}
+              canTestMedia={true}
               onOpenTestMedia={() => setMediaTestModalOpen(true)}
             />
           ) : null}
diff --git a/apps/ui/src/utils/overlayProcessResult.ts b/apps/ui/src/utils/overlayProcessResult.ts
@@ -1,8 +1,9 @@
-import type { OverlayProcessResult } from '../api/overlays'
+import type { OverlayProcessorRunResult } from '@maintainerr/contracts'
 
 export const formatOverlayProcessSummary = ({
   processed,
   reverted,
+  skipped,
   errors,
-}: Pick<OverlayProcessResult, 'processed' | 'reverted' | 'errors'>) =>
-  `Processed: ${processed}, Reverted: ${reverted}, Errors: ${errors}`
+}: OverlayProcessorRunResult) =>
+  `Processed: ${processed}, Reverted: ${reverted}, Skipped: ${skipped}, Errors: ${errors}`
diff --git a/packages/contracts/src/overlays/index.ts b/packages/contracts/src/overlays/index.ts
@@ -1,5 +1,6 @@
 export * from './frame-config'
 export * from './overlay-element'
+export * from './overlay-processor'
 export * from './overlay-provider-dtos'
 export * from './overlay-render'
 export * from './overlay-settings'
diff --git a/packages/contracts/src/overlays/overlay-processor.ts b/packages/contracts/src/overlays/overlay-processor.ts
@@ -0,0 +1,14 @@
+import z from 'zod'
+
+export const overlayProcessRequestSchema = z.object({
+  force: z.boolean().optional(),
+})
+
+export type OverlayProcessRequest = z.infer<typeof overlayProcessRequestSchema>
+
+export interface OverlayProcessorRunResult {
+  processed: number
+  reverted: number
+  skipped: number
+  errors: number
+}
