diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts index f43fb40d6e93..cae5890845f4 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts @@ -56,9 +56,11 @@ test.describe('Binary field image editor — new editor', () => { } }); - // The unified Binary field shows "Edit image" for image files. The new - // editor opens the legacy image editor JSP inside a PrimeNG dialog iframe. - test('import image and Edit opens legacy image editor dialog @critical', async ({ page }) => { + // The unified Binary field shows "Edit image" for image files. In the new + // Edit Content the new Angular image editor opens (the FEATURE_FLAG_NEW_IMAGE_EDITOR + // gate was removed, so it is always used here; the legacy Dojo editor only runs in + // the legacy Edit Content — see the describe block below). + test('import image and Edit opens the new Angular image editor @critical', async ({ page }) => { const formPage = new NewEditContentFormPage(page); await formPage.goToNew(contentTypeVariable); diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts index 6ee2601f43db..5a6f2e3c9ade 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts @@ -96,20 +96,17 @@ export class BinaryField extends FileField { } /** - * New editor: the legacy image editor JSP is embedded in a PrimeNG dialog - * iframe (`dot-legacy-image-editor-dialog`), not the Dojo `#dotImageDialog`. + * New editor: clicking "Edit image" opens the Angular image editor + * ({@link DotImageEditorComponent}, `data-testid="image-editor-root"`) inside a + * PrimeNG dialog. The flag that used to gate it (FEATURE_FLAG_NEW_IMAGE_EDITOR) was + * removed, so the new editor is always used in the new Edit Content — there is no + * legacy Dojo iframe here. */ async expectImageEditorOpen() { const dialog = this.page.getByRole('dialog'); await expect(dialog).toBeVisible({ timeout: 15000 }); - const iframe = dialog.getByTestId('legacy-image-editor-iframe'); - await expect(iframe).toBeVisible({ timeout: 30000 }); - - const editorFrame = this.page.frameLocator('[data-testid="legacy-image-editor-iframe"]'); - await expect(editorFrame.locator('#dotImageDialog, #imageToolIframe').first()).toBeVisible({ - timeout: 30000 - }); + await expect(dialog.getByTestId('image-editor-root')).toBeVisible({ timeout: 30000 }); } async openImageEditorInNewEditor() { diff --git a/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts b/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts index 9396e6299f60..497ddf60f351 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts @@ -1,4 +1,6 @@ -import { DotCMSContentType, DotPageContainer, DotPageContent } from '@dotcms/dotcms-models'; +import { DotCMSContentType } from './dot-content-types.model'; +import { DotPageContainer } from './dot-page-container.model'; +import { DotPageContent } from './dot-page-content.model'; export interface DotContentletEvent> { name: string; diff --git a/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts b/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts index f441f4ea3c8d..0416232edfb7 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts @@ -11,4 +11,9 @@ export interface DotFileMetadata { height?: number; width?: number; editableAsText?: boolean; + /** + * Focal point as an `"x,y"` string (normalized 0..1), exposed by the backend on image + * binary metadata and used to re-seed the image editor's focal marker on reopen. + */ + focalPoint?: string; } diff --git a/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts b/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts index 462a51f37dd0..3c5b3702fa1b 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts @@ -1,5 +1,5 @@ export interface DotHttpRequestOptions { method: string; headers: { [key: string]: string }; - body: any; + body: XMLHttpRequestBodyInit | null; } diff --git a/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts b/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts index 263457f288a5..03a624ecb2d2 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts @@ -70,7 +70,7 @@ export interface DotCMSSystemAction { export interface DotCMSWorkflowInput { id: string; - body: any; + body: Record; } export interface DotCMSContentletWorkflowActions { diff --git a/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts b/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts index a5edb3d90b5a..6ead8e412ead 100644 --- a/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts +++ b/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts @@ -1,5 +1,4 @@ -import { DotPageContainer } from '@dotcms/dotcms-models'; - +import { DotPageContainer } from './dot-page-container.model'; import { PageModelChangeEventType } from './page-model-change-event.type'; export interface PageModelChangeEvent { diff --git a/core-web/libs/dotcms-models/src/lib/shared-models.ts b/core-web/libs/dotcms-models/src/lib/shared-models.ts index fe86425a80ee..2da10347ece1 100644 --- a/core-web/libs/dotcms-models/src/lib/shared-models.ts +++ b/core-web/libs/dotcms-models/src/lib/shared-models.ts @@ -36,8 +36,7 @@ export const enum FeaturedFlags { FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION = 'FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION', FEATURE_FLAG_NEW_BLOCK_EDITOR = 'FEATURE_FLAG_NEW_BLOCK_EDITOR', FEATURE_FLAG_REPORT_ISSUE_ENABLED = 'FEATURE_FLAG_REPORT_ISSUE_ENABLED', - FEATURE_FLAG_LOCALE_SELECTOR_V2 = 'FEATURE_FLAG_LOCALE_SELECTOR_V2', - FEATURE_FLAG_NEW_IMAGE_EDITOR = 'FEATURE_FLAG_NEW_IMAGE_EDITOR' + FEATURE_FLAG_LOCALE_SELECTOR_V2 = 'FEATURE_FLAG_LOCALE_SELECTOR_V2' } export const enum DotConfigurationVariables { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts index ec8f3db83f5a..ad87bf885063 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts @@ -570,11 +570,18 @@ describe('DotEditContentCalendarFieldUtil - TDD Approach', () => { const todayInServerTz = getCurrentServerTime(SERVER_TIMEZONE_MOCKS.GULF); expect(result?.displayValue.getDate()).toBe(todayInServerTz.getDate()); - // formValue (UTC) might be different day due to timezone conversion, allow ±2 days - const expectedFormDate = todayInServerTz.getDate(); - const actualFormDate = result?.formValue.getDate(); - expect(actualFormDate).toBeDefined(); - expect(Math.abs(actualFormDate - expectedFormDate)).toBeLessThanOrEqual(2); + // formValue (UTC) might fall on a different calendar day due to timezone + // conversion; allow ±2 days. Compare the whole-day distance, not getDate() + // day-of-month subtraction — the latter wraps at month boundaries (e.g. + // Jun 30 vs Jul 1 would read as |30 - 1| = 29 instead of a 1-day difference). + const formValue = result?.formValue; + expect(formValue).toBeDefined(); + const dayMs = 24 * 60 * 60 * 1000; + const dayAtMidnight = (date: Date) => + new Date(date.getFullYear(), date.getMonth(), date.getDate()).getTime(); + const dayDistance = + Math.abs(dayAtMidnight(formValue!) - dayAtMidnight(todayInServerTz)) / dayMs; + expect(dayDistance).toBeLessThanOrEqual(2); }); it('should handle "now" defaultValue correctly for DATE fields', () => { 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 995b48b7dd77..9b1f531332eb 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 @@ -48,6 +48,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 { 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'; @@ -104,10 +105,10 @@ export class DotFileFieldComponent readonly #legacyDojoImageEditorLauncher = inject(LegacyDojoImageEditorLauncher); /** * New Angular image editor launcher. Provided by the Angular edit-content shell - * ({@link EditContentShellComponent}) and gated behind the - * `FEATURE_FLAG_NEW_IMAGE_EDITOR` flag via its `isAvailable()`. Injected as - * `{ optional: true }`: when absent (e.g. the legacy web-component host) or its - * flag is off, `onEditImage()` falls back to the legacy launchers. + * ({@link EditContentShellComponent}); its `isAvailable()` is always true now that + * the new editor is GA in the new Edit Content. Injected as `{ optional: true }`: + * when absent (e.g. the legacy web-component host), `onEditImage()` falls back to + * the legacy launchers. */ readonly #imageEditorLauncher = inject(IMAGE_EDITOR_LAUNCHER, { optional: true }); /** @@ -408,13 +409,16 @@ export class DotFileFieldComponent : variable; // Prefer the new Angular image editor when its launcher is provided (Angular - // edit-content shell) and its feature flag is on. Otherwise fall back to the - // legacy editor: Dojo DOM events for the web-component bridge, dialog iframe - // elsewhere. + // edit-content shell). Otherwise fall back to the legacy editor: Dojo DOM events + // for the web-component bridge, dialog iframe elsewhere. const newLauncher = this.#imageEditorLauncher; 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); this.#applyEditedImage( newLauncher.open({ @@ -424,7 +428,8 @@ export class DotFileFieldComponent fieldName: editorVariable, byInode: !!inode, fileName: this.$currentFileName() || undefined, - mimeType: metadata?.contentType + mimeType: metadata?.contentType, + focalPoint }) ); 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 new file mode 100644 index 000000000000..111bc6c2bbaa --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts @@ -0,0 +1,25 @@ +import { parseFocalPoint } from './focal-point.util'; + +describe('parseFocalPoint', () => { + it('parses an "x,y" focal point string into a point', () => { + expect(parseFocalPoint('0.88,0.31')).toEqual({ x: 0.88, y: 0.31 }); + }); + + it('returns undefined for an unset/empty value', () => { + expect(parseFocalPoint(undefined)).toBeUndefined(); + expect(parseFocalPoint(null)).toBeUndefined(); + expect(parseFocalPoint('')).toBeUndefined(); + }); + + it('treats the (0,0) "no focal point" sentinel as undefined', () => { + // The backend uses (0,0) to mean "no focal point" -> editor opens centred. + expect(parseFocalPoint('0,0')).toBeUndefined(); + }); + + it('returns undefined for a malformed or single-value string', () => { + // "0.0" has no comma -> one token -> the y axis parses to NaN. + expect(parseFocalPoint('0.0')).toBeUndefined(); + expect(parseFocalPoint('not-a-point')).toBeUndefined(); + expect(parseFocalPoint('abc,xyz')).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 new file mode 100644 index 000000000000..a5b59d65e62d --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts @@ -0,0 +1,32 @@ +import { NormalizedPoint } from '@dotcms/image-editor'; + +/** + * 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 + * point for seeding the image editor. + * + * Returns `undefined` for an unset/empty value, the `(0,0)` "no focal point" sentinel, + * or a malformed/single-value string, so the editor opens centred instead of seeding a + * bogus marker. + * + * @param value - The raw `focalPoint` metadata string (e.g. `"0.88,0.31"`). + * @returns The parsed point, or `undefined` when there is no usable focal point. + */ +export function parseFocalPoint(value: string | null | undefined): NormalizedPoint | undefined { + if (!value) { + return undefined; + } + + const [x, y] = value.split(',').map(Number); + + if (!Number.isFinite(x) || !Number.isFinite(y)) { + return undefined; + } + + // (0,0) is the backend's "no focal point" sentinel -> open centred. + if (x === 0 && y === 0) { + return undefined; + } + + return { x, y }; +} diff --git a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts index 101b290129b3..12cb3e239fb8 100644 --- a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts @@ -1,11 +1,10 @@ import { expect } from '@jest/globals'; -import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; -import { BehaviorSubject, Subject } from 'rxjs'; +import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest'; +import { Subject } from 'rxjs'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotPropertiesService } from '@dotcms/data-access'; -import { DotCMSTempFile, FeaturedFlags } from '@dotcms/dotcms-models'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent, ImageEditorOpenParams } from '@dotcms/image-editor'; import { AngularImageEditorLauncher } from './angular-image-editor.launcher'; @@ -13,10 +12,6 @@ import { AngularImageEditorLauncher } from './angular-image-editor.launcher'; describe('AngularImageEditorLauncher', () => { let spectator: SpectatorService; let onClose: Subject; - // Drives the new-image-editor flag; `next()` flows through `toSignal` so the same - // service instance reflects on/off without re-creating it. - const featureFlag$ = new BehaviorSubject(true); - const getFeatureFlag = jest.fn(() => featureFlag$); const params: ImageEditorOpenParams = { inode: 'inode-1', @@ -26,28 +21,17 @@ describe('AngularImageEditorLauncher', () => { const createService = createServiceFactory({ service: AngularImageEditorLauncher, - providers: [mockProvider(DotPropertiesService, { getFeatureFlag })], mocks: [DialogService] }); beforeEach(() => { - // Default the flag ON so the open() tests below run the Angular path; the - // gating itself is covered by the dedicated tests. - featureFlag$.next(true); onClose = new Subject(); spectator = createService(); spectator.inject(DialogService).open.mockReturnValue({ onClose }); }); - it('should be available when FEATURE_FLAG_NEW_IMAGE_EDITOR is on', () => { + it('should always be available (the new editor is the editor for the new Edit Content)', () => { expect(spectator.service.isAvailable()).toBe(true); - expect(getFeatureFlag).toHaveBeenCalledWith(FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR); - }); - - it('should NOT be available when the feature flag is off', () => { - featureFlag$.next(false); - - expect(spectator.service.isAvailable()).toBe(false); }); it('should open the DotImageEditorComponent with a headerless, closable dialog that owns Esc', () => { diff --git a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts index c6720f2146b9..95b9fa29da0b 100644 --- a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts +++ b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts @@ -1,12 +1,10 @@ import { Observable, map, take } from 'rxjs'; import { inject, Injectable } from '@angular/core'; -import { toSignal } from '@angular/core/rxjs-interop'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotPropertiesService } from '@dotcms/data-access'; -import { DotCMSTempFile, FeaturedFlags } from '@dotcms/dotcms-models'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent, DotImageEditorLauncher, @@ -16,24 +14,17 @@ import { /** * Launches the Angular `@dotcms/image-editor` modal through PrimeNG's `DialogService`. * - * Gated behind the {@link FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR} feature flag: - * `isAvailable()` resolves to the server-configured value, which ships as `false` - * for rollback safety (mirroring the other new-editor flags), so the binary field - * falls back to the legacy Dojo editor until an admin enables it. + * Provided by the new Edit Content shell, so the new image editor is used whenever this + * launcher is injected. The binary field falls back to the legacy Dojo editor only when + * no launcher is present (i.e. the field renders outside the new Edit Content). */ @Injectable() export class AngularImageEditorLauncher implements DotImageEditorLauncher { readonly #dialogService = inject(DialogService); - readonly #propertiesService = inject(DotPropertiesService); - - /** Resolved value of the new-image-editor feature flag (off until the server replies). */ - readonly #enabled = toSignal( - this.#propertiesService.getFeatureFlag(FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR), - { initialValue: false } - ); + /** Always available: the new image editor is the editor for the new Edit Content. */ isAvailable(): boolean { - return this.#enabled(); + return true; } /** diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts index 5b6ce098b747..4544c936d6ab 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts @@ -515,7 +515,7 @@ export class DotImageEditorCanvasComponent { }; } - /** Lazily attaches a single ResizeObserver to the displayed image element. */ + /** Lazily attaches a single ResizeObserver to the displayed image and its stage. */ #observeDisplayImg(): void { const img = this.$displayImg()?.nativeElement; @@ -525,6 +525,19 @@ export class DotImageEditorCanvasComponent { this.#resizeObserver = new ResizeObserver(() => this.#measureImageRect()); this.#resizeObserver.observe(img); + + // Also observe the stage. Toggling full-screen resizes the dialog, which + // re-centres the image within the stage WITHOUT changing the image's own size + // — a ResizeObserver on the image alone never reports that, leaving + // imageRect.x/y (img.offsetLeft/Top) stale and the focal/crop overlays + // mispositioned (notably on the full-screen -> windowed transition). The stage + // (the image's offsetParent) always resizes with the dialog, so observing it + // re-measures the image's offset box and keeps the overlays aligned. + const stage = this.$stage()?.nativeElement; + + if (stage) { + this.#resizeObserver.observe(stage); + } } /** diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss b/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss index 1345ef265ec3..3aa1b8a8b988 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss @@ -38,11 +38,13 @@ width: 6px; height: 6px; transform: translate(-50%, -50%); - background-color: var(--p-primary-color, #426bf0); + // Red target dot: high-contrast against arbitrary image content and the + // conventional colour for a focal-point marker (distinct from the UI primary). + background-color: var(--p-red-500, #ef4444); border-radius: 9999px; } &:focus-visible { - border-color: var(--p-primary-color, #426bf0); + border-color: var(--p-red-500, #ef4444); } } diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html index 96fef02f7ea4..dc84678f4cd2 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html @@ -16,4 +16,11 @@ download + + diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts index 854d439579ac..afcbb9ba2fc8 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts @@ -27,25 +27,28 @@ describe('DotImageEditorFooterComponent', () => { let dispatcher: SpyObject; const isBusy = signal(false); + const saveStatus = signal<'idle' | 'saving' | 'error'>('idle'); const createComponent = createComponentFactory({ component: DotImageEditorFooterComponent, imports: [DotMessagePipe], providers: [mockProvider(DotMessageService, { get: jest.fn((key: string) => key) })], - componentProviders: [Dispatcher, mockProvider(ImageEditorStore, { isBusy })] + componentProviders: [Dispatcher, mockProvider(ImageEditorStore, { isBusy, saveStatus })] }); beforeEach(() => { isBusy.set(false); + saveStatus.set('idle'); spectator = createComponent(); dispatcher = spectator.inject(Dispatcher, true); jest.spyOn(dispatcher, 'dispatch'); }); - it('should render the cancel and download actions', () => { + it('should render the cancel, download and save actions', () => { expect(spectator.query(byTestId('image-editor-cancel-btn'))).toBeTruthy(); expect(spectator.query(byTestId('image-editor-download-btn'))).toBeTruthy(); + expect(spectator.query(byTestId('image-editor-save-btn'))).toBeTruthy(); }); it('should emit cancel when Cancel is clicked', () => { @@ -71,4 +74,31 @@ describe('DotImageEditorFooterComponent', () => { expect(nativeButton(spectator, 'image-editor-download-btn').disabled).toBe(true); }); + + it('should dispatch saveRequested when Save is clicked', () => { + spectator.click(nativeButton(spectator, 'image-editor-save-btn')); + + expect(dispatcher.dispatch).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveRequested(), + { scope: 'self' } + ); + }); + + it('should show a loading spinner and disable Save while a save is in flight', () => { + saveStatus.set('saving'); + spectator.detectChanges(); + + const button = nativeButton(spectator, 'image-editor-save-btn'); + expect(button.disabled).toBe(true); + expect(button.classList).toContain('p-button-loading'); + }); + + it('should show a loading spinner and disable Save while a filter is applying (busy)', () => { + isBusy.set(true); + spectator.detectChanges(); + + const button = nativeButton(spectator, 'image-editor-save-btn'); + expect(button.disabled).toBe(true); + expect(button.classList).toContain('p-button-loading'); + }); }); diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts index d7a3b7fe1c7a..e8e31ada3fcb 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts @@ -11,9 +11,9 @@ import { ImageEditorStore } from '../../store/image-editor.store'; /** * Footer action bar of the image editor dialog. Reads readiness from the - * {@link ImageEditorStore} (`isBusy`) and dispatches the download lifecycle event. - * Cancel is surfaced as an output so the owning dialog component controls closing. - * (Saving the edited image back to the field is handled in a separate issue.) + * {@link ImageEditorStore} (`isBusy`/`saveStatus`) and dispatches the download and + * save lifecycle events. Cancel is surfaced as an output so the owning dialog + * component controls closing. */ @Component({ selector: 'dot-image-editor-footer', @@ -25,7 +25,7 @@ export class DotImageEditorFooterComponent { /** Image editor state store, provided by the owning dialog component. */ protected readonly store = inject(ImageEditorStore); - /** Lifecycle event dispatcher for the download action. */ + /** Lifecycle event dispatcher for the download and save actions. */ protected readonly dispatch = injectDispatch(imageEditorLifecycleEvents); /** Emitted when the user clicks Cancel; the dialog owner closes the editor. */ @@ -35,4 +35,9 @@ export class DotImageEditorFooterComponent { protected onDownload(): void { this.dispatch.downloadRequested(); } + + /** Dispatches a save of the current edits into a temp file. */ + protected onSave(): void { + this.dispatch.saveRequested(); + } } diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html index 911f69efdf52..7fa7901c4dfd 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html @@ -10,6 +10,12 @@
+ @if (store.saveStatus() === 'error') { + + } +
diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts index 351a0e5d372f..5847ae23bdee 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts @@ -15,6 +15,7 @@ import { Confirmation, ConfirmationService, ConfirmEventType } from 'primeng/api import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; import { DotMessageService } from '@dotcms/data-access'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent } from './dot-image-editor.component'; @@ -41,6 +42,8 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { const canUndo = signal(false); const canRedo = signal(false); const isFullscreen = signal(false); + const saveStatus = signal<'idle' | 'saving' | 'error'>('idle'); + const saveError = signal(null); const createComponent = createComponentFactory({ component: DotImageEditorComponent, @@ -57,7 +60,14 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { // mocking only the store. componentProviders: [ ConfirmationService, - mockProvider(ImageEditorStore, { isDirty, canUndo, canRedo, isFullscreen }) + mockProvider(ImageEditorStore, { + isDirty, + canUndo, + canRedo, + isFullscreen, + saveStatus, + saveError + }) ], // Isolate the shell from the children's own store/dispatch wiring. overrideComponents: [ @@ -94,6 +104,8 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { canUndo.set(false); canRedo.set(false); isFullscreen.set(false); + saveStatus.set('idle'); + saveError.set(null); // Spy before creation so the constructor's assetRequested dispatch is // captured (injectDispatch dispatches through Dispatcher.prototype). @@ -215,6 +227,48 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { expect(confirmationService.confirm).toHaveBeenCalledTimes(1); }); + describe('save lifecycle', () => { + const tempFile: DotCMSTempFile = { + fileName: 'edited.png', + folder: 'shared', + id: 'temp_578026b7cc', + image: true, + length: 1024, + mimeType: 'image/png', + referenceUrl: '/dA/temp_578026b7cc', + thumbnailUrl: '/dA/temp_578026b7cc/thumb' + }; + + it('should close with the temp file when a save succeeds', () => { + dispatcher.dispatch(imageEditorLifecycleEvents.saveSucceeded(tempFile)); + + expect(dialogRef.close).toHaveBeenCalledWith(tempFile); + }); + + it('should NOT close when a save fails', () => { + dispatcher.dispatch(imageEditorLifecycleEvents.saveFailed(new Error('boom'))); + + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it('should surface the save error when saveStatus is error', () => { + saveStatus.set('error'); + saveError.set('Failed to save image'); + spectator.detectChanges(); + + const error = spectator.query(byTestId('image-editor-save-error')); + expect(error).toExist(); + expect(error?.textContent?.trim()).toBe('Failed to save image'); + }); + + it('should not render the save error when saveStatus is idle', () => { + saveStatus.set('idle'); + spectator.detectChanges(); + + expect(spectator.query(byTestId('image-editor-save-error'))).not.toExist(); + }); + }); + describe('undo/redo shortcuts', () => { it('should dispatch undoRequested on Ctrl/Cmd+Z when undo is available', () => { canUndo.set(true); diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts index adfca0f5c9f7..9afe4c5806a4 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts @@ -1,7 +1,8 @@ -import { injectDispatch } from '@ngrx/signals/events'; +import { Events, injectDispatch } from '@ngrx/signals/events'; import { DOCUMENT } from '@angular/common'; import { ChangeDetectionStrategy, Component, effect, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { ConfirmationService, ConfirmEventType } from 'primeng/api'; import { ConfirmDialogModule } from 'primeng/confirmdialog'; @@ -27,8 +28,8 @@ import { DotImageEditorPanelsComponent } from '../dot-image-editor-panels/dot-im * Full-screen "Edit image" modal shell, opened through PrimeNG's `DialogService`. * Assembles the header, canvas, side panels and footer over a single * {@link ImageEditorStore} instance scoped to this dialog. It owns the dialog - * lifecycle: it requests the asset on init and guards close/cancel against unsaved - * edits (saving the edited image is deferred to a separate issue). + * lifecycle: it requests the asset on init, closes with the saved temp file when a + * save succeeds, and guards close/cancel against unsaved edits. */ @Component({ selector: 'dot-image-editor', @@ -66,6 +67,7 @@ export class DotImageEditorComponent { readonly #dotMessageService = inject(DotMessageService); readonly #dispatch = injectDispatch(imageEditorLifecycleEvents); readonly #historyDispatch = injectDispatch(imageEditorHistoryEvents); + readonly #events = inject(Events); readonly #document = inject(DOCUMENT); // The PrimeNG dialog hosting this editor: injectable because the dialog content // is declared inside `` in DynamicDialog's template, so we sit in the @@ -84,6 +86,13 @@ export class DotImageEditorComponent { // The editor owns its dialog, so it owns full-screen too: resize the host // `.p-dialog` to fill the viewport whenever `isFullscreen` flips. effect(() => this.#applyFullscreen(this.store.isFullscreen())); + + // A successful save resolves the editor with the staged temp file; a failed + // save keeps the dialog open and surfaces the error in the template. + this.#events + .on(imageEditorLifecycleEvents.saveSucceeded) + .pipe(takeUntilDestroyed()) + .subscribe(({ payload }) => this.#dialogRef.close(payload)); } /** diff --git a/core-web/libs/image-editor/src/lib/models/image-editor.models.ts b/core-web/libs/image-editor/src/lib/models/image-editor.models.ts index b4d8f1fd9e64..058fd273b0b8 100644 --- a/core-web/libs/image-editor/src/lib/models/image-editor.models.ts +++ b/core-web/libs/image-editor/src/lib/models/image-editor.models.ts @@ -30,6 +30,9 @@ export interface ImageRect { /** Loading lifecycle of the preview image. */ export type PreviewStatus = 'idle' | 'loading' | 'loaded' | 'error'; +/** Lifecycle of the Save (temp-file) request. */ +export type SaveStatus = 'idle' | 'saving' | 'error'; + /** Logical category an applied edit belongs to, used for grouping and labels. */ export type FilterCategory = | 'adjust' @@ -51,6 +54,9 @@ export type FilterName = | 'Jpeg' | 'WebP' | 'Quality' + // Focal point is injected into the chain only when saving (never in the preview + // URL); the registry key resolves case-insensitively to `focalpoint`. + | 'FocalPoint' // libvips-only modern format (AV1); registered lowercase as `avif`. | 'avif'; @@ -187,6 +193,11 @@ export interface ImageEditorOpenParams { byInode?: boolean; fileName?: string; mimeType?: string; + /** + * Normalized 0..1 focal point already stored on the asset, used to seed the + * editor so an existing focal point is preserved on Save unless the user moves it. + */ + focalPoint?: NormalizedPoint; } /** @@ -226,17 +237,26 @@ export interface ImageEditorState { zoom: ZoomState; /** * Normalized 0..1 focal point held as editor state only. It is NOT a filter slice - * (it never enters the preview filter chain nor the edit history) and is NOT - * persisted on its own — it would be committed alongside the other edits during - * the (separate) Save flow. See {@link withFocalPoint}. + * (it never enters the preview filter chain nor the edit history); it is folded + * into the chain only when saving and committed alongside the other edits. See + * {@link withFocalPoint} and {@link withSave}. */ focalPoint: NormalizedPoint; + /** + * The focal point the asset was opened with, used as the dirty/save baseline so a + * move-and-back is detected as pristine. + */ + seededFocalPoint: NormalizedPoint; /** Tool currently selected on the canvas. */ activeTool: ActiveTool; /** Loading lifecycle of the preview image. */ previewStatus: PreviewStatus; /** Consecutive silent retries of the current failing preview (reset on success). */ previewRetries: number; + /** Lifecycle of the Save (temp-file) request. */ + saveStatus: SaveStatus; + /** Last Save error message surfaced to the user, or `null`. */ + saveError: string | null; /** Last error message surfaced to the user, or `null`. */ error: string | null; /** Ordered undo/redo history of applied edits. */ diff --git a/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts b/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts index 0ab8dec5fcf0..9cc5bb5e56a5 100644 --- a/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts +++ b/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts @@ -6,6 +6,8 @@ import { Injectable, inject } from '@angular/core'; import { catchError, map } from 'rxjs/operators'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + import { AssetMeta, ImageEditorAssetContext, @@ -14,9 +16,9 @@ import { /** * Data-access service for the image editor: resolves asset metadata, queries the - * verified preview blob and file sizes, and triggers client-side downloads. All - * read-only/metadata calls are non-fatal and never throw. (Saving the edited image - * is handled in a separate issue.) + * verified preview blob and file sizes, triggers client-side downloads, and saves + * the edited image into a temp file. All read-only/metadata calls are non-fatal and + * never throw. */ @Injectable({ providedIn: 'root' }) export class DotImageEditorService { @@ -127,6 +129,16 @@ export class DotImageEditorService { anchor.remove(); } + /** + * Saves the edited image by GET-ting the filter/save URL, which makes the + * servlet stage the filtered render into a temp file and return its descriptor. + * @param url - The Save URL built by `buildSaveUrl` + * @returns The staged temp file descriptor + */ + saveEditedImage(url: string): Observable { + return this.#http.get(url); + } + #resolveNaturalDimensions(url: string): Observable { return new Observable((subscriber) => { const image = this.#document.createElement('img'); diff --git a/core-web/libs/image-editor/src/lib/store/features/index.ts b/core-web/libs/image-editor/src/lib/store/features/index.ts index f323309868b4..befc99ecec6c 100644 --- a/core-web/libs/image-editor/src/lib/store/features/index.ts +++ b/core-web/libs/image-editor/src/lib/store/features/index.ts @@ -6,5 +6,6 @@ export { withFileInfo } from './with-file-info.feature'; export { withFocalPoint } from './with-focal-point.feature'; export { withHistory } from './with-history.feature'; export { withPreview } from './with-preview.feature'; +export { withSave } from './with-save.feature'; export { withTransform } from './with-transform.feature'; export { withView } from './with-view.feature'; diff --git a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts new file mode 100644 index 000000000000..0d5432a6ecab --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts @@ -0,0 +1,66 @@ +import { signalStore, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { of } from 'rxjs'; + +import { Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { withAsset } from './with-asset.feature'; + +import { ImageEditorOpenParams } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { initialFocalPointState, initialImageEditorState } from '../image-editor.state'; + +const AssetStore = signalStore(withState(initialImageEditorState), withAsset()); + +const BASE_PARAMS: ImageEditorOpenParams = { + inode: 'inode-1', + variable: 'fileAsset', + fieldName: 'fileAsset', + fileName: 'photo.png', + mimeType: 'image/png' +}; + +describe('withAsset', () => { + let store: InstanceType; + let lifecycle: ReturnType>; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + AssetStore, + Dispatcher, + { + provide: DotImageEditorService, + useValue: { + loadAssetMeta: jest + .fn() + .mockReturnValue( + of({ naturalWidth: 800, naturalHeight: 600, originalBytes: 5000 }) + ) + } + } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(AssetStore); + runInInjectionContext(injector, () => { + lifecycle = injectDispatch(imageEditorLifecycleEvents); + }); + }); + + it('seeds focalPoint and seededFocalPoint from the open params', () => { + lifecycle.assetRequested({ ...BASE_PARAMS, focalPoint: { x: 0.3, y: 0.7 } }); + + expect(store.focalPoint()).toEqual({ x: 0.3, y: 0.7 }); + expect(store.seededFocalPoint()).toEqual({ x: 0.3, y: 0.7 }); + }); + + it('defaults focalPoint and seededFocalPoint to the center when none is provided', () => { + lifecycle.assetRequested(BASE_PARAMS); + + expect(store.focalPoint()).toEqual(initialFocalPointState); + expect(store.seededFocalPoint()).toEqual(initialFocalPointState); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts index 3f2508c149ff..774ec15dee0b 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts @@ -9,7 +9,7 @@ import { switchMap } from 'rxjs/operators'; import { ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; import { imageEditorLifecycleEvents } from '../image-editor.events'; -import { initialImageEditorState } from '../image-editor.state'; +import { initialFocalPointState, initialImageEditorState } from '../image-editor.state'; import { contextFromParams, errorMessage } from '../image-editor.store-utils'; /** @@ -24,6 +24,10 @@ export function withAsset() { on(imageEditorLifecycleEvents.assetRequested, ({ payload }, _state) => ({ ...initialImageEditorState, assetContext: contextFromParams(payload), + // Seed the focal point from the asset so an existing focal point is + // preserved on Save; the baseline lets a move-and-back read as pristine. + focalPoint: payload.focalPoint ?? initialFocalPointState, + seededFocalPoint: payload.focalPoint ?? initialFocalPointState, previewStatus: 'loading' as const })), on(imageEditorLifecycleEvents.assetLoaded, ({ payload }, state) => ({ diff --git a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts index fdc2954b7917..5239bce9e15a 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts @@ -10,7 +10,8 @@ import { CropState } from '../../models/image-editor.models'; import { imageEditorToolEvents } from '../image-editor.events'; import { initialImageEditorState } from '../image-editor.state'; -// Seed a pending resize so applying a crop can be shown to clear it. +// Seed a pending resize so applying a crop can be shown to PRESERVE it (legacy +// parity: a crop drawn on the scaled preview keeps the resize, chain Resize->Crop). const RESIZED = { ...initialImageEditorState, transform: { ...initialImageEditorState.transform, scale: 50, outputWidth: 500 } @@ -35,7 +36,7 @@ describe('withCrop', () => { runInInjectionContext(injector, () => (tool = injectDispatch(imageEditorToolEvents))); }); - it('applies a crop, clears resize, returns to move and adds a crop entry', () => { + it('applies a crop, preserves resize, returns to move and adds a crop entry', () => { tool.cropApplied({ x: 10, y: 10, w: 200, h: 150, active: false, aspect: null }); expect(store.crop()).toEqual({ @@ -46,8 +47,9 @@ describe('withCrop', () => { active: true, aspect: null }); - expect(store.transform().scale).toBe(100); - expect(store.transform().outputWidth).toBeNull(); + // Legacy parity: a crop keeps the resize (it was drawn on the scaled preview). + expect(store.transform().scale).toBe(50); + expect(store.transform().outputWidth).toBe(500); expect(store.activeTool()).toBe('move'); expect(store.history().at(-1)?.category).toBe('crop'); expect(store.previewStatus()).toBe('loading'); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts index c26055c10797..86f509f1d74d 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts @@ -1,16 +1,23 @@ import { signalStoreFeature, type } from '@ngrx/signals'; import { on, withReducer } from '@ngrx/signals/events'; -import { CropState, ImageEditorState, TransformState } from '../../models/image-editor.models'; +import { CropState, ImageEditorState } from '../../models/image-editor.models'; import { imageEditorToolEvents } from '../image-editor.events'; import { initialCropState } from '../image-editor.state'; import { coalesceHistory, editableSlicesOf } from '../image-editor.store-utils'; /** - * Crop feature: applies or cancels a manual crop selection. Applying a crop is - * mutually exclusive with resize (it clears scale/output dims), returns to the - * move tool and records a coalesced history entry; cancelling clears the pending - * selection. + * Crop feature: applies or cancels a manual crop selection. Applying a crop + * returns to the move tool and records a coalesced history entry; cancelling + * clears the pending selection. + * + * Crop / resize interplay mirrors the legacy editor, which is intentionally + * ASYMMETRIC: applying a resize clears any crop (handled in `withTransform`), + * but applying a crop KEEPS the resize — the crop box is drawn on the already + * scaled preview, so its pixels are in the scaled image's space and the server + * runs `Resize` then `Crop` (see `buildFilterChain`). Clearing the scale here + * would drop the resize and re-apply the crop against the full-resolution + * original, cutting the wrong region. */ export function withCrop() { return signalStoreFeature( @@ -18,17 +25,9 @@ export function withCrop() { withReducer( on(imageEditorToolEvents.cropApplied, ({ payload }, state) => { const crop: CropState = { ...payload, active: true }; - // Crop and resize are mutually exclusive: applying a crop clears resize. - const transform: TransformState = { - ...state.transform, - scale: 100, - outputWidth: null, - outputHeight: null - }; const next: ImageEditorState = { ...state, crop, - transform, activeTool: 'move', previewStatus: 'loading', cacheBust: state.cacheBust + 1 diff --git a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts new file mode 100644 index 000000000000..c988692c3ae0 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts @@ -0,0 +1,75 @@ +import { patchState, signalStore, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { of } from 'rxjs'; + +import { Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { withFocalPoint } from './with-focal-point.feature'; +import { withPreview } from './with-preview.feature'; + +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorToolEvents } from '../image-editor.events'; +import { initialImageEditorState } from '../image-editor.state'; + +// Compose preview + focal point so focal moves can be dispatched and read through +// `isDirty`, which compares the live focal point against the seeded baseline. +const PreviewStore = signalStore( + withState(initialImageEditorState), + withFocalPoint(), + withPreview() +); + +describe('withPreview - isDirty with focal point', () => { + let store: InstanceType; + let tools: ReturnType>; + + beforeEach(() => { + // The debounced `resolveSize$` effect schedules timers; isolate them. + jest.useFakeTimers(); + + TestBed.configureTestingModule({ + providers: [ + PreviewStore, + Dispatcher, + { + provide: DotImageEditorService, + useValue: { getFileSize: jest.fn().mockReturnValue(of(1000)) } + } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(PreviewStore); + runInInjectionContext(injector, () => { + tools = injectDispatch(imageEditorToolEvents); + }); + + // Seed a baseline focal point so a move-and-back can be detected as pristine. + patchState(store, { + focalPoint: { x: 0.4, y: 0.6 }, + seededFocalPoint: { x: 0.4, y: 0.6 } + }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('is not dirty when the focal point matches the seeded baseline', () => { + expect(store.isDirty()).toBe(false); + }); + + it('is dirty when the focal point moves away from the seeded baseline', () => { + tools.focalPointSet({ x: 0.1, y: 0.9 }); + + expect(store.isDirty()).toBe(true); + }); + + it('is not dirty when the focal point moves and then returns to the seeded baseline', () => { + tools.focalPointSet({ x: 0.1, y: 0.9 }); + expect(store.isDirty()).toBe(true); + + tools.focalPointSet({ x: 0.4, y: 0.6 }); + expect(store.isDirty()).toBe(false); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts index aa3bc7242fd6..324983eb5db3 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts @@ -10,7 +10,11 @@ import { catchError, debounceTime, distinctUntilChanged, switchMap, tap } from ' import { AUTO_PREVIEW_RETRY_LIMIT } from '../../image-editor.constants'; import { ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; -import { buildFilterChain, buildPreviewUrl } from '../../utils/image-filter-url.builder'; +import { + buildFilterChain, + buildPreviewUrl, + focalPointsEqual +} from '../../utils/image-filter-url.builder'; import { imageEditorLifecycleEvents } from '../image-editor.events'; /** @@ -102,8 +106,15 @@ export function withPreview() { previewUrl: computed(() => buildPreviewUrl(store.assetContext(), appliedFilters(), store.cacheBust()) ), - /** Whether any edit produces a non-empty filter chain. */ - isDirty: computed(() => appliedFilters().length > 0), + /** + * Whether the editor has unsaved changes: any preview filter, or a + * focal point moved away from the one the asset was opened with. + */ + isDirty: computed( + () => + appliedFilters().length > 0 || + !focalPointsEqual(store.focalPoint(), store.seededFocalPoint()) + ), /** Whether the editor is mid-flight loading a preview. */ isBusy: computed(() => store.previewStatus() === 'loading') }; 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 new file mode 100644 index 000000000000..88f3b58d98c7 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts @@ -0,0 +1,161 @@ +import { signalStore, signalStoreFeature, type, withComputed, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { NEVER, of, throwError } from 'rxjs'; + +import { computed, Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { withSave } from './with-save.feature'; + +import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { initialImageEditorState } from '../image-editor.state'; + +// `withSave` consumes the `appliedFilters` prop from `withPreview`; stub it with a +// fixed empty chain so the feature can be exercised in isolation. +function withAppliedFiltersStub() { + return signalStoreFeature( + type<{ state: ImageEditorState }>(), + withComputed(() => ({ appliedFilters: computed(() => []) })) + ); +} + +// Seed a realistic asset context + a moved focal point so the URL the feature builds +// (store signals -> buildSaveUrl) can be asserted, not only the status transitions. +const SaveStore = signalStore( + withState({ + ...initialImageEditorState, + assetContext: { + ...initialImageEditorState.assetContext, + variable: 'binary', + originalUrl: '/contentAsset/image/abc123/binary' + }, + focalPoint: { x: 0.25, y: 0.75 } + }), + withAppliedFiltersStub(), + withSave() +); + +const TEMP_FILE: DotCMSTempFile = { + fileName: 'edited.png', + folder: 'shared', + id: 'temp_578026b7cc', + image: true, + length: 1024, + mimeType: 'image/png', + referenceUrl: '/dA/temp_578026b7cc', + thumbnailUrl: '/dA/temp_578026b7cc/thumb' +}; + +describe('withSave', () => { + let store: InstanceType; + let service: { saveEditedImage: jest.Mock }; + let lifecycle: ReturnType>; + + function setup(): jest.SpyInstance { + const dispatchSpy = jest.spyOn(Dispatcher.prototype, 'dispatch'); + + TestBed.configureTestingModule({ + providers: [ + SaveStore, + Dispatcher, + { provide: DotImageEditorService, useValue: service } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(SaveStore); + runInInjectionContext(injector, () => { + lifecycle = injectDispatch(imageEditorLifecycleEvents); + }); + + return dispatchSpy; + } + + beforeEach(() => { + service = { saveEditedImage: jest.fn().mockReturnValue(of(TEMP_FILE)) }; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('marks the editor saving on saveRequested while the save is in flight', () => { + // A never-completing save holds the request pending so 'saving' is observable. + service.saveEditedImage.mockReturnValue(NEVER); + setup(); + + lifecycle.saveRequested(); + + expect(store.saveStatus()).toBe('saving'); + expect(store.saveError()).toBeNull(); + }); + + it('dispatches saveSucceeded and returns to idle on success', () => { + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(service.saveEditedImage).toHaveBeenCalledTimes(1); + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveSucceeded(TEMP_FILE) + ); + expect(store.saveStatus()).toBe('idle'); + expect(store.saveError()).toBeNull(); + }); + + 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. + const metadata = { + contentType: 'image/png', + fileSize: 1024, + isImage: true, + length: 1024, + modDate: 0, + name: 'edited.png', + sha256: 'abc', + title: 'edited.png', + version: 1 + }; + service.saveEditedImage.mockReturnValue(of({ ...TEMP_FILE, metadata })); + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveSucceeded({ + ...TEMP_FILE, + metadata: { ...metadata, focalPoint: '0.25,0.75' } + }) + ); + }); + + it('builds the Save URL from the asset context, filter chain and focal point', () => { + setup(); + + lifecycle.saveRequested(); + + // assetContext.variable + originalUrl, the (empty) stubbed chain, and the moved + // focal point all flow through buildSaveUrl into the single GET. + expect(service.saveEditedImage).toHaveBeenCalledWith( + '/contentAsset/image/abc123/binary/filter/FocalPoint/fp/0.25,0.75' + + '?binaryFieldId=binary&_imageToolSaveFile=true&overwrite=true' + ); + }); + + it('dispatches saveFailed and surfaces the error on failure', () => { + service.saveEditedImage.mockReturnValue(throwError(() => new Error('boom'))); + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveFailed(expect.any(Error)) + ); + expect(store.saveStatus()).toBe('error'); + expect(store.saveError()).toBe('boom'); + }); +}); 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 new file mode 100644 index 000000000000..c12fa101d964 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts @@ -0,0 +1,96 @@ +import { tapResponse } from '@ngrx/operators'; +import { signalStoreFeature, type } from '@ngrx/signals'; +import { Dispatcher, Events, on, withEventHandlers, withReducer } from '@ngrx/signals/events'; + +import { inject, Signal } from '@angular/core'; + +import { exhaustMap } from 'rxjs/operators'; + +import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { buildSaveUrl } from '../../utils/image-filter-url.builder'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { errorMessage } from '../image-editor.store-utils'; + +/** + * Save feature: stages the current edits (preview filter chain + focal point) into a + * temp file. A `saveRequested` marks the editor saving; the `save$` effect GETs the + * Save URL and folds the resulting temp file (`saveSucceeded`) or error (`saveFailed`) + * back in. Consumes the `appliedFilters` selector from {@link withPreview}, so it + * composes after it. + */ +export function withSave() { + return signalStoreFeature( + type<{ state: ImageEditorState; props: { appliedFilters: Signal } }>(), + withReducer( + on(imageEditorLifecycleEvents.saveRequested, (_event, state) => ({ + ...state, + saveStatus: 'saving' as const, + saveError: null + })), + on(imageEditorLifecycleEvents.saveSucceeded, (_event, state) => ({ + ...state, + saveStatus: 'idle' as const + })), + on(imageEditorLifecycleEvents.saveFailed, ({ payload }, state) => ({ + ...state, + saveStatus: 'error' as const, + saveError: errorMessage(payload, 'Failed to save image') + })) + ), + withEventHandlers((store) => { + const events = inject(Events); + const dispatcher = inject(Dispatcher); + const service = inject(DotImageEditorService); + + return { + // Save the edited image whenever the user requests it. `exhaustMap` (not + // switchMap) so a re-triggered save is ignored while one is in flight, rather + // than cancelling the in-flight request. The Save button is already disabled + // while saving, so this is defense-in-depth for non-UI triggers. + save$: events.on(imageEditorLifecycleEvents.saveRequested).pipe( + exhaustMap(() => { + // Capture the focal point at save time and fold it into the returned + // temp's metadata. The servlet writes the focal to the temp's metadata + // STORAGE, but the serialized DotCMSTempFile.metadata is basic (no + // focalPoint), so an in-session reopen would read the temp metadata and + // seed the marker at centre. Injecting it here mirrors the contentlet + // read-back used after a page refresh, keeping both paths consistent. + const focalPoint = store.focalPoint(); + + return service + .saveEditedImage( + buildSaveUrl( + store.assetContext(), + store.appliedFilters(), + focalPoint, + store.seededFocalPoint(), + store.assetContext().variable + ) + ) + .pipe( + tapResponse({ + next: (tempFile) => + dispatcher.dispatch( + imageEditorLifecycleEvents.saveSucceeded({ + ...tempFile, + metadata: tempFile.metadata + ? { + ...tempFile.metadata, + focalPoint: `${focalPoint.x},${focalPoint.y}` + } + : tempFile.metadata + }) + ), + error: (error) => + dispatcher.dispatch( + imageEditorLifecycleEvents.saveFailed(error) + ) + }) + ); + }) + ) + }; + }) + ); +} diff --git a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts index da0276cb831b..612656f1c7eb 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts @@ -83,6 +83,21 @@ describe('withTransform', () => { ]); }); + it('records each flip of the same axis as its own history step (discrete toggle)', () => { + const { store, transform } = setup(TransformStore); + transform.flipHToggled(); + transform.flipHToggled(); + // Two clicks net back to the un-flipped state... + expect(store.transform().flipH).toBe(false); + // ...but each click is a distinct undoable step — it must NOT coalesce into + // one silently-updated entry (the QA bug: applied twice, shown once). + expect(store.history().map((entry) => entry.label)).toEqual([ + 'Flip horizontal', + 'Flip horizontal' + ]); + expect(store.historyIndex()).toBe(1); + }); + it('clears an active crop when explicit output dimensions are set', () => { const { store, transform } = setup(TransformStoreCropped); transform.outputDimsChanged({ width: 500, height: null }); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts index c41d248643e7..eda195d2a16c 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts @@ -42,13 +42,16 @@ export function withTransform() { flipH: !state.transform.flipH }; + // A flip is a discrete toggle, not a continuous control: pass a null + // coalesceKey so each click is its own history step (two flips of the + // same axis are two entries, not one silently updated in place). return transformPatch( state, transform, state.crop, 'flip', 'Flip horizontal', - 'flipH' + null ); }), on(imageEditorTransformEvents.flipVToggled, (_event, state) => { @@ -57,14 +60,8 @@ export function withTransform() { flipV: !state.transform.flipV }; - return transformPatch( - state, - transform, - state.crop, - 'flip', - 'Flip vertical', - 'flipV' - ); + // Discrete toggle — see flipHToggled: null disables coalescing. + return transformPatch(state, transform, state.crop, 'flip', 'Flip vertical', null); }), on(imageEditorTransformEvents.outputDimsChanged, ({ payload }, state) => { const transform: TransformState = { diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.events.ts b/core-web/libs/image-editor/src/lib/store/image-editor.events.ts index 203d1e0b3fdf..920a85fd038f 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.events.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.events.ts @@ -1,6 +1,8 @@ import { type } from '@ngrx/signals'; import { eventGroup } from '@ngrx/signals/events'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + import { ActiveTool, CompressionMode, @@ -71,7 +73,7 @@ export const imageEditorHistoryEvents = eventGroup({ } }); -/** Events covering the editor lifecycle: load, preview and download. */ +/** Events covering the editor lifecycle: load, preview, download and save. */ export const imageEditorLifecycleEvents = eventGroup({ source: 'Image Editor Lifecycle', events: { @@ -80,6 +82,9 @@ export const imageEditorLifecycleEvents = eventGroup({ previewErrored: type(), retryRequested: type(), downloadRequested: type(), + saveRequested: type(), + saveSucceeded: type(), + saveFailed: type(), assetLoaded: type<{ naturalWidth: number; naturalHeight: number; diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.state.ts b/core-web/libs/image-editor/src/lib/store/image-editor.state.ts index 2c6afa0a0b9c..47f04c250adc 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.state.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.state.ts @@ -80,9 +80,12 @@ export const initialImageEditorState: ImageEditorState = { fileInfo: initialFileInfoState, zoom: initialZoomState, focalPoint: initialFocalPointState, + seededFocalPoint: initialFocalPointState, activeTool: 'move', previewStatus: 'idle', previewRetries: 0, + saveStatus: 'idle', + saveError: null, error: null, history: [], historyIndex: -1, diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts index f14698e9898c..f6208bef7ba4 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts @@ -57,11 +57,17 @@ let historyEntrySeq = 0; * {@link FilterCategory} therefore starts a new entry rather than overwriting the * previous one. Either way a new value invalidates forward history built against * the old one. + * + * Passing a `null` coalesceKey marks a *discrete* action (e.g. a flip toggle, + * which is a button click, not a continuous drag): it never merges into the head, + * so every invocation is its own undoable step — two flips of the same axis are + * two history entries, not one silently updated in place. * @param state - The current editor state * @param category - The category the edit belongs to (used for grouping/labels) * @param label - The human-readable label for the entry * @param snapshot - The editable slices to capture - * @param coalesceKey - The control identity edits merge on; defaults to `category` + * @param coalesceKey - The control identity edits merge on; defaults to `category`. + * `null` disables coalescing (each call appends a distinct entry). * @returns The new `history` array and `historyIndex` */ export function coalesceHistory( @@ -69,11 +75,11 @@ export function coalesceHistory( category: FilterCategory, label: string, snapshot: EditableSlices, - coalesceKey: string = category + coalesceKey: string | null = category ): Pick { const head = state.history[state.historyIndex]; - if (head && head.coalesceKey === coalesceKey) { + if (coalesceKey !== null && head && head.coalesceKey === coalesceKey) { // Update the head in place AND drop any redo tail: a new value for the same // control invalidates forward history built against the old value (matching // the redo-tail truncation the new-entry branch below performs). @@ -85,12 +91,15 @@ export function coalesceHistory( return { history, historyIndex: state.historyIndex }; } + // A monotonic counter keeps ids collision-free even for two entries created + // in the same millisecond (Date.now() could collide, e.g. under fake timers). + const id = `${category}-${++historyEntrySeq}`; const entry: ImageEditorHistoryEntry = { - // A monotonic counter keeps ids collision-free even for two entries created - // in the same millisecond (Date.now() could collide, e.g. under fake timers). - id: `${category}-${++historyEntrySeq}`, + id, category, - coalesceKey, + // A discrete action (null key) falls back to its unique id so it never + // coalesces with a later action sharing the same category. + coalesceKey: coalesceKey ?? id, label, snapshot }; @@ -238,7 +247,7 @@ export function transformPatch( crop: CropState, category: FilterCategory, label: string, - coalesceKey: string = category + coalesceKey: string | null = category ): ImageEditorState { const next: ImageEditorState = { ...state, diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts index 7008ce2ec72b..c2a1b7cbe548 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts @@ -162,7 +162,7 @@ describe('ImageEditorStore', () => { expect(store.history()).toHaveLength(0); }); - it('should apply a crop, reset resize and add a crop entry', () => { + it('should apply a crop, preserve resize and add a crop entry', () => { transform.scaleChanged(50); tool.cropApplied({ x: 10, y: 10, w: 200, h: 150, active: false, aspect: null }); @@ -174,7 +174,12 @@ describe('ImageEditorStore', () => { active: true, aspect: null }); - expect(store.transform().scale).toBe(100); + // Legacy parity: a crop drawn on the scaled preview keeps the resize + // (the server runs Resize then Crop), so the scale must NOT reset. + // (The Resize->Crop chain ordering is covered in the builder spec, which + // has real natural dimensions; this store has none, so scale alone can't + // emit a Resize filter here.) + expect(store.transform().scale).toBe(50); expect(store.activeTool()).toBe('move'); expect(store.history().at(-1)?.category).toBe('crop'); }); diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store.ts index b15eb74bf769..aafef6f9688a 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store.ts @@ -9,6 +9,7 @@ import { withFocalPoint, withHistory, withPreview, + withSave, withTransform, withView } from './features'; @@ -21,9 +22,9 @@ import { initialImageEditorState } from './image-editor.state'; * single place (`features/with-*.feature.ts`). * * Order matters only where features consume each other's selectors: `withPreview` - * derives `previewUrl`, which `withDownload` reads — so download composes after - * preview. The store is NOT provided in root; the editor dialog supplies it so - * each editor instance is isolated. + * derives `previewUrl`/`appliedFilters`, which `withDownload` and `withSave` read — + * so download and save compose after preview. The store is NOT provided in root; the + * editor dialog supplies it so each editor instance is isolated. */ export const ImageEditorStore = signalStore( withState(initialImageEditorState), @@ -36,5 +37,6 @@ export const ImageEditorStore = signalStore( withHistory(), withAsset(), withPreview(), - withDownload() + withDownload(), + withSave() ); diff --git a/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts b/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts index ea113dbeac94..edca0d70ed12 100644 --- a/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts @@ -92,7 +92,7 @@ describe('dimensions.util', () => { }); }); - it('uses the crop size when an active crop is present and not resizing', () => { + it('uses the crop size when an active crop is present', () => { expect( computeOutputDimensions( ctx(1000, 800), @@ -102,14 +102,16 @@ describe('dimensions.util', () => { ).toEqual({ width: 200, height: 151 }); }); - it('ignores the crop when resizing (resize supersedes crop)', () => { + it('uses the crop size even when resizing (crop runs after resize)', () => { + // Legacy parity: the crop was drawn on the scaled preview, so its w/h are + // already in the resized space and dictate the final size. expect( computeOutputDimensions( ctx(1000, 800), transform({ scale: 50 }), crop({ active: true, w: 200, h: 150 }) ) - ).toEqual({ width: 500, height: 400 }); + ).toEqual({ width: 200, height: 150 }); }); it('uses explicit output width and height when both are set', () => { diff --git a/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts b/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts index 061960b4cdba..7dc6628eb2a7 100644 --- a/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts +++ b/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts @@ -62,13 +62,10 @@ export function computeOutputDimensions( let width = ctx.naturalWidth; let height = ctx.naturalHeight; - const isResizing = - transform.outputWidth != null || transform.outputHeight != null || transform.scale !== 100; - - // Resizing supersedes crop, mirroring the filter-chain rule. - if (!isResizing && crop.active && crop.w > 0 && crop.h > 0) { - width = Math.round(crop.w); - height = Math.round(crop.h); + // A crop runs last in the chain (after any resize), so when one is active it + // dictates the final size — its w/h are already in the scaled image's space. + if (crop.active && crop.w > 0 && crop.h > 0) { + return { width: Math.round(crop.w), height: Math.round(crop.h) }; } if (transform.outputWidth != null || transform.outputHeight != null) { diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts index 73c5440ff806..a86b2a83ae5c 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts @@ -1,11 +1,19 @@ -import { buildFilterChain, buildPreviewUrl, cleanUrl, toHsb } from './image-filter-url.builder'; +import { + buildFilterChain, + buildPreviewUrl, + buildSaveUrl, + cleanUrl, + toHsb +} from './image-filter-url.builder'; import { AdjustState, + AppliedFilter, CompressionMode, CropState, FileInfoState, ImageEditorAssetContext, + NormalizedPoint, TransformState } from '../models/image-editor.models'; @@ -180,22 +188,24 @@ describe('image-filter-url.builder', () => { }); }); - describe('buildFilterChain - resize removes crop', () => { - it('drops the Crop filter when resizing', () => { + describe('buildFilterChain - resize and crop coexist (legacy parity)', () => { + it('keeps the Crop after the Resize when explicit output dimensions are set', () => { const result = chain({ transform: { outputWidth: 600 }, crop: { active: true, x: 0, y: 0, w: 100, h: 100 } }); - expect(result).toEqual([{ name: 'Resize', args: '/resize_w/600' }]); - expect(result.some((f) => f.name === 'Crop')).toBe(false); + // Crop is drawn on the scaled preview, so it runs after the resize. + expect(result.map((f) => f.name)).toEqual(['Resize', 'Crop']); }); - it('treats a non-100 scale as resizing and drops crop', () => { + it('keeps the Crop after a non-100 scale resize', () => { const result = chain({ transform: { scale: 50 }, crop: { active: true, x: 0, y: 0, w: 100, h: 100 } }); - expect(result.some((f) => f.name === 'Crop')).toBe(false); + expect(result.map((f) => f.name)).toEqual(['Resize', 'Crop']); + // The crop coords stay in the scaled space (they are emitted verbatim). + expect(result[1].args).toBe('/crop_w/100/crop_h/100/crop_x/0/crop_y/0'); }); it('builds a Resize filter from scale% × the natural size', () => { @@ -319,4 +329,84 @@ describe('image-filter-url.builder', () => { expect(url).toBe('/contentAsset/image/abc123/fileAsset?test=5'); }); }); + + describe('buildSaveUrl', () => { + const center: NormalizedPoint = { x: 0.5, y: 0.5 }; + + it('appends the save flags to the base url for an empty chain and centered focal', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset?binaryFieldId=fileAsset&_imageToolSaveFile=true' + ); + }); + + it('keeps the filter segment and adds no overwrite when only filters are applied', () => { + const filters = chain({ adjust: { grayscale: true } }); + const url = buildSaveUrl(baseContext, filters, center, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset/filter/Grayscale/grayscale/1' + + '?binaryFieldId=fileAsset&_imageToolSaveFile=true' + ); + expect(url).not.toContain('overwrite'); + expect(url).not.toContain('FocalPoint'); + }); + + it('does not inject a FocalPoint filter for an untouched centered focal point', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'fileAsset'); + expect(url).not.toContain('FocalPoint'); + expect(url).not.toContain('overwrite'); + }); + + it('injects a FocalPoint filter and overwrite flag when the focal point moved from centre', () => { + const url = buildSaveUrl(baseContext, [], { x: 0.25, y: 0.75 }, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset/filter/FocalPoint/fp/0.25,0.75' + + '?binaryFieldId=fileAsset&_imageToolSaveFile=true&overwrite=true' + ); + }); + + it('re-stages an untouched non-centre focal point so it is preserved on save', () => { + const seeded: NormalizedPoint = { x: 0.3, y: 0.7 }; + const url = buildSaveUrl(baseContext, [], seeded, seeded, 'fileAsset'); + expect(url).toContain('/filter/FocalPoint/fp/0.3,0.7'); + expect(url).toContain('&overwrite=true'); + }); + + it('sends the focal point when recentring a previously-set focal point', () => { + // Opened at (0.3,0.7), dragged back to centre: must still send so the server overwrites. + const url = buildSaveUrl(baseContext, [], center, { x: 0.3, y: 0.7 }, 'fileAsset'); + expect(url).toContain('/filter/FocalPoint/fp/0.5,0.5'); + expect(url).toContain('&overwrite=true'); + }); + + it('treats a sub-epsilon nudge back to the seeded centre as unchanged (no focal entry)', () => { + const nudged: NormalizedPoint = { x: 0.5 + 1e-6, y: 0.5 - 1e-6 }; + const url = buildSaveUrl(baseContext, [], nudged, center, 'fileAsset'); + expect(url).not.toContain('FocalPoint'); + expect(url).not.toContain('overwrite'); + }); + + it('appends FocalPoint to an existing chain as a single comma-joined filter segment', () => { + const filters: AppliedFilter[] = chain({ + crop: { active: true, x: 0, y: 0, w: 100, h: 50 } + }); + const url = buildSaveUrl(baseContext, filters, { x: 0.1, y: 0.2 }, center, 'fileAsset'); + expect(url).toContain( + '/filter/Crop,FocalPoint/crop_w/100/crop_h/50/crop_x/0/crop_y/0/fp/0.1,0.2' + ); + expect(url).toContain('&overwrite=true'); + }); + + it('includes the binaryFieldId in the query', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'myImage'); + expect(url).toContain('binaryFieldId=myImage'); + }); + + it('URL-encodes the binaryFieldId so special characters cannot inject query params', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'a&b=c'); + // The raw `&`/`=` must be percent-encoded, not leak into the query string. + expect(url).toContain('binaryFieldId=a%26b%3Dc'); + expect(url).not.toContain('binaryFieldId=a&b=c'); + }); + }); }); diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts index bd7abd16d820..20755c460156 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts @@ -4,7 +4,8 @@ import { AppliedFilter, CompressionMode, FilterChainInput, - ImageEditorAssetContext + ImageEditorAssetContext, + NormalizedPoint } from '../models/image-editor.models'; /** @@ -43,10 +44,12 @@ function compressionFilter(mode: CompressionMode, quality: number): AppliedFilte /** * Builds the ordered list of server filters from the current edit state, - * mirroring the legacy ImageEditor rules: resizing removes crop, vertical flip - * is expressed as a 180deg rotation plus flip-token cancellation, and the - * compression filter is always applied last and exclusively. Crop is placed - * relative to the rotate/flip transforms by {@link FilterChainInput.cropBeforeTransforms}: + * mirroring the legacy ImageEditor rules: resize and crop coexist (the legacy + * rule is asymmetric — adding a resize clears any crop, but a crop drawn on the + * scaled preview keeps the resize and runs after it, so the crop pixels stay in + * the scaled image's space), vertical flip is expressed as a 180deg rotation plus + * flip-token cancellation, and the compression filter is always applied last and + * exclusively. Crop is placed relative to the rotate/flip transforms by {@link FilterChainInput.cropBeforeTransforms}: * a crop drawn before rotating must run first (on the un-rotated image it was drawn * against), while a crop drawn on an already-rotated preview runs after — so the * server applies it in the same coordinate space the user saw. @@ -73,9 +76,13 @@ export function buildFilterChain(input: FilterChainInput): AppliedFilter[] { filters.push({ name: 'Resize', args }); } - // Resize and crop stay mutually exclusive: resize wins. + // Resize and crop coexist: the crop box was drawn on the already-scaled + // preview, so its pixels are in the resized image's space. Emitting the Crop + // after the Resize (below) makes the server crop the scaled image — the region + // the user actually selected. (The reverse guard — a resize clearing an + // existing crop — lives in withTransform, mirroring the legacy editor.) const cropFilter = - !hasResize && crop.active && crop.w > 0 && crop.h > 0 + crop.active && crop.w > 0 && crop.h > 0 ? { name: 'Crop' as const, args: @@ -176,3 +183,78 @@ export function buildPreviewUrl( return url; } + +/** + * Compares two normalized focal points within a small tolerance. Focal coordinates are + * floats (pointer math and 0.01 keyboard nudges that never round), so an exact `===` would + * report a point nudged back to its origin as still-moved — leaving `isDirty` stuck true and + * the discard guard armed, and the Save URL emitting a focal entry for a visually-unchanged point. + * @param a - First focal point + * @param b - Second focal point + * @param epsilon - Maximum per-axis difference treated as equal (default 1e-4) + * @returns Whether the two points are equal within the tolerance + */ +export function focalPointsEqual(a: NormalizedPoint, b: NormalizedPoint, epsilon = 1e-4): boolean { + return Math.abs(a.x - b.x) < epsilon && Math.abs(a.y - b.y) < epsilon; +} + +/** The neutral, "no focal point set" position the editor opens with by default. */ +const CENTER_FOCAL_POINT: NormalizedPoint = { x: 0.5, y: 0.5 }; + +/** + * Builds the Save URL: the same `/filter/` segment the preview uses, but + * without the cache-bust and with the `_imageToolSaveFile` flag so the servlet + * stages the filtered render into a temp file (returned as a {@link DotCMSTempFile}). + * + * The focal point is folded in as a trailing `FocalPoint` chain entry (the registry + * resolves the name case-insensitively) and forces `overwrite=true` — which the + * `FocalPointImageFilter` requires to stage the focal metadata onto the result — whenever + * it holds a real (non-centre) value OR differs from the point the asset opened with (so + * recentring a previously-set focal point still overwrites it, and an existing focal point + * is re-staged onto the new temp rather than dropped). It is intentionally NOT part of the + * preview chain, so it only ever reaches the server on Save. + * @param ctx - Resolved asset context (provides the base URL) + * @param chain - The ordered preview filters produced by {@link buildFilterChain} + * @param focalPoint - The current normalized 0..1 focal point + * @param seededFocalPoint - The focal point the asset was opened with (the dirty baseline) + * @param binaryFieldId - The binary field variable (sent for legacy parity) + * @returns The fully-qualified Save URL + */ +export function buildSaveUrl( + ctx: ImageEditorAssetContext, + chain: AppliedFilter[], + focalPoint: NormalizedPoint, + seededFocalPoint: NormalizedPoint, + binaryFieldId: string +): string { + // Persist the focal point (and force the server to commit it via `overwrite`) when it + // holds a real, non-centre value OR the user moved it from what the asset opened with + // (the latter also covers recentring a previously-set focal point back to the middle). + const focalApplied = + !focalPointsEqual(focalPoint, seededFocalPoint) || + !focalPointsEqual(focalPoint, CENTER_FOCAL_POINT); + const saveChain: AppliedFilter[] = focalApplied + ? [...chain, { name: 'FocalPoint', args: `/fp/${focalPoint.x},${focalPoint.y}` }] + : chain; + + let url = ctx.originalUrl; + + if (saveChain.length > 0) { + const names = saveChain.map((filter) => filter.name).join(','); + const args = saveChain.map((filter) => filter.args).join(''); + url = `${url}/filter/${names}${args}`; + } + + url = cleanUrl(url); + url += url.includes('?') ? '&' : '?'; + // Encode the field variable defensively: dotCMS field variables are constrained to + // identifier-safe characters today, but encoding guarantees a stray `&`/`=`/`?` could + // never break out of the value and inject extra query parameters. + url += `binaryFieldId=${encodeURIComponent(binaryFieldId)}&_imageToolSaveFile=true`; + + if (focalApplied) { + url += '&overwrite=true'; + } + + return url; +} diff --git a/core-web/libs/sdk/ai/scripts/generate-spec.ts b/core-web/libs/sdk/ai/scripts/generate-spec.ts index e48a31947d22..6692590b1e01 100644 --- a/core-web/libs/sdk/ai/scripts/generate-spec.ts +++ b/core-web/libs/sdk/ai/scripts/generate-spec.ts @@ -58,7 +58,7 @@ const EXCLUDED_PATTERNS = [ ]; const DEFAULT_SPEC_PATH = '/api/openapi.json'; -const DEFAULT_SPEC_URL = `https://demo.dotcms.com${DEFAULT_SPEC_PATH}`; +const DEFAULT_SPEC_URL = `https://corpsites-headless.dotcms.cloud${DEFAULT_SPEC_PATH}`; /** * Matches a path against a pattern. Pattern syntax: diff --git a/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java b/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java index 24260456bfb2..449302b4036e 100644 --- a/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java +++ b/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java @@ -75,14 +75,6 @@ public interface FeatureFlagName { String FEATURE_FLAG_LOCALE_SELECTOR_V2 = "FEATURE_FLAG_LOCALE_SELECTOR_V2"; - /** - * Enables the new Angular image editor (@dotcms/image-editor) in the Edit Content v2 - * binary field. Off by default; when disabled the binary field falls back to the - * legacy Dojo image editor. - * Frontend equivalent: {@code FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR}. - */ - String FEATURE_FLAG_NEW_IMAGE_EDITOR = "FEATURE_FLAG_NEW_IMAGE_EDITOR"; - /** * libvips image-engine toggle (off by default; the legacy Java2D engine is used * otherwise). The new image editor reads this through the configuration endpoint diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java index b6a0c1457fbe..ef5af4f764fa 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java @@ -93,7 +93,6 @@ public class ConfigurationResource implements Serializable { FeatureFlagName.FEATURE_FLAG_NEW_BLOCK_EDITOR, FeatureFlagName.FEATURE_FLAG_CONTENT_EDITOR2_ENABLED, FeatureFlagName.FEATURE_FLAG_LOCALE_SELECTOR_V2, - FeatureFlagName.FEATURE_FLAG_NEW_IMAGE_EDITOR, // libvips engine toggle: the new image editor reads it to gate AVIF output. FeatureFlagName.IMAGE_API_USE_LIBVIPS); @@ -112,7 +111,6 @@ public class ConfigurationResource implements Serializable { REPORT_ISSUE_INCLUDE_USER_PII, FeatureFlagName.FEATURE_FLAG_REPORT_ISSUE_ENABLED, FeatureFlagName.FEATURE_FLAG_LOCALE_SELECTOR_V2, - FeatureFlagName.FEATURE_FLAG_NEW_IMAGE_EDITOR, // libvips engine toggle: the new image editor reads it to gate AVIF output. FeatureFlagName.IMAGE_API_USE_LIBVIPS })); 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 f4a400192539..616e6a6772d9 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 @@ -14,6 +14,7 @@ import com.dotmarketing.business.IdentifierAPI; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.image.focalpoint.FocalPointAPI; import com.dotmarketing.portlets.categories.model.Category; import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo; @@ -264,9 +265,19 @@ private void addBinaries(final Contentlet contentlet, final Map if (null != metadata) { Map metaMap = new HashMap<>(metadata.getMap()); metaMap.remove("path"); + // Focal point lives in the binary's custom metadata under a prefixed key, so it + // is not exposed under the clean "focalPoint" key by metadata.getMap(). Surface + // it explicitly so the image editor can re-seed its marker when reopened. + final String focalPoint = Try.of(() -> + metadata.getCustomMeta().getOrDefault(FocalPointAPI.FOCAL_POINT, "0.0").toString() + ).onFailure(e -> Logger.debug(this, + "Unable to read focal point for field '" + velocityVarName + "': " + + e.getMessage())) + .getOrElse("0.0"); + metaMap.put(FocalPointAPI.FOCAL_POINT, focalPoint); map.put(velocityVarName + "MetaData", metaMap); putBinaryLinks(velocityVarName, metadata.getName(), contentlet, map); - } + } } catch (final Exception e) { Logger.warn(this, String.format("An error occurred when retrieving the Binary file from field" diff --git a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java index 5be5ac02c95e..8b871090d828 100644 --- a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java +++ b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java @@ -51,6 +51,7 @@ import com.dotmarketing.beans.Identifier; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.web.WebAPILocator; +import com.dotmarketing.image.focalpoint.FocalPointAPI; import com.dotmarketing.db.DbConnectionFactory; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; @@ -108,6 +109,7 @@ public class BinaryExporterServlet extends HttpServlet { private final ContentletAPI contentAPI = APILocator.getContentletAPI(); private final TempFileAPI tempFileAPI = APILocator.getTempFileAPI(); private final FileMetadataAPI fileMetadataAPI = APILocator.getFileMetadataAPI(); + private final FocalPointAPI focalPointAPI = APILocator.getFocalPointAPI(); private final WebResource webResource = new WebResource(); Map exportersByPathMapping; @@ -412,11 +414,28 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl // THIS IS WHERE THE MAGIC HAPPENS // this creates a temp resource using the altered file if (req.getParameter(WebKeys.IMAGE_TOOL_SAVE_FILES) != null && user!=null && !user.equals(APILocator.getUserAPI().getAnonymousUser())) { - final DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req); + DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req); FileUtil.copyFile(data.getDataFile(), temp.file); //Temp files time-mark must be updated so they can be recognized by the tempFileAPI temp.file.setLastModified(System.currentTimeMillis()); + // createEmptyTempFile derives metadata from the still-empty file, so the rendered + // image would otherwise come back as metadata:null/image:false/mimeType:unknown. + // Re-wrap to regenerate metadata from the now-populated file (mirrors + // TempFileAPI.createTempFile) so consumers get a usable image preview. + if (temp.metadata == null && temp.file.exists()) { + temp = new DotTempFile(temp.id, temp.file); + } copyMetadata(uuid, fieldVarName, temp); + // The focal point is authoritative in the request URL. Write it straight onto the saved + // temp AFTER copyMetadata, so it persists even when the image exporter served the rendition + // from cache and never ran FocalPointImageFilter (the side effect that otherwise persists + // the focal). No-op when the request carries no fp (plain saves / non-focal images); + // putCustomMetadataAttributes merges, so only the focalPoint key is set and the metadata + // copied above is preserved. + final String savedTempId = temp.id; + final String focalFieldVar = fieldVarName; + focalPointAPI.parseFocalPointFromParams(params) + .ifPresent(focalPoint -> focalPointAPI.writeFocalPoint(savedTempId, focalFieldVar, focalPoint)); resp.getWriter().println(DotObjectMapperProvider.getInstance().getDefaultObjectMapper().writeValueAsString(temp)); resp.getWriter().close(); resp.flushBuffer(); diff --git a/dotCMS/src/main/resources/dotmarketing-config.properties b/dotCMS/src/main/resources/dotmarketing-config.properties index 90aa703758df..741b88578cc5 100644 --- a/dotCMS/src/main/resources/dotmarketing-config.properties +++ b/dotCMS/src/main/resources/dotmarketing-config.properties @@ -873,10 +873,6 @@ FEATURE_FLAG_NEW_BLOCK_EDITOR=false ## Enhanced locale selector v2 in the edit-content sidebar FEATURE_FLAG_LOCALE_SELECTOR_V2=true -## New Angular image editor in the Edit Content v2 binary field (rollback safety: -## the legacy Dojo image editor opens by default until this is explicitly enabled) -FEATURE_FLAG_NEW_IMAGE_EDITOR=false - ## libvips image engine toggle. Off by default (legacy Java2D engine). The new image ## editor reads this (via the configuration endpoint) to gate the libvips-only AVIF ## output format. Declared here so the endpoint returns an explicit boolean instead diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index 23429882a010..39369ae68fe2 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -6479,6 +6479,8 @@ edit.content.image-editor.close.aria=Close image editor edit.content.image-editor.footer.cancel=Cancel edit.content.image-editor.footer.download=Download edit.content.image-editor.footer.download.aria=Download image +edit.content.image-editor.footer.save=Save +edit.content.image-editor.footer.save.aria=Save edited image edit.content.image-editor.canvas.alt=Image preview edit.content.image-editor.canvas.loading=Applying preview… edit.content.image-editor.canvas.error.title=Could not load image 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 new file mode 100644 index 000000000000..e4a202d1356c --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java @@ -0,0 +1,117 @@ +package com.dotmarketing.portlets.contentlet.transform.strategy; + +import static com.dotmarketing.portlets.contentlet.transform.strategy.TransformOptions.BINARIES; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.dotcms.api.APIProvider; +import com.dotcms.contenttype.model.field.BinaryField; +import com.dotcms.contenttype.model.field.Field; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.storage.model.Metadata; +import com.dotmarketing.image.focalpoint.FocalPointAPI; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.junit.Test; +import org.mockito.Mockito; + +public class DefaultTransformStrategyTest { + + private static final String FIELD_VAR = "fileField"; + private static final String META_KEY = FIELD_VAR + "MetaData"; + + /** + * Custom metadata is persisted in the backing map under the {@link Metadata#CUSTOM_PROP_PREFIX} + * ("dot:") prefix; {@code Metadata.getCustomMeta()} strips that prefix when exposing it. + */ + private static final String CUSTOM_FOCAL_POINT_KEY = + Metadata.CUSTOM_PROP_PREFIX + FocalPointAPI.FOCAL_POINT; + + /** + * Invokes the private {@code addBinaries} method in isolation so the focal-point behavior can be + * exercised without standing up the rest of the transform pipeline. + */ + @SuppressWarnings("unchecked") + private void invokeAddBinaries(final DefaultTransformStrategy strategy, + final Contentlet contentlet, final Map map) throws Exception { + final Method addBinaries = DefaultTransformStrategy.class.getDeclaredMethod( + "addBinaries", Contentlet.class, Map.class, Set.class); + addBinaries.setAccessible(true); + addBinaries.invoke(strategy, contentlet, map, Set.of(BINARIES)); + } + + private Contentlet mockContentletWithBinary(final Metadata metadata) throws Exception { + final Field field = Mockito.mock(BinaryField.class); + Mockito.when(field.variable()).thenReturn(FIELD_VAR); + + 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(false); + Mockito.when(contentlet.getIdentifier()).thenReturn("identifier-1"); + Mockito.when(contentlet.getInode()).thenReturn("inode-1"); + Mockito.when(contentlet.getBinaryMetadata(FIELD_VAR)).thenReturn(metadata); + return contentlet; + } + + /** + * When the binary's custom metadata carries a focal point, it must be surfaced under + * {@code {field}MetaData.focalPoint} on the REST read path so the image editor can re-seed its + * marker. Regression coverage for #36067. + */ + @Test + public void testAddBinaries_whenCustomMetaHasFocalPoint_surfacesItInMetaDataMap() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + fieldsMeta.put(CUSTOM_FOCAL_POINT_KEY, "0.25,0.75"); + final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox); + final Contentlet contentlet = mockContentletWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = (Map) map.get(META_KEY); + assertEquals("Focal point from custom metadata must be exposed under the focalPoint key", + "0.25,0.75", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } + + /** + * When the binary has no focal point in its custom metadata, the read path must default the + * {@code focalPoint} entry to "0.0" rather than omit it, matching the GraphQL/Velocity view. + */ + @Test + public void testAddBinaries_whenCustomMetaHasNoFocalPoint_defaultsToZero() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox); + final Contentlet contentlet = mockContentletWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = (Map) map.get(META_KEY); + assertEquals("focalPoint must default to 0.0 when absent from custom metadata", + "0.0", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } +} diff --git a/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java b/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java index 4a102ce0782a..dc934b25b1b5 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java @@ -177,6 +177,44 @@ public void Test_Write_Focal_Point_From_Temp_Asset_Id() throws Exception { assertFalse(focalPointAPI.readFocalPoint(invalidAssetID, "fileAsset").isPresent()); } - - + + /** + * Method to test: the focal-point persistence wired into the image-tool save flow. On Save, + * {@link com.dotmarketing.servlets.BinaryExporterServlet} reads the focal point from the + * request params ({@link FocalPointAPI#parseFocalPointFromParams(Map)}) and writes it directly + * onto the generated temp file ({@link FocalPointAPI#writeFocalPoint(String, String, FocalPoint)}). + *

+ * Test scenario: parse a focal point from a request param map, write it onto a freshly created + * temp file, and read it back — mirroring exactly what the servlet does on Save. The direct + * write is what makes the focal survive even when the image exporter serves the rendition from + * cache and never runs the FocalPoint filter (the side effect that otherwise persists it). + *

+ * Expected: the focal point is persisted on the saved temp and reads back equal. + */ + @Test + public void Test_Persist_Focal_Point_On_Temp_From_Save_Params() throws Exception { + final String fieldVar = "fileAsset"; + final FocalPoint expected = new FocalPoint(0.2f, 0.3f); + + // 1. The servlet pulls the focal point out of the request params (the /fp/x,y URL segment). + final Map params = ImmutableMap.of("fp", new String[] {"0.2,0.3"}); + final Optional parsed = focalPointAPI.parseFocalPointFromParams(params); + assertTrue("Focal point parsed from save params", parsed.isPresent()); + + // 2. A temp file is generated for the edited image. + final HttpServletRequest request = mockHttpServletRequest(); + final DotTempFile dotTempFile = APILocator.getTempFileAPI().createEmptyTempFile("temp", request); + assertTrue(dotTempFile.file.createNewFile()); + assertTrue(dotTempFile.file.setLastModified(System.currentTimeMillis())); + + // 3. The servlet writes the focal point straight onto that temp (independent of the filter). + focalPointAPI.writeFocalPoint(dotTempFile.id, fieldVar, parsed.get()); + + // 4. Reopening the editor reads the focal back from the temp's metadata. + final Optional persisted = focalPointAPI.readFocalPoint(dotTempFile.id, fieldVar); + assertTrue("Focal point persisted on the saved temp", persisted.isPresent()); + assertEquals(expected, persisted.get()); + } + + }