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-7436-e03d97108ddd47befc78a87f467f781dd50a56e6 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 16463d53d587d3cdb5efc08671a4d34ef8ba212c Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Aug 9 07:29:49 2026 -0700 test(frontend): render the drag-and-drop repeat section's rows (#7436) ### What changes were proposed in this PR? The existing suite drives `onDrop` directly and never renders, so everything the template owns was unpinned. Adds 8 tests. The remove index is the one that matters: it comes from the `ngFor` loop variable, and a fixed or off-by-one index deletes someone else's row while every row looks identical on screen. Also covered: one row per entry, the drag handle, the add button's wiring, its label falling back to `"Add"`, and the add button locking for a disabled section. **Verified by mutation**, all reverted (template diff empty): | Mutation | Result | |---|---| | remove uses a fixed index | red | | add button unwired | red | | add label ignores the field's own | red | | add label loses its default | red | | add button never disabled | red | | only the first row rendered | red | | drag handle removed | red | The drag-handle test **survived its first mutation**: it asserted the `.drag-handle` class, which is styling and survives `cdkDragHandle` being dropped — leaving the row undraggable with the test still green. It now asserts the directive. ### A production bug this surfaced The per-row remove button's `[disabled]` guard never takes effect. The rows are `*ngFor="let field of field.fieldGroup"`, which **shadows** the component's `field`, so inside a row `field.templateOptions?.disabled` reads the sub-field's options and is always `undefined`. The add button, outside the loop, reads the same expression correctly and does disable. Filed as #7431. This PR deliberately asserts the add button's gating and **not** the remove buttons', so the current behaviour is not cemented before the fix. No production file is touched. ### Any related issues, documentation, discussions? Closes #7433 ### How was this PR tested? ``` npx ng test --watch=false --include="**/repeat-dnd.component.spec.ts" ``` ``` Test Files 1 passed (1) Tests 12 passed (12) ``` 8 new on top of the existing 4. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../formly/repeat-dnd/repeat-dnd.component.spec.ts | 94 +++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/common/formly/repeat-dnd/repeat-dnd.component.spec.ts b/frontend/src/app/common/formly/repeat-dnd/repeat-dnd.component.spec.ts index 86d74a1a96..4589fa76ef 100644 --- a/frontend/src/app/common/formly/repeat-dnd/repeat-dnd.component.spec.ts +++ b/frontend/src/app/common/formly/repeat-dnd/repeat-dnd.component.spec.ts @@ -17,7 +17,8 @@ * under the License. */ -import { CdkDragDrop } from "@angular/cdk/drag-drop"; +import { CdkDragDrop, CdkDragHandle } from "@angular/cdk/drag-drop"; +import { By } from "@angular/platform-browser"; import { FormArray, FormControl } from "@angular/forms"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { FormlyRepeatDndComponent } from "./repeat-dnd.component"; @@ -92,4 +93,95 @@ describe("FormlyRepeatDndComponent", () => { expect((component.formControl as FormArray).controls.map(control => control.value)).toEqual(["b", "c", "a"]); expect(reorder).toHaveBeenCalledOnce(); }); + /** + * The class-level tests above drive onDrop directly and never render. The template owns the rest + * of the control: one row per entry, which index a row's remove button carries, and whether the + * section is editable at all. + */ + describe("rendered rows", () => { + /** Renders the repeat section with the given template options. */ + function render(templateOptions: Record<string, unknown> = {}): HTMLElement { + setComponentState(); + component.field = { + ...component.field, + fieldGroup: [{ key: "a" }, { key: "b" }, { key: "c" }], + templateOptions, + } as any; + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + function removeButtons(): HTMLButtonElement[] { + return Array.from( + (fixture.nativeElement as HTMLElement).querySelectorAll<HTMLButtonElement>(".dnd-remove-button") + ); + } + + function addButton(): HTMLButtonElement { + // The add button is the only one outside a row; it carries no class of its own. + return Array.from((fixture.nativeElement as HTMLElement).querySelectorAll<HTMLButtonElement>("button")).find( + b => !b.closest(".dnd-row") + )!; + } + + it("renders one row per entry", () => { + const el = render(); + + expect(el.querySelectorAll(".dnd-row").length).toBe(3); + }); + + it("gives each row a drag handle", () => { + // Asserted on the cdkDragHandle directive, not the .drag-handle class: the class is styling + // and survives the directive being dropped, which would leave the row undraggable. + render(); + + expect(fixture.debugElement.queryAll(By.directive(CdkDragHandle)).length).toBe(3); + }); + + it("removes the row whose button was pressed", () => { + // The index comes from the ngFor loop variable; a fixed or off-by-one index would delete + // someone else's row, and every row looks the same on screen. + const spy = vi.spyOn(component, "remove").mockImplementation(() => {}); + render(); + + removeButtons()[1].click(); + + expect(spy).toHaveBeenCalledWith(1); + }); + + it("appends a row from the add button", () => { + const spy = vi.spyOn(component, "add").mockImplementation(() => {}); + render(); + + addButton().click(); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("labels the add button Add when the field does not name it", () => { + render(); + + expect(addButton().textContent?.trim()).toBe("Add"); + }); + + it("uses the field's own label for the add button when it has one", () => { + render({ addText: "Add a column" }); + + expect(addButton().textContent?.trim()).toBe("Add a column"); + }); + + it("locks the add button for a disabled section", () => { + // A read-only operator property must not offer edits it cannot persist. nz-button reflects + // the state as an attribute rather than the DOM property, so it is read that way. + render({ disabled: true }); + + expect(addButton().getAttribute("disabled")).not.toBeNull(); + }); + + it("leaves the add button available otherwise", () => { + render({ disabled: false }); + + expect(addButton().getAttribute("disabled")).toBeNull(); + }); + }); });
