From 36ee8495d53b93f78e1ac67fd3b9cdb0b1a49236 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Mon, 13 Jul 2026 14:01:49 -0400 Subject: [PATCH] fix(edit-content): allow saving code files with warnings in file editor (#36543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file/code editor blocked Save whenever the Monaco model had any marker. ngx-monaco-editor registers as an NG_VALIDATORS and reports ANY marker (including informational TS diagnostics like "declared but never read") as a `monaco` form error, silently invalidating the form — the template only ever surfaced errors for the `name` field, so Save appeared to do nothing. Now only genuine `Error`-severity markers block saving; hints/warnings pass through. When a real syntax error is present the editor gets a red outline (PrimeNG invalid token) and a message below it, replacing the mime-type hint. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../dot-form-file-editor.component.html | 21 ++- .../dot-form-file-editor.component.scss | 8 ++ .../dot-form-file-editor.component.spec.ts | 130 +++++++++++++++++- .../dot-form-file-editor.component.ts | 57 +++++++- .../WEB-INF/messages/Language.properties | 1 + 5 files changed, 207 insertions(+), 10 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.html b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.html index c879d12d4734..593b5fe92ec1 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.html +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.html @@ -74,19 +74,26 @@

{{ $header() }}

- @let file = store.file(); -
- - Mime Type: {{ file.mimeType }} -
+ @if ($hasSyntaxError()) { + + {{ 'dot.file.field.error.syntax' | dm }} + + } @else { + @let file = store.file(); +
+ + Mime Type: {{ file.mimeType }} +
+ }
diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.scss b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.scss index 7c46b9f8a955..84ba18035c9c 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.scss +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.scss @@ -92,6 +92,14 @@ opacity: 0.5; } +// Red outline shown when the content has real (Error-severity) syntax errors that +// block saving. Uses PrimeNG's invalid-field border token so it matches `p-invalid` +// and adapts to dark mode; the hex fallback (Lara `red.400`) is what renders in the +// legacy Dojo bundle, where the theme CSS variables aren't injected. +.dot-form-file-editor__editor--error { + border-color: var(--p-form-field-invalid-border-color, #f87171); +} + .dot-form-file-editor__mime-type { display: flex; align-items: center; diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.spec.ts index 5f203f73c88c..74455477e95d 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.spec.ts @@ -16,10 +16,13 @@ import { DotFileFieldUploadService } from '../../services/upload-file/upload-fil // monacoMock doesn't expose `getLanguages`, which getInfoByLang / the velocity // registration call. Provide a no-op so the component's Monaco hooks don't throw. +// It also doesn't expose `MarkerSeverity`, which #hasErrorSeverityMarker relies on to +// tell a real syntax error apart from an informational hint/warning marker. // eslint-disable-next-line @typescript-eslint/no-explicit-any (global as any).monaco = { ...monacoMock, - languages: { ...monacoMock.languages, getLanguages: () => [] } + languages: { ...monacoMock.languages, getLanguages: () => [] }, + MarkerSeverity: { Hint: 1, Info: 2, Warning: 4, Error: 8 } }; describe('DotFormFileEditorComponent', () => { @@ -115,4 +118,129 @@ describe('DotFormFileEditorComponent', () => { expect(spectator.component.$header()).toBe('Edit File'); }); }); + + describe('Content validation (Monaco markers)', () => { + // Builds a single Monaco marker of the given severity. Only `severity`/`message` + // matter to the gate; the position fields just satisfy the marker shape. + const marker = (severity: number, message = 'diagnostic') => ({ + severity, + message, + startLineNumber: 1, + startColumn: 1, + endLineNumber: 1, + endColumn: 1, + owner: 'javascript', + resource: null + }); + + // Drives the editor to a given set of markers AND mirrors what ngx-monaco-editor's + // own Validator would then set on the control (any marker -> a single `monaco` error). + // The real TS language service that produces these markers can't run in jsdom, so we + // inject them; this exercises our gate, not Monaco's marker generation. + const setMarkers = (markers: ReturnType[]) => { + jest.spyOn(monaco.editor, 'getModelMarkers').mockReturnValue(markers); + spectator.component.contentField.setErrors( + markers.length ? { monaco: { value: markers.map((m) => m.message) } } : null + ); + }; + + const spyUpload = () => + jest.spyOn(spectator.component.store, 'uploadFile').mockImplementation(() => undefined); + + beforeEach(() => { + spectator.component.ngOnInit(); + spectator.component.form.controls.name.setValue('script.js'); + + const editor = monaco.editor.create(); + spectator.component.onEditorInit( + editor as unknown as monaco.editor.IStandaloneCodeEditor + ); + }); + + it('should save when the content has no markers and the name is valid', () => { + setMarkers([]); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).toHaveBeenCalled(); + expect(spectator.component.$hasSyntaxError()).toBe(false); + }); + + it('should save when the content only has Hint-severity markers (e.g. unused-variable diagnostics)', () => { + setMarkers( + [monaco.MarkerSeverity.Hint, monaco.MarkerSeverity.Hint].map((s) => + marker(s, "'foo' is declared but its value is never read.") + ) + ); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).toHaveBeenCalled(); + expect(spectator.component.$hasSyntaxError()).toBe(false); + }); + + it('should save when the content only has Warning-severity markers', () => { + setMarkers([marker(monaco.MarkerSeverity.Warning, 'Unreachable code detected.')]); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).toHaveBeenCalled(); + expect(spectator.component.$hasSyntaxError()).toBe(false); + }); + + it('should block saving and flag $hasSyntaxError when the content has an Error-severity marker', () => { + setMarkers([marker(monaco.MarkerSeverity.Error, "'}' expected.")]); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).not.toHaveBeenCalled(); + expect(spectator.component.$hasSyntaxError()).toBe(true); + }); + + it('should block saving when markers mix hints/warnings with at least one Error', () => { + setMarkers([ + marker( + monaco.MarkerSeverity.Hint, + "'foo' is declared but its value is never read." + ), + marker(monaco.MarkerSeverity.Warning, 'Unreachable code detected.'), + marker(monaco.MarkerSeverity.Error, "'}' expected.") + ]); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).not.toHaveBeenCalled(); + expect(spectator.component.$hasSyntaxError()).toBe(true); + }); + + it('should clear $hasSyntaxError and allow saving once the error markers are resolved', () => { + // Start with a real syntax error. + setMarkers([marker(monaco.MarkerSeverity.Error, "'}' expected.")]); + expect(spectator.component.$hasSyntaxError()).toBe(true); + + // User fixes it: markers clear and ngx-monaco-editor drops the `monaco` error. + setMarkers([]); + expect(spectator.component.$hasSyntaxError()).toBe(false); + + const uploadFileSpy = spyUpload(); + spectator.component.onSubmit(); + + expect(uploadFileSpy).toHaveBeenCalled(); + }); + + it('should still block saving when the name field is invalid, regardless of content markers', () => { + spectator.component.form.controls.name.setValue('nodotextension'); + setMarkers([]); + const uploadFileSpy = spyUpload(); + + spectator.component.onSubmit(); + + expect(uploadFileSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.ts index bba9d4115df1..46e8fa54efe2 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-form-file-editor/dot-form-file-editor.component.ts @@ -116,6 +116,13 @@ export class DotFormFileEditorComponent implements OnInit { this.$isFullscreen() ? 'close_fullscreen' : 'open_in_full' ); + /** + * Whether the current Monaco model has at least one `Error`-severity marker. Drives the + * `content` field's error slot in the template — see {@link #hasBlockingErrors} for why only + * `Error` severity (not hints/warnings) counts. + */ + readonly $hasSyntaxError = signal(false); + /** * Form group for the file editor component. * @@ -174,6 +181,13 @@ export class DotFormFileEditorComponent implements OnInit { .subscribe((value) => { this.store.setFileName(value); }); + + // ngx-monaco-editor recomputes `content`'s validity (and re-emits statusChanges) + // every time the Monaco model's markers change, so this is the reliable trigger to + // refresh whether the template should show the syntax-error message. + this.contentField.statusChanges.pipe(takeUntilDestroyed()).subscribe(() => { + this.$hasSyntaxError.set(this.#hasErrorSeverityMarker()); + }); } /** @@ -227,13 +241,14 @@ export class DotFormFileEditorComponent implements OnInit { * Handles the form submission event. * * This method performs the following actions: - * 1. Checks if the form is invalid. If so, marks the form as dirty and updates its validity status. + * 1. Checks if the form is invalid, ignoring the `content` field's `monaco` error (see + * {@link #hasBlockingErrors}). If so, marks the form as dirty and updates its validity status. * 2. If the form is valid, retrieves the raw values from the form and triggers the file upload process via the store. * * @returns {void} */ onSubmit(): void { - if (this.form.invalid) { + if (this.#hasBlockingErrors()) { this.form.markAsDirty(); this.form.updateValueAndValidity(); @@ -244,6 +259,44 @@ export class DotFormFileEditorComponent implements OnInit { this.store.uploadFile(values); } + /** + * Whether the form has validation errors that should actually block saving. + * + * `ngx-monaco-editor` registers itself as an `NG_VALIDATORS` for the `content` control + * and surfaces ANY marker on the Monaco model — syntax/semantic errors, but also purely + * informational TS diagnostics like "declared but never read" — as a single `monaco` form + * error, with no severity info attached. Those low-severity hints are shown to the user as + * underlines in the editor itself, but shouldn't block Save. Only genuine `Error`-severity + * markers (red squiggly) block it, so we read the real markers on the model directly rather + * than trusting the presence of the `monaco` form error. Any other error on `content`, or an + * invalid `name`, always blocks submission. + */ + #hasBlockingErrors(): boolean { + if (this.nameField.invalid) { + return true; + } + + const contentErrors = this.contentField.errors ?? {}; + const hasNonMonacoContentErrors = Object.keys(contentErrors).some( + (key) => key !== 'monaco' + ); + + return hasNonMonacoContentErrors || this.#hasErrorSeverityMarker(); + } + + /** Whether the current Monaco model has at least one `Error`-severity marker. */ + #hasErrorSeverityMarker(): boolean { + const model = this.#editorRef?.getModel(); + + if (!model || typeof monaco === 'undefined') { + return false; + } + + return monaco.editor + .getModelMarkers({ resource: model.uri }) + .some((marker) => marker.severity === monaco.MarkerSeverity.Error); + } + /** * Getter for the 'name' field control from the form. * diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index 1ce3c26116fc..7bd0cd70d39a 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -1254,6 +1254,7 @@ dot.file.field.drag.and.drop.error.file.maxsize.exceeded.message=The file weight dot.file.field.drag.and.drop.error.server.error.message=Something went wrong, please try again or contact our support team. dot.file.field.error.type.file.not.supported.message=This type of file is not supported. Please use a {0} file. dot.file.field.error.type.file.not.extension=Please add the file's extension +dot.file.field.error.syntax=This file has syntax errors. Click the red markers on the right to jump to them. dot.file.field.file.size=File Size dot.file.field.file.dimension=Dimension dot.file.field.file.bytes=Bytes