Skip to content

fix: Transform # character to - in slug#10896

Merged
pumfleet merged 2 commits intocalcom:mainfrom
ujwalkumar1995:Fix-#-symbol-in-slug
Aug 24, 2023
Merged

fix: Transform # character to - in slug#10896
pumfleet merged 2 commits intocalcom:mainfrom
ujwalkumar1995:Fix-#-symbol-in-slug

Conversation

@ujwalkumar1995
Copy link
Copy Markdown
Contributor

@ujwalkumar1995 ujwalkumar1995 commented Aug 22, 2023

What does this PR do?

In slugs, # character was not being replaced by - which could lead to 404 page not found. Fixed this issue as part of this PR.

Fixes #10443

Recording.2023-08-23.014832.mp4

Requirement/Documentation

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Try to create an event type with # in the name. # will be replaced by -.

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

I noticed couple of things as part of this change. Have added my observations as comments in the PR review section for better understanding.

@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ❌ Failed (Inspect) Aug 24, 2023 10:11am

@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 22, 2023

@ujwalkumar1995 is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 22, 2023

Thank you for following the naming conventions! 🙏

@github-actions github-actions Bot added event-types area: event types, event-types Medium priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Aug 22, 2023
Comment thread packages/lib/slugify.ts
.replace(/[\s_#]+/g, "-") // Replace whitespace, # and underscores with a single dash
.replace(/^-+/, "") // Remove dashes from start
.replace(/-+$/, "") // Remove dashes from end
.replace(/\+/g, "-"); // Transform all + to -
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was not required as + was already being replaced as part of 'Replace any non-alphanumeric characters (including Unicode) with a dash' regex.
Have verified and added tests for it.

Comment thread packages/lib/slugify.ts
@@ -5,10 +5,9 @@ export const slugify = (str: string) => {
.normalize("NFD") // Normalize to decomposed form for handling accents
.replace(/\p{Diacritic}/gu, "") // Remove any diacritics (accents) from characters
.replace(/[^\p{L}\p{N}\p{Zs}\p{Emoji}]+/gu, "-") // Replace any non-alphanumeric characters (including Unicode) with a dash
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I verified this regex, it should not have ideally allowed # character. I am not sure why it is doing so. Therefore had to add an explicit check to replace # character

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 22, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM. works fine now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working event-types area: event types, event-types Medium priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-2271] Can create event type with # in the slug, page visit = 404

3 participants