Skip to content

validate-stack-name-regexp #915

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
wants to merge 3 commits into
base: staging
Choose a base branch
from
Open

validate-stack-name-regexp #915

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6d5f48
validate-stack-name-regexp
tzador Oct 31, 2025
1159759
fix error message
tzador Oct 31, 2025
20bc0cd
back to 255 limit
tzador Oct 31, 2025
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
10 changes: 7 additions & 3 deletions src/app/stacks/create/components/sharedSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { validateStackName } from "@/lib/stacks";
import { validateStackNameNotInUse } from "@/lib/stacks";
import { z } from "zod";

export const stackNameSchema = z
.string()
.trim()
.min(1, "Stack name is required")
.max(255, "Stack name must be less than 255 characters")
.refine((name) => validateStackName(name), "Stack name is already in use");
.max(32, "Stack name must be less than 32 characters")
Copy link
Contributor

@Cahllagerfeld Cahllagerfeld Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you come to the conclusion of this?

grafik

I just checked the Swagger-Docs and the only restriction is the length of 255 characters that we had before 🤔

Copy link
Collaborator Author

@tzador tzador Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the designs it was 32, but yes, api is king, so back to 255

.refine((name) => validateStackNameNotInUse(name), "Stack name is already in use")
Copy link
Contributor

@Cahllagerfeld Cahllagerfeld Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As sending the request is the most expensive operation during the validation, we should run it as the last validator. Imo only after all the other checks were good, we should run it. Wdyt?

Copy link
Collaborator Author

@tzador tzador Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, swapped the order, not sure how much it improves, but if earlier rule fails first, it will show up sooner.

.refine(
(name) => name.match(/^[a-zA-Z0-9-]+$/),
"Stack name can only contain only letters, numbers and dashes"
);
2 changes: 1 addition & 1 deletion src/lib/stacks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { fetchStacks } from "@/data/stacks/stacklist-query";
import AwesomeDebouncePromise from "awesome-debounce-promise";

export const validateStackName = AwesomeDebouncePromise(async (name: string) => {
export const validateStackNameNotInUse = AwesomeDebouncePromise(async (name: string) => {
const data = await fetchStacks({ name });
return data.total === 0;
}, 500);