Skip to content

fix: don't allow "new" or "create" as url-friendly names #13596

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.

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

Merged
merged 5 commits into from
Jun 18, 2024
Merged

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Jun 18, 2024

  • Currently, you can create a template named "new", which you'll be unable to view in the dashboard because of a routing conflict. /templates/new is the route of the CreateTemplatePage component.

  • Groups, on the other hand, currently use /groups/create as the route for the CreateGroupPage. It's also technically not an issue right now, because groups are routed by UUID on the frontend instead of by name, but we don't do that anywhere else in the app and should probably fix that later. As such, we should make sure that any groups created won't conflict when we fix that down the road.

  • Organizations will soon be similar, when I add the /organizations/new page soon.

It would also be nice to consolidate on just /new or just /create as the route that these sorts of pages should live at, (ie. change groups to /groups/new). In the mean time, let's disallow using either as a name, regardless of the resource type.

Given all this, and for consistency, no resource which has a "url-friendly name" should allow "new" or "create" as that name. If we consolidate all of the routes to just one of these options down the road, we can loosen the restriction.

@aslilac aslilac requested a review from Emyrk June 18, 2024 18:12
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

Good catch!

Unfortunately the actual error text will be quite cryptic.

field: name detail: Validation failed for tag "template_name" with value: "new"

This is an issue with the validate library. It's no worse than the current name validation.

@aslilac
Copy link
Member Author

aslilac commented Jun 18, 2024

Unfortunately the actual error text will be quite cryptic.

boo. unfortunate that it just throws the actual error text away. 🙃

@aslilac aslilac merged commit e987ad1 into main Jun 18, 2024
28 checks passed
@aslilac aslilac deleted the no-new-name branch June 18, 2024 21:36
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants