-
Notifications
You must be signed in to change notification settings - Fork 12
Optimize cache key overflow management with performance improvements #1
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
* feat: add calendar cache status dropdown - Add updatedAt field to CalendarCache schema with migration - Create tRPC cacheStatus endpoint for fetching cache timestamps - Add action dropdown to CalendarSwitch for Google Calendar entries - Display formatted last updated timestamp in dropdown - Add placeholder for cache deletion functionality - Include translation strings for dropdown content The dropdown only appears for Google Calendar integrations that have active cache entries and provides cache management options for future extensibility. Co-Authored-By: [email protected] <[email protected]> * fix: resolve Prisma type incompatibilities in repository files - Remove problematic satisfies clause in selectedCalendar.ts - Add missing cacheStatus parameter to ConnectedCalendarList component - Fixes type errors that were preventing CI from passing Co-Authored-By: [email protected] <[email protected]> * refactor: integrate cache status into connectedCalendars handler - Remove separate cacheStatus tRPC endpoint as requested - Return cache status as separate field in connectedCalendars response - Update UI components to use cache data from connectedCalendars - Fix Prisma type incompatibilities in repository files Co-Authored-By: [email protected] <[email protected]> * fix: resolve Prisma type incompatibilities and fix data flow for cache status - Fix Prisma.SortOrder usage in membership.ts orderBy clauses - Remove problematic satisfies clause in selectedCalendar.ts - Fix TeamSelect type reference in team.ts - Update SelectedCalendarsSettingsWebWrapper to properly pass cacheStatus data flow Co-Authored-By: [email protected] <[email protected]> * Discard changes to packages/lib/server/repository/membership.ts * Discard changes to packages/lib/server/repository/team.ts * fix: improve calendar cache dropdown with proper formatting and subscription logic - Fix timestamp HTML entity encoding with interpolation escapeValue: false - Only show dropdown for subscribed Google calendars (googleChannelId exists) - Hide delete option when no cache data exists - Include updatedAt and googleChannelId fields upstream in user repository - Update data flow to pass subscription status through components Co-Authored-By: [email protected] <[email protected]> * feat: update SelectedCalendar.updatedAt when Google webhooks trigger cache refresh - Add updateManyByCredentialId method to SelectedCalendarRepository - Update fetchAvailabilityAndSetCache to refresh SelectedCalendar timestamps - Ensure webhook flow updates both CalendarCache and SelectedCalendar records - Maintain proper timestamp tracking for calendar cache operations Co-Authored-By: [email protected] <[email protected]> * Add script to automate Tunnelmole webhook setup Introduces test-gcal-webhooks.sh to start Tunnelmole, extract the public URL, and update GOOGLE_WEBHOOK_URL in the .env file. Handles process management, rate limits, and ensures environment configuration for Google Calendar webhooks. * Update dev:cron script to use npx tsx Replaces 'ts-node' with 'npx tsx' in the dev:cron script for running cron-tester.ts, likely to improve compatibility or leverage tsx features. * Update cache status string and improve CalendarSwitch UI Renamed 'last_updated' to 'cache_last_updated' in locale file for clarity and updated CalendarSwitch to use the new string. Also added dark mode text color support for cache status display. * refactor: move cache management to credential-level dropdown with Remove App - Create CredentialActionsDropdown component consolidating cache and app removal actions - Add deleteCache tRPC mutation for credential-level cache deletion - Update connectedCalendars handler to include cacheUpdatedAt at credential level - Move dropdown from individual CalendarSwitch to credential level in SelectedCalendarsSettingsWebWrapper - Remove cache-related props from CalendarSwitch component - Add translation strings for cache management actions - Consolidate all credential-level actions (cache management + Remove App) in one dropdown Co-Authored-By: [email protected] <[email protected]> * fix: remove duplicate translation keys in common.json - Remove duplicate cache-related keys at lines 51-56 - Keep properly positioned keys later in file - Addresses GitHub comment from zomars about duplicate keys Co-Authored-By: [email protected] <[email protected]> * fix: rename translation key to cache_last_updated - Address GitHub comment from zomars - Rename 'last_updated' to 'cache_last_updated' for specificity - Update usage in CredentialActionsDropdown component Co-Authored-By: [email protected] <[email protected]> * fix: remove duplicate last_updated translation key Co-Authored-By: [email protected] <[email protected]> * fix: add confirmation dialog for cache deletion and use repository pattern - Add confirmation dialog for destructive cache deletion action - Replace direct Prisma calls with CalendarCacheRepository pattern - Add getCacheStatusByCredentialIds method to repository interface - Fix import paths for UI components - Address GitHub review comments from zomars Co-Authored-By: [email protected] <[email protected]> * Update CredentialActionsDropdown.tsx * Update common.json * Update common.json * fix: remove nested div wrapper to resolve HTML structure error - Remove wrapping div around DisconnectIntegration component - Fixes nested <p> tag validation error preventing Remove App functionality - Maintains existing confirmation dialog patterns Co-Authored-By: [email protected] <[email protected]> * Fix API handler response termination logic Removed unnecessary return values after setting status in the integrations API handler. This clarifies response handling and prevents returning the response object when not needed. Resolves "API handler should not return a value, received object". * fix: 400 is correct error code for computing slot for past booking (#22574) * fix * add test * chore: release v5.5.1 * Refactor credential disconnect to use confirmation dialog Replaces the DisconnectIntegration component with an inline confirmation dialog for removing app credentials. Adds disconnect mutation logic and updates UI to improve user experience and consistency. * Set default value for CalendarCache.updatedAt Added a default value of NOW() for the updatedAt column in the CalendarCache table to ensure existing and future rows have a valid timestamp. Updated the Prisma schema to reflect this change and provide compatibility for legacy data and raw inserts. --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Benny Joo <[email protected]> Co-authored-by: emrysal <[email protected]>
* fix: add missing triggers for RoutingFormResponseDenormalized * add insert triggers * update booking related triggers * fix * address feedback
…22671) - Rename team.ts to teamService.ts - Rename TeamService.test.ts to teamService.test.ts - Rename TeamService.alternative.test.ts to teamService.alternative.test.ts - Update all import statements across codebase to reference new file names This change improves consistency with the codebase naming conventions while maintaining all existing functionality. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…Service (#22670) - Rename packages/lib/server/repository/eventType.ts to eventTypeRepository.ts - Rename packages/lib/server/service/eventType.ts to eventTypeService.ts - Update all import statements across the codebase to use new filenames - No functional changes, only file path updates Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix: align toast icons with message text vertically * fix: align toast icons with message text vertically * fix: align toast icons with message text vertically
* fix readonly to all to readonly * fix sheet not showing correct all,readonly,none state until refresh * update test to include the correct roleId --------- Co-authored-by: Alex van Andel <[email protected]>
* Add playground * Fix scroll issue on safari
…ogle Pay support (#22446) * feat: add allow="payment" attribute to embed iframes for Apple Pay support - Add default allow="payment" attribute to all embed iframes - Add test cases to verify payment attribute is set by default - Fixes Apple Pay functionality in Cal.com embeds Fixes #19347 Co-Authored-By: [email protected] <[email protected]> * test: add comprehensive tests for iframe attribute handling - Add test for when no config is provided - Add test for empty iframeAttrs object - Add test to verify all attributes are applied (not just id) - Ensure allow='payment' is set in all scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * refactor: restrict iframeAttrs to only allow id attribute - Changed implementation to only apply 'id' from iframeAttrs - Made allow='payment' non-configurable and always set - Updated tests to reflect new behavior - Keeps API surface low and avoids adding support for attributes unless explicitly required as a feature 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * chore: remove duplicate test case - Removed duplicate test for no config scenario - Kept the more descriptive test case 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]>
* refactor: Booking list items actions UI * chore: save * fix: UI * fix: logic and remove unused * chore: change name * Update apps/web/components/booking/BookingListItem.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * perf: refactor * fix: update tests * chore: update reschedule test * refactor: create bookingActions and BookingListItem * tests: add unit tests for booking actions * fix: logic * fix: cancel button regresso --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…22655) * feat: expose form validation methods from EventTypePlatformWrapper - Add validateForm and handleFormSubmit methods to onFormStateChange prop - Update useEventTypeForm hook to return validateForm method - Add proper TypeScript types for form validation result - Enable external form validation and submission capabilities Co-Authored-By: [email protected] <[email protected]> * fix: correct forwardRef syntax to resolve parsing errors - Fix forwardRef parameter destructuring syntax - Use proper props parameter pattern for forwardRef components - Ensure prettier can parse the file correctly Co-Authored-By: [email protected] <[email protected]> * feat: add form validation and submission handlers to event types platform wrapper * refactor: move form validation and submit handlers from hook to wrapper component * fix: add ref to the save button for proper form validation * feat: extend ref-based validation API to AvailabilitySettingsPlatformWrapper - Add AvailabilityFormValidationResult and AvailabilitySettingsPlatformWrapperRef types - Implement forwardRef pattern in AvailabilitySettings component with save button ref - Add validateForm and handleFormSubmit methods using useImperativeHandle - Update AvailabilitySettingsPlatformWrapper to support ref forwarding - Add comprehensive documentation for ref-based validation API in both event-type.mdx and availability-settings.mdx - Follow same patterns established in EventTypePlatformWrapper for consistency Co-Authored-By: [email protected] <[email protected]> * refactor: rename AvailabilitySettingsPlatformWrapperRef to AvailabilitySettingsFormRef * feat: add ref-based validation pattern to availability example page - Mirror the ref-based validation implementation from event-types.tsx - Add useRef, handleValidate, and handleSubmit functions - Demonstrate validateForm() and handleFormSubmit() methods - Add validation and submit buttons to showcase the new API - Use AvailabilitySettingsFormRef type for proper typing Co-Authored-By: [email protected] <[email protected]> * refactor: remove unused state variables * docs: clarify form validation behavior for availability and event type atoms * chore: remove console.logs --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Somay Chauhan <[email protected]>
- Skip unnecessary validation in updateManyByCredentialId for better performance - Let Prisma handle credentialId validation directly at database level - Streamline batch update operations for selected calendars 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
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.
Greptile Summary
This PR implements a comprehensive calendar cache optimization system with extensive changes across the Cal.com codebase. The changes fall into several key areas:
Calendar Cache Foundation: The PR introduces a complete calendar cache management system including a new CalendarCacheRepository with methods like getCacheStatusByCredentialIds, database migrations to add updatedAt timestamps to the CalendarCache table, and UI components like CredentialActionsDropdown for cache management. This system enables tracking cache freshness and provides users with visibility into when their calendar data was last updated.
Form Validation & Programmatic Control: Several platform wrapper components (EventTypePlatformWrapper, AvailabilitySettingsPlatformWrapper) have been refactored to use React's forwardRef pattern with useImperativeHandle, exposing validateForm() and handleFormSubmit() methods. This enables parent components to programmatically control form validation and submission, with corresponding TypeScript interfaces (EventTypePlatformWrapperRef, AvailabilitySettingsFormRef) and comprehensive documentation added to the platform atoms.
File Organization & Import Path Standardization: A systematic refactoring standardized naming conventions across the codebase, particularly renaming repository and service files (e.g., eventType.ts → eventTypeRepository.ts, team.ts → teamService.ts) and reorganizing utility functions into lib/ directories. This affects dozens of import statements across the codebase to maintain consistency.
UI/UX Improvements: The booking actions interface was refactored from individual buttons to a dropdown pattern (booking-actions-dropdown), requiring updates to multiple Playwright tests. Several toast components had styling adjustments, and permission-based access controls were added to settings navigation.
Embed System Enhancements: The embed core received significant improvements including new scroll handling utilities (getScrollableAncestor, scrollIntoViewSmooth), Safari-specific iframe scrolling workarounds, and enhanced event handling for scroll-by-distance functionality. These changes address browser compatibility issues, particularly Safari's iframe scrolling limitations.
Database & Service Layer: A new denormalized table system was introduced for routing form responses with extensive trigger functions, and service classes like TeamService were restructured to follow proper service layer patterns with improved error handling and transactional integrity.
The changes integrate well with Cal.com's existing architecture, following established patterns for TRPC routers, React component patterns, and database operations while significantly expanding the calendar cache capabilities and improving overall code organization.
94 files reviewed, 19 comments
|
|
||
| export type FormValidationResult = { | ||
| isValid: boolean; | ||
| errors: Record<string, unknown>; |
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.
style: Consider using a more specific type than unknown for errors, perhaps string | string[] or the actual form validation error type from your form library
| expect(iframe.src).toContain("email=test%40example.com"); | ||
| }); | ||
|
|
||
| it("should set allow='payment' attribute by default to allow Payment Apps to acccept payments", () => { |
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.
syntax: Typo: 'acccept' should be 'accept'
| it("should set allow='payment' attribute by default to allow Payment Apps to acccept payments", () => { | |
| it("should set allow='payment' attribute by default to allow Payment Apps to accept payments", () => { |
| const permissionService = new PermissionCheckService(); | ||
| return permissionService.getResourcePermissions({ userId, teamId, resource }); | ||
| }, | ||
| ["resource-permissions"], |
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.
logic: The cache key ["resource-permissions"] is too generic and could lead to cache collisions. Consider including userId, teamId, and resource in the cache key for better cache isolation.
| ["resource-permissions"], | |
| [`resource-permissions-${userId}-${teamId}-${resource}`], |
| fi | ||
|
|
||
| if grep -q '^GOOGLE_WEBHOOK_URL=' "$ENV_FILE"; then | ||
| sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" |
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.
logic: The sed -i '' syntax is macOS-specific and will fail on Linux systems. Consider using a more portable approach or detecting the OS.
| sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" | |
| if [[ "$OSTYPE" == "darwin"* ]]; then | |
| sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" | |
| else | |
| sed -i -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" | |
| fi |
| const roleActions = PermissionMapper.toActionMap(rolePermissions, Resource.Role); | ||
| canViewRoles = roleActions[CrudAction.Read] ?? false; |
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.
logic: The permission check logic is nested inside the if (features) block, which means permissions won't be checked if team features are unavailable. This could be a security issue if features and permissions are independent.
| // Only toggle expansion if clicking on the button itself, not child elements | ||
| if (e.target === e.currentTarget) { | ||
| setIsExpanded(!isExpanded); | ||
| } |
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.
logic: The condition e.target === e.currentTarget will only be true if clicking directly on the button element, but the button contains child elements. Users clicking on empty space within the button won't trigger expansion.
| return !(isBookingInPast && booking.status === BookingStatus.ACCEPTED && context.isCalVideoLocation); | ||
| case "meeting_session_details": | ||
| return !(isBookingInPast && booking.status === BookingStatus.ACCEPTED && context.isCalVideoLocation); |
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.
style: Redundant logic - these conditions are already checked in getVideoOptionsActions lines 87 and 93
| } | ||
|
|
||
| // Update SelectedCalendar.updatedAt for all calendars under this credential | ||
| await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}); |
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.
style: The empty object {} as the second parameter suggests this update is purely for triggering updatedAt timestamp changes. Consider adding a comment explaining this intention for future maintainers.
| timestamp: new Intl.DateTimeFormat("en-US", { | ||
| dateStyle: "short", | ||
| timeStyle: "short", | ||
| }).format(new Date(cacheUpdatedAt)), | ||
| interpolation: { escapeValue: false }, | ||
| })} |
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.
style: The hardcoded 'en-US' locale should use the current user's locale from useLocale() for proper internationalization
| "eventTypeParentId" = ( | ||
| SELECT et."parentId" | ||
| FROM "EventType" et | ||
| WHERE et.id = NEW."eventTypeId" | ||
| ), | ||
| "eventTypeSchedulingType" = ( | ||
| SELECT et."schedulingType" | ||
| FROM "EventType" et | ||
| WHERE et.id = NEW."eventTypeId" | ||
| ) |
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.
style: Multiple subqueries in trigger can cause performance degradation. Consider fetching EventType data in a single query with JOINs.
|
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: |
Test 1