-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add timezone support for Google Sheets sync #8568
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
feat: add timezone support for Google Sheets sync #8568
Conversation
- Introduced timezone field in GoogleSheetsSync model to store IANA timezone identifier. - Updated appendEventToGoogleSheets function to use stored timezone for date formatting. - Modified journeyVisitorExportToGoogleSheet mutation to capture user's timezone for consistent date handling. - Added timezone parameter in relevant tests for validation.
WalkthroughThe PR adds timezone support to the Google Sheets export feature. It introduces a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 6345002
☁️ Nx Cloud last updated this comment at |
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 (2)
apis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.spec.ts (1)
245-257: Test correctly verifies default timezone persistence.The assertion properly expects
timezone: 'UTC'when no timezone argument is provided. Consider adding a test case that passes a custom timezone (e.g.,'America/New_York') to verify non-default timezone handling is correctly persisted.apis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.ts (1)
195-207: Consider validating the timezone identifier.The timezone argument accepts any string with a default of 'UTC'. An invalid IANA timezone (e.g.,
'Invalid/Zone') would be persisted and could cause issues whenformatDateYmdInTimeZoneis called during live sync. The function has a fallback, but storing invalid data is suboptimal.♻️ Optional: Add validation before using the timezone
+ // Validate timezone is a valid IANA identifier + const validateTimezone = (tz: string): boolean => { + try { + Intl.DateTimeFormat(undefined, { timeZone: tz }) + return true + } catch { + return false + } + } + // Use user's timezone or default to UTC - const userTimezone = timezone ?? 'UTC' + const userTimezone = timezone && validateTimezone(timezone) ? timezone : 'UTC'
📜 Review details
Configuration used: Repository UI
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 (5)
apis/api-journeys-modern/src/schema/event/utils.tsapis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.spec.tsapis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.tslibs/prisma/journeys/db/migrations/20260107193142_20260107193140/migration.sqllibs/prisma/journeys/db/schema.prisma
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.spec.tsapis/api-journeys-modern/src/schema/event/utils.tsapis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.spec.tsapis/api-journeys-modern/src/schema/event/utils.tsapis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.ts
🧠 Learnings (1)
📚 Learning: 2025-11-11T23:22:02.196Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 8156
File: apis/api-journeys-modern/src/lib/google/googleAuth.ts:0-0
Timestamp: 2025-11-11T23:22:02.196Z
Learning: In apis/api-journeys-modern, use the validated `env` object from `../../env` instead of accessing `process.env` directly for environment variables that are defined in env.ts (e.g., GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, INTEGRATION_ACCESS_KEY_ENCRYPTION_SECRET). This eliminates the need for runtime validation checks since Zod validates them at application startup.
Applied to files:
apis/api-journeys-modern/src/schema/event/utils.ts
🧬 Code graph analysis (1)
apis/api-journeys-modern/src/schema/event/utils.ts (1)
apis/api-journeys-modern/src/schema/journeyVisitor/export/date.ts (1)
formatDateYmdInTimeZone(4-15)
⏰ 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). (7)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: affected (22)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
libs/prisma/journeys/db/migrations/20260107193142_20260107193140/migration.sql (1)
1-2: LGTM!The migration correctly adds a nullable
timezonecolumn to store IANA timezone identifiers. The TEXT type is appropriate for this use case.libs/prisma/journeys/db/schema.prisma (1)
291-291: LGTM!The optional
timezonefield with the inline comment explaining the IANA format and example is well-documented. Making it nullable ensures backward compatibility with existing sync records.apis/api-journeys-modern/src/schema/journeyVisitor/journeyVisitorExportToGoogleSheet.mutation.ts (1)
598-602: Timezone persistence looks correct.The
timezone: userTimezoneis properly included in the sync data payload, ensuring the user's timezone preference is stored for consistent date formatting in subsequent live sync operations.apis/api-journeys-modern/src/schema/event/utils.ts (3)
348-350: LGTM!The fallback to
'UTC'whensync.timezoneis null ensures backward compatibility for existing sync records created before this feature.
418-429: Date formatting with timezone is correctly implemented.The code safely handles date parsing and formatting with proper guards:
- Checks for non-empty input
- Validates the Date object with
isNaNcheck- Falls back to the original value if formatting fails
The empty catch block at line 426-428 is acceptable here since preserving the original value is a sensible fallback.
431-436: Row mapping correctly uses formatted date.The
formattedDatevariable properly replacescreatedAtRawin the row map, ensuring timezone-aware dates are written to Google Sheets.
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.