Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@goldflag has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (15)
WalkthroughThis pull request systematically refactors query parameter naming from camelCase to snake_case across client and server code. Changes include renaming time-related fields ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
client/src/api/analytics/events/useGetEvents.ts(2 hunks)client/src/api/analytics/useGetJourneys.ts(2 hunks)client/src/api/analytics/useGetMetric.ts(1 hunks)client/src/api/analytics/useGetOverview.ts(1 hunks)client/src/api/analytics/useGetOverviewBucketed.ts(2 hunks)client/src/api/analytics/useGetUserSessions.ts(1 hunks)client/src/api/utils.ts(1 hunks)client/src/app/[site]/journeys/page.tsx(1 hunks)client/src/app/[site]/main/components/MainSection/Chart.tsx(2 hunks)client/src/app/[site]/main/components/MainSection/Overview.tsx(1 hunks)client/src/app/[site]/main/components/MainSection/PreviousChart.tsx(3 hunks)client/src/app/[site]/realtime/RealtimeChart/RealtimeChart.tsx(1 hunks)client/src/components/BucketSelection.tsx(2 hunks)client/src/components/DateSelector/DateSelector.tsx(5 hunks)client/src/components/DateSelector/types.ts(1 hunks)client/src/components/SiteCard.tsx(2 hunks)client/src/lib/store.ts(1 hunks)client/src/lib/urlParams.ts(3 hunks)server/src/api/analytics/events/getEventNames.ts(1 hunks)server/src/api/analytics/events/getEventProperties.ts(1 hunks)server/src/api/analytics/events/getEvents.ts(1 hunks)server/src/api/analytics/getErrorBucketed.ts(2 hunks)server/src/api/analytics/getMetric.ts(1 hunks)server/src/api/analytics/getOverview.ts(1 hunks)server/src/api/analytics/getOverviewBucketed.ts(5 hunks)server/src/api/analytics/getPageTitles.ts(1 hunks)server/src/api/analytics/getUsers.ts(1 hunks)server/src/api/analytics/performance/getPerformanceByDimension.ts(1 hunks)server/src/api/analytics/performance/getPerformanceTimeSeries.ts(2 hunks)server/src/api/analytics/query-validation.ts(5 hunks)server/src/api/analytics/utils.ts(3 hunks)server/src/api/sessionReplay/getSessionReplays.ts(1 hunks)shared/src/params.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
client/src/api/analytics/useGetJourneys.ts (1)
client/src/api/utils.ts (2)
getQueryParams(36-52)authedFetch(54-96)
server/src/api/analytics/performance/getPerformanceTimeSeries.ts (1)
server/src/api/analytics/utils.ts (3)
TimeBucketToFn(315-325)bucketIntervalMap(327-337)getTimeStatement(7-65)
server/src/api/analytics/events/getEvents.ts (1)
server/src/api/analytics/utils.ts (1)
getTimeStatement(7-65)
client/src/api/analytics/events/useGetEvents.ts (1)
client/src/api/utils.ts (1)
getQueryParams(36-52)
client/src/api/utils.ts (2)
client/src/components/DateSelector/types.ts (1)
Time(44-44)client/src/lib/dateTimeUtils.ts (1)
timeZone(14-14)
server/src/api/analytics/utils.ts (1)
shared/src/params.ts (1)
FilterParams(10-10)
server/src/api/analytics/getOverviewBucketed.ts (1)
server/src/api/analytics/utils.ts (4)
TimeBucketToFn(315-325)bucketIntervalMap(327-337)getTimeStatement(7-65)getFilterStatement(141-288)
client/src/app/[site]/main/components/MainSection/Chart.tsx (1)
client/src/lib/dateTimeUtils.ts (1)
userLocale(5-5)
server/src/api/analytics/getErrorBucketed.ts (1)
server/src/api/analytics/utils.ts (2)
TimeBucketToFn(315-325)bucketIntervalMap(327-337)
⏰ 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). (2)
- GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
🔇 Additional comments (39)
server/src/api/analytics/getMetric.ts (2)
55-56: Formatting improvement: blank line enhances readability.The added blank line between
timeStatementandfilterStatementdeclarations improves visual separation and code clarity. Formatting-only change; no functional impact.
51-57: No issues found — snake_case parameter handling is correctly implemented.The
FilterParamstype from@rybbit/shareddefines all required time parameters with snake_case names (start_date,end_date,time_zone,past_minutes_start,past_minutes_end). The call togetTimeStatement(request.query)on line 55 receives these properties directly from the Fastify request, and thegetTimeStatementfunction correctly destructures and validates these snake_case parameters before building SQL queries. No conversion or special handling is required—the parameters flow through correctly from the request through the utility functions.server/src/api/analytics/getPageTitles.ts (1)
32-32: LGTM! Parameter destructuring updated correctly.The removal of time-related parameter destructuring is correct. The utility functions
getTimeStatementandgetFilterStatementnow receiverequest.querydirectly with snake_case field names, maintaining functionality while aligning with the new naming convention.server/src/api/analytics/getUsers.ts (1)
36-36: LGTM! Consistent parameter handling.The destructuring change follows the same pattern as other analytics endpoints, removing explicit time parameter extraction while maintaining the functional parameters needed locally. The utility functions correctly handle the snake_case parameters from
request.query.server/src/api/sessionReplay/getSessionReplays.ts (1)
27-31: LGTM! Snake_case migration applied correctly.The explicit parameter access has been correctly updated to snake_case naming. The numeric conversion for
past_minutes_startandpast_minutes_endis properly maintained, ensuring type safety when passing to the session replay service.shared/src/params.ts (1)
1-8: LGTM! Core type definition updated to snake_case.This is the foundational change that enables the snake_case migration across the codebase. The interface update is clean and consistent, standardizing all time-related parameter names to snake_case while preserving the type structure.
server/src/api/analytics/query-validation.ts (2)
16-46: LGTM! Date parameter validation schema updated thoroughly.The
fillDateParamsSchemahas been correctly updated to use snake_case field names. The validation logic, regex patterns, and error messages are all consistent with the new naming convention.
77-154: LGTM! Filter params validation comprehensively migrated.The
filterParamsTimeStatementFillSchemaupdate is thorough and consistent:
- All field names converted to snake_case
- Transform functions correctly reference new field names
- Refine validators updated with proper field checks
- Error messages align with the new naming convention
server/src/api/analytics/utils.ts (2)
7-65: LGTM! Core time statement function migrated successfully.The
getTimeStatementfunction has been comprehensively updated:
- Function signature correctly uses snake_case parameter names
- All destructuring, variable construction, and SQL template usage updated consistently
- Validation and date calculation logic preserved
- SQL escaping and formatting maintained
This is a critical utility function, and the migration is complete and correct.
168-171: Formatting improvement for readability.The event_name condition has been reformatted to split the ternary expression across multiple lines. This improves readability without changing functionality.
server/src/api/analytics/events/getEvents.ts (2)
36-36: LGTM! Query parameter destructuring updated.The destructuring correctly uses snake_case for time-related parameters, aligning with the migration. Other parameters remain unchanged.
44-44: LGTM! Time statement condition updated.The condition correctly checks for
start_dateorend_dateto determine whether to use custom time ranges or default to the last 30 minutes. Logic preserved, naming updated.client/src/app/[site]/journeys/page.tsx (1)
31-31: LGTM! Property shorthand applied.The change from
timeZone: timeZoneto justtimeZoneis a standard JavaScript property shorthand optimization. Clean and idiomatic.client/src/api/analytics/useGetUserSessions.ts (1)
155-155: LGTM! Clean snake_case migration.The property rename from
pastMinutesStarttopast_minutes_startis consistent with the broader refactoring effort and maintains the existing logic.client/src/app/[site]/main/components/MainSection/Overview.tsx (1)
87-87: LGTM! Consistent refactoring.The property access update aligns with the snake_case naming convention while preserving the time range filtering logic.
client/src/app/[site]/realtime/RealtimeChart/RealtimeChart.tsx (1)
14-15: LGTM! Consistent object key updates.The
realtimeTimeconfiguration object correctly uses the new snake_case keys, maintaining compatibility with the updateduseGetOverviewBucketedhook interface.client/src/components/BucketSelection.tsx (1)
10-28: LGTM! Consistent conditional updates.Both conditional checks correctly reference the renamed
past_minutes_startproperty while preserving the bucket selection logic.client/src/components/SiteCard.tsx (2)
34-35: LGTM! Consistent overrideTime object updates.The snake_case keys align with the updated hook interfaces while maintaining the 24-hour time window configuration.
50-51: LGTM! Consistent overrideTime object updates.Matches the previous update, ensuring both
useGetOverviewBucketedanduseGetOverviewcalls use consistent snake_case parameter naming.server/src/api/analytics/events/getEventProperties.ts (1)
22-22: LGTM! Cleaner destructuring pattern.The updated destructuring only extracts parameters used directly in the function body (
eventName,filters), while still passing the fullreq.querytogetTimeStatement()which handles time-related parameters internally. This is cleaner and more maintainable.client/src/app/[site]/main/components/MainSection/PreviousChart.tsx (3)
29-29: LGTM! Thorough refactoring includes commented code.Good practice to update the property reference even in commented code, preventing confusion if this code is uncommented in the future.
61-61: LGTM! Consistent property access.The
maxPastMinutescalculation correctly uses the renamed property while maintaining the DateTime manipulation logic.
103-103: LGTM! Consistent conditional check.The axis formatting conditional correctly references the renamed property while preserving the time range threshold logic (>= 1440 minutes).
client/src/lib/store.ts (1)
82-92: LGTM! Consistent store updates.All three property references (
past_minutes_start,past_minutes_end) are correctly updated within the past-minutes mode logic, preserving both thetimeDiffcalculation andpreviousTimecomputation.server/src/api/analytics/getOverview.ts (1)
96-109: LGTM! Consistent snake_case parameter migration.The destructuring and parameter passing have been correctly updated to use snake_case naming (start_date, end_date, time_zone, past_minutes_start, past_minutes_end). The change is consistent with the broader refactoring across the codebase.
client/src/api/analytics/useGetJourneys.ts (1)
26-42: LGTM! Excellent consolidation usinggetQueryParams.The refactoring successfully consolidates parameter construction via the
getQueryParamsutility, removing manual time field extraction and reducing code duplication. The queryKey is also simplified to use the rawtimeobject instead of individual date fields, which is cleaner and more maintainable.client/src/api/analytics/useGetMetric.ts (1)
42-49: LGTM! Correct snake_case migration for previous period calculation.The field names have been correctly updated to snake_case (
past_minutes_start,past_minutes_end), and the previous period calculation logic is maintained correctly. The transformation properly doubles the start value and uses the original start as the end to create the appropriate previous period window.server/src/api/analytics/performance/getPerformanceTimeSeries.ts (1)
9-108: LGTM! Comprehensive and consistent snake_case migration.The file demonstrates thorough migration to snake_case across all time-related parameters:
- Parameter destructuring (lines 12-13, 32-33, 69)
- SQL template literals (lines 15-26, 77)
- Conditional checks (line 73)
All changes are consistent and properly aligned with the broader refactoring effort.
server/src/api/analytics/events/getEventNames.ts (1)
21-24: LGTM! Simplified parameter handling.The refactoring correctly delegates time parameter extraction to
getTimeStatement, which handles the fullreq.queryobject internally. This eliminates unnecessary destructuring and aligns with the centralized parameter handling pattern established in this PR.client/src/api/utils.ts (1)
8-52: LGTM! Comprehensive snake_case parameter migration.The changes systematically migrate all time-related parameters to snake_case:
start_date,end_dateinstead ofstartDate,endDatetime_zoneinstead oftimeZonepast_minutes_start,past_minutes_endinstead ofpastMinutesStart,pastMinutesEndThe function
getStartAndEndDateis correctly made internal (non-exported) sincegetQueryParamsnow serves as the public API. All branches handle the naming consistently.client/src/api/analytics/useGetOverviewBucketed.ts (1)
46-62: LGTM! Proper use ofgetQueryParamsand snake_case in queryKey.The refactoring correctly:
- Uses
getQueryParamsutility for centralized parameter handling- Updates the queryKey to use snake_case fields (
past_minutes_start,past_minutes_end)- Maintains the conditional queryKey structure based on time mode
client/src/components/DateSelector/DateSelector.tsx (1)
49-53: Snake_case migration executed correctly.All references to
pastMinutesStartandpastMinutesEndhave been consistently updated topast_minutes_startandpast_minutes_end. The logic remains unchanged.Also applies to: 190-191, 202-203, 214-215, 226-227
client/src/api/analytics/events/useGetEvents.ts (1)
59-67: Excellent refactoring with getQueryParams utility.Consolidating parameter construction into
getQueryParamsimproves maintainability and ensures consistent time parameter handling across API calls.client/src/components/DateSelector/types.ts (1)
39-40: Core type definition updated consistently.The field rename from camelCase to snake_case in
PastMinutesModealigns with REST API naming conventions and will be enforced by TypeScript across the codebase.server/src/api/analytics/performance/getPerformanceByDimension.ts (1)
57-57: Simplified parameter destructuring aligns with helper usage.Removing explicit time-related parameter destructuring is appropriate since
getTimeStatementandgetFilterStatementaccessreq.querydirectly. This reduces redundancy.client/src/app/[site]/main/components/MainSection/Chart.tsx (1)
89-90: Field references updated consistently throughout chart logic.All usages of
past_minutes_startare correct and maintain the original logic for time calculations and conditional formatting.Also applies to: 277-277, 284-284
server/src/api/analytics/getErrorBucketed.ts (1)
12-28: Server-side snake_case migration aligned with client changes.Time parameter handling correctly updated to use
start_date,end_date,time_zone,past_minutes_start, andpast_minutes_end. SQL generation and query parameters are consistent.Also applies to: 86-86
server/src/api/analytics/getOverviewBucketed.ts (1)
12-33: Comprehensive snake_case migration across all query construction.All time-related parameters consistently updated throughout:
- Parameter extraction and validation
- SQL query construction with proper field references
- Time statement fill logic for past-minutes and date ranges
The changes maintain the original logic while standardizing on snake_case naming.
Also applies to: 69-78, 127-127, 138-138, 167-178
client/src/lib/urlParams.ts (1)
59-82: URL state synchronization correctly updated for snake_case parameters.The changes ensure bidirectional consistency:
wellKnownPresetsreturn Time objects withpast_minutes_start/past_minutes_end- Serialization writes snake_case URL parameters
- Deserialization correctly reads and reconstructs Time objects
This maintains URL parameter compatibility throughout the migration.
Also applies to: 115-116, 176-183
| overrideTime?: | ||
| | { mode: "past-minutes"; pastMinutesStart: number; pastMinutesEnd: number } | ||
| | { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number } | ||
| | { mode: "range"; startDate: string; endDate: string }; |
There was a problem hiding this comment.
Inconsistent naming: range mode should use snake_case.
The overrideTime type uses snake_case for past-minutes mode (past_minutes_start, past_minutes_end) but retains camelCase for range mode (startDate, endDate). Based on changes in client/src/api/utils.ts where getStartAndEndDate returns start_date and end_date, the range mode should also use snake_case for consistency.
Apply this diff to fix the inconsistency:
type UseGetOverviewOptions = {
periodTime?: PeriodTime;
site?: number | string;
overrideTime?:
| { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number }
- | { mode: "range"; startDate: string; endDate: string };
+ | { mode: "range"; start_date: string; end_date: string };
};🤖 Prompt for AI Agents
In client/src/api/analytics/useGetOverview.ts around lines 19 to 21, the
overrideTime union is inconsistent: the "past-minutes" variant uses snake_case
keys (past_minutes_start, past_minutes_end) while the "range" variant uses
camelCase (startDate, endDate); change the "range" variant to use snake_case
keys (start_date, end_date) so it matches getStartAndEndDate and the rest of the
codebase, and update any corresponding usages/types to reference start_date and
end_date.
| overrideTime?: | ||
| | { mode: "past-minutes"; pastMinutesStart: number; pastMinutesEnd: number } | ||
| | { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number } | ||
| | { mode: "range"; startDate: string; endDate: string }; |
There was a problem hiding this comment.
Inconsistent naming: range mode should use snake_case.
The overrideTime type uses snake_case for past-minutes mode (past_minutes_start, past_minutes_end) but retains camelCase for range mode (startDate, endDate). This is inconsistent with the broader migration to snake_case and will cause type mismatches when interacting with utilities that expect snake_case field names.
Apply this diff to fix the inconsistency:
overrideTime?:
| { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number }
- | { mode: "range"; startDate: string; endDate: string };
+ | { mode: "range"; start_date: string; end_date: string };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| overrideTime?: | |
| | { mode: "past-minutes"; pastMinutesStart: number; pastMinutesEnd: number } | |
| | { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number } | |
| | { mode: "range"; startDate: string; endDate: string }; | |
| overrideTime?: | |
| | { mode: "past-minutes"; past_minutes_start: number; past_minutes_end: number } | |
| | { mode: "range"; start_date: string; end_date: string }; |
🤖 Prompt for AI Agents
In client/src/api/analytics/useGetOverviewBucketed.ts around lines 33 to 35, the
overrideTime union mixes snake_case for the past-minutes variant with camelCase
for the range variant; change the range variant field names to snake_case
(start_date, end_date) so both variants consistently use snake_case, update any
related type references/usages to the new field names, and run type checks to
ensure no remaining references to startDate/endDate remain.
…es for consistency
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/src/api/gsc/getData.ts(2 hunks)server/src/api/gsc/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/src/api/gsc/getData.ts (1)
server/src/index.ts (2)
req(137-144)res(145-149)
⏰ 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). (2)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
- GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
🔇 Additional comments (3)
server/src/api/gsc/getData.ts (2)
17-17: LGTM!The destructuring correctly uses the new snake_case parameter names, consistent with the updated type definition.
61-62: LGTM!The mapping correctly transforms internal snake_case parameters to the camelCase format expected by the Google Search Console API. This is the appropriate approach when interfacing with external APIs that have their own conventions.
server/src/api/gsc/types.ts (1)
20-21: LGTM!The type definition correctly migrates query parameters to snake_case, aligning with the implementation in
getData.tsand the broader snake_case standardization effort.
…ake_case to camelCase for consistency across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/app/[site]/main/components/MainSection/PreviousChart.tsx (2)
61-61: Incomplete snake_case migration: updatepastMinutesStarttopast_minutes_start.Line 61 still uses the camelCase property
time.pastMinutesStart, which is inconsistent with the migration objective and the change made to the commented code on line 29. If theTimetype definition has been updated to use snake_case properties, this will cause a runtime error.Apply this diff to complete the migration:
- ? DateTime.now().setZone("UTC").minus({ minutes: time.pastMinutesStart }).startOf("hour").toJSDate() + ? DateTime.now().setZone("UTC").minus({ minutes: time.past_minutes_start }).startOf("hour").toJSDate()
103-103: Incomplete snake_case migration: updatepastMinutesStarttopast_minutes_start.Line 103 still uses the camelCase property
time.pastMinutesStart, which is inconsistent with the migration objective. This must be updated to match the snake_case convention.Apply this diff to complete the migration:
- if ((time.mode === "past-minutes" && time.pastMinutesStart >= 1440) || time.mode === "day") { + if ((time.mode === "past-minutes" && time.past_minutes_start >= 1440) || time.mode === "day") {
🧹 Nitpick comments (1)
client/src/lib/urlParams.ts (1)
175-184: Consider adding validation for numeric conversions.The deserialization correctly translates snake_case URL parameters back to camelCase internal format. However, consider validating that
Number(past_minutes_start)andNumber(past_minutes_end)produce valid numbers to prevent invalid Time objects if the URL contains malformed values.Apply this diff to add validation:
} else if (timeMode === "past-minutes") { const past_minutes_start = searchParams.get("past_minutes_start"); const past_minutes_end = searchParams.get("past_minutes_end"); if (past_minutes_start && past_minutes_end) { + const startNum = Number(past_minutes_start); + const endNum = Number(past_minutes_end); + if (!isNaN(startNum) && !isNaN(endNum)) { - result.time = { - mode: "past-minutes", - pastMinutesStart: Number(past_minutes_start), - pastMinutesEnd: Number(past_minutes_end), - }; + result.time = { + mode: "past-minutes", + pastMinutesStart: startNum, + pastMinutesEnd: endNum, + }; + } } } else if (timeMode === "all-time") {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/src/api/analytics/useGetMetric.ts(1 hunks)client/src/api/analytics/useGetOverviewBucketed.ts(0 hunks)client/src/api/utils.ts(1 hunks)client/src/app/[site]/main/components/MainSection/PreviousChart.tsx(1 hunks)client/src/lib/urlParams.ts(3 hunks)
💤 Files with no reviewable changes (1)
- client/src/api/analytics/useGetOverviewBucketed.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/api/analytics/useGetMetric.ts
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/api/utils.ts (2)
client/src/components/DateSelector/types.ts (1)
Time(44-44)client/src/lib/dateTimeUtils.ts (1)
timeZone(14-14)
⏰ 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). (2)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
- GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
🔇 Additional comments (4)
client/src/api/utils.ts (2)
36-52: LGTM! Snake_case migration is consistent.The query parameter naming has been successfully updated to snake_case (
time_zone,past_minutes_start,past_minutes_end,start_date,end_date), aligning with backend conventions.
8-34: No breaking changes from export removal.The function
getStartAndEndDateis only used internally withinclient/src/api/utils.ts(line 48) and has no external consumers. Removing the export is safe and introduces no breaking changes.client/src/lib/urlParams.ts (2)
59-82: LGTM! New time presets are correctly implemented.The new past-minutes presets are correctly calculated (30, 60, 360, and 1440 minutes) and follow the established pattern for wellKnown presets.
114-117: LGTM! Correct boundary translation from internal to URL format.The serialization correctly translates internal camelCase (
pastMinutesStart/pastMinutesEnd) to snake_case URL parameters (past_minutes_start/past_minutes_end) for backend compatibility.
…lCase to snake_case for consistency across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/src/api/admin/useGetAdminServiceEventCount.ts(1 hunks)client/src/api/analytics/useGetOrgEventCount.ts(1 hunks)server/src/api/admin/getAdminServiceEventCount.ts(2 hunks)server/src/api/analytics/getOrgEventCount.ts(2 hunks)server/src/api/gsc/getData.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/api/admin/useGetAdminServiceEventCount.ts (1)
client/src/lib/dateTimeUtils.ts (1)
timeZone(14-14)
client/src/api/analytics/useGetOrgEventCount.ts (1)
client/src/lib/dateTimeUtils.ts (1)
timeZone(14-14)
⏰ 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). (2)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
- GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
🔇 Additional comments (9)
client/src/api/admin/useGetAdminServiceEventCount.ts (1)
26-28: Query parameters correctly migrated to snake_case.The client changes are verified to be compatible with the server. The endpoint
server/src/api/admin/getAdminServiceEventCount.tsalready expects snake_case parameters (start_date,end_date,time_zone), and the client now sends them correctly. All three parameters are consistently formatted and follow the established pattern across other analytics endpoints in the codebase.client/src/api/analytics/useGetOrgEventCount.ts (1)
28-30: LGTM! Clean parameter mapping to snake_case.The URL query parameters are correctly mapped from camelCase function parameters to snake_case keys, aligning with the server-side expectations and maintaining clean separation between internal TypeScript conventions and external API conventions.
server/src/api/gsc/getData.ts (2)
17-17: LGTM! Query parameters correctly migrated to snake_case.The destructuring and validation logic have been properly updated to use snake_case parameter names, and the error message now correctly references "start_date or end_date".
Also applies to: 24-26
61-62: LGTM! Correct API mapping to Google's camelCase convention.The transformation from snake_case parameters to camelCase keys for the Google Search Console API body is correct, as Google APIs expect camelCase naming.
server/src/api/analytics/getOrgEventCount.ts (2)
21-23: LGTM! Type definition and destructuring correctly updated.The Querystring type definition and parameter destructuring have been properly migrated to snake_case, maintaining the UTC default for timezone.
Also applies to: 29-29
49-76: SQL construction correctly updated to snake_case (pending typo fix).All variable references in the SQL query construction have been correctly updated to use the snake_case parameters. The time filter logic and date boundary calculations remain intact. Once the typo on line 57 is fixed, this implementation will be complete.
server/src/api/admin/getAdminServiceEventCount.ts (3)
31-31: LGTM - destructuring consistent with type definition.The parameter destructuring correctly uses the new snake_case names and preserves the UTC default for
time_zone.
39-62: LGTM - time filter logic correctly updated.All variable references have been consistently updated to snake_case throughout the time filter construction and FILL date boundary logic. SQL injection protection via
SqlString.escape()is properly maintained, and the conditional logic remains functionally unchanged.
18-20: API consumer verification shows all internal callers updated correctly.All in-repository consumers of this endpoint have been properly updated to use the new snake_case parameters. The client hook (
client/src/api/admin/useGetAdminServiceEventCount.ts) correctly converts parameters to snake_case when making API calls, and the server endpoint expects these parameters.However, you should verify:
- Whether this API has external consumers outside this repository
- If a deprecation strategy or documentation update exists for any external users of the old camelCase parameters
…ake_case for consistency across the codebase.
…ake_case for consistency across multiple endpoints.
Summary by CodeRabbit