From 0c26cb1abf69f588995d3501c6cdd9380f47b465 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 15:08:18 -0400 Subject: [PATCH 01/12] feat(edit-content): enable image editor in Image and File fields Show the image editor for Image fields, and for File fields only when the referenced asset is an image. Image/File resolve a separate dotAsset by identifier, so availability is driven by a shared isImageFile() predicate over the referenced asset metadata (isImage flag, then image/* mime, then extension). Restricted to the new Angular Edit Content: Image/File never expose the editor in the legacy Dojo host. Binary behavior is unchanged. Introduce an ImageEditSaveStrategy resolved by input type so the save path is selected per field type: Binary applies the edit inline (current behavior); Image/File get a scaffolded dotAsset strategy for the upcoming versioned check-in (safe no-op that never corrupts the identifier reference). Refs #36363 --- ...ield.component.legacy-availability.spec.ts | 118 ++++++++++++++++++ .../dot-file-field.component.spec.ts | 89 ++++++++++++- .../dot-file-field.component.ts | 43 +++++-- .../binary-image-edit-save.strategy.spec.ts | 29 +++++ .../binary-image-edit-save.strategy.ts | 23 ++++ .../dotasset-image-edit-save.strategy.spec.ts | 26 ++++ .../dotasset-image-edit-save.strategy.ts | 32 +++++ .../image-edit-save-strategy.model.ts | 27 ++++ .../image-edit-save-strategy.resolver.spec.ts | 47 +++++++ .../image-edit-save-strategy.resolver.ts | 23 ++++ .../services/save-strategy/index.ts | 4 + .../src/lib/shared/contentlet.utils.spec.ts | 56 ++++++++- .../utils/src/lib/shared/contentlet.utils.ts | 64 ++++++++++ 13 files changed, 569 insertions(+), 12 deletions(-) create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts new file mode 100644 index 000000000000..87e0a95ff3e5 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts @@ -0,0 +1,118 @@ +import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest'; +import { of } from 'rxjs'; + +import { provideHttpClient } from '@angular/common/http'; +import { provideHttpClientTesting } from '@angular/common/http/testing'; +import { ReactiveFormsModule } from '@angular/forms'; + +import { DialogService } from 'primeng/dynamicdialog'; + +import { DotAiService, DotMessageService } from '@dotcms/data-access'; +import { createFakeContentlet } from '@dotcms/utils-testing'; + +import { DotFileFieldComponent } from './dot-file-field.component'; + +import { BINARY_FIELD_MOCK, FILE_FIELD_MOCK, IMAGE_FIELD_MOCK } from '../../../../utils/mocks'; +import { + LegacyDialogImageEditorLauncher, + LegacyDojoImageEditorLauncher +} from '../../services/image-editor'; +import { + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver +} from '../../services/save-strategy'; +import { DotFileFieldUploadService } from '../../services/upload-file/upload-file.service'; +import { FileFieldStore } from '../../store/file-field.store'; +import { DotFileFieldPreviewComponent } from '../dot-file-field-preview/dot-file-field-preview.component'; +import { DotFileFieldUiMessageComponent } from '../dot-file-field-ui-message/dot-file-field-ui-message.component'; + +/** + * Availability of the image editor in the legacy Dojo host — i.e. when the + * Angular image-editor launcher (`IMAGE_EDITOR_LAUNCHER`) is NOT provided (the + * token is only supplied by the new Angular Edit Content shell). + * + * Binary fields keep the binary inline and fall back to the legacy editor, so + * they still expose the action. Image/File fields reference a separate dotAsset + * and are new-Edit-Content-only, so they must NOT expose the editor here. + * + * Kept in a dedicated spec because Spectator allows a single + * `createComponentFactory` per file, and this scenario needs a factory that + * omits the launcher token. + */ +describe('DotFileFieldComponent — legacy host availability (no Angular launcher)', () => { + let spectator: Spectator; + + const createComponent = createComponentFactory({ + component: DotFileFieldComponent, + imports: [ReactiveFormsModule], + componentMocks: [DotFileFieldPreviewComponent, DotFileFieldUiMessageComponent], + providers: [ + FileFieldStore, + mockProvider(DotFileFieldUploadService), + mockProvider(DialogService), + LegacyDialogImageEditorLauncher, + LegacyDojoImageEditorLauncher, + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver, + mockProvider(DotMessageService, { get: jest.fn().mockReturnValue('Test Message') }), + mockProvider(DotAiService, { + checkPluginInstallation: jest.fn().mockReturnValue(of(false)) + }), + provideHttpClient(), + provideHttpClientTesting() + // IMAGE_EDITOR_LAUNCHER intentionally not provided (legacy host). + ] + }); + + const setReferencedImageAsset = (field: typeof IMAGE_FIELD_MOCK) => { + spectator = createComponent({ + props: { + field, + contentlet: createFakeContentlet({ [field.variable]: 'ref-identifier' }), + hasError: false + } as never + }); + spectator.detectChanges(); + spectator.component.store.setPreviewFile({ + source: 'contentlet', + file: { + identifier: 'ref-identifier', + assetMetaData: { isImage: true, contentType: 'image/png', name: 'beach.png' } + } as never + }); + spectator.detectChanges(); + }; + + it('hides the editor for an Image field even when the asset is an image', () => { + setReferencedImageAsset(IMAGE_FIELD_MOCK); + expect(spectator.component.$canEditImage()).toBe(false); + }); + + it('hides the editor for a File field even when the asset is an image', () => { + setReferencedImageAsset(FILE_FIELD_MOCK); + expect(spectator.component.$canEditImage()).toBe(false); + }); + + it('still exposes the editor for a Binary image field (legacy fallback)', () => { + spectator = createComponent({ + props: { + field: BINARY_FIELD_MOCK, + contentlet: createFakeContentlet({ [BINARY_FIELD_MOCK.variable]: null }), + hasError: false + } as never + }); + spectator.detectChanges(); + spectator.component.store.setPreviewFile({ + source: 'temp', + file: { + id: 'temp-1', + fileName: 'img.png', + metadata: { isImage: true, contentType: 'image/png', name: 'img.png' } + } + } as never); + spectator.detectChanges(); + expect(spectator.component.$canEditImage()).toBe(true); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts index d483e45b992e..79c528df85c1 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts @@ -13,12 +13,17 @@ import { createFakeContentlet } from '@dotcms/utils-testing'; import { DotFileFieldComponent } from './dot-file-field.component'; -import { BINARY_FIELD_MOCK, IMAGE_FIELD_MOCK } from '../../../../utils/mocks'; +import { BINARY_FIELD_MOCK, FILE_FIELD_MOCK, IMAGE_FIELD_MOCK } from '../../../../utils/mocks'; import { IMAGE_EDITOR_LAUNCHER } from '../../../shared/image-editor-launcher'; import { LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher } from '../../services/image-editor'; +import { + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver +} from '../../services/save-strategy'; import { DotFileFieldUploadService } from '../../services/upload-file/upload-file.service'; import { FileFieldStore } from '../../store/file-field.store'; import { DotFileFieldPreviewComponent } from '../dot-file-field-preview/dot-file-field-preview.component'; @@ -45,6 +50,9 @@ describe('DotFileFieldComponent', () => { mockProvider(DialogService), LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver, { provide: IMAGE_EDITOR_LAUNCHER, useValue: mockImageEditorLauncher }, mockProvider(DotMessageService, { get: jest.fn().mockReturnValue('Test Message') @@ -215,6 +223,85 @@ describe('DotFileFieldComponent', () => { }); }); + describe('edit image availability', () => { + const IMAGE_ASSET_META = { isImage: true, contentType: 'image/png', name: 'beach.png' }; + const NON_IMAGE_ASSET_META = { + isImage: false, + contentType: 'application/pdf', + name: 'doc.pdf' + }; + + // Hydrates the preview from a referenced dotAsset contentlet (Image/File + // fields), whose metadata lives in `assetMetaData`. + const setReferencedAsset = ( + field: typeof IMAGE_FIELD_MOCK, + assetMetaData: Record + ) => { + spectator = createComponent({ + props: { + field, + contentlet: createFakeContentlet({ [field.variable]: 'ref-identifier' }), + hasError: false + } as never + }); + spectator.detectChanges(); + + spectator.component.store.setPreviewFile({ + source: 'contentlet', + file: { identifier: 'ref-identifier', inode: 'ref-inode', assetMetaData } as never + }); + spectator.detectChanges(); + }; + + it('shows the editor for an Image field pointing at an image asset', () => { + setReferencedAsset(IMAGE_FIELD_MOCK, IMAGE_ASSET_META); + expect(spectator.component.$canEditImage()).toBe(true); + }); + + it('shows the editor for a File field when the referenced asset is an image', () => { + setReferencedAsset(FILE_FIELD_MOCK, IMAGE_ASSET_META); + expect(spectator.component.$canEditImage()).toBe(true); + }); + + it('hides the editor for a File field when the referenced asset is not an image', () => { + setReferencedAsset(FILE_FIELD_MOCK, NON_IMAGE_ASSET_META); + expect(spectator.component.$canEditImage()).toBe(false); + }); + + it('hides the editor for an empty Image field', () => { + spectator = createComponent({ + props: { + field: IMAGE_FIELD_MOCK, + contentlet: createFakeContentlet({ [IMAGE_FIELD_MOCK.variable]: null }), + hasError: false + } as never + }); + spectator.detectChanges(); + expect(spectator.component.$canEditImage()).toBe(false); + }); + + it('keeps Binary hidden for a non-image file', () => { + spectator = createComponent({ + props: { + field: BINARY_FIELD_MOCK, + contentlet: createFakeContentlet({ [BINARY_FIELD_MOCK.variable]: null }), + hasError: false + } as never + }); + spectator.detectChanges(); + spectator.component.store.setPreviewFile({ + source: 'temp', + file: { + id: 'temp-1', + fileName: 'doc.pdf', + metadata: { isImage: false, contentType: 'application/pdf', name: 'doc.pdf' } + } + } as never); + spectator.detectChanges(); + expect(spectator.component.$canEditImage()).toBe(false); + }); + }); + describe('vertical layout', () => { it('should use horizontal layout by default', () => { spectator.detectChanges(); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 9b1f531332eb..bccd303ef0e6 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -40,12 +40,17 @@ import { DropZoneFileEvent, DropZoneFileValidity } from '@dotcms/ui'; -import { getFileMetadata } from '@dotcms/utils'; +import { getFileMetadata, isImageFile } from '@dotcms/utils'; import { LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher } from './../../services/image-editor'; +import { + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver +} from './../../services/save-strategy'; import { DotFileFieldUploadService } from './../../services/upload-file/upload-file.service'; import { FileFieldStore } from './../../store/file-field.store'; import { parseFocalPoint } from './../../utils/focal-point.util'; @@ -80,6 +85,9 @@ import { IMAGE_EDITOR_LAUNCHER } from '../../../shared/image-editor-launcher'; DialogService, LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + ImageEditSaveStrategyResolver, { multi: true, provide: NG_VALUE_ACCESSOR, @@ -111,6 +119,11 @@ export class DotFileFieldComponent * the legacy launchers. */ readonly #imageEditorLauncher = inject(IMAGE_EDITOR_LAUNCHER, { optional: true }); + /** + * Resolves how an edited image is persisted based on the field's input type: + * Binary applies the edit inline, Image/File version the referenced dotAsset. + */ + readonly #imageEditSaveStrategyResolver = inject(ImageEditSaveStrategyResolver); /** * A readonly private field that holds an instance of the DialogService. * This service is injected using Angular's dependency injection mechanism. @@ -201,20 +214,26 @@ export class DotFileFieldComponent /** * Whether the "Edit image" action is available for the current file. * - * Only Binary fields expose the image editor when enabled, and only when - * the previewed file is actually an image. File/Image fields never show it. + * Shown when image editing is enabled and the previewed file is an image + * (see {@link isImageFile}). Binary fields keep the binary inline and have a + * legacy fallback, so they work in any host. Image/File fields reference a + * separate dotAsset and are only supported in the new Angular Edit Content — + * where the image-editor launcher is provided — never in the legacy Dojo host. */ $canEditImage = computed(() => { if (!this.$enableImageEditor()) { return false; } - // Image editing is only supported for Binary fields, not File or Image fields. - if (this.store.inputType() !== INPUT_TYPES.Binary) { + if (!isImageFile(this.#currentMetadata())) { return false; } - return !!this.#currentMetadata()?.isImage; + if (this.store.inputType() !== INPUT_TYPES.Binary) { + return !!this.#imageEditorLauncher; + } + + return true; }); /** @@ -451,9 +470,11 @@ export class DotFileFieldComponent } /** - * Applies the edited image emitted by an image-editor launcher to the preview, - * shared by the new Angular editor and the legacy launchers. Ignores a closed - * editor (no temp file) and surfaces a server error if the stream fails. + * Applies the edited image emitted by an image-editor launcher, delegating to + * the {@link ImageEditSaveStrategy} resolved for the field's input type (Binary + * applies inline; Image/File version the referenced dotAsset). Shared by the + * new Angular editor and the legacy launchers. Ignores a closed editor (no temp + * file) and surfaces a server error if the stream fails. * * @param result$ the launcher's close stream, emitting the edited temp file or null */ @@ -465,7 +486,9 @@ export class DotFileFieldComponent ) .subscribe({ next: (tempFile) => { - this.store.applyTempFile(tempFile); + this.#imageEditSaveStrategyResolver + .resolve(this.store.inputType()) + .apply(tempFile); }, error: () => { this.store.setUIMessage(getUiMessage('SERVER_ERROR')); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts new file mode 100644 index 000000000000..ba3b10fa6f5f --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts @@ -0,0 +1,29 @@ +import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; + +import { FileFieldStore } from '../../store/file-field.store'; + +describe('BinaryImageEditSaveStrategy', () => { + let spectator: SpectatorService; + + const createService = createServiceFactory({ + service: BinaryImageEditSaveStrategy, + providers: [mockProvider(FileFieldStore, { applyTempFile: jest.fn() })] + }); + + beforeEach(() => { + spectator = createService(); + }); + + it('applies the edited temp file inline via the store', () => { + const tempFile = { id: 'temp-1', fileName: 'edited.png' } as DotCMSTempFile; + const store = spectator.inject(FileFieldStore); + + spectator.service.apply(tempFile); + + expect(store.applyTempFile).toHaveBeenCalledWith(tempFile); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts new file mode 100644 index 000000000000..fb302d570ccd --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts @@ -0,0 +1,23 @@ +import { inject, Injectable } from '@angular/core'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; + +import { FileFieldStore } from '../../store/file-field.store'; + +/** + * Binary field save strategy — the existing behavior. + * + * The binary lives inline on the contentlet, so applying the edit just swaps the + * preview to the edited temp file; its temp id becomes the field value and is + * resolved to a binary on the standard contentlet check-in. + */ +@Injectable() +export class BinaryImageEditSaveStrategy implements ImageEditSaveStrategy { + readonly #store = inject(FileFieldStore); + + apply(tempFile: DotCMSTempFile): void { + this.#store.applyTempFile(tempFile); + } +} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts new file mode 100644 index 000000000000..ed009de0c5ec --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts @@ -0,0 +1,26 @@ +import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; + +describe('DotAssetImageEditSaveStrategy', () => { + let spectator: SpectatorService; + + const createService = createServiceFactory({ + service: DotAssetImageEditSaveStrategy + }); + + beforeEach(() => { + spectator = createService(); + }); + + // Scaffold: the versioned check-in of the referenced dotAsset lands in a + // follow-up step. For now apply() must be a safe no-op that never falls back + // to the Binary inline write (which would corrupt the identifier reference). + it('does not throw when applying (scaffolded no-op)', () => { + const tempFile = { id: 'temp-1', fileName: 'edited.png' } as DotCMSTempFile; + + expect(() => spectator.service.apply(tempFile)).not.toThrow(); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts new file mode 100644 index 000000000000..087a60df5ffc --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts @@ -0,0 +1,32 @@ +import { Injectable } from '@angular/core'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; + +/** + * Image/File field save strategy. + * + * Image and File fields store only an `identifier` that references a separate + * `dotAsset` contentlet (the binary lives in that asset's `asset` field). Saving + * an edit must check in a NEW VERSION of the referenced `dotAsset` — preserving + * version history — and refresh the preview WITHOUT mutating the field value, + * which must keep pointing at the same identifier. + * + * Scaffolded intentionally: the versioned check-in of the referenced asset is + * implemented in a follow-up step. It deliberately does NOT fall back to the + * Binary inline write, which would overwrite the field's identifier reference + * with a temp id and corrupt the reference. + * + * Only ever reached from the new Angular Edit Content: the image-editor entry + * point for Image/File fields is gated on the presence of the Angular + * image-editor launcher, so this never runs in the legacy Dojo host. + */ +@Injectable() +export class DotAssetImageEditSaveStrategy implements ImageEditSaveStrategy { + apply(_tempFile: DotCMSTempFile): void { + // TODO(#36363): upload/reuse the edited temp file and check in a new + // version of the referenced dotAsset (via the workflow fire endpoint), + // then re-hydrate the preview without changing the field value. + } +} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts new file mode 100644 index 000000000000..3d6d7c750585 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts @@ -0,0 +1,27 @@ +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +/** + * Strategy for persisting the result of an image-editor session back to a file + * field. + * + * {@link DotFileFieldComponent} renders Binary, Image and File fields with a + * single component, but each stores its binary differently, so the "apply edited + * image" step differs per input type: + * + * - **Binary** keeps the binary inline on the contentlet, so the edited temp file + * becomes the field value directly (resolved to a binary on the contentlet + * check-in). + * - **Image/File** reference a separate `dotAsset` contentlet by identifier (the + * binary lives in that asset's `asset` field), so the edit must be checked in as + * a new version of the referenced asset without changing the field value. + * + * Implementations are selected by {@link ImageEditSaveStrategyResolver}. + */ +export interface ImageEditSaveStrategy { + /** + * Applies the temp file returned by the image editor to the field. + * + * @param tempFile - The edited image staged by the editor as a temp file. + */ + apply(tempFile: DotCMSTempFile): void; +} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts new file mode 100644 index 000000000000..8c24068d7aff --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts @@ -0,0 +1,47 @@ +import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; + +import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; +import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; +import { ImageEditSaveStrategyResolver } from './image-edit-save-strategy.resolver'; + +import { INPUT_TYPES } from '../../../../models/dot-edit-content-file.model'; +import { FileFieldStore } from '../../store/file-field.store'; + +describe('ImageEditSaveStrategyResolver', () => { + let spectator: SpectatorService; + + const createService = createServiceFactory({ + service: ImageEditSaveStrategyResolver, + providers: [ + BinaryImageEditSaveStrategy, + DotAssetImageEditSaveStrategy, + mockProvider(FileFieldStore, { applyTempFile: jest.fn() }) + ] + }); + + beforeEach(() => { + spectator = createService(); + }); + + it('resolves the Binary strategy for Binary fields', () => { + expect(spectator.service.resolve(INPUT_TYPES.Binary)).toBeInstanceOf( + BinaryImageEditSaveStrategy + ); + }); + + it('resolves the dotAsset strategy for Image fields', () => { + expect(spectator.service.resolve(INPUT_TYPES.Image)).toBeInstanceOf( + DotAssetImageEditSaveStrategy + ); + }); + + it('resolves the dotAsset strategy for File fields', () => { + expect(spectator.service.resolve(INPUT_TYPES.File)).toBeInstanceOf( + DotAssetImageEditSaveStrategy + ); + }); + + it('falls back to the dotAsset strategy when the input type is null', () => { + expect(spectator.service.resolve(null)).toBeInstanceOf(DotAssetImageEditSaveStrategy); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts new file mode 100644 index 000000000000..6ff23a816f38 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts @@ -0,0 +1,23 @@ +import { inject, Injectable } from '@angular/core'; + +import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; +import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; +import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; + +import { INPUT_TYPE, INPUT_TYPES } from '../../../../models/dot-edit-content-file.model'; + +/** + * Resolves the {@link ImageEditSaveStrategy} for a field's input type. + * + * Binary fields apply the edit inline; everything else (Image/File) is a + * reference-backed field and versions the referenced `dotAsset`. + */ +@Injectable() +export class ImageEditSaveStrategyResolver { + readonly #binary = inject(BinaryImageEditSaveStrategy); + readonly #dotAsset = inject(DotAssetImageEditSaveStrategy); + + resolve(inputType: INPUT_TYPE | null): ImageEditSaveStrategy { + return inputType === INPUT_TYPES.Binary ? this.#binary : this.#dotAsset; + } +} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts new file mode 100644 index 000000000000..702e18175a37 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts @@ -0,0 +1,4 @@ +export * from './image-edit-save-strategy.model'; +export * from './binary-image-edit-save.strategy'; +export * from './dotasset-image-edit-save.strategy'; +export * from './image-edit-save-strategy.resolver'; diff --git a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts index 6a4a63bbba72..8646b009ffc6 100644 --- a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts +++ b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts @@ -1,6 +1,12 @@ import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; -import { getFileMetadata, getFileVersion, cleanMimeTypes, checkMimeType } from './contentlet.utils'; +import { + getFileMetadata, + getFileVersion, + cleanMimeTypes, + checkMimeType, + isImageFile +} from './contentlet.utils'; const NEW_FILE_MOCK: { entity: DotCMSContentlet } = { entity: { @@ -190,4 +196,52 @@ describe('utils', () => { expect(checkMimeType(file, acceptedFiles)).toBe(true); }); }); + + describe('isImageFile', () => { + it('returns false for null or undefined metadata', () => { + expect(isImageFile(null)).toBe(false); + expect(isImageFile(undefined)).toBe(false); + }); + + it('returns false for empty metadata', () => { + expect(isImageFile({})).toBe(false); + }); + + it('trusts the authoritative isImage flag', () => { + expect(isImageFile({ isImage: true })).toBe(true); + expect( + isImageFile({ isImage: false, contentType: 'application/pdf', name: 'a.pdf' }) + ).toBe(false); + }); + + it('falls back to an image/* content type when isImage is absent', () => { + expect(isImageFile({ contentType: 'image/png' })).toBe(true); + expect(isImageFile({ contentType: 'IMAGE/PNG' })).toBe(true); + expect(isImageFile({ contentType: 'application/pdf' })).toBe(false); + }); + + it('falls back to a known image extension as a last resort', () => { + expect(isImageFile({ name: 'photo.PNG' })).toBe(true); + expect(isImageFile({ name: 'archive.tar.gz' })).toBe(false); + expect(isImageFile({ name: 'noextension' })).toBe(false); + }); + + it.each(['jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp', 'svg', 'ico', 'tiff', 'avif', 'heic'])( + 'detects "%s" as an image extension', + (ext) => { + expect(isImageFile({ name: `file.${ext}` })).toBe(true); + } + ); + + it.each(['pdf', 'docx', 'txt', 'mp4', 'zip', 'json'])( + 'does not detect "%s" as an image extension', + (ext) => { + expect(isImageFile({ name: `file.${ext}` })).toBe(false); + } + ); + + it('detects a referenced dotAsset via its assetMetaData', () => { + expect(isImageFile(getFileMetadata(NEW_FILE_MOCK.entity))).toBe(true); + }); + }); }); diff --git a/core-web/libs/utils/src/lib/shared/contentlet.utils.ts b/core-web/libs/utils/src/lib/shared/contentlet.utils.ts index c3f5ed9fac60..775ab7d419cf 100644 --- a/core-web/libs/utils/src/lib/shared/contentlet.utils.ts +++ b/core-web/libs/utils/src/lib/shared/contentlet.utils.ts @@ -72,3 +72,67 @@ export function checkMimeType( return false; } + +/** + * Common image file extensions, used as a last-resort fallback when metadata + * carries neither an explicit `isImage` flag nor a recognizable `image/*` + * content type. + */ +const IMAGE_FILE_EXTENSIONS = new Set([ + 'jpg', + 'jpeg', + 'png', + 'gif', + 'webp', + 'bmp', + 'svg', + 'ico', + 'tif', + 'tiff', + 'avif', + 'heic', + 'heif' +]); + +/** + * Whether a file name ends with a known image extension. + * + * @param name The file name to inspect. + * @returns true when the name has a recognized image extension. + */ +function hasImageExtension(name: string | undefined): boolean { + const extension = name?.split('.').pop()?.toLowerCase(); + + return !!extension && IMAGE_FILE_EXTENSIONS.has(extension); +} + +/** + * Determines whether a file's metadata describes an image, using layered signals + * so detection stays correct even when the metadata is partial: + * + * 1. the authoritative server-computed `isImage` flag; + * 2. an `image/*` content type; + * 3. a known image file extension on the file name (last-resort fallback). + * + * Use this to gate image-only affordances (e.g. the image editor) uniformly + * across Binary fields and the `dotAsset` referenced by Image/File fields. Pair + * it with {@link getFileMetadata} to resolve the metadata first. + * + * @param metadata The resolved file metadata (may be partial, null or undefined). + * @returns true when the metadata describes an image. + */ +export function isImageFile(metadata: Partial | null | undefined): boolean { + if (!metadata) { + return false; + } + + if (metadata.isImage) { + return true; + } + + if (metadata.contentType?.toLowerCase().startsWith('image/')) { + return true; + } + + return hasImageExtension(metadata.name); +} From 966b97097a6ac760b12f520a6174d4a92d24d4ab Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 15:08:19 -0400 Subject: [PATCH 02/12] fix(image-editor): keep edited image recognized after save The Save servlet can return the edited temp file as metadata: null, image: false, mimeType: "unknown". Downstream that stopped treating it as an image: blank thumbnail, hidden edit pencil, and a crash in the file-info dialog header (metadata.title on null). Enrich the edited temp file as an image (enrichEditedImage util) so every consumer and host stays consistent, and null-guard the dialog header binding. Refs #36363 --- .../dot-file-field-preview.component.html | 8 +- .../dot-file-field-preview.component.spec.ts | 13 +++ .../store/features/with-save.feature.spec.ts | 41 ++++++- .../lib/store/features/with-save.feature.ts | 17 ++- .../utils/enrich-edited-image.util.spec.ts | 104 ++++++++++++++++++ .../src/lib/utils/enrich-edited-image.util.ts | 47 ++++++++ 6 files changed, 216 insertions(+), 14 deletions(-) create mode 100644 core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.spec.ts create mode 100644 core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.html b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.html index 2a8e2a1ae6b4..de2681dd11ca 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.html +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.html @@ -162,22 +162,22 @@ [closeOnEscape]="true" [dismissableMask]="true" [modal]="true" - [header]="metadata.title" + [header]="metadata?.title" [appendTo]="'body'" [style]="{ maxWidth: '582px', width: '100%' }">
{{ 'Size' | dm }}:
- @if (metadata.width && metadata.height) { + @if (metadata?.width && metadata?.height) {
- {{ metadata.width }}px, {{ metadata.height }}px + {{ metadata?.width }}px, {{ metadata?.height }}px
}
- {{ metadata.fileSize | dotFileSizeFormat }} + {{ metadata?.fileSize | dotFileSizeFormat }}
diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.spec.ts index b984fd55dc63..986dc53e3983 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field-preview/dot-file-field-preview.component.spec.ts @@ -89,6 +89,19 @@ describe('DotFileFieldPreviewComponent', () => { expect(spectator.query(byTestId('download-btn'))).toBeFalsy(); }); + + it('renders without crashing when the temp file has no metadata', () => { + // Regression: the image-editor Save servlet can return a temp file with + // `metadata: null`. The file-info dialog header bound `metadata.title` + // unguarded and threw "Cannot read properties of null (reading 'title')". + spectator.setInput('previewFile', { + source: 'temp', + file: { ...TEMP_FILE_MOCK, image: false, mimeType: 'unknown', metadata: null } + } as unknown); + + expect(() => spectator.detectChanges()).not.toThrow(); + expect(spectator.component).toBeTruthy(); + }); }); describe('contentlet without content preview file', () => { diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts index 88f3b58d98c7..a101346bca54 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts @@ -99,13 +99,52 @@ describe('withSave', () => { lifecycle.saveRequested(); expect(service.saveEditedImage).toHaveBeenCalledTimes(1); + // The editor only edits images, so the result is always flagged as an image and + // seeded with the current focal point (see `enrichEditedImage`). expect(dispatchSpy).toHaveBeenCalledWith( - imageEditorLifecycleEvents.saveSucceeded(TEMP_FILE) + imageEditorLifecycleEvents.saveSucceeded({ + ...TEMP_FILE, + image: true, + metadata: { + contentType: 'image/png', + fileSize: 1024, + length: 1024, + modDate: 0, + name: 'edited.png', + sha256: '', + title: 'edited.png', + version: 0, + isImage: true, + focalPoint: '0.25,0.75' + } + }) ); expect(store.saveStatus()).toBe('idle'); expect(store.saveError()).toBeNull(); }); + it('recognizes the edit as an image even when the servlet returns no metadata', () => { + // Regression: the Save servlet can return the edited temp file as + // `metadata: null, image: false, mimeType: "unknown"`. Without enrichment the + // thumbnail, the "edit image" gate and the file-info dialog stop treating it as + // an image (blank thumb / hidden pencil / crash on `metadata.title`). + service.saveEditedImage.mockReturnValue( + of({ ...TEMP_FILE, image: false, mimeType: 'unknown', metadata: null } as never) + ); + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + const event = dispatchSpy.mock.calls + .map(([arg]) => arg) + .find((arg) => arg?.type === imageEditorLifecycleEvents.saveSucceeded.type); + // Payload shape is normalized to tolerate nested (`{ type, payload }`) or spread. + const payload = event.payload ?? event; + expect(payload.image).toBe(true); + expect(payload.metadata.isImage).toBe(true); + expect(payload.metadata.focalPoint).toBe('0.25,0.75'); + }); + it('folds the current focal point into the returned temp metadata', () => { // The servlet returns a temp whose serialized metadata is basic (no focalPoint), // so the feature injects the focal so an in-session reopen can re-seed the marker. diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts index c12fa101d964..49693a77b950 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts @@ -8,6 +8,7 @@ import { exhaustMap } from 'rxjs/operators'; import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { enrichEditedImage } from '../../utils/enrich-edited-image.util'; import { buildSaveUrl } from '../../utils/image-filter-url.builder'; import { imageEditorLifecycleEvents } from '../image-editor.events'; import { errorMessage } from '../image-editor.store-utils'; @@ -72,15 +73,13 @@ export function withSave() { tapResponse({ next: (tempFile) => dispatcher.dispatch( - imageEditorLifecycleEvents.saveSucceeded({ - ...tempFile, - metadata: tempFile.metadata - ? { - ...tempFile.metadata, - focalPoint: `${focalPoint.x},${focalPoint.y}` - } - : tempFile.metadata - }) + imageEditorLifecycleEvents.saveSucceeded( + enrichEditedImage( + tempFile, + store.assetContext(), + focalPoint + ) + ) ), error: (error) => dispatcher.dispatch( diff --git a/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.spec.ts b/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.spec.ts new file mode 100644 index 000000000000..c8ed826e9aa4 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.spec.ts @@ -0,0 +1,104 @@ +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { enrichEditedImage } from './enrich-edited-image.util'; + +import { ImageEditorAssetContext, NormalizedPoint } from '../models/image-editor.models'; + +const CTX: ImageEditorAssetContext = { + idOrTempId: 'abc123', + inode: 'abc123', + tempId: null, + variable: 'binary', + fieldName: 'binary', + fileName: 'beach.png', + mimeType: 'image/png', + isTempFile: false, + byInode: true, + naturalWidth: 2752, + naturalHeight: 1536, + originalUrl: '/contentAsset/image/abc123/binary' +}; + +const FOCAL: NormalizedPoint = { x: 0.25, y: 0.75 }; + +const BASE_TEMP: DotCMSTempFile = { + fileName: 'edited.png', + folder: 'shared', + id: 'temp_1', + image: false, + length: 1024, + mimeType: 'image/png', + referenceUrl: '/dA/temp_1', + thumbnailUrl: '' +}; + +describe('enrichEditedImage', () => { + it('flags the temp file as an image and seeds the focal point', () => { + const result = enrichEditedImage(BASE_TEMP, CTX, FOCAL); + + expect(result.image).toBe(true); + expect(result.metadata?.isImage).toBe(true); + expect(result.metadata?.focalPoint).toBe('0.25,0.75'); + }); + + it('synthesizes metadata from the context when the servlet returns none', () => { + const tempFile = { ...BASE_TEMP, mimeType: 'unknown', metadata: null } as never; + + const result = enrichEditedImage(tempFile, CTX, FOCAL); + + expect(result.metadata).toEqual({ + contentType: 'image/png', // ctx.mimeType wins over the temp's "unknown" + fileSize: 1024, + length: 1024, + modDate: 0, + name: 'edited.png', + sha256: '', + title: 'edited.png', + version: 0, + width: 2752, + height: 1536, + isImage: true, + focalPoint: '0.25,0.75' + }); + }); + + it('preserves real server metadata and only forces isImage + focal point', () => { + const metadata = { + contentType: 'image/jpeg', + fileSize: 5000, + isImage: true, + length: 5000, + modDate: 123, + name: 'server.jpg', + sha256: 'deadbeef', + title: 'Server Title', + version: 7, + width: 800, + height: 600 + }; + + const result = enrichEditedImage({ ...BASE_TEMP, metadata }, CTX, FOCAL); + + expect(result.metadata).toEqual({ ...metadata, isImage: true, focalPoint: '0.25,0.75' }); + }); + + it('falls back to the context file name when the temp file has none', () => { + const result = enrichEditedImage( + { ...BASE_TEMP, fileName: '', metadata: null } as never, + CTX, + FOCAL + ); + + expect(result.metadata?.name).toBe('beach.png'); + expect(result.metadata?.title).toBe('beach.png'); + }); + + it('omits width/height when the context has no natural dimensions', () => { + const ctx = { ...CTX, naturalWidth: 0, naturalHeight: 0 }; + + const result = enrichEditedImage({ ...BASE_TEMP, metadata: null } as never, ctx, FOCAL); + + expect(result.metadata).not.toHaveProperty('width'); + expect(result.metadata).not.toHaveProperty('height'); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.ts b/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.ts new file mode 100644 index 000000000000..e1c7a35aa66e --- /dev/null +++ b/core-web/libs/image-editor/src/lib/utils/enrich-edited-image.util.ts @@ -0,0 +1,47 @@ +import { DotCMSTempFile, DotFileMetadata } from '@dotcms/dotcms-models'; + +import { ImageEditorAssetContext, NormalizedPoint } from '../models/image-editor.models'; + +/** + * Enriches the temp file returned by the Save servlet so downstream consumers + * keep recognizing it as an image. + * + * The editor only ever edits images, but the Save endpoint can return a temp + * file without metadata (`metadata: null`, `image: false`, `mimeType: "unknown"`). + * Left untouched, the thumbnail, the "edit image" gate and the file-info dialog + * all stop treating it as an image (blank thumb / hidden pencil / crash on + * `metadata.title`). Synthesizing minimal metadata here — the one place that + * knows the result is an edited image — keeps every consumer and host consistent, + * and always folds in the current focal point. + * + * @param tempFile - The temp file returned by the Save servlet. + * @param ctx - The asset context the editor opened with (source of name/mime/size). + * @param focalPoint - The focal point at save time, seeded into the metadata. + * @returns A temp file flagged as an image with complete, focal-seeded metadata. + */ +export function enrichEditedImage( + tempFile: DotCMSTempFile, + ctx: ImageEditorAssetContext, + focalPoint: NormalizedPoint +): DotCMSTempFile { + const name = tempFile.fileName || ctx.fileName || ''; + const metadata: DotFileMetadata = { + contentType: ctx.mimeType || tempFile.mimeType || 'image/*', + fileSize: tempFile.length ?? 0, + length: tempFile.length ?? 0, + modDate: 0, + name, + sha256: '', + title: name, + version: 0, + ...(ctx.naturalWidth ? { width: ctx.naturalWidth } : {}), + ...(ctx.naturalHeight ? { height: ctx.naturalHeight } : {}), + // Preserve real server metadata when present; then force the invariants: + // the result is an image, seeded with the current focal point. + ...tempFile.metadata, + isImage: true, + focalPoint: `${focalPoint.x},${focalPoint.y}` + }; + + return { ...tempFile, image: true, metadata }; +} From 25bee85d4453c60e829dfb818982bbf8893e94cf Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 15:17:07 -0400 Subject: [PATCH 03/12] refactor(image-editor): encapsulate isImageFile in image-editor lib Move the image-detection predicate out of libs/utils and into the image-editor lib (alongside enrichEditedImage), exported from its public API. Groups the image-ness logic with its owner and leaves libs/utils untouched. Drop the file-extension fallback: rely on the authoritative isImage flag and the image/* content type, both reliably present on the metadata the gate runs against (dotAsset assetMetaData and enriched Binary temp files). File names are not a trusted signal. Refs #36363 --- .../dot-file-field.component.ts | 3 +- core-web/libs/image-editor/src/index.ts | 1 + .../src/lib/utils/is-image-file.util.spec.ts | 37 +++++++++++ .../src/lib/utils/is-image-file.util.ts | 26 ++++++++ .../src/lib/shared/contentlet.utils.spec.ts | 56 +--------------- .../utils/src/lib/shared/contentlet.utils.ts | 64 ------------------- 6 files changed, 67 insertions(+), 120 deletions(-) create mode 100644 core-web/libs/image-editor/src/lib/utils/is-image-file.util.spec.ts create mode 100644 core-web/libs/image-editor/src/lib/utils/is-image-file.util.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index bccd303ef0e6..0d389b51c668 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -31,6 +31,7 @@ import { DotFileMetadata, DotGeneratedAIImage } from '@dotcms/dotcms-models'; +import { isImageFile } from '@dotcms/image-editor'; import { DotAIImagePromptComponent, DotBrowserSelectorComponent, @@ -40,7 +41,7 @@ import { DropZoneFileEvent, DropZoneFileValidity } from '@dotcms/ui'; -import { getFileMetadata, isImageFile } from '@dotcms/utils'; +import { getFileMetadata } from '@dotcms/utils'; import { LegacyDialogImageEditorLauncher, diff --git a/core-web/libs/image-editor/src/index.ts b/core-web/libs/image-editor/src/index.ts index 3899a11ec43f..7be46854cbda 100644 --- a/core-web/libs/image-editor/src/index.ts +++ b/core-web/libs/image-editor/src/index.ts @@ -2,6 +2,7 @@ export { DotImageEditorComponent } from './lib/components/dot-image-editor/dot-i export * from './lib/models/image-editor.models'; export { RANGES } from './lib/image-editor.constants'; export { buildFilterChain, buildPreviewUrl, cleanUrl } from './lib/utils/image-filter-url.builder'; +export { isImageFile } from './lib/utils/is-image-file.util'; export * from './lib/store/image-editor.events'; export { ImageEditorStore } from './lib/store/image-editor.store'; export { initialImageEditorState } from './lib/store/image-editor.state'; diff --git a/core-web/libs/image-editor/src/lib/utils/is-image-file.util.spec.ts b/core-web/libs/image-editor/src/lib/utils/is-image-file.util.spec.ts new file mode 100644 index 000000000000..e8ed8c84beb4 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/utils/is-image-file.util.spec.ts @@ -0,0 +1,37 @@ +import { isImageFile } from './is-image-file.util'; + +describe('isImageFile', () => { + it('returns false for null or undefined metadata', () => { + expect(isImageFile(null)).toBe(false); + expect(isImageFile(undefined)).toBe(false); + }); + + it('returns false for empty metadata', () => { + expect(isImageFile({})).toBe(false); + }); + + it('trusts the authoritative isImage flag', () => { + expect(isImageFile({ isImage: true })).toBe(true); + expect(isImageFile({ isImage: false, contentType: 'application/pdf', name: 'a.pdf' })).toBe( + false + ); + }); + + it('falls back to an image/* content type when isImage is absent', () => { + expect(isImageFile({ contentType: 'image/png' })).toBe(true); + expect(isImageFile({ contentType: 'IMAGE/PNG' })).toBe(true); + expect(isImageFile({ contentType: 'application/pdf' })).toBe(false); + }); + + it('does not infer image-ness from the file name alone', () => { + // Extension sniffing is intentionally avoided; name is not a trusted signal. + expect(isImageFile({ name: 'photo.png' })).toBe(false); + }); + + it('detects image metadata coming from a referenced dotAsset', () => { + // Shape of a dotAsset's assetMetaData resolved via getFileMetadata upstream. + expect(isImageFile({ isImage: true, contentType: 'image/jpeg', name: 'image 2.jpg' })).toBe( + true + ); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/utils/is-image-file.util.ts b/core-web/libs/image-editor/src/lib/utils/is-image-file.util.ts new file mode 100644 index 000000000000..b8bd3826a988 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/utils/is-image-file.util.ts @@ -0,0 +1,26 @@ +import { DotFileMetadata } from '@dotcms/dotcms-models'; + +/** + * Determines whether a file's metadata describes an image. + * + * Trusts the authoritative server-computed `isImage` flag first, then falls back + * to the `image/*` content type. Both signals are reliably present on the + * metadata this gate runs against: the `assetMetaData` of the `dotAsset` + * referenced by Image/File fields, and the (enriched) metadata of Binary temp + * files. Extension sniffing is intentionally avoided — file names are unreliable + * and the mime type is always available here. + * + * @param metadata The resolved file metadata (may be partial, null or undefined). + * @returns true when the metadata describes an image. + */ +export function isImageFile(metadata: Partial | null | undefined): boolean { + if (!metadata) { + return false; + } + + if (metadata.isImage) { + return true; + } + + return !!metadata.contentType?.toLowerCase().startsWith('image/'); +} diff --git a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts index 8646b009ffc6..6a4a63bbba72 100644 --- a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts +++ b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts @@ -1,12 +1,6 @@ import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; -import { - getFileMetadata, - getFileVersion, - cleanMimeTypes, - checkMimeType, - isImageFile -} from './contentlet.utils'; +import { getFileMetadata, getFileVersion, cleanMimeTypes, checkMimeType } from './contentlet.utils'; const NEW_FILE_MOCK: { entity: DotCMSContentlet } = { entity: { @@ -196,52 +190,4 @@ describe('utils', () => { expect(checkMimeType(file, acceptedFiles)).toBe(true); }); }); - - describe('isImageFile', () => { - it('returns false for null or undefined metadata', () => { - expect(isImageFile(null)).toBe(false); - expect(isImageFile(undefined)).toBe(false); - }); - - it('returns false for empty metadata', () => { - expect(isImageFile({})).toBe(false); - }); - - it('trusts the authoritative isImage flag', () => { - expect(isImageFile({ isImage: true })).toBe(true); - expect( - isImageFile({ isImage: false, contentType: 'application/pdf', name: 'a.pdf' }) - ).toBe(false); - }); - - it('falls back to an image/* content type when isImage is absent', () => { - expect(isImageFile({ contentType: 'image/png' })).toBe(true); - expect(isImageFile({ contentType: 'IMAGE/PNG' })).toBe(true); - expect(isImageFile({ contentType: 'application/pdf' })).toBe(false); - }); - - it('falls back to a known image extension as a last resort', () => { - expect(isImageFile({ name: 'photo.PNG' })).toBe(true); - expect(isImageFile({ name: 'archive.tar.gz' })).toBe(false); - expect(isImageFile({ name: 'noextension' })).toBe(false); - }); - - it.each(['jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp', 'svg', 'ico', 'tiff', 'avif', 'heic'])( - 'detects "%s" as an image extension', - (ext) => { - expect(isImageFile({ name: `file.${ext}` })).toBe(true); - } - ); - - it.each(['pdf', 'docx', 'txt', 'mp4', 'zip', 'json'])( - 'does not detect "%s" as an image extension', - (ext) => { - expect(isImageFile({ name: `file.${ext}` })).toBe(false); - } - ); - - it('detects a referenced dotAsset via its assetMetaData', () => { - expect(isImageFile(getFileMetadata(NEW_FILE_MOCK.entity))).toBe(true); - }); - }); }); diff --git a/core-web/libs/utils/src/lib/shared/contentlet.utils.ts b/core-web/libs/utils/src/lib/shared/contentlet.utils.ts index 775ab7d419cf..c3f5ed9fac60 100644 --- a/core-web/libs/utils/src/lib/shared/contentlet.utils.ts +++ b/core-web/libs/utils/src/lib/shared/contentlet.utils.ts @@ -72,67 +72,3 @@ export function checkMimeType( return false; } - -/** - * Common image file extensions, used as a last-resort fallback when metadata - * carries neither an explicit `isImage` flag nor a recognizable `image/*` - * content type. - */ -const IMAGE_FILE_EXTENSIONS = new Set([ - 'jpg', - 'jpeg', - 'png', - 'gif', - 'webp', - 'bmp', - 'svg', - 'ico', - 'tif', - 'tiff', - 'avif', - 'heic', - 'heif' -]); - -/** - * Whether a file name ends with a known image extension. - * - * @param name The file name to inspect. - * @returns true when the name has a recognized image extension. - */ -function hasImageExtension(name: string | undefined): boolean { - const extension = name?.split('.').pop()?.toLowerCase(); - - return !!extension && IMAGE_FILE_EXTENSIONS.has(extension); -} - -/** - * Determines whether a file's metadata describes an image, using layered signals - * so detection stays correct even when the metadata is partial: - * - * 1. the authoritative server-computed `isImage` flag; - * 2. an `image/*` content type; - * 3. a known image file extension on the file name (last-resort fallback). - * - * Use this to gate image-only affordances (e.g. the image editor) uniformly - * across Binary fields and the `dotAsset` referenced by Image/File fields. Pair - * it with {@link getFileMetadata} to resolve the metadata first. - * - * @param metadata The resolved file metadata (may be partial, null or undefined). - * @returns true when the metadata describes an image. - */ -export function isImageFile(metadata: Partial | null | undefined): boolean { - if (!metadata) { - return false; - } - - if (metadata.isImage) { - return true; - } - - if (metadata.contentType?.toLowerCase().startsWith('image/')) { - return true; - } - - return hasImageExtension(metadata.name); -} From 1ea1b9706669fb57a27993f8d55a5e9736ad4f88 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 15:46:06 -0400 Subject: [PATCH 04/12] feat(edit-content): version referenced dotAsset on Image/File image save Implement the dotAsset save strategy: on saving an image edit from an Image/File field, check in and publish a new version of the referenced dotAsset via the default PUBLISH workflow action, in the asset's own language, then refresh the preview. The field value (the identifier) is unchanged, so the reference is preserved and other content sharing the asset sees the update. The edited binary is passed as the staged temp file id in the asset field; the check-in resolves it server-side. Add DotWorkflowActionsFireService.publishContentletByIdentifier with optional language support (sent as ?language=). Verified end-to-end: PUBLISH fire returns 200, the field reflects the new version, and the reference is preserved. Refs #36363 --- .../dot-workflow-actions-fire.service.ts | 31 +++++++- ...ield.component.legacy-availability.spec.ts | 7 +- .../dot-file-field.component.spec.ts | 7 +- .../dot-file-field.component.ts | 7 +- .../dotasset-image-edit-save.strategy.spec.ts | 71 ++++++++++++++++--- .../dotasset-image-edit-save.strategy.ts | 57 +++++++++++---- .../image-edit-save-strategy.resolver.spec.ts | 5 +- 7 files changed, 158 insertions(+), 27 deletions(-) diff --git a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts index f95b342bf1d8..deae5161f80e 100644 --- a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts +++ b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts @@ -17,6 +17,8 @@ export interface DotActionRequestOptions { action: ActionToFire; individualPermissions?: { [key: string]: string[] }; formData?: FormData; + /** Language of the contentlet version to fire against (sent as `?language=`). */ + language?: string | number; } export interface DotFireActionOptions { @@ -195,6 +197,28 @@ export class DotWorkflowActionsFireService { }); } + /** + * Fire the default PUBLISH action against an existing contentlet, resolved by the + * `identifier` in `data` and the given language. Used to check in and publish a new + * version of the `dotAsset` referenced by an Image/File field from the image editor. + * + * @template T + * @param {{ [key: string]: string }} data contentlet fields (must include `identifier`) + * @param {string | number} [language] language of the version to fire against + * @returns {Observable} + * @memberof DotWorkflowActionsFireService + */ + publishContentletByIdentifier( + data: { [key: string]: string }, + language?: string | number + ): Observable { + return this.request({ + data, + action: ActionToFire.PUBLISH, + language + }); + } + /** * Fire an "DELETE" action over the content type received with the specified data * @@ -239,7 +263,8 @@ export class DotWorkflowActionsFireService { data, action, individualPermissions, - formData + formData, + language }: DotActionRequestOptions): Observable { let url = `${this.BASE_URL}/actions/default/fire/${action}`; @@ -249,6 +274,10 @@ export class DotWorkflowActionsFireService { : { contentlet }; const params = new URLSearchParams({}); + if (language !== undefined && language !== null && `${language}` !== '') { + params.append('language', `${language}`); + } + // It's not best approach but this legacy code if (contentlet['inode']) { params.append('inode', contentlet['inode']); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts index 87e0a95ff3e5..cd6aa7db60cd 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts @@ -7,7 +7,11 @@ import { ReactiveFormsModule } from '@angular/forms'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotAiService, DotMessageService } from '@dotcms/data-access'; +import { + DotAiService, + DotMessageService, + DotWorkflowActionsFireService +} from '@dotcms/data-access'; import { createFakeContentlet } from '@dotcms/utils-testing'; import { DotFileFieldComponent } from './dot-file-field.component'; @@ -56,6 +60,7 @@ describe('DotFileFieldComponent — legacy host availability (no Angular launche BinaryImageEditSaveStrategy, DotAssetImageEditSaveStrategy, ImageEditSaveStrategyResolver, + mockProvider(DotWorkflowActionsFireService), mockProvider(DotMessageService, { get: jest.fn().mockReturnValue('Test Message') }), mockProvider(DotAiService, { checkPluginInstallation: jest.fn().mockReturnValue(of(false)) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts index 79c528df85c1..09b0ca29d73b 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts @@ -7,7 +7,11 @@ import { ReactiveFormsModule } from '@angular/forms'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotAiService, DotMessageService } from '@dotcms/data-access'; +import { + DotAiService, + DotMessageService, + DotWorkflowActionsFireService +} from '@dotcms/data-access'; import { DotGeneratedAIImage, PromptType } from '@dotcms/dotcms-models'; import { createFakeContentlet } from '@dotcms/utils-testing'; @@ -53,6 +57,7 @@ describe('DotFileFieldComponent', () => { BinaryImageEditSaveStrategy, DotAssetImageEditSaveStrategy, ImageEditSaveStrategyResolver, + mockProvider(DotWorkflowActionsFireService), { provide: IMAGE_EDITOR_LAUNCHER, useValue: mockImageEditorLauncher }, mockProvider(DotMessageService, { get: jest.fn().mockReturnValue('Test Message') diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 0d389b51c668..fdacf78aee95 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -23,7 +23,11 @@ import { TooltipModule } from 'primeng/tooltip'; import { filter, map } from 'rxjs/operators'; -import { DotAiService, DotMessageService } from '@dotcms/data-access'; +import { + DotAiService, + DotMessageService, + DotWorkflowActionsFireService +} from '@dotcms/data-access'; import { DotCMSContentlet, DotCMSContentTypeField, @@ -89,6 +93,7 @@ import { IMAGE_EDITOR_LAUNCHER } from '../../../shared/image-editor-launcher'; BinaryImageEditSaveStrategy, DotAssetImageEditSaveStrategy, ImageEditSaveStrategyResolver, + DotWorkflowActionsFireService, { multi: true, provide: NG_VALUE_ACCESSOR, diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts index ed009de0c5ec..ffef9fcc624d 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts @@ -1,26 +1,79 @@ -import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest'; +import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; +import { of, throwError } from 'rxjs'; -import { DotCMSTempFile } from '@dotcms/dotcms-models'; +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; +import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; +import { FileFieldStore } from '../../store/file-field.store'; + +const DOTASSET = { identifier: 'ref-id', inode: 'ref-inode', languageId: 1 } as DotCMSContentlet; +const TEMP_FILE = { id: 'temp_1', fileName: 'edited.png' } as DotCMSTempFile; + describe('DotAssetImageEditSaveStrategy', () => { let spectator: SpectatorService; + let fire: DotWorkflowActionsFireService; + let store: FileFieldStore; const createService = createServiceFactory({ - service: DotAssetImageEditSaveStrategy + service: DotAssetImageEditSaveStrategy, + providers: [ + mockProvider(FileFieldStore, { + uploadedFile: jest.fn().mockReturnValue({ source: 'contentlet', file: DOTASSET }), + getAssetData: jest.fn(), + setUIMessage: jest.fn() + }), + mockProvider(DotWorkflowActionsFireService, { + publishContentletByIdentifier: jest.fn().mockReturnValue(of(DOTASSET)) + }) + ] }); beforeEach(() => { + jest.clearAllMocks(); spectator = createService(); + fire = spectator.inject(DotWorkflowActionsFireService); + store = spectator.inject(FileFieldStore); + // mockProvider jest.fns are shared across tests; re-establish defaults each run. + (store.uploadedFile as jest.Mock).mockReturnValue({ source: 'contentlet', file: DOTASSET }); + (fire.publishContentletByIdentifier as jest.Mock).mockReturnValue(of(DOTASSET)); }); - // Scaffold: the versioned check-in of the referenced dotAsset lands in a - // follow-up step. For now apply() must be a safe no-op that never falls back - // to the Binary inline write (which would corrupt the identifier reference). - it('does not throw when applying (scaffolded no-op)', () => { - const tempFile = { id: 'temp-1', fileName: 'edited.png' } as DotCMSTempFile; + it('publishes a new version of the referenced dotAsset in its own language', () => { + spectator.service.apply(TEMP_FILE); + + expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( + { identifier: 'ref-id', asset: 'temp_1' }, + 1 + ); + }); + + it('refreshes the preview from the new version without changing the field value', () => { + spectator.service.apply(TEMP_FILE); + + expect(store.getAssetData).toHaveBeenCalledWith('ref-id'); + }); + + it('surfaces a server error when the publish fails', () => { + (fire.publishContentletByIdentifier as jest.Mock).mockReturnValue( + throwError(() => new Error('boom')) + ); + + spectator.service.apply(TEMP_FILE); + + expect(store.setUIMessage).toHaveBeenCalled(); + expect(store.getAssetData).not.toHaveBeenCalled(); + }); + + it('does nothing when there is no referenced asset in preview', () => { + (store.uploadedFile as jest.Mock).mockReturnValue({ + source: 'temp', + file: TEMP_FILE + }); + + spectator.service.apply(TEMP_FILE); - expect(() => spectator.service.apply(tempFile)).not.toThrow(); + expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts index 087a60df5ffc..9fd96f86f43f 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts @@ -1,22 +1,29 @@ -import { Injectable } from '@angular/core'; +import { DestroyRef, inject, Injectable } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { DotCMSTempFile } from '@dotcms/dotcms-models'; +import { take } from 'rxjs/operators'; + +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; +import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; +import { FileFieldStore } from '../../store/file-field.store'; +import { getUiMessage } from '../../utils/messages'; + /** * Image/File field save strategy. * * Image and File fields store only an `identifier` that references a separate * `dotAsset` contentlet (the binary lives in that asset's `asset` field). Saving - * an edit must check in a NEW VERSION of the referenced `dotAsset` — preserving - * version history — and refresh the preview WITHOUT mutating the field value, - * which must keep pointing at the same identifier. + * an edit checks in and publishes a NEW VERSION of the referenced `dotAsset` — + * preserving version history — via the default PUBLISH workflow action, then + * refreshes the preview. The field value (the identifier) never changes, so the + * reference is preserved and other content pointing at the same asset sees the + * updated image. * - * Scaffolded intentionally: the versioned check-in of the referenced asset is - * implemented in a follow-up step. It deliberately does NOT fall back to the - * Binary inline write, which would overwrite the field's identifier reference - * with a temp id and corrupt the reference. + * The edited binary is passed as the staged temp file id in the `asset` field; + * the check-in resolves it to the real binary server-side. * * Only ever reached from the new Angular Edit Content: the image-editor entry * point for Image/File fields is gated on the presence of the Angular @@ -24,9 +31,33 @@ import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; */ @Injectable() export class DotAssetImageEditSaveStrategy implements ImageEditSaveStrategy { - apply(_tempFile: DotCMSTempFile): void { - // TODO(#36363): upload/reuse the edited temp file and check in a new - // version of the referenced dotAsset (via the workflow fire endpoint), - // then re-hydrate the preview without changing the field value. + readonly #store = inject(FileFieldStore); + readonly #workflowActionsFire = inject(DotWorkflowActionsFireService); + readonly #destroyRef = inject(DestroyRef); + + apply(tempFile: DotCMSTempFile): void { + const uploaded = this.#store.uploadedFile(); + + // Only reference-backed previews (the resolved dotAsset) can be versioned; + // guard defensively even though the editor entry point already ensures this. + if (uploaded?.source !== 'contentlet') { + return; + } + + const { identifier, languageId } = uploaded.file; + + this.#workflowActionsFire + .publishContentletByIdentifier( + { identifier, asset: tempFile.id }, + languageId + ) + .pipe(take(1), takeUntilDestroyed(this.#destroyRef)) + .subscribe({ + // Re-hydrate the preview from the new version. The field value (the + // identifier) is unchanged, so this refreshes the image without + // touching the reference. + next: () => this.#store.getAssetData(identifier), + error: () => this.#store.setUIMessage(getUiMessage('SERVER_ERROR')) + }); } } diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts index 8c24068d7aff..33f69537ca4f 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts @@ -1,5 +1,7 @@ import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; + import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; import { ImageEditSaveStrategyResolver } from './image-edit-save-strategy.resolver'; @@ -15,7 +17,8 @@ describe('ImageEditSaveStrategyResolver', () => { providers: [ BinaryImageEditSaveStrategy, DotAssetImageEditSaveStrategy, - mockProvider(FileFieldStore, { applyTempFile: jest.fn() }) + mockProvider(FileFieldStore, { applyTempFile: jest.fn() }), + mockProvider(DotWorkflowActionsFireService) ] }); From 07c4236003fab3a31254eaf4af3645b5dd36a72a Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 16:02:54 -0400 Subject: [PATCH 05/12] fix(edit-content): address self-review findings on Image/File save - Version the correct binary field: use the referenced contentlet's titleImage (asset for dotAsset, fileAsset for a legacy FileAsset) instead of hardcoding 'asset', so saves work for File fields backed by a FileAsset. - Keep the Binary edit-image gate strict (isImage flag only) so its behavior is unchanged; the mime fallback in isImageFile applies only to Image/File. - Surface a message instead of silently discarding the edit when the save strategy is reached without a resolved reference. - Tighten the with-save regression assertion to the exact enriched payload. Refs #36363 --- .../dot-file-field.component.ts | 15 ++++++----- .../dotasset-image-edit-save.strategy.spec.ts | 17 +++++++++++- .../dotasset-image-edit-save.strategy.ts | 15 ++++++++--- .../store/features/with-save.feature.spec.ts | 27 +++++++++++++------ 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index fdacf78aee95..a6c8882b0cf2 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -231,15 +231,16 @@ export class DotFileFieldComponent return false; } - if (!isImageFile(this.#currentMetadata())) { - return false; - } - - if (this.store.inputType() !== INPUT_TYPES.Binary) { - return !!this.#imageEditorLauncher; + // Binary keeps its original, strict gate: the authoritative isImage flag + // only. It works in any host via the legacy fallback. + if (this.store.inputType() === INPUT_TYPES.Binary) { + return !!this.#currentMetadata()?.isImage; } - return true; + // Image/File resolve a separate dotAsset/FileAsset and are only supported + // in the new Angular Edit Content, where the image-editor launcher is + // provided — never in the legacy Dojo host. + return isImageFile(this.#currentMetadata()) && !!this.#imageEditorLauncher; }); /** diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts index ffef9fcc624d..d881d52834c3 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts @@ -49,6 +49,20 @@ describe('DotAssetImageEditSaveStrategy', () => { ); }); + it('targets the fileAsset field when the reference is a legacy FileAsset', () => { + (store.uploadedFile as jest.Mock).mockReturnValue({ + source: 'contentlet', + file: { ...DOTASSET, titleImage: 'fileAsset' } + }); + + spectator.service.apply(TEMP_FILE); + + expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( + { identifier: 'ref-id', fileAsset: 'temp_1' }, + 1 + ); + }); + it('refreshes the preview from the new version without changing the field value', () => { spectator.service.apply(TEMP_FILE); @@ -66,7 +80,7 @@ describe('DotAssetImageEditSaveStrategy', () => { expect(store.getAssetData).not.toHaveBeenCalled(); }); - it('does nothing when there is no referenced asset in preview', () => { + it('surfaces a message and does not publish when there is no referenced asset', () => { (store.uploadedFile as jest.Mock).mockReturnValue({ source: 'temp', file: TEMP_FILE @@ -75,5 +89,6 @@ describe('DotAssetImageEditSaveStrategy', () => { spectator.service.apply(TEMP_FILE); expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); + expect(store.setUIMessage).toHaveBeenCalled(); }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts index 9fd96f86f43f..1020296eae4e 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts @@ -38,17 +38,26 @@ export class DotAssetImageEditSaveStrategy implements ImageEditSaveStrategy { apply(tempFile: DotCMSTempFile): void { const uploaded = this.#store.uploadedFile(); - // Only reference-backed previews (the resolved dotAsset) can be versioned; - // guard defensively even though the editor entry point already ensures this. + // Only reference-backed previews (the resolved dotAsset/FileAsset) can be + // versioned. The editor entry point already ensures this, so reaching here + // without one is an unexpected state — surface it instead of silently + // discarding the user's edit. if (uploaded?.source !== 'contentlet') { + this.#store.setUIMessage(getUiMessage('SERVER_ERROR')); + return; } const { identifier, languageId } = uploaded.file; + // The binary field variable differs by referenced type: `asset` for a + // dotAsset, `fileAsset` for a legacy FileAsset. `titleImage` carries the + // server-computed field name (the same signal onEditImage uses to open the + // editor); default to `asset`. + const fieldVariable = (uploaded.file['titleImage'] as string) || 'asset'; this.#workflowActionsFire .publishContentletByIdentifier( - { identifier, asset: tempFile.id }, + { identifier, [fieldVariable]: tempFile.id }, languageId ) .pipe(take(1), takeUntilDestroyed(this.#destroyRef)) diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts index a101346bca54..203d9c8c16c6 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts @@ -135,14 +135,25 @@ describe('withSave', () => { lifecycle.saveRequested(); - const event = dispatchSpy.mock.calls - .map(([arg]) => arg) - .find((arg) => arg?.type === imageEditorLifecycleEvents.saveSucceeded.type); - // Payload shape is normalized to tolerate nested (`{ type, payload }`) or spread. - const payload = event.payload ?? event; - expect(payload.image).toBe(true); - expect(payload.metadata.isImage).toBe(true); - expect(payload.metadata.focalPoint).toBe('0.25,0.75'); + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveSucceeded({ + ...TEMP_FILE, + image: true, + mimeType: 'unknown', + metadata: { + contentType: 'unknown', + fileSize: 1024, + length: 1024, + modDate: 0, + name: 'edited.png', + sha256: '', + title: 'edited.png', + version: 0, + isImage: true, + focalPoint: '0.25,0.75' + } + }) + ); }); it('folds the current focal point into the returned temp metadata', () => { From be03a3815f9e698ca67ed1839007f4c4abd1576c Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 16:19:50 -0400 Subject: [PATCH 06/12] refactor(edit-content): replace image-save Strategy with a store method The Strategy pattern (interface + 2 impls + resolver + DI wiring) was over-engineered for two stable branches. Replace it with a publishEditedAsset rxMethod on FileFieldStore, next to applyTempFile/getAssetData where the asset logic already lives, and branch inline in the component (Binary -> applyTempFile, Image/File -> publishEditedAsset). The rxMethod's switchMap also serializes concurrent saves (a re-triggered edit cancels the in-flight publish+refresh), closing the concurrency gap the review flagged. Behavior is unchanged: same PUBLISH-by-identifier in the asset's language, same titleImage-based field selection (asset vs fileAsset), same refresh without mutating the field value. Removes the save-strategy folder and its specs; adds store coverage. Refs #36363 --- ...ield.component.legacy-availability.spec.ts | 8 -- .../dot-file-field.component.spec.ts | 8 -- .../dot-file-field.component.ts | 32 +++---- .../binary-image-edit-save.strategy.spec.ts | 29 ------ .../binary-image-edit-save.strategy.ts | 23 ----- .../dotasset-image-edit-save.strategy.spec.ts | 94 ------------------- .../dotasset-image-edit-save.strategy.ts | 72 -------------- .../image-edit-save-strategy.model.ts | 27 ------ .../image-edit-save-strategy.resolver.spec.ts | 50 ---------- .../image-edit-save-strategy.resolver.ts | 23 ----- .../services/save-strategy/index.ts | 4 - .../store/file-field.store.spec.ts | 82 +++++++++++++++- .../store/file-field.store.ts | 66 ++++++++++++- 13 files changed, 157 insertions(+), 361 deletions(-) delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts index cd6aa7db60cd..5bf7a846c383 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.legacy-availability.spec.ts @@ -21,11 +21,6 @@ import { LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher } from '../../services/image-editor'; -import { - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver -} from '../../services/save-strategy'; import { DotFileFieldUploadService } from '../../services/upload-file/upload-file.service'; import { FileFieldStore } from '../../store/file-field.store'; import { DotFileFieldPreviewComponent } from '../dot-file-field-preview/dot-file-field-preview.component'; @@ -57,9 +52,6 @@ describe('DotFileFieldComponent — legacy host availability (no Angular launche mockProvider(DialogService), LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver, mockProvider(DotWorkflowActionsFireService), mockProvider(DotMessageService, { get: jest.fn().mockReturnValue('Test Message') }), mockProvider(DotAiService, { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts index 09b0ca29d73b..8733324ca76e 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts @@ -23,11 +23,6 @@ import { LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher } from '../../services/image-editor'; -import { - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver -} from '../../services/save-strategy'; import { DotFileFieldUploadService } from '../../services/upload-file/upload-file.service'; import { FileFieldStore } from '../../store/file-field.store'; import { DotFileFieldPreviewComponent } from '../dot-file-field-preview/dot-file-field-preview.component'; @@ -54,9 +49,6 @@ describe('DotFileFieldComponent', () => { mockProvider(DialogService), LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver, mockProvider(DotWorkflowActionsFireService), { provide: IMAGE_EDITOR_LAUNCHER, useValue: mockImageEditorLauncher }, mockProvider(DotMessageService, { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index a6c8882b0cf2..af76779f0c58 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -51,11 +51,6 @@ import { LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher } from './../../services/image-editor'; -import { - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver -} from './../../services/save-strategy'; import { DotFileFieldUploadService } from './../../services/upload-file/upload-file.service'; import { FileFieldStore } from './../../store/file-field.store'; import { parseFocalPoint } from './../../utils/focal-point.util'; @@ -90,9 +85,6 @@ import { IMAGE_EDITOR_LAUNCHER } from '../../../shared/image-editor-launcher'; DialogService, LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - ImageEditSaveStrategyResolver, DotWorkflowActionsFireService, { multi: true, @@ -125,11 +117,6 @@ export class DotFileFieldComponent * the legacy launchers. */ readonly #imageEditorLauncher = inject(IMAGE_EDITOR_LAUNCHER, { optional: true }); - /** - * Resolves how an edited image is persisted based on the field's input type: - * Binary applies the edit inline, Image/File version the referenced dotAsset. - */ - readonly #imageEditSaveStrategyResolver = inject(ImageEditSaveStrategyResolver); /** * A readonly private field that holds an instance of the DialogService. * This service is injected using Angular's dependency injection mechanism. @@ -477,11 +464,10 @@ export class DotFileFieldComponent } /** - * Applies the edited image emitted by an image-editor launcher, delegating to - * the {@link ImageEditSaveStrategy} resolved for the field's input type (Binary - * applies inline; Image/File version the referenced dotAsset). Shared by the - * new Angular editor and the legacy launchers. Ignores a closed editor (no temp - * file) and surfaces a server error if the stream fails. + * Applies the edited image emitted by an image-editor launcher. Binary applies + * the edit inline; Image/File version the referenced dotAsset/FileAsset. Shared + * by the new Angular editor and the legacy launchers. Ignores a closed editor + * (no temp file) and surfaces a server error if the stream fails. * * @param result$ the launcher's close stream, emitting the edited temp file or null */ @@ -493,9 +479,13 @@ export class DotFileFieldComponent ) .subscribe({ next: (tempFile) => { - this.#imageEditSaveStrategyResolver - .resolve(this.store.inputType()) - .apply(tempFile); + // Binary keeps the binary inline; Image/File reference a separate + // dotAsset/FileAsset that must be versioned via a check-in + publish. + if (this.store.inputType() === INPUT_TYPES.Binary) { + this.store.applyTempFile(tempFile); + } else { + this.store.publishEditedAsset(tempFile); + } }, error: () => { this.store.setUIMessage(getUiMessage('SERVER_ERROR')); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts deleted file mode 100644 index ba3b10fa6f5f..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.spec.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; - -import { DotCMSTempFile } from '@dotcms/dotcms-models'; - -import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; - -import { FileFieldStore } from '../../store/file-field.store'; - -describe('BinaryImageEditSaveStrategy', () => { - let spectator: SpectatorService; - - const createService = createServiceFactory({ - service: BinaryImageEditSaveStrategy, - providers: [mockProvider(FileFieldStore, { applyTempFile: jest.fn() })] - }); - - beforeEach(() => { - spectator = createService(); - }); - - it('applies the edited temp file inline via the store', () => { - const tempFile = { id: 'temp-1', fileName: 'edited.png' } as DotCMSTempFile; - const store = spectator.inject(FileFieldStore); - - spectator.service.apply(tempFile); - - expect(store.applyTempFile).toHaveBeenCalledWith(tempFile); - }); -}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts deleted file mode 100644 index fb302d570ccd..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/binary-image-edit-save.strategy.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { inject, Injectable } from '@angular/core'; - -import { DotCMSTempFile } from '@dotcms/dotcms-models'; - -import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; - -import { FileFieldStore } from '../../store/file-field.store'; - -/** - * Binary field save strategy — the existing behavior. - * - * The binary lives inline on the contentlet, so applying the edit just swaps the - * preview to the edited temp file; its temp id becomes the field value and is - * resolved to a binary on the standard contentlet check-in. - */ -@Injectable() -export class BinaryImageEditSaveStrategy implements ImageEditSaveStrategy { - readonly #store = inject(FileFieldStore); - - apply(tempFile: DotCMSTempFile): void { - this.#store.applyTempFile(tempFile); - } -} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts deleted file mode 100644 index d881d52834c3..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.spec.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; -import { of, throwError } from 'rxjs'; - -import { DotWorkflowActionsFireService } from '@dotcms/data-access'; -import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; - -import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; - -import { FileFieldStore } from '../../store/file-field.store'; - -const DOTASSET = { identifier: 'ref-id', inode: 'ref-inode', languageId: 1 } as DotCMSContentlet; -const TEMP_FILE = { id: 'temp_1', fileName: 'edited.png' } as DotCMSTempFile; - -describe('DotAssetImageEditSaveStrategy', () => { - let spectator: SpectatorService; - let fire: DotWorkflowActionsFireService; - let store: FileFieldStore; - - const createService = createServiceFactory({ - service: DotAssetImageEditSaveStrategy, - providers: [ - mockProvider(FileFieldStore, { - uploadedFile: jest.fn().mockReturnValue({ source: 'contentlet', file: DOTASSET }), - getAssetData: jest.fn(), - setUIMessage: jest.fn() - }), - mockProvider(DotWorkflowActionsFireService, { - publishContentletByIdentifier: jest.fn().mockReturnValue(of(DOTASSET)) - }) - ] - }); - - beforeEach(() => { - jest.clearAllMocks(); - spectator = createService(); - fire = spectator.inject(DotWorkflowActionsFireService); - store = spectator.inject(FileFieldStore); - // mockProvider jest.fns are shared across tests; re-establish defaults each run. - (store.uploadedFile as jest.Mock).mockReturnValue({ source: 'contentlet', file: DOTASSET }); - (fire.publishContentletByIdentifier as jest.Mock).mockReturnValue(of(DOTASSET)); - }); - - it('publishes a new version of the referenced dotAsset in its own language', () => { - spectator.service.apply(TEMP_FILE); - - expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( - { identifier: 'ref-id', asset: 'temp_1' }, - 1 - ); - }); - - it('targets the fileAsset field when the reference is a legacy FileAsset', () => { - (store.uploadedFile as jest.Mock).mockReturnValue({ - source: 'contentlet', - file: { ...DOTASSET, titleImage: 'fileAsset' } - }); - - spectator.service.apply(TEMP_FILE); - - expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( - { identifier: 'ref-id', fileAsset: 'temp_1' }, - 1 - ); - }); - - it('refreshes the preview from the new version without changing the field value', () => { - spectator.service.apply(TEMP_FILE); - - expect(store.getAssetData).toHaveBeenCalledWith('ref-id'); - }); - - it('surfaces a server error when the publish fails', () => { - (fire.publishContentletByIdentifier as jest.Mock).mockReturnValue( - throwError(() => new Error('boom')) - ); - - spectator.service.apply(TEMP_FILE); - - expect(store.setUIMessage).toHaveBeenCalled(); - expect(store.getAssetData).not.toHaveBeenCalled(); - }); - - it('surfaces a message and does not publish when there is no referenced asset', () => { - (store.uploadedFile as jest.Mock).mockReturnValue({ - source: 'temp', - file: TEMP_FILE - }); - - spectator.service.apply(TEMP_FILE); - - expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); - expect(store.setUIMessage).toHaveBeenCalled(); - }); -}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts deleted file mode 100644 index 1020296eae4e..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/dotasset-image-edit-save.strategy.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { DestroyRef, inject, Injectable } from '@angular/core'; -import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; - -import { take } from 'rxjs/operators'; - -import { DotWorkflowActionsFireService } from '@dotcms/data-access'; -import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; - -import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; - -import { FileFieldStore } from '../../store/file-field.store'; -import { getUiMessage } from '../../utils/messages'; - -/** - * Image/File field save strategy. - * - * Image and File fields store only an `identifier` that references a separate - * `dotAsset` contentlet (the binary lives in that asset's `asset` field). Saving - * an edit checks in and publishes a NEW VERSION of the referenced `dotAsset` — - * preserving version history — via the default PUBLISH workflow action, then - * refreshes the preview. The field value (the identifier) never changes, so the - * reference is preserved and other content pointing at the same asset sees the - * updated image. - * - * The edited binary is passed as the staged temp file id in the `asset` field; - * the check-in resolves it to the real binary server-side. - * - * Only ever reached from the new Angular Edit Content: the image-editor entry - * point for Image/File fields is gated on the presence of the Angular - * image-editor launcher, so this never runs in the legacy Dojo host. - */ -@Injectable() -export class DotAssetImageEditSaveStrategy implements ImageEditSaveStrategy { - readonly #store = inject(FileFieldStore); - readonly #workflowActionsFire = inject(DotWorkflowActionsFireService); - readonly #destroyRef = inject(DestroyRef); - - apply(tempFile: DotCMSTempFile): void { - const uploaded = this.#store.uploadedFile(); - - // Only reference-backed previews (the resolved dotAsset/FileAsset) can be - // versioned. The editor entry point already ensures this, so reaching here - // without one is an unexpected state — surface it instead of silently - // discarding the user's edit. - if (uploaded?.source !== 'contentlet') { - this.#store.setUIMessage(getUiMessage('SERVER_ERROR')); - - return; - } - - const { identifier, languageId } = uploaded.file; - // The binary field variable differs by referenced type: `asset` for a - // dotAsset, `fileAsset` for a legacy FileAsset. `titleImage` carries the - // server-computed field name (the same signal onEditImage uses to open the - // editor); default to `asset`. - const fieldVariable = (uploaded.file['titleImage'] as string) || 'asset'; - - this.#workflowActionsFire - .publishContentletByIdentifier( - { identifier, [fieldVariable]: tempFile.id }, - languageId - ) - .pipe(take(1), takeUntilDestroyed(this.#destroyRef)) - .subscribe({ - // Re-hydrate the preview from the new version. The field value (the - // identifier) is unchanged, so this refreshes the image without - // touching the reference. - next: () => this.#store.getAssetData(identifier), - error: () => this.#store.setUIMessage(getUiMessage('SERVER_ERROR')) - }); - } -} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts deleted file mode 100644 index 3d6d7c750585..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.model.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { DotCMSTempFile } from '@dotcms/dotcms-models'; - -/** - * Strategy for persisting the result of an image-editor session back to a file - * field. - * - * {@link DotFileFieldComponent} renders Binary, Image and File fields with a - * single component, but each stores its binary differently, so the "apply edited - * image" step differs per input type: - * - * - **Binary** keeps the binary inline on the contentlet, so the edited temp file - * becomes the field value directly (resolved to a binary on the contentlet - * check-in). - * - **Image/File** reference a separate `dotAsset` contentlet by identifier (the - * binary lives in that asset's `asset` field), so the edit must be checked in as - * a new version of the referenced asset without changing the field value. - * - * Implementations are selected by {@link ImageEditSaveStrategyResolver}. - */ -export interface ImageEditSaveStrategy { - /** - * Applies the temp file returned by the image editor to the field. - * - * @param tempFile - The edited image staged by the editor as a temp file. - */ - apply(tempFile: DotCMSTempFile): void; -} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts deleted file mode 100644 index 33f69537ca4f..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.spec.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; - -import { DotWorkflowActionsFireService } from '@dotcms/data-access'; - -import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; -import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; -import { ImageEditSaveStrategyResolver } from './image-edit-save-strategy.resolver'; - -import { INPUT_TYPES } from '../../../../models/dot-edit-content-file.model'; -import { FileFieldStore } from '../../store/file-field.store'; - -describe('ImageEditSaveStrategyResolver', () => { - let spectator: SpectatorService; - - const createService = createServiceFactory({ - service: ImageEditSaveStrategyResolver, - providers: [ - BinaryImageEditSaveStrategy, - DotAssetImageEditSaveStrategy, - mockProvider(FileFieldStore, { applyTempFile: jest.fn() }), - mockProvider(DotWorkflowActionsFireService) - ] - }); - - beforeEach(() => { - spectator = createService(); - }); - - it('resolves the Binary strategy for Binary fields', () => { - expect(spectator.service.resolve(INPUT_TYPES.Binary)).toBeInstanceOf( - BinaryImageEditSaveStrategy - ); - }); - - it('resolves the dotAsset strategy for Image fields', () => { - expect(spectator.service.resolve(INPUT_TYPES.Image)).toBeInstanceOf( - DotAssetImageEditSaveStrategy - ); - }); - - it('resolves the dotAsset strategy for File fields', () => { - expect(spectator.service.resolve(INPUT_TYPES.File)).toBeInstanceOf( - DotAssetImageEditSaveStrategy - ); - }); - - it('falls back to the dotAsset strategy when the input type is null', () => { - expect(spectator.service.resolve(null)).toBeInstanceOf(DotAssetImageEditSaveStrategy); - }); -}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts deleted file mode 100644 index 6ff23a816f38..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/image-edit-save-strategy.resolver.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { inject, Injectable } from '@angular/core'; - -import { BinaryImageEditSaveStrategy } from './binary-image-edit-save.strategy'; -import { DotAssetImageEditSaveStrategy } from './dotasset-image-edit-save.strategy'; -import { ImageEditSaveStrategy } from './image-edit-save-strategy.model'; - -import { INPUT_TYPE, INPUT_TYPES } from '../../../../models/dot-edit-content-file.model'; - -/** - * Resolves the {@link ImageEditSaveStrategy} for a field's input type. - * - * Binary fields apply the edit inline; everything else (Image/File) is a - * reference-backed field and versions the referenced `dotAsset`. - */ -@Injectable() -export class ImageEditSaveStrategyResolver { - readonly #binary = inject(BinaryImageEditSaveStrategy); - readonly #dotAsset = inject(DotAssetImageEditSaveStrategy); - - resolve(inputType: INPUT_TYPE | null): ImageEditSaveStrategy { - return inputType === INPUT_TYPES.Binary ? this.#binary : this.#dotAsset; - } -} diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts deleted file mode 100644 index 702e18175a37..000000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/services/save-strategy/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -export * from './image-edit-save-strategy.model'; -export * from './binary-image-edit-save.strategy'; -export * from './dotasset-image-edit-save.strategy'; -export * from './image-edit-save-strategy.resolver'; diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts index 460991a7e575..0674bffbabea 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts @@ -3,6 +3,9 @@ import { of, throwError } from 'rxjs'; import { TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; +import { DotCMSContentlet, DotCMSTempFile } from '@dotcms/dotcms-models'; + import { FileFieldStore } from './file-field.store'; import { UIMessage } from '../../../models/dot-edit-content-file.model'; @@ -16,7 +19,11 @@ describe('FileFieldStore', () => { beforeEach(() => { TestBed.configureTestingModule({ - providers: [FileFieldStore, mockProvider(DotFileFieldUploadService)] + providers: [ + FileFieldStore, + mockProvider(DotFileFieldUploadService), + mockProvider(DotWorkflowActionsFireService) + ] }); store = TestBed.inject(FileFieldStore); @@ -257,4 +264,77 @@ describe('FileFieldStore', () => { expect(store.uiMessage()).toEqual(getUiMessage('SERVER_ERROR')); }); }); + + describe('Method: publishEditedAsset', () => { + let fire: SpyObject; + const TEMP = { id: 'temp_1', fileName: 'edited.png' } as DotCMSTempFile; + const ASSET = { + identifier: 'ref-id', + inode: 'ref-inode', + languageId: 1 + } as DotCMSContentlet; + + beforeEach(() => { + fire = TestBed.inject( + DotWorkflowActionsFireService + ) as SpyObject; + // Hydrate the preview with a referenced dotAsset. + service.getContentById.mockReturnValue(of(ASSET)); + store.getAssetData('ref-id'); + }); + + it('publishes a new version of the referenced dotAsset in its own language', () => { + fire.publishContentletByIdentifier.mockReturnValue(of(ASSET)); + + store.publishEditedAsset(TEMP); + + expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( + { identifier: 'ref-id', asset: 'temp_1' }, + 1 + ); + }); + + it('targets the fileAsset field for a legacy FileAsset reference', () => { + service.getContentById.mockReturnValue(of({ ...ASSET, titleImage: 'fileAsset' })); + store.getAssetData('ref-id'); + fire.publishContentletByIdentifier.mockReturnValue(of(ASSET)); + + store.publishEditedAsset(TEMP); + + expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( + { identifier: 'ref-id', fileAsset: 'temp_1' }, + 1 + ); + }); + + it('refreshes the preview from the new version without changing the value', () => { + const newVersion = { ...ASSET, inode: 'new-inode' }; + fire.publishContentletByIdentifier.mockReturnValue(of(ASSET)); + service.getContentById.mockReturnValue(of(newVersion)); + + store.publishEditedAsset(TEMP); + + expect(service.getContentById).toHaveBeenCalledWith('ref-id'); + expect(store.uploadedFile()).toEqual({ source: 'contentlet', file: newVersion }); + expect(store.value()).toBe('ref-id'); + expect(store.fileStatus()).toBe('preview'); + }); + + it('surfaces a server error when the publish fails', () => { + fire.publishContentletByIdentifier.mockReturnValue(throwError(() => 'error')); + + store.publishEditedAsset(TEMP); + + expect(store.uiMessage()).toEqual(getUiMessage('SERVER_ERROR')); + }); + + it('surfaces a message and does not publish without a referenced asset', () => { + store.removeFile(); + + store.publishEditedAsset(TEMP); + + expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); + expect(store.uiMessage()).toEqual(getUiMessage('SERVER_ERROR')); + }); + }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts index d250c324703f..c3eeda4678a0 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts @@ -1,13 +1,14 @@ import { tapResponse } from '@ngrx/operators'; import { patchState, signalStore, withComputed, withMethods, withState } from '@ngrx/signals'; import { rxMethod } from '@ngrx/signals/rxjs-interop'; -import { Observable, of, pipe } from 'rxjs'; +import { EMPTY, Observable, of, pipe } from 'rxjs'; import { HttpClient } from '@angular/common/http'; import { computed, inject } from '@angular/core'; import { filter, map, switchMap, tap } from 'rxjs/operators'; +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; import { DotCMSContentlet, DotCMSTempFile, DotFileMetadata } from '@dotcms/dotcms-models'; import { @@ -114,6 +115,7 @@ export const FileFieldStore = signalStore( })), withMethods((store) => { const uploadService = inject(DotFileFieldUploadService); + const workflowActionsFire = inject(DotWorkflowActionsFireService); const http = inject(HttpClient); /** @@ -333,6 +335,68 @@ export const FileFieldStore = signalStore( ) ) ), + /** + * publishEditedAsset versions the referenced asset (Image/File fields). + * + * Image/File fields store only an `identifier` referencing a separate + * `dotAsset`/`FileAsset` contentlet (the binary lives in its `asset` / + * `fileAsset` field). Saving an edit checks in and publishes a NEW version + * of that referenced contentlet in its own language, then re-hydrates the + * preview. The field value (the identifier) is unchanged, so the reference + * is preserved and other content sharing the asset sees the update. + * + * `switchMap` serializes concurrent saves: a re-triggered edit cancels an + * in-flight publish+refresh instead of racing it. + * @param tempFile edited image staged as a temp file by the image editor + */ + publishEditedAsset: rxMethod( + pipe( + switchMap((tempFile) => { + const uploaded = store.uploadedFile(); + + // Only a resolved dotAsset/FileAsset reference can be versioned; + // reaching here without one is unexpected — surface it instead of + // silently discarding the edit. + if (uploaded?.source !== 'contentlet') { + patchState(store, { uiMessage: getUiMessage('SERVER_ERROR') }); + + return EMPTY; + } + + const { identifier, languageId } = uploaded.file; + // The binary field variable differs by referenced type: `asset` + // for a dotAsset, `fileAsset` for a legacy FileAsset. titleImage + // carries the server-computed field name; default to `asset`. + const fieldVariable = (uploaded.file['titleImage'] as string) || 'asset'; + + patchState(store, { fileStatus: 'uploading' }); + + return workflowActionsFire + .publishContentletByIdentifier( + { identifier, [fieldVariable]: tempFile.id }, + languageId + ) + .pipe( + switchMap(() => uploadService.getContentById(identifier)), + tapResponse({ + next: (file) => { + patchState(store, { + fileStatus: 'preview', + value: file.identifier, + uploadedFile: { source: 'contentlet', file } + }); + }, + error: () => { + patchState(store, { + fileStatus: 'preview', + uiMessage: getUiMessage('SERVER_ERROR') + }); + } + }) + ); + }) + ) + ), /** * setFileFromContentlet hydrates the preview from a saved contentlet. * Used by the binary web component, which receives the contentlet From 2805c7d3b86d33185b91df8652c9ef607d4d592f Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 2 Jul 2026 16:34:26 -0400 Subject: [PATCH 07/12] refactor(edit-content): move the apply-edited-image flow into the store The component no longer subscribes to the launcher stream or routes by input type. onEditImage just hands the launcher's close stream to a store method, store.applyEditedImage(result$), which owns the whole flow: filter the closed editor (null), route by input type (Binary inline vs Image/File versioned), and surface a server error on stream failure. Subscription is torn down with the store. Keeps the component thin (no #applyEditedImage, no manual subscribe/#destroyRef for this path) and the persistence logic cohesive in the store. Refs #36363 --- .../dot-file-field.component.spec.ts | 12 ++-- .../dot-file-field.component.ts | 35 +----------- .../store/file-field.store.spec.ts | 56 +++++++++++++++++++ .../store/file-field.store.ts | 39 ++++++++++++- 4 files changed, 104 insertions(+), 38 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts index 8733324ca76e..332e3f5aec9b 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.spec.ts @@ -185,7 +185,9 @@ describe('DotFileFieldComponent', () => { mockImageEditorLauncher.open.mockReturnValue(of(EDITED_TEMP_FILE)); setupBinaryWithImage(); - const applySpy = jest.spyOn(spectator.component.store, 'applyTempFile'); + const applySpy = jest + .spyOn(spectator.component.store, 'applyEditedImage') + .mockImplementation(); spectator.component.onEditImage(); @@ -197,7 +199,7 @@ describe('DotFileFieldComponent', () => { mimeType: 'image/png' }) ); - expect(applySpy).toHaveBeenCalledWith(EDITED_TEMP_FILE); + expect(applySpy).toHaveBeenCalled(); }); it('should fall back to the legacy editor when the new launcher is unavailable', () => { @@ -210,13 +212,15 @@ describe('DotFileFieldComponent', () => { const legacyOpenSpy = jest .spyOn(legacy, 'open') .mockReturnValue(of(EDITED_TEMP_FILE) as never); - const applySpy = jest.spyOn(spectator.component.store, 'applyTempFile'); + const applySpy = jest + .spyOn(spectator.component.store, 'applyEditedImage') + .mockImplementation(); spectator.component.onEditImage(); expect(mockImageEditorLauncher.open).not.toHaveBeenCalled(); expect(legacyOpenSpy).toHaveBeenCalled(); - expect(applySpy).toHaveBeenCalledWith(EDITED_TEMP_FILE); + expect(applySpy).toHaveBeenCalled(); }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index af76779f0c58..759f4d134368 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -1,5 +1,4 @@ import { signalMethod } from '@ngrx/signals'; -import { Observable } from 'rxjs'; import { AfterViewInit, @@ -433,7 +432,7 @@ export class DotFileFieldComponent // the marker instead of resetting it to centre. const focalPoint = parseFocalPoint(metadata?.focalPoint); - this.#applyEditedImage( + this.store.applyEditedImage( newLauncher.open({ inode, tempId, @@ -453,7 +452,7 @@ export class DotFileFieldComponent ? this.#legacyDojoImageEditorLauncher : this.#legacyDialogImageEditorLauncher; - this.#applyEditedImage( + this.store.applyEditedImage( legacyLauncher.open({ inode, tempId, @@ -463,36 +462,6 @@ export class DotFileFieldComponent ); } - /** - * Applies the edited image emitted by an image-editor launcher. Binary applies - * the edit inline; Image/File version the referenced dotAsset/FileAsset. Shared - * by the new Angular editor and the legacy launchers. Ignores a closed editor - * (no temp file) and surfaces a server error if the stream fails. - * - * @param result$ the launcher's close stream, emitting the edited temp file or null - */ - #applyEditedImage(result$: Observable): void { - result$ - .pipe( - filter((tempFile): tempFile is DotCMSTempFile => !!tempFile), - takeUntilDestroyed(this.#destroyRef) - ) - .subscribe({ - next: (tempFile) => { - // Binary keeps the binary inline; Image/File reference a separate - // dotAsset/FileAsset that must be versioned via a check-in + publish. - if (this.store.inputType() === INPUT_TYPES.Binary) { - this.store.applyTempFile(tempFile); - } else { - this.store.publishEditedAsset(tempFile); - } - }, - error: () => { - this.store.setUIMessage(getUiMessage('SERVER_ERROR')); - } - }); - } - /** * Handle file drop event. * diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts index 0674bffbabea..777fdfc7b0b0 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.spec.ts @@ -337,4 +337,60 @@ describe('FileFieldStore', () => { expect(store.uiMessage()).toEqual(getUiMessage('SERVER_ERROR')); }); }); + + describe('Method: applyEditedImage', () => { + const TEMP = { id: 'temp_1', fileName: 'edited.png' } as DotCMSTempFile; + const ASSET = { + identifier: 'ref-id', + inode: 'ref-inode', + languageId: 1 + } as DotCMSContentlet; + let fire: SpyObject; + + beforeEach(() => { + fire = TestBed.inject( + DotWorkflowActionsFireService + ) as SpyObject; + fire.publishContentletByIdentifier.mockReturnValue(of(ASSET)); + service.getContentById.mockReturnValue(of(ASSET)); + }); + + it('routes Binary edits to the inline apply (no asset publish)', () => { + store.initLoad({ fieldVariable: 'bin', inputType: 'Binary' }); + + store.applyEditedImage(of(TEMP)); + + expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); + expect(store.value()).toBe('temp_1'); + expect(store.uploadedFile()).toEqual({ source: 'temp', file: TEMP }); + }); + + it('routes Image/File edits to the referenced-asset publish', () => { + store.initLoad({ fieldVariable: 'img', inputType: 'Image' }); + store.getAssetData('ref-id'); // hydrate the referenced contentlet + + store.applyEditedImage(of(TEMP)); + + expect(fire.publishContentletByIdentifier).toHaveBeenCalledWith( + { identifier: 'ref-id', asset: 'temp_1' }, + 1 + ); + }); + + it('ignores a closed editor (null) without persisting', () => { + store.initLoad({ fieldVariable: 'img', inputType: 'Image' }); + + store.applyEditedImage(of(null)); + + expect(fire.publishContentletByIdentifier).not.toHaveBeenCalled(); + }); + + it('surfaces a server error when the launcher stream fails', () => { + store.initLoad({ fieldVariable: 'img', inputType: 'Image' }); + + store.applyEditedImage(throwError(() => 'error')); + + expect(store.uiMessage()).toEqual(getUiMessage('SERVER_ERROR')); + }); + }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts index c3eeda4678a0..7bfeac7dd822 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/store/file-field.store.ts @@ -4,7 +4,8 @@ import { rxMethod } from '@ngrx/signals/rxjs-interop'; import { EMPTY, Observable, of, pipe } from 'rxjs'; import { HttpClient } from '@angular/common/http'; -import { computed, inject } from '@angular/core'; +import { computed, DestroyRef, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { filter, map, switchMap, tap } from 'rxjs/operators'; @@ -488,5 +489,41 @@ export const FileFieldStore = signalStore( ) ) }; + }), + withMethods((store) => { + const destroyRef = inject(DestroyRef); + + return { + /** + * Consumes an image-editor launcher's close stream and persists the edit. + * + * Owns the whole apply flow so callers just hand over the launcher stream: + * ignores a closed editor (null), routes by input type (Binary applies the + * edit inline; Image/File version the referenced dotAsset/FileAsset), and + * surfaces a server error if the stream fails. The subscription is torn + * down with the store. + * + * @param result$ the launcher close stream emitting the edited temp file or null + */ + applyEditedImage(result$: Observable): void { + result$ + .pipe( + filter((tempFile): tempFile is DotCMSTempFile => !!tempFile), + takeUntilDestroyed(destroyRef) + ) + .subscribe({ + next: (tempFile) => { + if (store.inputType() === INPUT_TYPES.Binary) { + store.applyTempFile(tempFile); + } else { + store.publishEditedAsset(tempFile); + } + }, + error: () => { + patchState(store, { uiMessage: getUiMessage('SERVER_ERROR') }); + } + }); + } + }; }) ); From f3d59a1af6b4d47dca177f5319ffb4e957207e10 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Fri, 3 Jul 2026 10:53:43 -0400 Subject: [PATCH 08/12] fix(edit-content): provide DotWorkflowActionsFireService where FileFieldStore is FileFieldStore now injects DotWorkflowActionsFireService at construction, so every injector that provides the store must satisfy that dependency. The wrapper field component and its spec provided FileFieldStore without it, breaking the full edit-content unit suite in CI (NG0201). Provide the service alongside the store (component + spec) and assert the edit flow via the store's applyEditedImage entry point. Refs #36363 --- .../dot-edit-content-file-field.component.spec.ts | 8 +++++--- .../dot-edit-content-file-field.component.ts | 8 +++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.spec.ts index 79a43adb2f4c..f785fa6ce2fb 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.spec.ts @@ -12,7 +12,8 @@ import { DotContentletService, DotMessageService, DotUploadFileService, - DotUploadService + DotUploadService, + DotWorkflowActionsFireService } from '@dotcms/data-access'; import { DotCMSContentlet, DotCMSContentTypeField } from '@dotcms/dotcms-models'; import { DotDropZoneComponent, DropZoneErrorType, DropZoneFileEvent } from '@dotcms/ui'; @@ -74,6 +75,7 @@ describe('DotFileFieldComponent', () => { DotFileFieldUploadService, LegacyDialogImageEditorLauncher, LegacyDojoImageEditorLauncher, + mockProvider(DotWorkflowActionsFireService), provideHttpClient(), provideHttpClientTesting(), mockProvider(DotUploadFileService), @@ -565,13 +567,13 @@ describe('DotFileFieldComponent', () => { setImagePreview(true); - const spyApply = jest.spyOn(store, 'applyTempFile'); + const spyApply = jest.spyOn(store, 'applyEditedImage').mockImplementation(); spectator.component.onEditImage(); expect(dialogLauncher.open).toHaveBeenCalled(); expect(dojoLauncher.open).not.toHaveBeenCalled(); - expect(spyApply).toHaveBeenCalledWith(tempFile); + expect(spyApply).toHaveBeenCalled(); }); it('should open the Dojo launcher when useLegacyDojoImageEditor is true', () => { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.ts index 00dcc94e74dd..02359eb1272b 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/dot-edit-content-file-field.component.ts @@ -3,6 +3,7 @@ import { ControlContainer, ReactiveFormsModule } from '@angular/forms'; import { DialogService } from 'primeng/dynamicdialog'; +import { DotWorkflowActionsFireService } from '@dotcms/data-access'; import { DotCMSContentTypeField, DotCMSContentlet } from '@dotcms/dotcms-models'; import { DotMessagePipe } from '@dotcms/ui'; @@ -28,7 +29,12 @@ import { BaseWrapperField } from '../shared/base-wrapper-field'; DotMessagePipe, ReactiveFormsModule ], - providers: [DotFileFieldUploadService, FileFieldStore, DialogService], + providers: [ + DotFileFieldUploadService, + FileFieldStore, + DialogService, + DotWorkflowActionsFireService + ], templateUrl: './dot-edit-content-file-field.component.html', changeDetection: ChangeDetectionStrategy.OnPush, viewProviders: [ From 424d7e37fc999f1d72076e0314590953995e31d7 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 6 Jul 2026 13:28:29 -0400 Subject: [PATCH 09/12] fix(edit-content): restore focal point on reopen for referenced assets The focal point persists correctly on save, but reopening the editor for an Image/File field's referenced dotAsset showed it centred. A Binary field's metadata surfaces the clean focalPoint key, but a referenced dotAsset's assetMetaData exposes the raw namespaced dot:focalPoint custom attribute; the editor only read focalPoint, so the marker reset to centre. Read both keys via focalPointFromMetadata when seeding the editor. Refs #36363 --- .../dot-file-field.component.ts | 10 +++--- .../utils/focal-point.util.spec.ts | 33 ++++++++++++++++++- .../utils/focal-point.util.ts | 22 +++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 759f4d134368..6b678f698d60 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -52,7 +52,7 @@ import { } from './../../services/image-editor'; import { DotFileFieldUploadService } from './../../services/upload-file/upload-file.service'; import { FileFieldStore } from './../../store/file-field.store'; -import { parseFocalPoint } from './../../utils/focal-point.util'; +import { focalPointFromMetadata, parseFocalPoint } from './../../utils/focal-point.util'; import { getUiMessage } from './../../utils/messages'; import { DotFileFieldPreviewComponent } from './../dot-file-field-preview/dot-file-field-preview.component'; import { DotFileFieldUiMessageComponent } from './../dot-file-field-ui-message/dot-file-field-ui-message.component'; @@ -427,10 +427,10 @@ export class DotFileFieldComponent if (newLauncher?.isAvailable()) { const metadata = this.#currentMetadata(); - // Seed the editor with the asset's stored focal point (exposed on the binary - // metadata as an "x,y" string by DefaultTransformStrategy) so reopening restores - // the marker instead of resetting it to centre. - const focalPoint = parseFocalPoint(metadata?.focalPoint); + // Seed the editor with the asset's stored focal point so reopening restores + // the marker instead of resetting it to centre. Binary fields expose the + // clean `focalPoint` key; a referenced dotAsset exposes `dot:focalPoint`. + const focalPoint = parseFocalPoint(focalPointFromMetadata(metadata)); this.store.applyEditedImage( newLauncher.open({ diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts index 111bc6c2bbaa..126da218d981 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts @@ -1,4 +1,35 @@ -import { parseFocalPoint } from './focal-point.util'; +import { DotFileMetadata } from '@dotcms/dotcms-models'; + +import { focalPointFromMetadata, parseFocalPoint } from './focal-point.util'; + +describe('focalPointFromMetadata', () => { + it('reads the clean focalPoint key (Binary field metadata)', () => { + expect(focalPointFromMetadata({ focalPoint: '0.4,0.6' } as DotFileMetadata)).toBe( + '0.4,0.6' + ); + }); + + it('falls back to the namespaced dot:focalPoint key (referenced dotAsset assetMetaData)', () => { + expect( + focalPointFromMetadata({ 'dot:focalPoint': '0.14,0.29' } as unknown as DotFileMetadata) + ).toBe('0.14,0.29'); + }); + + it('prefers the clean key when both are present', () => { + expect( + focalPointFromMetadata({ + focalPoint: '0.4,0.6', + 'dot:focalPoint': '0.1,0.2' + } as unknown as DotFileMetadata) + ).toBe('0.4,0.6'); + }); + + it('returns undefined for null/undefined or when neither key is present', () => { + expect(focalPointFromMetadata(null)).toBeUndefined(); + expect(focalPointFromMetadata(undefined)).toBeUndefined(); + expect(focalPointFromMetadata({} as DotFileMetadata)).toBeUndefined(); + }); +}); describe('parseFocalPoint', () => { it('parses an "x,y" focal point string into a point', () => { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts index a5b59d65e62d..c7346462cddb 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts @@ -1,5 +1,27 @@ +import { DotFileMetadata } from '@dotcms/dotcms-models'; import { NormalizedPoint } from '@dotcms/image-editor'; +/** + * Resolves the focal point string from file metadata across its two shapes. + * + * A Binary field's own metadata surfaces the clean `focalPoint` key (added by the + * backend view transform), whereas a referenced `dotAsset`'s `assetMetaData` + * exposes the raw namespaced custom attribute `dot:focalPoint`. Reading both lets + * the editor restore the marker on reopen regardless of which shape backs the field. + * + * @param metadata - The resolved file metadata (binary metaData or dotAsset assetMetaData). + * @returns The focal point `"x,y"` string, or undefined when none is present. + */ +export function focalPointFromMetadata( + metadata: DotFileMetadata | null | undefined +): string | undefined { + if (!metadata) { + return undefined; + } + + return metadata.focalPoint ?? (metadata as unknown as Record)['dot:focalPoint']; +} + /** * Parses a focal point stored as an `"x,y"` string (the backend exposes it on the * binary field metadata as a custom `focalPoint` attribute) into a normalized 0..1 From 9bf677ee21821932b8e31be4107210727b91e38b Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 6 Jul 2026 14:55:42 -0400 Subject: [PATCH 10/12] test(e2e): update Image/File edit-button expectations for #36363 Two E2E tests asserted the pre-#36363 behavior (Edit image button hidden after uploading an image to an Image field / importing an image URL to a File field). This PR intentionally exposes the image editor for Image fields and for File fields when the file is an image, so flip those assertions to expect the button visible and update the stale helper doc. Refs #36363 --- .../fields/file-upload-fields/file-field/file-field.spec.ts | 5 +++-- .../file-upload-fields/file-field/helpers/file-field.ts | 3 ++- .../file-upload-fields/image-field/image-field.spec.ts | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/file-field.spec.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/file-field.spec.ts index 17f78daaf6b5..a94f2fa9d38e 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/file-field.spec.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/file-field.spec.ts @@ -107,14 +107,15 @@ test('import from URL completes without 400 and shows preview @critical', async await field.importFromUrl(E2E_IMPORT_URL); }); -test('import image URL does not show Edit image button', async ({ page }) => { +test('import image URL shows Edit image button', async ({ page }) => { const formPage = new NewEditContentFormPage(page); await formPage.goToNew(contentTypeVariable); const field = new FileField(page, FILE_FIELD_VARIABLE); await field.expectVisible(); await field.importFromUrl(E2E_IMPORT_URL); - await field.expectEditButtonHidden(); + // File fields expose the image editor when the file is an image (#36363). + await field.expectEditButtonVisible(); }); test.describe('required file field', () => { diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/helpers/file-field.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/helpers/file-field.ts index a9757f7bbf2b..34e5fd7e99b2 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/helpers/file-field.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/file-field/helpers/file-field.ts @@ -77,7 +77,8 @@ export class FileField { } /** - * The "Edit image" action is only available for Binary fields with an image file. + * The "Edit image" action is available for Binary, Image and File fields when the + * previewed file is an image (#36363). */ async expectEditButtonVisible() { await expect(this.editButton.or(this.editButtonResponsive).first()).toBeVisible({ diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/image-field/image-field.spec.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/image-field/image-field.spec.ts index 328187f2d233..71f6104a39e2 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/image-field/image-field.spec.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/image-field/image-field.spec.ts @@ -81,14 +81,15 @@ test('upload an image, save, reload, and thumbnail still displayed @critical', a await field.expectPreviewShowsFileName(TEST_IMAGE.name); }); -test('upload image does not show Edit image button', async ({ page }) => { +test('upload image shows Edit image button', async ({ page }) => { const formPage = new NewEditContentFormPage(page); await formPage.goToNew(contentTypeVariable); const field = new ImageField(page, IMAGE_FIELD_VARIABLE); await field.expectVisible(); await field.uploadFile(TEST_IMAGE); - await field.expectEditButtonHidden(); + // Image fields expose the image editor for images (#36363). + await field.expectEditButtonVisible(); }); test.describe('required image field', () => { From 70c7e27339061083f095e4b3f492fe7ba7d88169 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 6 Jul 2026 15:39:05 -0400 Subject: [PATCH 11/12] fix(edit-content): persist focal point for FileAsset-referenced fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Focal point restored to a single mechanism: it lives as the custom-metadata `focalPoint` on the binary field, surfaced by the backend as `{fieldVar}MetaData.focalPoint` — the same path Binary and dotAsset fields use. Backend: DefaultTransformStrategy.addBinaries skipped this surfacing for a FileAsset's `fileAsset` field (early continue after putBinaryLinks), so the focal never reached the API for legacy FileAsset content. Surface it there too, mirroring the dotAsset path. Frontend: read the focal uniformly from the referenced asset's `{titleImage}MetaData.focalPoint` (asset for dotAsset, fileAsset for FileAsset) and drop the ad-hoc dot:focalPoint fallback now that the clean key is surfaced for every type. Refs #36363 --- .../dot-file-field.component.ts | 16 +++-- .../utils/focal-point.util.spec.ts | 59 +++++++++++++------ .../utils/focal-point.util.ts | 38 +++++++++--- .../strategy/DefaultTransformStrategy.java | 16 +++++ 4 files changed, 98 insertions(+), 31 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 6b678f698d60..5587af417c46 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -52,7 +52,11 @@ import { } from './../../services/image-editor'; import { DotFileFieldUploadService } from './../../services/upload-file/upload-file.service'; import { FileFieldStore } from './../../store/file-field.store'; -import { focalPointFromMetadata, parseFocalPoint } from './../../utils/focal-point.util'; +import { + focalPointFromContentlet, + focalPointFromMetadata, + parseFocalPoint +} from './../../utils/focal-point.util'; import { getUiMessage } from './../../utils/messages'; import { DotFileFieldPreviewComponent } from './../dot-file-field-preview/dot-file-field-preview.component'; import { DotFileFieldUiMessageComponent } from './../dot-file-field-ui-message/dot-file-field-ui-message.component'; @@ -428,9 +432,13 @@ export class DotFileFieldComponent if (newLauncher?.isAvailable()) { const metadata = this.#currentMetadata(); // Seed the editor with the asset's stored focal point so reopening restores - // the marker instead of resetting it to centre. Binary fields expose the - // clean `focalPoint` key; a referenced dotAsset exposes `dot:focalPoint`. - const focalPoint = parseFocalPoint(focalPointFromMetadata(metadata)); + // the marker instead of resetting it to centre. A referenced dotAsset/FileAsset + // exposes it on assetMetaData/fileAssetMetaData; an inline binary temp on metaData. + const focalPoint = parseFocalPoint( + uploaded?.source === 'contentlet' + ? focalPointFromContentlet(uploaded.file) + : focalPointFromMetadata(metadata) + ); this.store.applyEditedImage( newLauncher.open({ diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts index 126da218d981..71c1fd92a504 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts @@ -1,30 +1,53 @@ -import { DotFileMetadata } from '@dotcms/dotcms-models'; +import { DotCMSContentlet, DotFileMetadata } from '@dotcms/dotcms-models'; -import { focalPointFromMetadata, parseFocalPoint } from './focal-point.util'; +import { + focalPointFromContentlet, + focalPointFromMetadata, + parseFocalPoint +} from './focal-point.util'; -describe('focalPointFromMetadata', () => { - it('reads the clean focalPoint key (Binary field metadata)', () => { - expect(focalPointFromMetadata({ focalPoint: '0.4,0.6' } as DotFileMetadata)).toBe( - '0.4,0.6' - ); +describe('focalPointFromContentlet', () => { + it('reads a dotAsset focal from assetMetaData (titleImage=asset)', () => { + const file = { + titleImage: 'asset', + assetMetaData: { focalPoint: '0.4,0.6' } + } as unknown as DotCMSContentlet; + expect(focalPointFromContentlet(file)).toBe('0.4,0.6'); }); - it('falls back to the namespaced dot:focalPoint key (referenced dotAsset assetMetaData)', () => { - expect( - focalPointFromMetadata({ 'dot:focalPoint': '0.14,0.29' } as unknown as DotFileMetadata) - ).toBe('0.14,0.29'); + it('reads a legacy FileAsset focal from fileAssetMetaData (titleImage=fileAsset)', () => { + const file = { + titleImage: 'fileAsset', + metaData: { name: 'a.png' }, + fileAssetMetaData: { focalPoint: '0.76,0.13' } + } as unknown as DotCMSContentlet; + expect(focalPointFromContentlet(file)).toBe('0.76,0.13'); + }); + + it('defaults the field variable to asset when titleImage is absent', () => { + const file = { assetMetaData: { focalPoint: '0.1,0.2' } } as unknown as DotCMSContentlet; + expect(focalPointFromContentlet(file)).toBe('0.1,0.2'); }); - it('prefers the clean key when both are present', () => { + it('returns undefined when the field metadata carries no focal', () => { + expect(focalPointFromContentlet(null)).toBeUndefined(); expect( - focalPointFromMetadata({ - focalPoint: '0.4,0.6', - 'dot:focalPoint': '0.1,0.2' - } as unknown as DotFileMetadata) - ).toBe('0.4,0.6'); + focalPointFromContentlet({ + titleImage: 'fileAsset', + fileAssetMetaData: { name: 'a.png' } + } as unknown as DotCMSContentlet) + ).toBeUndefined(); + }); +}); + +describe('focalPointFromMetadata', () => { + it('reads the focalPoint key', () => { + expect(focalPointFromMetadata({ focalPoint: '0.4,0.6' } as DotFileMetadata)).toBe( + '0.4,0.6' + ); }); - it('returns undefined for null/undefined or when neither key is present', () => { + it('returns undefined for null/undefined or when the key is absent', () => { expect(focalPointFromMetadata(null)).toBeUndefined(); expect(focalPointFromMetadata(undefined)).toBeUndefined(); expect(focalPointFromMetadata({} as DotFileMetadata)).toBeUndefined(); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts index c7346462cddb..06d9bde92b82 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts @@ -1,25 +1,45 @@ -import { DotFileMetadata } from '@dotcms/dotcms-models'; +import { DotCMSContentlet, DotFileMetadata } from '@dotcms/dotcms-models'; import { NormalizedPoint } from '@dotcms/image-editor'; /** - * Resolves the focal point string from file metadata across its two shapes. + * Reads the focal point string from a resolved binary metadata object. * - * A Binary field's own metadata surfaces the clean `focalPoint` key (added by the - * backend view transform), whereas a referenced `dotAsset`'s `assetMetaData` - * exposes the raw namespaced custom attribute `dot:focalPoint`. Reading both lets - * the editor restore the marker on reopen regardless of which shape backs the field. + * The backend surfaces it under the single clean `focalPoint` key on the binary + * field's metadata — the same mechanism Binary fields have always used. * - * @param metadata - The resolved file metadata (binary metaData or dotAsset assetMetaData). + * @param metadata - The resolved file metadata. * @returns The focal point `"x,y"` string, or undefined when none is present. */ export function focalPointFromMetadata( metadata: DotFileMetadata | null | undefined ): string | undefined { - if (!metadata) { + return metadata?.focalPoint; +} + +/** + * Resolves the focal point from a referenced asset contentlet. + * + * The focal lives on the binary field's metadata, exposed by the backend as + * `{fieldVar}MetaData`, where `fieldVar` is the asset's binary field (`asset` for a + * dotAsset, `fileAsset` for a legacy FileAsset) — carried by the contentlet's + * `titleImage`. This is the same `{fieldVar}MetaData.focalPoint` mechanism a Binary + * field uses, applied uniformly. + * + * @param file - The referenced asset contentlet (dotAsset or FileAsset). + * @returns The focal point `"x,y"` string, or undefined when none is present. + */ +export function focalPointFromContentlet( + file: DotCMSContentlet | null | undefined +): string | undefined { + if (!file) { return undefined; } - return metadata.focalPoint ?? (metadata as unknown as Record)['dot:focalPoint']; + const raw = file as unknown as Record; + const fieldVar = + ((file as unknown as Record)['titleImage'] ?? 'asset') || 'asset'; + + return focalPointFromMetadata(raw[`${fieldVar}MetaData`]); } /** diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java index 616e6a6772d9..84bfc5838ba1 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java @@ -253,6 +253,22 @@ private void addBinaries(final Contentlet contentlet, final Map : contentlet.getBinaryMetadata(FILE_ASSET).getName(); putBinaryLinks(FILE_ASSET, name, contentlet, map); + + // Surface the binary metadata (including the focal point from custom + // metadata) for the FileAsset's binary field, mirroring the dotAsset path + // below. Without this the image editor cannot re-seed the focal marker when + // reopening a File/Image field that references a legacy FileAsset. + final Metadata fileAssetMetadata = contentlet.getBinaryMetadata(FILE_ASSET); + if (null != fileAssetMetadata) { + final Map metaMap = new HashMap<>( + fileAssetMetadata.getMap()); + metaMap.remove("path"); + final String focalPoint = Try.of(() -> fileAssetMetadata.getCustomMeta() + .getOrDefault(FocalPointAPI.FOCAL_POINT, "0.0").toString()) + .getOrElse("0.0"); + metaMap.put(FocalPointAPI.FOCAL_POINT, focalPoint); + map.put(FILE_ASSET + "MetaData", metaMap); + } continue; } From 66795012398afd705ec35b4149f462293ac6b0be Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 6 Jul 2026 17:05:13 -0400 Subject: [PATCH 12/12] test(transform): cover FileAsset focal-point surfacing in DefaultTransformStrategy Add regression coverage for the FileAsset branch of addBinaries that now surfaces the focal point under fileAssetMetaData.focalPoint (defaulting to 0.0 when absent), mirroring the dotAsset path. Closes #36363 --- .../DefaultTransformStrategyTest.java | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java index e4a202d1356c..3823c46d3058 100644 --- a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java @@ -9,8 +9,11 @@ import com.dotcms.contenttype.model.field.Field; import com.dotcms.contenttype.model.type.ContentType; import com.dotcms.storage.model.Metadata; +import com.dotmarketing.beans.Identifier; +import com.dotmarketing.business.IdentifierAPI; import com.dotmarketing.image.focalpoint.FocalPointAPI; import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.fileassets.business.FileAssetAPI; import java.io.Serializable; import java.lang.reflect.Method; import java.util.HashMap; @@ -114,4 +117,100 @@ public void testAddBinaries_whenCustomMetaHasNoFocalPoint_defaultsToZero() assertEquals("focalPoint must default to 0.0 when absent from custom metadata", "0.0", metaMap.get(FocalPointAPI.FOCAL_POINT)); } + + private static final String FILE_ASSET_META_KEY = FileAssetAPI.BINARY_FIELD + "MetaData"; + + /** + * Mocks an {@link APIProvider} whose {@code identifierAPI} resolves to an asset name, so the + * FileAsset branch of {@code addBinaries} can build its binary links. The field is injected via + * reflection because the real {@code APIProvider.Builder} eagerly resolves APILocator defaults, + * which is not available in a pure unit test. + */ + private APIProvider toolBoxResolvingAssetName(final String assetName) throws Exception { + final Identifier identifier = Mockito.mock(Identifier.class); + Mockito.when(identifier.getAssetName()).thenReturn(assetName); + final IdentifierAPI identifierAPI = Mockito.mock(IdentifierAPI.class); + Mockito.when(identifierAPI.find(Mockito.anyString())).thenReturn(identifier); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final java.lang.reflect.Field field = APIProvider.class.getDeclaredField("identifierAPI"); + field.setAccessible(true); + field.set(toolBox, identifierAPI); + return toolBox; + } + + private Contentlet mockFileAssetWithBinary(final Metadata metadata) throws Exception { + final Field field = Mockito.mock(BinaryField.class); + Mockito.when(field.variable()).thenReturn(FileAssetAPI.BINARY_FIELD); + + final ContentType contentType = Mockito.mock(ContentType.class); + Mockito.when(contentType.fields(BinaryField.class)).thenReturn(List.of(field)); + + final Contentlet contentlet = Mockito.mock(Contentlet.class); + Mockito.when(contentlet.getContentType()).thenReturn(contentType); + Mockito.when(contentlet.isFileAsset()).thenReturn(true); + Mockito.when(contentlet.getIdentifier()).thenReturn("identifier-1"); + Mockito.when(contentlet.getInode()).thenReturn("inode-1"); + Mockito.when(contentlet.getBinaryMetadata(FileAssetAPI.BINARY_FIELD)).thenReturn(metadata); + return contentlet; + } + + /** + * A legacy FileAsset's {@code fileAsset} binary field must also surface its focal point under + * {@code fileAssetMetaData.focalPoint}, mirroring the dotAsset path, so the image editor can + * re-seed the marker when a File/Image field references a FileAsset. Regression coverage for + * #36363. + */ + @Test + public void testAddBinaries_fileAsset_whenCustomMetaHasFocalPoint_surfacesItInMetaDataMap() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + fieldsMeta.put(CUSTOM_FOCAL_POINT_KEY, "0.76,0.13"); + final Metadata metadata = new Metadata(FileAssetAPI.BINARY_FIELD, fieldsMeta); + + final DefaultTransformStrategy strategy = + new DefaultTransformStrategy(toolBoxResolvingAssetName("image.png")); + final Contentlet contentlet = mockFileAssetWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected fileAssetMetaData to be surfaced for a FileAsset", + map.containsKey(FILE_ASSET_META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = + (Map) map.get(FILE_ASSET_META_KEY); + assertEquals("Focal point must be exposed under the focalPoint key for a FileAsset", + "0.76,0.13", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } + + /** + * A FileAsset with no focal point in its custom metadata still surfaces the + * {@code focalPoint} entry defaulting to "0.0", matching the dotAsset path. + */ + @Test + public void testAddBinaries_fileAsset_whenCustomMetaHasNoFocalPoint_defaultsToZero() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + final Metadata metadata = new Metadata(FileAssetAPI.BINARY_FIELD, fieldsMeta); + + final DefaultTransformStrategy strategy = + new DefaultTransformStrategy(toolBoxResolvingAssetName("image.png")); + final Contentlet contentlet = mockFileAssetWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected fileAssetMetaData to be surfaced for a FileAsset", + map.containsKey(FILE_ASSET_META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = + (Map) map.get(FILE_ASSET_META_KEY); + assertEquals("focalPoint must default to 0.0 when absent from custom metadata", + "0.0", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } }