Skip to content

Commit 7c9c71a

Browse files
authored
Updates correct python component (#1424)
## Description Implemented component name preservation in the Python component editor. When editing a component using the Python editor, the original component name is now preserved even when the YAML is regenerated from the Python code. This prevents unintended name changes during component editing. ## Type of Change - [x] Bug fix - [x] Improvement ## Checklist - [x] I have tested this does not break current pipelines / runs functionality - [x] I have tested the changes on staging ## Test Instructions 1. Create a new python component (use default `Filter text` for now) 2. Edit the component 3. Confirm you are able to edit PYTHON in the editor 4. Save the component and import as new (new name `Filter text two` or something) 5. Edit the new component, Filter text two, and replace the existing component 6. Confirm the component being replaced is `Filter text two` and not `Filter text` 7. Do the same steps but for JS (or something else) and confirm you can replace or import as new
1 parent adce36c commit 7c9c71a

File tree

8 files changed

+109
-17
lines changed

8 files changed

+109
-17
lines changed

src/components/shared/ComponentEditor/ComponentEditorDialog.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ type PythonCodeDetection =
6363
isPython: true;
6464
pythonOriginalCode: string;
6565
options: YamlGeneratorOptions;
66+
componentName?: string;
6667
}
6768
| { isPython: false };
6869

@@ -108,6 +109,7 @@ export const ComponentEditorDialog = withSuspenseWrapper(
108109
return {
109110
isPython: true,
110111
pythonOriginalCode,
112+
componentName: hydratedComponent.name,
111113
options: {
112114
baseImage:
113115
hydratedComponent.spec.implementation.container.image ??
@@ -219,6 +221,7 @@ export const ComponentEditorDialog = withSuspenseWrapper(
219221
options={pythonCodeDetection.options}
220222
onComponentTextChange={handleComponentTextChange}
221223
onErrorsChange={setErrors}
224+
preserveComponentName={pythonCodeDetection.componentName}
222225
/>
223226
) : (
224227
<YamlComponentEditor

src/components/shared/ComponentEditor/components/PythonComponentEditor.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import MonacoEditor from "@monaco-editor/react";
2-
import { useCallback, useEffect, useState } from "react";
2+
import { useCallback, useEffect, useRef, useState } from "react";
33

44
import { withSuspenseWrapper } from "@/components/shared/SuspenseWrapper";
55
import { BlockStack, InlineStack } from "@/components/ui/layout";
@@ -9,6 +9,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
99
import { DEFAULT_MONACO_OPTIONS } from "../constants";
1010
import { usePythonYamlGenerator } from "../generators/python";
1111
import type { YamlGeneratorOptions } from "../types";
12+
import { preserveComponentName } from "../utils/preserveComponentName";
1213
import { ComponentSpecErrorsList } from "./ComponentSpecErrorsList";
1314
import { PreviewTaskNodeCard } from "./PreviewTaskNodeCard";
1415
import { TogglePreview } from "./TogglePreview";
@@ -48,25 +49,36 @@ export const PythonComponentEditor = withSuspenseWrapper(
4849
options,
4950
onComponentTextChange,
5051
onErrorsChange,
52+
preserveComponentName: initialComponentName,
5153
}: {
5254
text: string;
5355
options: YamlGeneratorOptions;
5456
onComponentTextChange: (yaml: string) => void;
5557
onErrorsChange: (errors: string[]) => void;
58+
preserveComponentName?: string;
5659
}) => {
5760
const [componentText, setComponentText] = useState("");
5861
const [validationErrors, setValidationErrors] = useState<string[]>([]);
5962
const [showPreview, setShowPreview] = useState(true);
6063
const [yamlGeneratorOptions, setYamlGeneratorOptions] = useState(options);
6164

6265
const yamlGenerator = usePythonYamlGenerator();
66+
const preservedNameRef = useRef(initialComponentName);
67+
68+
useEffect(() => {
69+
preservedNameRef.current = initialComponentName;
70+
}, [initialComponentName]);
6371

6472
const handleFunctionTextChange = useCallback(
6573
async (value: string | undefined) => {
6674
try {
6775
const yaml = await yamlGenerator(value ?? "", yamlGeneratorOptions);
68-
setComponentText(yaml);
69-
onComponentTextChange(yaml);
76+
const yamlWithPreservedName = preserveComponentName(
77+
yaml,
78+
preservedNameRef.current,
79+
);
80+
setComponentText(yamlWithPreservedName);
81+
onComponentTextChange(yamlWithPreservedName);
7082
setValidationErrors([]);
7183
onErrorsChange([]);
7284
} catch (error) {
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { preserveComponentName } from "./preserveComponentName";
4+
5+
describe("preserveComponentName", () => {
6+
const baseYaml = `name: Original Name
7+
description: Sample
8+
implementation:
9+
container:
10+
image: python:3.12
11+
`;
12+
13+
it("returns original yaml when no name provided", () => {
14+
expect(preserveComponentName(baseYaml)).toBe(baseYaml);
15+
expect(preserveComponentName(baseYaml, "")).toBe(baseYaml);
16+
});
17+
18+
it("replaces the name when provided", () => {
19+
const result = preserveComponentName(baseYaml, "Preserved Name");
20+
21+
expect(result).toContain("name: Preserved Name");
22+
});
23+
24+
it("falls back to original yaml on parse errors", () => {
25+
const malformedYaml = ":- invalid";
26+
27+
expect(preserveComponentName(malformedYaml, "New Name")).toBe(
28+
malformedYaml,
29+
);
30+
});
31+
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import yaml from "js-yaml";
2+
3+
import type { ComponentSpec } from "@/utils/componentSpec";
4+
import { isValidComponentSpec } from "@/utils/componentSpec";
5+
6+
export const preserveComponentName = (
7+
yamlText: string,
8+
preservedName?: string,
9+
): string => {
10+
if (!preservedName || !preservedName.trim()) {
11+
return yamlText;
12+
}
13+
14+
try {
15+
const parsed = yaml.load(yamlText);
16+
17+
if (isValidComponentSpec(parsed)) {
18+
const updatedSpec: ComponentSpec = {
19+
...parsed,
20+
name: preservedName,
21+
};
22+
23+
return yaml.dump(updatedSpec, { lineWidth: 10000 });
24+
}
25+
} catch (error) {
26+
console.error("Failed to preserve component name:", error);
27+
}
28+
29+
return yamlText;
30+
};

src/providers/ComponentLibraryProvider/ComponentLibraryProvider.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ describe("ComponentLibraryProvider - Component Management", () => {
381381

382382
const consoleSpy = vi
383383
.spyOn(console, "error")
384-
.mockImplementation(() => { });
384+
.mockImplementation(() => {});
385385

386386
const { result } = renderHook(() => useComponentLibrary(), {
387387
wrapper: createWrapper,
@@ -414,7 +414,7 @@ describe("ComponentLibraryProvider - Component Management", () => {
414414
mockDeleteComponentFileFromList.mockRejectedValue(error);
415415
const consoleSpy = vi
416416
.spyOn(console, "error")
417-
.mockImplementation(() => { });
417+
.mockImplementation(() => {});
418418

419419
const { result } = renderHook(() => useComponentLibrary(), {
420420
wrapper: createWrapper,

src/providers/ComponentLibraryProvider/ComponentLibraryProvider.tsx

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,8 @@ export const ComponentLibraryProvider = ({
264264
});
265265
} else {
266266
console.warn(
267-
`Component "${component.name
267+
`Component "${
268+
component.name
268269
}" does not have spec or text, cannot favorite.`,
269270
);
270271
}
@@ -450,8 +451,8 @@ export const ComponentLibraryProvider = ({
450451
async (component: ComponentReference) => {
451452
const duplicate = userComponentsFolder
452453
? flattenFolders(userComponentsFolder).find(
453-
(c) => getComponentName(c) === getComponentName(component),
454-
)
454+
(c) => getComponentName(c) === getComponentName(component),
455+
)
455456
: undefined;
456457

457458
if (duplicate?.name) {
@@ -463,14 +464,13 @@ export const ComponentLibraryProvider = ({
463464
return;
464465
}
465466

466-
await importComponent(component)
467-
.then(async () => {
468-
await refreshComponentLibrary();
469-
await refreshUserComponents();
470-
})
471-
.catch((error) => {
472-
console.error("Error adding component to library:", error);
473-
});
467+
try {
468+
await importComponent(component);
469+
await refreshComponentLibrary();
470+
await refreshUserComponents();
471+
} catch (error) {
472+
console.error("Error adding component to library:", error);
473+
}
474474
},
475475
[
476476
userComponentsFolder,

src/services/componentService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ async function hydrateFromContentfulComponentReference(
408408

409409
const digest = await generateDigest(text);
410410
// we always want to have a name, so we generate a default one if it is not provided
411-
const name = component.name ?? spec.name ?? `component-${digest.slice(0, 8)}`;
411+
const name = spec.name ?? component.name ?? `component-${digest.slice(0, 8)}`;
412412

413413
return {
414414
...component,

src/services/hydrateComponentReference.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,22 @@ describe("hydrateComponentReference()", () => {
910910
expect(result?.text).toBe(yaml.dump(componentSpec)); // Text regenerated from spec
911911
});
912912

913+
it("should prefer YAML spec name over provided component name", async () => {
914+
const { text: componentText, spec: componentSpec } =
915+
prepareComponentContent("Filter text two", "filter:v2");
916+
917+
const contentfulRef = {
918+
text: componentText,
919+
spec: componentSpec,
920+
name: "Filter text",
921+
};
922+
923+
const result = await hydrateComponentReference(contentfulRef);
924+
925+
expect(result).not.toBeNull();
926+
expect(result?.name).toBe("Filter text two");
927+
});
928+
913929
it("should preserve URL when provided with text and spec", async () => {
914930
// Arrange
915931
const testUrl = "https://example.com/contentful.yaml";

0 commit comments

Comments
 (0)