Skip to content

chore(frontend): remove spec-only PresetService members #8614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aglinxinyuan wants to merge 1 commit into
base: main
Choose a base branch
from
+0 −175
Open

chore(frontend): remove spec-only PresetService members #8614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
109 changes: 0 additions & 109 deletions frontend/src/app/workspace/service/preset/preset.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
66 changes: 0 additions & 66 deletions frontend/src/app/workspace/service/preset/preset.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ const PresetArraySchema: CustomJSONSchema7 = {

export type Preset = { [key: string]: string | number | boolean };

export type PresetDictionary = {
[Key: string]: Preset[];
};
@Injectable({
providedIn: "root",
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
Expand Down
Loading