Skip to content

Conversation

@akshayutture-augment
Copy link
Contributor

* 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]>
@akshayutture-augment
Copy link
Contributor Author

@claude review this PR

1 similar comment
@akshayutture-augment
Copy link
Contributor Author

@claude review this PR

@claude
Copy link

claude bot commented Nov 20, 2025

Claude finished @akshayutture-augment's task —— View job


Code Review Complete ✅

  • Read repository guidelines (CLAUDE.md)
  • Review database schema changes
  • Review new calendar cache repository methods
  • Review new CredentialActionsDropdown component
  • Review deleteCache handler and tRPC router changes
  • Review UI integration changes
  • Review test script
  • Compile and post comprehensive feedback

Overview

This PR adds calendar cache status display and deletion capabilities to the Cal.com UI. The implementation allows users to see when their Google Calendar cache was last updated and provides an option to manually delete cached data.

Positive Aspects ✨

  1. Well-structured architecture - Clean separation of concerns with repository pattern
  2. Security-conscious - Proper authorization checks in deleteCache.handler.ts:17-26
  3. User experience - Nice UI with cache status visibility and confirmation dialogs
  4. Backward compatibility - Migration handles existing rows with a default value

Critical Issues 🔴

1. Security: Missing Authorization for Team Credentials (High Priority)

File: packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:17-22

The handler only checks if the credential belongs to the user:

const credential = await prisma.credential.findFirst({
  where: {
    id: credentialId,
    userId: user.id,
  },
});

Problem: This doesn't handle team credentials (teamId field). A user could potentially delete cache for team credentials they don't have access to.

Recommendation:

const credential = await prisma.credential.findFirst({
  where: {
    id: credentialId,
    OR: [
      { userId: user.id },
      {
        teamId: {
          in: user.teams.map(t => t.teamId)
        }
      }
    ]
  },
});

2. Type Safety: Missing Return Type (Medium Priority)

File: packages/features/calendar-cache/calendar-cache.repository.ts:173-188

The getCacheStatusByCredentialIds method lacks an explicit return type annotation.

Recommendation:

async getCacheStatusByCredentialIds(
  credentialIds: number[]
): Promise<{ credentialId: number; updatedAt: Date | null }[]> {

3. Database Performance: Missing Index (Medium Priority)

File: packages/prisma/schema.prisma:1719

The migration adds updatedAt field but doesn't add an index. The getCacheStatusByCredentialIds method uses groupBy on credentialId with _max.updatedAt, which could be slow for large datasets.

Recommendation: Consider adding a composite index:

@@index([credentialId, updatedAt])

Major Issues 🟡

4. Error Handling: Generic Error Messages (Medium Priority)

File: packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts:25

throw new Error("Credential not found or access denied");

Problem: This generic error message doesn't distinguish between "credential not found" and "access denied", making debugging harder and potentially leaking information.

Recommendation: Use specific error types:

if (!credential) {
  throw new TRPCError({
    code: "NOT_FOUND",
    message: "Credential not found"
  });
}

5. Code Quality: Unused Method in Mock (Low Priority)

File: packages/features/calendar-cache/calendar-cache.repository.mock.ts:23-25

The mock includes deleteManyByCredential() method which isn't in the interface (ICalendarCacheRepository).

Recommendation: Remove it or add it to the interface if needed.

6. UI/UX: No Loading State (Low Priority)

File: packages/features/apps/components/CredentialActionsDropdown.tsx:42-50

The delete cache mutation doesn't show a loading state while the operation is in progress.

Recommendation: Add loading state to the button:

const [isDeleting, setIsDeleting] = useState(false);
// ... in mutation
onMutate: () => setIsDeleting(true),
onSettled: () => setIsDeleting(false),

Minor Issues 🔵

7. Code Consistency: Inconsistent Date Formatting (Low Priority)

File: packages/features/apps/components/CredentialActionsDropdown.tsx:89-92

The date formatting is hardcoded to "en-US" locale, but the app uses useLocale() hook which should provide the user's locale.

Recommendation:

{t("cache_last_updated", {
  timestamp: new Intl.DateTimeFormat(locale, {
    dateStyle: "short",
    timeStyle: "short",
  }).format(new Date(cacheUpdatedAt)),
})}

8. Documentation: Missing JSDoc Comments (Low Priority)

File: packages/features/calendar-cache/calendar-cache.repository.ts:173

The getCacheStatusByCredentialIds method is a public API but lacks documentation.

Recommendation: Add JSDoc:

/**
 * Retrieves the most recent cache update time for each credential.
 * @param credentialIds - Array of credential IDs to check
 * @returns Array of objects containing credentialId and updatedAt timestamp
 */
async getCacheStatusByCredentialIds(credentialIds: number[])

9. Test Script: macOS-Specific sed Command (Low Priority)

File: scripts/test-gcal-webhooks.sh:68

sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE"

The -i '' syntax is macOS-specific. This won't work on Linux.

Recommendation:

# Portable version
sed -i.bak -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" && rm "$ENV_FILE.bak"

10. Database Migration: Default Value Comment Mismatch (Low Priority)

File: packages/prisma/migrations/20250715160635_add_calendar_cache_updated_at/migration.sql:2-4

The warning says "without a default value" but the code shows DEFAULT NOW() is added. The comment is misleading.

Testing Recommendations 🧪

  1. Test team credential deletion - Verify authorization works for team credentials
  2. Test concurrent deletions - What happens if multiple users try to delete the same cache?
  3. Test with large datasets - Verify performance with many credentials
  4. Test error scenarios - Invalid credential IDs, network failures, etc.

Summary

Overall, this is a solid feature addition with good code quality. The main concerns are:

  1. Security fix needed for team credential authorization
  2. Performance consideration for the database query
  3. Minor improvements for error handling and UX

The changes follow the existing codebase patterns and introduce useful functionality for users to manage their calendar cache.


@github-actions
Copy link

github-actions bot commented Dec 5, 2025

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants