feat: add calendar cache status and actions (#22532)

[ShashankFC]

Test 1nn---nReplicated from ai-code-review-evaluation/cal.com-greptile#11

[ShashankFC]

@claude

[claude]

Claude finished @ShashankFC's task in 2m 27s —— View job

---

Code Review Complete

I've reviewed all 17 changed files in this PR. Here's my comprehensive feedback:

Tasks

• Read repository guidelines (CLAUDE.md)
• Review database schema changes and migration
• Review calendar cache repository changes
• Review new tRPC endpoints
• Review UI components and frontend changes
• Review integration with existing calendar services
• Check for security, performance, and best practice issues
• Provide comprehensive feedback

---

Overall Assessment

This PR adds calendar cache management UI capabilities, allowing users to view cache status and delete cached calendar data for Google Calendar integrations. The implementation is generally solid with good separation of concerns.

Critical Issues

1. Missing Authorization Check in deleteCache Handler 🔴

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

The authorization check only verifies that the credential belongs to the user, but doesn't validate that the user has permission to delete cache for that credential type or team context.

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

Issue: If delegated credentials are involved, this might not properly handle permission boundaries.

Recommendation: Verify team membership and feature flags before allowing cache deletion, similar to how other calendar operations handle delegated credentials.

Fix this →

2. Database Migration Has Warning Comment 🟡

File: packages/prisma/migrations/20250715160635_add_calendar_cache_updated_at/migration.sql:1-6

/*
  Warnings:

  - Added the required column `updatedAt` to the `CalendarCache` table without a default value. This is not possible if the table is not empty.

*/

The migration warning indicates the column is marked as required without a default, but the actual migration adds DEFAULT NOW(). The warning comment is misleading and should be removed or updated to reflect the actual migration behavior.

3. Race Condition in Cache Repository 🟡

File: packages/features/calendar-cache/calendar-cache.repository.ts:106-119

When querying cache for in-memory delegation credentials, there's a potential race condition:

cached = await prisma.calendarCache.findFirst({
  where: {
    userId,
    key,
    expiresAt: { gte: new Date(Date.now()) },
  },
  orderBy: {
    expiresAt: "desc",
  },
});

Issue: Between checking expiresAt and returning the result, the cache entry could expire. The Date.now() timestamp could be stale by milliseconds.

Recommendation: Add a small buffer (e.g., 1 second) when checking expiry or revalidate after fetching.

Security Concerns

4. Missing Error Messages Sanitization 🟡

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

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

Generic error message is good for security, but the catch block in the component shows raw error messages to users:

File: packages/features/apps/components/CredentialActionsDropdown.tsx:47-49

onError: () => {
  showToast(t("error_deleting_cache"), "error");
},

Good practice: Error messages are properly sanitized in the UI layer.

Performance Issues

5. N+1 Query Pattern in connectedCalendars Handler 🟡

File: packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts:27-29

const credentialIds = connectedCalendars.map((cal) => cal.credentialId);
const cacheRepository = new CalendarCacheRepository();
const cacheStatuses = await cacheRepository.getCacheStatusByCredentialIds(credentialIds);

While this uses a single query with groupBy, consider that connectedCalendars data is fetched separately. If there are many calendars, this could be optimized by joining or including cache status in the initial query.

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

The getCacheStatusByCredentialIds uses groupBy which is good, but consider adding an index on (credentialId, updatedAt) for better performance.

Code Quality Issues

6. Inconsistent Component Naming 🟢

File: packages/features/apps/components/CredentialActionsDropdown.tsx

The component is located in features/apps/components/ but is specifically for calendar credential actions. Consider:

• Renaming to CalendarCredentialActionsDropdown for clarity
• Moving to features/calendars/components/ to be co-located with related calendar components

7. Magic Numbers Without Constants 🟢

File: packages/features/calendar-cache/calendar-cache.repository.ts:14-16

const MS_PER_DAY = 24 * 60 * 60 * 1000;
const ONE_MONTH_IN_MS = 30 * MS_PER_DAY;
const CACHING_TIME = ONE_MONTH_IN_MS;

Good: Constants are defined. However, consider extracting these to a shared config file since caching duration is a business rule that might need adjustment.

8. Missing Error Handling in UI 🟢

File: packages/features/apps/components/CredentialActionsDropdown.tsx:136

deleteCacheMutation.mutate({ credentialId });

The mutation is called but there's no loading state handling. Consider disabling the button or showing a spinner while the deletion is in progress.

9. TypeScript Type Safety Could Be Improved 🟢

File: packages/features/apps/components/CredentialActionsDropdown.tsx:23

cacheUpdatedAt?: Date | null;

Consider using a stricter type like Date | null (without undefined) or document why all three states are needed.

10. Console.log Left in Production Code 🟡

File: packages/lib/getConnectedDestinationCalendars.ts:236-240

console.log(
  `Upsert the selectedCalendar record to the DB for user ${user.id} with details ${JSON.stringify(
    selectedCalendar
  )}`
);

Issue: This should use the logger instance instead of console.log for consistency and proper log management.

Fix this →

Minor Issues & Suggestions

11. Duplicate HR Element Logic 🟢

File: packages/features/apps/components/CredentialActionsDropdown.tsx:112

{canDisconnect && hasCache && <hr className="my-1" />}

Good: Conditional separator is clean and follows React patterns.

12. Test Script Has Hardcoded Paths 🟢

File: scripts/test-gcal-webhooks.sh:5-6

LOG_FILE="/tmp/tmole.log"
ENV_FILE="../.env"

The script assumes it's run from a specific directory. Consider making it more robust:

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
ENV_FILE="$SCRIPT_DIR/../.env"

13. Missing Translations for Other Locales 🟢

Only English translations were added in apps/web/public/static/locales/en/common.json. Remember to add translations for other supported locales.

14. Good: Proper Use of Repository Pattern ✅

The PR correctly uses the repository pattern (CalendarCacheRepository) instead of direct Prisma calls in most places, maintaining good architectural separation.

15. Good: Proper Type Safety in tRPC Router ✅

File: packages/trpc/server/routers/viewer/calendars/_router.tsx:28-33

The tRPC endpoint properly validates input with Zod schema:

deleteCache: authedProcedure
  .input(z.object({ credentialId: z.number() }))
  .mutation(async ({ ctx, input }) => {

Database Schema Review

16. Schema Change is Well-Designed ✅

File: packages/prisma/schema.prisma:1719

updatedAt    DateTime    @default(now()) @updatedAt

The addition of @updatedAt decorator ensures automatic timestamp updates, which is perfect for cache tracking.

Testing Recommendations

1. Add Unit Tests: The deleteCache.handler.ts needs unit tests covering:

   • Successful cache deletion
   • Unauthorized access attempts
   • Non-existent credential ID
   • Delegated credential scenarios

2. Add Integration Tests: Test the complete flow:

   • Create cache entries
   • View cache status in UI
   • Delete cache
   • Verify cache is cleared

3. Add E2E Tests: Test the dropdown UI interaction and confirmation dialogs.

Documentation

The PR lacks documentation for:

• The new cache management feature (user-facing docs)
• API documentation for the deleteCache endpoint
• Migration guide if there are breaking changes

Summary

Strengths:

• Good separation of concerns with repository pattern
• Proper type safety with tRPC and Zod
• Clean UI implementation with confirmation dialogs
• Well-structured database migration

Must Fix Before Merge:

• Authorization checks for delegated credentials (Async import of the appStore packages #1)
• Console.log in production code (feat: add calendar cache status and actions (#22532) #10)
• Migration warning comment (feat: 2fa backup codes #2)

Should Fix:

• Race condition in cache expiry check (fix: handle collective multiple host on destinationCalendar #3)
• Performance optimization opportunities (Comprehensive workflow reminder management for booking lifecycle events #5)
• Test script path handling (#12)

Nice to Have:

• Component location and naming (Advanced date override handling and timezone compatibility improvements #6)
• Loading states in UI (SMS workflow reminder retry count tracking #8)
• Cache duration as configurable value (OAuth credential sync and app integration enhancements #7)

---

| Branch: introduce-cache-key-overflow

[github-actions]

This PR is being marked as stale due to inactivity.