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-8614-09220c21fad973b1b9b53b77b91a8ecaa4a76c5e in repository https://gitbox.apache.org/repos/asf/texera.git
commit 1f693675f99c968d9a46d7b009e96475a4ce810b Author: Xinyuan Lin <[email protected]> AuthorDate: Thu Sep 24 03:43:55 2026 +0000 chore(frontend): remove spec-only PresetService members (#8614) ### What changes were proposed in this PR? Removes three `PresetService` members that no production code calls: `updateOrCreatePreset`, `isValidNewOperatorPreset`, and the exported `PresetDictionary` type. Pure deletion, no behaviour change: **−175 lines**. ### History | | | | --- | --- | | **Introduced by** | #1164 (2022-01-13) — "Add PresetService (User Presets Step 3)", which built out the preset API surface | | **Usage removed by** | **never wired up** — no production file has ever called `updateOrCreatePreset` or `isValidNewOperatorPreset`, and `PresetDictionary` has zero references worktree-wide. Only the service's own spec exercises them | > Reviewer note: the wired-up half of the service stays — `savePresets`, `getPresets`, `applyPreset`, `isValidOperatorPreset`, `isValidPreset` and the Ajv type guards all keep live consumers. `common/formly/preset-wrapper/**` calls none of the removed three (checked line by line in both the `.ts` and the `.html`). Note `isValidOperatorPreset` (live) and `isValidNewOperatorPreset` (removed) differ by one word — only the latter goes. ### Any related issues, documentation, discussions? Closes #8611 ### How was this PR tested? Existing tests only — this PR removes methods and the spec blocks that covered them. From `frontend/`: - `npx ng test --watch=false --include='**/preset.service.spec.ts'` — 48 tests pass. - `yarn --cwd frontend format:ci` — clean. Verification, re-runnable by a reviewer: ``` git grep -n "updateOrCreatePreset\|isValidNewOperatorPreset\|PresetDictionary" git grep -n isValidOperatorPreset # the live sibling, untouched ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../service/preset/preset.service.spec.ts | 109 --------------------- .../app/workspace/service/preset/preset.service.ts | 66 ------------- 2 files changed, 175 deletions(-) diff --git a/frontend/src/app/workspace/service/preset/preset.service.spec.ts b/frontend/src/app/workspace/service/preset/preset.service.spec.ts index f3b4807b64..69298f5341 100644 --- a/frontend/src/app/workspace/service/preset/preset.service.spec.ts +++ b/frontend/src/app/workspace/service/preset/preset.service.spec.ts @@ -380,33 +380,6 @@ describe("PresetService", () => { presetService.isValidOperatorPreset({ presetProperty: "applied" }, mockPresetEnabledPredicate.operatorID) ).toBe(true); }); - - it("isValidNewOperatorPreset returns false when the preset already exists", () => { - const existing: Preset = { presetProperty: "applied" }; - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([existing]))); - - let result: boolean | undefined; - presetService - .isValidNewOperatorPreset(existing, mockPresetEnabledPredicate.operatorID) - .subscribe(v => (result = v)); - expect(result).toBe(false); - }); - - it("isValidNewOperatorPreset returns true when the preset is novel", () => { - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "applied" }]))); - - let result: boolean | undefined; - presetService - .isValidNewOperatorPreset({ presetProperty: "novel" }, mockPresetEnabledPredicate.operatorID) - .subscribe(v => (result = v)); - expect(result).toBe(true); - }); - - it("isValidNewOperatorPreset short-circuits to false when the preset itself is invalid", () => { - let result: boolean | undefined; - presetService.isValidNewOperatorPreset({}, mockPresetEnabledPredicate.operatorID).subscribe(v => (result = v)); - expect(result).toBe(false); - }); }); describe("static schema helpers", () => { @@ -521,88 +494,6 @@ describe("PresetService", () => { }); }); - describe("updateOrCreatePreset", () => { - // fetchKey is backed by a synchronous `of(...)`, so the subscribe body (and - // the savePresets write-through it triggers) runs before the call returns. - it("writes the stored preset list back unchanged when the original and replacement presets are identical", () => { - const stored: Preset[] = [{ presetProperty: "v1" }]; - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify(stored))); - - presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "x" }, { presetProperty: "x" }); - - // list is written back unchanged: neither pushed, replaced, nor spliced. - expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify(stored)); - }); - - it("stores the replacement when the dictionary has no entry yet", () => { - // First write for this operator type: fetchKey resolves to null, so the - // missing entry has to read as an empty list rather than being parsed. - userConfigStub.fetchKey.mockReturnValue(of(null)); - - presetService.updateOrCreatePreset( - presetType, - presetTarget, - { presetProperty: "missing" }, - { presetProperty: "v2" } - ); - - expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify([{ presetProperty: "v2" }])); - }); - - it("appends the replacement when neither preset already exists", () => { - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }]))); - - presetService.updateOrCreatePreset( - presetType, - presetTarget, - { presetProperty: "missing" }, - { presetProperty: "v2" } - ); - - expect(userConfigStub.set).toHaveBeenCalledWith( - presetDictKey, - JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]) - ); - }); - - it("writes the stored preset list back unchanged when only the replacement preset already exists", () => { - const stored: Preset[] = [{ presetProperty: "v1" }, { presetProperty: "v2" }]; - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify(stored))); - - presetService.updateOrCreatePreset( - presetType, - presetTarget, - { presetProperty: "missing" }, - { presetProperty: "v2" } - ); - - expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify(stored)); - }); - - it("implicitly deletes a preset when both the original and the replacement exist", () => { - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]))); - - // Both presets are present (membership is checked deeply via isEqual), so the - // implicit-delete branch removes the original (v1) and leaves the replacement (v2). - presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "v1" }, { presetProperty: "v2" }); - - expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify([{ presetProperty: "v2" }])); - }); - - it("replaces the original preset in place when only the original exists", () => { - userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]))); - - // The original exists (deep match) but the replacement does not, so the replace - // branch swaps the original (v1) for the replacement (v3) at its index. - presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "v1" }, { presetProperty: "v3" }); - - expect(userConfigStub.set).toHaveBeenCalledWith( - presetDictKey, - JSON.stringify([{ presetProperty: "v3" }, { presetProperty: "v2" }]) - ); - }); - }); - describe("preset Ajv type guards", () => { describe("isValidPreset", () => { it("accepts an object whose values are all non-blank strings", () => { diff --git a/frontend/src/app/workspace/service/preset/preset.service.ts b/frontend/src/app/workspace/service/preset/preset.service.ts index a4de6d63a1..445cacd016 100644 --- a/frontend/src/app/workspace/service/preset/preset.service.ts +++ b/frontend/src/app/workspace/service/preset/preset.service.ts @@ -65,9 +65,6 @@ const PresetArraySchema: CustomJSONSchema7 = { export type Preset = { [key: string]: string | number | boolean }; -export type PresetDictionary = { - [Key: string]: Preset[]; -}; @Injectable({ providedIn: "root", }) @@ -158,47 +155,6 @@ export class PresetService { }); } - /** - * broadcast savePresets event and also save preset to presetDict, which is a *view* (in the database sense) of DictionaryService's dictionary that only stores presets - * @param type string, usually "operator" - * @param target string, usualy operatorType - * @param presets Preset[] - * @param displayMessage message to display when saving presets - * @param messageType see AlertMessageType, determines icon used in popup message - */ - public updateOrCreatePreset( - type: string, - target: string, - originalPreset: Preset, - replacementPreset: Preset, - displayMessage?: string | null, - messageType: AlertMessageType = "success" - ) { - this.userConfigService - .fetchKey(`${type}-${target}`) - .pipe(first()) - .subscribe(oldpresets => { - let presets = JSON.parse(oldpresets ?? "[]") as Preset[]; - if (isEqual(originalPreset, replacementPreset)) { - // no modification: no update required - } else if (!contains(presets, originalPreset) && !contains(presets, replacementPreset)) { - presets.push(replacementPreset); - } else if (!contains(presets, originalPreset) && contains(presets, replacementPreset)) { - // no modification: old preset doesn't exist to be updated, new preset already exists - } else if (contains(presets, originalPreset) && contains(presets, replacementPreset)) { - // implicit deletion by replacing original with existing preset - // deep-equality index: presets are freshly JSON-parsed, so reference-based indexOf would miss - presets.splice( - presets.findIndex(preset => isEqual(preset, originalPreset)), - 1 - ); - } else { - presets[presets.findIndex(preset => isEqual(preset, originalPreset))] = replacementPreset; - } - this.savePresets(type, target, presets, displayMessage, messageType); - }); - } - /** * broadcast savePresets event and also save preset to presetDict, which is a *view* (in the database sense) of DictionaryService's dictionary that only stores presets * removes preset if it exists @@ -262,28 +218,6 @@ export class PresetService { return fitsSchema && noEmptyProperties; } - /** - * extracts preset schema from operator schema and validates a preset with it. - * also checks if preset exists in presetDict already. - * @param preset - * @param operatorID - * @returns boolean - */ - public isValidNewOperatorPreset(preset: Preset, operatorID: string): Observable<boolean> { - if (!this.isValidOperatorPreset(preset, operatorID)) return of(false); - - return this.getPresets( - "operator", - this.workflowActionService.getTexeraGraph().getOperator(operatorID).operatorType - ).pipe( - first(), - map(presets => { - console.log(!presets.some(existingPreset => isEqual(preset, existingPreset)), "vn"); - return !presets.some(existingPreset => isEqual(preset, existingPreset)); - }) - ); - } - public isValidPreset(preset: any): preset is Preset { return asType(PresetService.isPreset(preset), "boolean"); }
