-
Notifications
You must be signed in to change notification settings - Fork 10.4k
fix: slugify event slug in duplicate dialog #22983
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
Conversation
WalkthroughA new Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/08/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/08/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/eventtypes/components/DuplicateDialog.tsx (1)
153-156
: Trigger dirty state and validation; compose with RHF instead of overriding onChange.Current handler only sets
shouldTouch
. To keep RHF state accurate and show validation promptly, also setshouldDirty
andshouldValidate
. Optionally, move the handler intoregister
options to avoid overriding RHF’s internalonChange
.Minimal change (keep current approach, just enhance state/validation):
- onChange={(e) => { - form.setValue("slug", slugify(e?.target.value), { shouldTouch: true }); - }} + onChange={(e) => { + form.setValue("slug", slugify(e?.target.value), { + shouldTouch: true, + shouldDirty: true, + shouldValidate: true, + }); + }}Alternative (compose with RHF by injecting onChange via register options):
- {...register("slug")} - onChange={(e) => { - form.setValue("slug", slugify(e?.target.value), { shouldTouch: true }); - }} + {...register("slug", { + onChange: (e) => { + form.setValue("slug", slugify(e?.target?.value ?? ""), { + shouldTouch: true, + shouldDirty: true, + shouldValidate: true, + }); + }, + })}Optional deduplication: define a shared
handleSlugChange
and reuse it in both branches to avoid divergence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/features/eventtypes/components/DuplicateDialog.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()
for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/eventtypes/components/DuplicateDialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/features/eventtypes/components/DuplicateDialog.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
Applied to files:
packages/features/eventtypes/components/DuplicateDialog.tsx
📚 Learning: 2025-07-21T21:33:23.371Z
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/features/eventtypes/components/DuplicateDialog.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Detect changes
🔇 Additional comments (1)
packages/features/eventtypes/components/DuplicateDialog.tsx (1)
153-156
: LGTM: Slugify on change ensures consistent URL formatting.Mirrors the behavior of the other branch and prevents title-based auto-slug updates after the user edits the slug.
* fix: flaky e2e * fix: slugify url
No description provided.