Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,26 @@ <h2 class="dot-form-file-editor__title">{{ $header() }}</h2>
<ngx-monaco-editor
class="dot-form-file-editor__editor"
[class.dot-form-file-editor__editor--disabled]="form.disabled"
[class.dot-form-file-editor__editor--error]="$hasSyntaxError()"
[class.code-editor-disabled]="form.disabled"
[options]="store.monacoConfig()"
(init)="onEditorInit($event)"
data-testid="code-editor"
formControlName="content" />

@let file = store.file();
<div
class="dot-form-file-editor__mime-type"
[class.dot-form-file-editor__mime-type--hidden]="!file.mimeType">
<i class="pi pi-info-circle"></i>
<small>Mime Type: {{ file.mimeType }}</small>
</div>
@if ($hasSyntaxError()) {
<small class="p-invalid" data-testid="content-error-msg">
{{ 'dot.file.field.error.syntax' | dm }}
</small>
} @else {
@let file = store.file();
<div
class="dot-form-file-editor__mime-type"
[class.dot-form-file-editor__mime-type--hidden]="!file.mimeType">
<i class="pi pi-info-circle"></i>
<small>Mime Type: {{ file.mimeType }}</small>
</div>
}
</div>
</div>
<div class="dot-form-file-editor__actions">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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<typeof marker>[]) => {
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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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());
});
}

/**
Expand Down Expand Up @@ -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();

Expand All @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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=<strong>Something went wrong</strong>, 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
Expand Down
Loading