Skip to content

Commit 7bd8e32

Browse files
authored
Loosen restrictions on Pipeline Naming (#1078)
## Description <!-- Please provide a brief description of the changes made in this pull request. Include any relevant context or reasoning for the changes. --> Remove restrictions on special characters in pipeline naming. Special characters will encode correctly in urls. Also trimmed pipeline name values and prevented empty string names. ## Related Issue and Pull requests <!-- Link to any related issues using the format #<issue-number> --> ## Type of Change <!-- Please delete options that are not relevant --> - [x] Bug fix - [x] Improvement ## Checklist <!-- Please ensure the following are completed before submitting the PR --> - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Screenshots (if applicable) <!-- Include any screenshots that might help explain the changes or provide visual context --> ![image.png](https://app.graphite.dev/user-attachments/assets/180211ed-3f01-489c-a05c-59cc14a4687f.png) ![image.png](https://app.graphite.dev/user-attachments/assets/96e7212a-421e-405e-bd03-6daafebd90a7.png) ## Test Instructions <!-- Detail steps and prerequisites for testing the changes in this PR --> 1. verify special characters can now be used in pipeline names 2. verify that special characters do not break pipeline urls 3. verify that pipeline editing cans till be accessed via the pipeline list, inspetc button & clone button ## Additional Comments <!-- Add any additional context or information that reviewers might need to know regarding this PR -->
1 parent 4a5e7ff commit 7bd8e32

File tree

5 files changed

+31
-31
lines changed

5 files changed

+31
-31
lines changed

src/components/PipelineRun/RunDetails.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export const RunDetails = () => {
2828
const { componentSpec } = useComponentSpec();
2929
const { details, state, runId, isLoading, error } = useRootExecutionContext();
3030

31-
const editorRoute = `/editor/${componentSpec.name}`;
31+
const editorRoute = componentSpec.name
32+
? `/editor/${encodeURIComponent(componentSpec.name)}`
33+
: "";
3234

3335
const canAccessEditorSpec = useCheckComponentSpecFromPath(
3436
editorRoute,
@@ -137,7 +139,7 @@ export const RunDetails = () => {
137139

138140
<div>
139141
<div className="flex gap-2">
140-
{canAccessEditorSpec && (
142+
{canAccessEditorSpec && componentSpec.name && (
141143
<InspectPipelineButton pipelineName={componentSpec.name} />
142144
)}
143145
<ClonePipelineButton componentSpec={componentSpec} />
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { useNavigate } from "@tanstack/react-router";
2-
import { Network } from "lucide-react";
32
import { useCallback } from "react";
43

54
import TooltipButton from "@/components/shared/Buttons/TooltipButton";
5+
import { Icon } from "@/components/ui/icon";
66

77
type InspectPipelineButtonProps = {
8-
pipelineName?: string;
8+
pipelineName: string;
99
};
1010

1111
export const InspectPipelineButton = ({
@@ -14,7 +14,7 @@ export const InspectPipelineButton = ({
1414
const navigate = useNavigate();
1515

1616
const handleInspect = useCallback(() => {
17-
navigate({ to: `/editor/${pipelineName}` });
17+
navigate({ to: `/editor/${encodeURIComponent(pipelineName)}` });
1818
}, [pipelineName, navigate]);
1919

2020
return (
@@ -24,7 +24,7 @@ export const InspectPipelineButton = ({
2424
tooltip="Inspect pipeline"
2525
data-testid="inspect-pipeline-button"
2626
>
27-
<Network className="w-4 h-4 rotate-270" />
27+
<Icon name="Network" className="rotate-270" />
2828
</TooltipButton>
2929
);
3030
};

src/components/shared/Dialogs/PipelineNameDialog.tsx

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { type ChangeEvent, type ReactNode, useCallback, useState } from "react";
1+
import {
2+
Activity,
3+
type ChangeEvent,
4+
type ReactNode,
5+
useCallback,
6+
useState,
7+
} from "react";
28

39
import { Alert, AlertDescription } from "@/components/ui/alert";
410
import { Button } from "@/components/ui/button";
@@ -16,7 +22,6 @@ import { Icon } from "@/components/ui/icon";
1622
import { Input } from "@/components/ui/input";
1723
import { BlockStack } from "@/components/ui/layout";
1824
import useLoadUserPipelines from "@/hooks/useLoadUserPipelines";
19-
import { VALID_NAME_MESSAGE, VALID_NAME_REGEX } from "@/utils/constants";
2025

2126
interface PipelineNameDialogProps {
2227
trigger: ReactNode;
@@ -57,9 +62,11 @@ const PipelineNameDialog = ({
5762
Array.from(userPipelines.keys()).map((name) => name.toLowerCase()),
5863
);
5964

60-
if (!VALID_NAME_REGEX.test(newName)) {
61-
setError(VALID_NAME_MESSAGE);
62-
} else if (existingPipelineNames.has(newName.trim().toLowerCase())) {
65+
const normalizedNewName = newName.trim().toLowerCase();
66+
67+
if (normalizedNewName === "") {
68+
setError("Name cannot be empty");
69+
} else if (existingPipelineNames.has(normalizedNewName)) {
6370
setError("Name already exists");
6471
} else {
6572
setError(null);
@@ -84,7 +91,7 @@ const PipelineNameDialog = ({
8491
);
8592

8693
const handleSubmit = useCallback(() => {
87-
onSubmit(name);
94+
onSubmit(name.trim());
8895
}, [name, onSubmit]);
8996

9097
const isDisabled =
@@ -103,19 +110,12 @@ const PipelineNameDialog = ({
103110
</DialogHeader>
104111
<BlockStack gap="2">
105112
<Input value={name} onChange={handleOnChange} />
106-
<Alert variant={error ? "destructive" : "default"}>
107-
{error ? (
108-
<>
109-
<Icon name="CircleAlert" />
110-
<AlertDescription>{error}</AlertDescription>
111-
</>
112-
) : (
113-
<>
114-
<Icon name="Info" />
115-
<AlertDescription>{VALID_NAME_MESSAGE}</AlertDescription>
116-
</>
117-
)}
118-
</Alert>
113+
<Activity mode={error ? "visible" : "hidden"}>
114+
<Alert variant="destructive">
115+
<Icon name="CircleAlert" />
116+
<AlertDescription>{error}</AlertDescription>
117+
</Alert>
118+
</Activity>
119119
</BlockStack>
120120
<DialogFooter className="sm:justify-end">
121121
<DialogClose asChild>

src/hooks/useCheckComponentSpecFromPath.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ import { useMemo } from "react";
33

44
import { RUNS_BASE_PATH } from "@/routes/router";
55
import { loadPipelineByName } from "@/services/pipelineService";
6+
import { TWENTY_FOUR_HOURS_IN_MS } from "@/utils/constants";
67
import { getIdOrTitleFromPath } from "@/utils/URL";
78

89
export const useCheckComponentSpecFromPath = (
910
url: string,
1011
disabled: boolean = false,
1112
) => {
1213
const isRunPath = url.includes(RUNS_BASE_PATH);
14+
const isEmptyPath = url.trim() === "" || url.trim() === "/";
1315

1416
const { title } = useMemo(() => getIdOrTitleFromPath(url), [url]);
1517

@@ -19,8 +21,8 @@ export const useCheckComponentSpecFromPath = (
1921
loadPipelineByName(title as string).then(
2022
(result) => !!result.experiment?.componentRef?.spec,
2123
),
22-
enabled: !disabled && !isRunPath && !!title,
23-
staleTime: 1000 * 60 * 60 * 24,
24+
enabled: !disabled && !isRunPath && !!title && !isEmptyPath,
25+
staleTime: TWENTY_FOUR_HOURS_IN_MS,
2426
});
2527

2628
return existsLocal ?? false;

src/utils/constants.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ implementation:
3434
outputValues: {}
3535
`;
3636

37-
export const VALID_NAME_REGEX = /^[a-zA-Z0-9\s]+$/;
38-
export const VALID_NAME_MESSAGE =
39-
"Name must be unique and contain only alphanumeric characters and spaces";
40-
4137
// IndexedDB constants
4238
export const DB_NAME = "components";
4339
export const PIPELINE_RUNS_STORE_NAME = "pipeline_runs";

0 commit comments

Comments
 (0)