This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7787-ee06e4ba05f937f43041f7dc7faaca11d6759f9b in repository https://gitbox.apache.org/repos/asf/texera.git
commit 5f95b671d3dd73e327a15d9561c9b601394f64d5 Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Aug 19 22:58:03 2026 +0000 fix(frontend): show validation errors on the Hugging Face field and drop two redundant guards (#7787) ### What changes were proposed in this PR? Three template fixes. The first is a real user-facing bug; the other two are safe no-ops. **1. The Hugging Face model field never showed validation errors.** ```diff - *ngIf="props.showError && formControl.errors"> + *ngIf="showError && formControl.errors"> ``` `showError` is a getter on `@ngx-formly`'s `FieldType` (`templates/field.type.d.ts:25`), a sibling of `get props()` — **not** a `props` field. The only other `showError` declarations in formly are predicate *functions* (`FormlyFormOptions.showError`, `FormlyConfig.extras.showError`). So `props.showError` was permanently `undefined` and `<formly-validation-message>` could never render. It typechecked only because formly's props type carries an index signature, which is why it went unnoticed. The two working siblings in this codebase already use the bare getter: `common/formly/object.type.ts:23` and `multischema.type.ts:24`. **2. `console-frame.component.html`: removed a shadowed duplicate template pair.** `#checkedTemplate` was declared at lines 36 *and* 53, `#unCheckedTemplate` at 41 *and* 58, with both `nz-switch`es referencing the same two names. First declaration wins, so the second pair was unreachable — and both bodies were byte-identical. **3. `dataset-detail.component.html`: removed a redundant conjunct.** Line 585's `userHasWriteAccess() &&` is dominated by the enclosing `nz-collapse` (line 452), already gated on `userDatasetAccessLevel === "WRITE"` — exactly what that method returns (`dataset-detail.component.ts:560-562`). Both read the same field, so they cannot disagree even at runtime. ### The fix is pinned by a test that fails without it A `validation message` describe in `hugging-face.component.spec.ts` asserts both directions. With the template reverted to `props.showError`, that test fails (**1 failed | 86 passed**); with the fix, **87 pass**. The sharpest detail: on the unfixed template, `expect(component.showError).toBe(true)` **passes** on the line above while the DOM query returns `null`. The getter says to show the error and the template ignores it — which is precisely the bug, and precisely why no existing test caught it. ### One fixture change was required, and it is fixture infidelity rather than fallout Applying the fix initially failed 78 tests with `TypeError: this.options.showError is not a function`, because the spec's field literals supplied `options: { detectChanges: vi.fn() }` only. In production `FormlyForm` always fills in `options.showError` and `options.fieldChanges`. Those fixtures were therefore modelling a state formly never produces. Added one `buildFormlyOptions()` helper and used it at the six existing `options:` literals; no test asserted on `options.detectChanges`, so behaviour is unchanged. ### Items 2 and 3 are verified as behaviour-preserving For **console-frame**, rather than assume the duplicate was inert, the existing settings-dropdown test was temporarily instrumented to dump `innerHTML` of *both* switches in both on and off states, run against the original and the fixed template, and diffed. The only delta is Angular's generated style-scope id (`_ngcontent-a-c3896271133` → `_ngcontent-a-c2948443161`), a template-content hash stamped consistently on DOM and CSS. Normalising it, the rendered on/off content is identical. The instrumentation was reverted and is not in this diff. For **dataset-detail**, the suite passes 173/173 **both with and without** the change — no test outcome moves, which is what makes it a safe cleanup rather than a behaviour change. The one case that looked risky (`offers the creator only once there is something to commit`) uses the merging render helper, so `userDatasetAccessLevel` stays `"WRITE"` across its second render. ### Verification - `hugging-face` + `console-frame` + `dataset-detail` in one run: **288 passed (288)**. - `operator-property-edit-frame` + `formly-config` as regression cover for the formly options path: **207 passed | 1 skipped**. - `npx ng build` exits 0 with `Browser application bundle generation complete` and zero `[ERROR]` lines — worth running here because `ng test` and `tsc --noEmit` both miss Angular template diagnostics. - `yarn format:ci` exits 0. ### Any related issues, documentation, discussions? Closes #7786 ### How was this PR tested? ``` npx ng test --watch=false --include="**/hugging-face.component.spec.ts" --include="**/console-frame.component.spec.ts" --include="**/dataset-detail.component.spec.ts" ``` ``` Test Files 3 passed (3) Tests 288 passed (288) ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../dataset-detail.component.html | 2 +- .../hugging-face/hugging-face.component.html | 2 +- .../hugging-face/hugging-face.component.spec.ts | 57 +++++++++++++++++++--- .../console-frame/console-frame.component.html | 10 ---- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html index 33a6182dc5..d0595642aa 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.html @@ -582,7 +582,7 @@ </texera-dataset-staged-objects-list> <div - *ngIf="userHasWriteAccess() && userHasPendingChanges" + *ngIf="userHasPendingChanges" class="version-creator"> <div class="version-input-container"> <label>Version:</label> diff --git a/frontend/src/app/workspace/component/hugging-face/hugging-face.component.html b/frontend/src/app/workspace/component/hugging-face/hugging-face.component.html index 777111cc3e..bcaaab3003 100644 --- a/frontend/src/app/workspace/component/hugging-face/hugging-face.component.html +++ b/frontend/src/app/workspace/component/hugging-face/hugging-face.component.html @@ -198,6 +198,6 @@ <div class="alert alert-danger" role="alert" - *ngIf="props.showError && formControl.errors"> + *ngIf="showError && formControl.errors"> <formly-validation-message [field]="field"></formly-validation-message> </div> diff --git a/frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts b/frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts index 201361767d..c3a7935cf9 100644 --- a/frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts +++ b/frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts @@ -22,6 +22,7 @@ import { HttpClientTestingModule, HttpTestingController } from "@angular/common/ import { By } from "@angular/platform-browser"; import { FormControl, FormGroup } from "@angular/forms"; import { FieldTypeConfig } from "@ngx-formly/core"; +import { Subject } from "rxjs"; import { AppSettings } from "../../../common/app-setting"; import { HuggingFaceComponent, @@ -49,6 +50,15 @@ function buildTaskResponse(): HuggingFaceTaskOption[] { ]; } +/** + * Build the `options` that Formly attaches to every field at runtime: `FormlyForm` + * always fills in `showError` (from `FormlyConfig.extras.showError`) and `fieldChanges`, + * which `FieldType.showError` and `formly-validation-message` respectively rely on. + */ +function buildFormlyOptions(showError = false) { + return { detectChanges: vi.fn(), showError: () => showError, fieldChanges: new Subject() }; +} + /** * Build a minimal FormlyFieldConfig with a FormGroup backing it, * similar to what Formly provides at runtime. @@ -82,7 +92,7 @@ function buildFieldWithFormGroup(taskValue = "", modelIdValue = ""): { field: Fi model, props: {}, parent: { fieldGroup: [] }, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; return { field, formGroup }; @@ -1239,7 +1249,7 @@ describe("HuggingFaceComponent (TestBed)", () => { model: { task: "" }, props: {}, parent: { fieldGroup: [] }, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; component.field = field; @@ -1268,7 +1278,7 @@ describe("HuggingFaceComponent (TestBed)", () => { model: { task: "" }, props: {}, parent: { fieldGroup: [] }, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; component.field = field; @@ -1318,7 +1328,7 @@ describe("HuggingFaceComponent (TestBed)", () => { model, props: {}, parent: { fieldGroup: [] }, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; component.field = field; @@ -1353,7 +1363,7 @@ describe("HuggingFaceComponent (TestBed)", () => { model: null, props: {}, parent: { fieldGroup: [] }, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; component.field = field; @@ -1559,7 +1569,7 @@ describe("HuggingFaceComponent (TestBed)", () => { form: formGroup, model: { modelId: "" } as Record<string, unknown>, props: {}, - options: { detectChanges: vi.fn() }, + options: buildFormlyOptions(), } as unknown as FieldTypeConfig; // no `parent` -> the `field.parent ?? field` fallback component.field = field; fixture.detectChanges(); @@ -1573,4 +1583,39 @@ describe("HuggingFaceComponent (TestBed)", () => { expect(component.selectedTaskTag).toBe("translation"); }); }); + + // ── Validation message ── + + describe("validation message", () => { + /** Initialize the component with the `showError` predicate that Formly delegates to. */ + function initWithShowError(showError: boolean) { + const { field } = buildFieldWithFormGroup(); + Object.assign(field.options!, { showError: () => showError }); + component.field = field; + + fixture.detectChanges(); + flushIconRequests(); + http.expectOne(`${API}/huggingface/tasks`).flush(buildTaskResponse()); + http.expectOne(req => req.url.startsWith(`${API}/huggingface/models`)).flush([]); + + // Set the error after init: ngOnInit re-runs updateValueAndValidity, which would clear it. + component.formControl.setErrors({ required: true }); + fixture.detectChanges(); + flushIconRequests(); + } + + it("renders the validation message when the field is in an error state", () => { + initWithShowError(true); + + expect(component.showError).toBe(true); + expect(fixture.debugElement.query(By.css("div.alert-danger formly-validation-message"))).toBeTruthy(); + }); + + it("does not render the validation message when showError is false", () => { + initWithShowError(false); + + expect(component.showError).toBe(false); + expect(fixture.debugElement.query(By.css("div.alert-danger"))).toBeNull(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.html b/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.html index 1c488090c8..74f8f9dc34 100644 --- a/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.html +++ b/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.html @@ -50,16 +50,6 @@ [nzCheckedChildren]="checkedTemplate" [nzUnCheckedChildren]="unCheckedTemplate"></nz-switch> Show Source - <ng-template #checkedTemplate - ><span - nz-icon - nzType="check"></span - ></ng-template> - <ng-template #unCheckedTemplate - ><span - nz-icon - nzType="close"></span - ></ng-template> </li> </ul> </nz-dropdown-menu>
