fix: changes to error on schedule name empty save#24044
fix: changes to error on schedule name empty save#24044anikdhabal merged 11 commits intocalcom:mainfrom
Conversation
WalkthroughThis PR prevents blank schedule names by adding client- and server-side validation: the AvailabilitySettingsWebWrapper.handleSubmit in apps/web/modules/availability/[schedule]/schedule-view.tsx trims the name and shows an error toast using i18n key schedule_name_cannot_be_empty, returning early instead of calling updateMutation. The English locale apps/web/public/static/locales/en/common.json adds Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
@KirankumarAmbati is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/modules/availability/[schedule]/schedule-view.tsx (1)
123-126: Defensive check and trim before mutate.values.name may be undefined; .trim() would throw. Also, pass the trimmed name to the mutation to avoid persisting whitespace.
Apply within this block:
- if (!values.name.trim()) { + const name = (values.name ?? "").trim(); + if (!name) { showToast(t("schedule_name_cannot_be_empty"), "error"); return; }Then use the trimmed name in the mutation payload:
updateMutation.mutate({ scheduleId, dateOverrides: dateOverrides.flatMap((override) => override.ranges), ...values, name, // ensures no leading/trailing spaces are saved });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/modules/availability/[schedule]/schedule-view.tsx(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:
apps/web/modules/availability/[schedule]/schedule-view.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:
apps/web/modules/availability/[schedule]/schedule-view.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/modules/availability/[schedule]/schedule-view.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Applied to files:
apps/web/modules/availability/[schedule]/schedule-view.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
apps/web/public/static/locales/en/common.json (1)
3735-3735: Avoid duplicate i18n strings — ensure translations exist for schedule_name_cannot_be_emptyReuse the existing key you_need_to_add_a_name if semantically identical; otherwise add schedule_name_cannot_be_empty to every locale to avoid missing translations at runtime.
Verification failed: the earlier script errored with "/bin/bash: line 5: !: command not found". Re-run this fixed check and paste the output:
#!/bin/bash set -euo pipefail base="apps/web/public/static/locales" fd -t f common.json "$base" | while IFS= read -r f; do if rg -q '"schedule_name_cannot_be_empty"\s*:' "$f"; then : else echo "Missing in: $f" fi done
|
please address code rabbit suggestions |
…m/KirankumarAmbati/cal.com into fix/schedule-name-cannot-be-empty
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/trpc/server/routers/viewer/availability/schedule/update.schema.ts (1)
8-8: LGTM! Validation logic is sound.The validation chain correctly enforces a non-empty trimmed name when the field is provided during updates. The
.optional()placement is appropriate—users may omit the name field, but if they supply it, it must be non-empty after trimming.The error message
"Schedule name cannot be empty"is duplicated increate.schema.ts(line 4). Consider extracting it to a shared constant:const SCHEDULE_NAME_EMPTY_ERROR = "Schedule name cannot be empty";Then reference it in both schemas:
- name: z.string().trim().min(1, "Schedule name cannot be empty").optional(), + name: z.string().trim().min(1, SCHEDULE_NAME_EMPTY_ERROR).optional(),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/trpc/server/routers/viewer/availability/schedule/create.schema.ts(1 hunks)packages/trpc/server/routers/viewer/availability/schedule/update.schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/trpc/server/routers/viewer/availability/schedule/create.schema.tspackages/trpc/server/routers/viewer/availability/schedule/update.schema.ts
**/*.{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/trpc/server/routers/viewer/availability/schedule/create.schema.tspackages/trpc/server/routers/viewer/availability/schedule/update.schema.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/availability/schedule/create.schema.tspackages/trpc/server/routers/viewer/availability/schedule/update.schema.ts
🔇 Additional comments (1)
packages/trpc/server/routers/viewer/availability/schedule/create.schema.ts (1)
4-4: LGTM! Required field validation is correct.The validation properly enforces a non-empty trimmed name for schedule creation. Making the
namefield required (not optional) is appropriate for CREATE operations—every new schedule must have a name.
pallava-joshi
left a comment
There was a problem hiding this comment.
please fix the i18n translations bug. rest LGTM.
- Add frontend validation with error toast and i18n support - Add backend validation with .trim().min(1) in Zod schemas - Add i18n translation for schedule_name_cannot_be_empty - Add comprehensive tests for create and update schemas (9 tests) - Sync previousValue with prop changes in EditableHeading - Follow PR calcom#24044 approach with all reviewer feedback addressed Fixes calcom#21709
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/availability/[schedule]/schedule-view.tsx">
<violation number="1" location="apps/web/modules/availability/[schedule]/schedule-view.tsx:124">
P2: New translation key `schedule_name_cannot_be_empty` is only present in en locale; missing in other locales so non-English users will see untranslated toast</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
What does this PR do?
Visual Demo (For contributors especially)
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist