chore: pass timezone through headers #351
Conversation
Reviewer's GuideThis PR adds timezone propagation from the client through HTTP headers into the Apollo context and downstream cycle progress utilities, updating all date aggregations and formatting to use timezone-aware functions for consistent charting. Sequence diagram for timezone propagation from client to backendsequenceDiagram
actor User
participant Frontend
participant Backend
participant ApolloContext
participant CycleUtils
User->>Frontend: Initiates request
Frontend->>Backend: Sends request with x-timezone header
Backend->>ApolloContext: Passes headers
ApolloContext->>CycleUtils: Passes timezone from headers
CycleUtils->>CycleUtils: Uses timezone in date calculations
Class diagram for fillMissingDays utility with timezone supportclassDiagram
class fillMissingDays {
+fillMissingDays(data, baseDate, totalDays, timezone)
}
fillMissingDays : uses toZonedTime(date, timezone)
fillMissingDays : formats date with timezone
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces timezone handling end-to-end: frontend sends x-timezone header, backend extracts timezone into Apollo context, GraphQL resolver propagates it to cycle progress utilities, and computations become timezone-aware. Also adds a new timezone header utility and minor UI chart interaction tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Frontend UI
participant ApolloC as Apollo Client
participant Net as Network
participant ApolloS as Apollo Server Context
participant Resolver as Cycle Resolver
participant Utils as getCycleProgressChart (utils)
participant DB as Database
UI->>ApolloC: Query getCycleProgressChart
Note right of ApolloC: Resolve client timezone via Intl API
ApolloC->>Net: HTTP POST GraphQL<br/>Headers: x-timezone=IANA TZ
Net->>ApolloS: Request with headers
ApolloS->>ApolloS: Extract timezone via getTimezone()<br/>default 'UTC'
ApolloS-->>Resolver: context { requestInfo.timezone, models, ... }
rect rgba(200, 230, 255, 0.3)
Note over Resolver,Utils: New/changed flow: pass timezone through
Resolver->>Utils: getCycleProgressChart(id, assigneeId, models, timezone)
Utils->>Utils: Compute zoned start/end (toZonedTime)
Utils->>DB: Aggregations with $dateToString { timezone }
DB-->>Utils: Aggregated chart data
Utils-->>Resolver: Chart series (zoned)
end
Resolver-->>UI: GraphQL response (chart data)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You’re threading timezone through a lot of utilities—consider centralizing timezone extraction and conversion at the context or data‐boundary layer so you don’t have to pass it through every function signature.
- Add validation or normalization for the incoming timezone header (e.g. against the IANA tz database) so that invalid or malformed values don’t break downstream date conversions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’re threading timezone through a lot of utilities—consider centralizing timezone extraction and conversion at the context or data‐boundary layer so you don’t have to pass it through every function signature.
- Add validation or normalization for the incoming timezone header (e.g. against the IANA tz database) so that invalid or malformed values don’t break downstream date conversions.
## Individual Comments
### Comment 1
<location> `backend/erxes-api-shared/src/utils/headers/timezone.ts:3-4` </location>
<code_context>
+import { IncomingHttpHeaders } from 'http';
+
+export const getTimezone = (req: IncomingHttpHeaders) => {
+ return req['x-timezone'] || req['timezone'] || 'UTC';
+};
</code_context>
<issue_to_address>
**suggestion:** Consider normalizing timezone header casing and value.
Direct string key access may miss headers with different casing. Normalize header keys to lowercase or use a header parsing library. Also, validate that the timezone value is a valid IANA timezone string to avoid errors.
</issue_to_address>
### Comment 2
<location> `backend/plugins/operation_api/src/modules/cycle/utils.ts:116-120` </location>
<code_context>
cycleId: string,
assigneeId: string | undefined,
models: IModels,
+ timezone: string,
) => {
const filter: { cycleId: Types.ObjectId; assigneeId?: string } = {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Timezone parameter is required but not validated.
Please add validation for the timezone parameter or handle errors from downstream date-fns-tz functions to prevent runtime exceptions.
```suggestion
cycleId: string,
assigneeId: string | undefined,
models: IModels,
timezone: string,
) => {
// Validate timezone parameter
if (
typeof timezone !== 'string' ||
!/^[A-Za-z_\/]+$/.test(timezone) // basic format check
) {
throw new Error(`Invalid timezone parameter: "${timezone}"`);
}
try {
// Try to convert a known date to the timezone to check validity
toZonedTime(new Date(), timezone);
} catch (err) {
throw new Error(`Invalid timezone parameter: "${timezone}". Error: ${err.message}`);
}
```
</issue_to_address>
### Comment 3
<location> `frontend/core-ui/src/providers/apollo-provider/apolloClient.ts:35-40` </location>
<code_context>
headers: {
...headers,
sessioncode: sessionStorage.getItem('sessioncode') || '',
+ 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone,
},
};
</code_context>
<issue_to_address>
**suggestion:** Client timezone detection may not be reliable in all environments.
Consider adding fallback logic or validation for environments where Intl.DateTimeFormat().resolvedOptions().timeZone is unavailable or inaccurate.
```suggestion
// Helper to safely get timezone, fallback to 'UTC'
function getClientTimezone(): string {
try {
const tz = Intl?.DateTimeFormat?.().resolvedOptions?.().timeZone;
if (typeof tz === 'string' && tz.length > 0) {
return tz;
}
} catch (e) {
// Ignore errors and fallback
}
return 'UTC';
}
headers: {
...headers,
sessioncode: sessionStorage.getItem('sessioncode') || '',
'x-timezone': getClientTimezone(),
},
};
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| export const getTimezone = (req: IncomingHttpHeaders) => { | ||
| return req['x-timezone'] || req['timezone'] || 'UTC'; |
There was a problem hiding this comment.
suggestion: Consider normalizing timezone header casing and value.
Direct string key access may miss headers with different casing. Normalize header keys to lowercase or use a header parsing library. Also, validate that the timezone value is a valid IANA timezone string to avoid errors.
| headers: { | ||
| ...headers, | ||
| sessioncode: sessionStorage.getItem('sessioncode') || '', | ||
| 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
suggestion: Client timezone detection may not be reliable in all environments.
Consider adding fallback logic or validation for environments where Intl.DateTimeFormat().resolvedOptions().timeZone is unavailable or inaccurate.
| headers: { | |
| ...headers, | |
| sessioncode: sessionStorage.getItem('sessioncode') || '', | |
| 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, | |
| }, | |
| }; | |
| // Helper to safely get timezone, fallback to 'UTC' | |
| function getClientTimezone(): string { | |
| try { | |
| const tz = Intl?.DateTimeFormat?.().resolvedOptions?.().timeZone; | |
| if (typeof tz === 'string' && tz.length > 0) { | |
| return tz; | |
| } | |
| } catch (e) { | |
| // Ignore errors and fallback | |
| } | |
| return 'UTC'; | |
| } | |
| headers: { | |
| ...headers, | |
| sessioncode: sessionStorage.getItem('sessioncode') || '', | |
| 'x-timezone': getClientTimezone(), | |
| }, | |
| }; |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to ad97300 in 1 minute and 24 seconds. Click for details.
- Reviewed
186lines of code in7files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. backend/erxes-api-shared/src/utils/headers/timezone.ts:4
- Draft comment:
Consider normalizing the header value (e.g. trimming whitespace) to ensure a clean timezone string. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. backend/plugins/operation_api/src/modules/cycle/utils.ts:182
- Draft comment:
Ensure the passed timezone is validated to avoid injecting invalid timezone strings into the aggregation pipeline. Consider providing a fallback (like 'UTC') if the timezone is not valid. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% 1. MongoDB's $dateToString handles invalid timezones by defaulting to UTC. 2. date-fns-tz's toZonedTime() also handles invalid timezones gracefully. 3. This is a speculative comment asking for validation without evidence of actual issues. 4. The comment starts with "Ensure that..." which is a red flag per our rules. I could be wrong about how MongoDB and date-fns-tz handle invalid timezones. There could be edge cases where invalid timezones cause runtime errors. Both MongoDB and date-fns-tz are well-tested libraries that handle timezone validation internally. If there were issues with invalid timezones, they would surface during normal testing. Delete this comment. It's speculative, starts with "Ensure that...", and suggests validation that's likely already handled by the underlying libraries.
3. backend/plugins/operation_api/src/modules/project/utils/charUtils.ts:38
- Draft comment:
Good incorporation of timezone conversion via toZonedTime. For clarity and type safety, consider explicitly typing the timezone parameter in the function signature. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. frontend/core-ui/src/providers/apollo-provider/apolloClient.ts:38
- Draft comment:
The 'x-timezone' header is set correctly using the browser's resolved timezone. Consider a fallback in case the browser does not support this feature. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. Intl.DateTimeFormat is supported in all modern browsers since IE11. 2. The code follows the same pattern as the sessioncode line above which also doesn't have a fallback. 3. If timezone detection fails, that likely indicates a serious browser compatibility issue that should be caught in testing. 4. The comment is speculative - "in case the browser does not support this feature" rather than identifying a real issue. The suggestion could prevent runtime errors in very old browsers. The fallback pattern does follow defensive programming practices. Given universal modern browser support and that this is a frontend app, the suggestion is overly cautious and doesn't address a real-world issue. Delete the comment as it suggests handling an edge case that is extremely unlikely in practice and doesn't represent a real issue.
Workflow ID: wflow_vbN5xGEm8QGRH4aN
You can customize 
| return {}; | ||
| } | ||
| const user: any = extractUserFromHeader(req.headers); | ||
| const timezone: any = getTimezone(req.headers); |
There was a problem hiding this comment.
Use an explicit type (string) for the timezone instead of 'any'.
| const timezone: any = getTimezone(req.headers); | |
| const timezone: string = getTimezone(req.headers); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
131-133: Return object when cycle is missing — keep GraphQL/consumer types consistentgetCycleProgressChart returns [] when no cycle is found; GraphQL schema exposes it as JSON and frontend expects an object shape { totalScope, chartData } — return an object instead to avoid runtime/type mismatches.
Locations:
- backend/plugins/operation_api/src/modules/cycle/utils.ts (if (!cycle) return [] — ~lines 131–133)
- GraphQL schema: backend/plugins/operation_api/src/modules/cycle/graphql/schemas/cycle.ts (getCycleProgressChart: JSON)
- Frontend consumer: frontend/plugins/operation_ui/src/modules/cycle/hooks/useGetCycleProgressChart.tsx (IGetCycleProgressChart)
Suggested change:
- if (!cycle) { - return []; - } + if (!cycle) { + return { totalScope: 0, chartData: [] }; + }
🧹 Nitpick comments (4)
frontend/core-ui/src/providers/apollo-provider/apolloClient.ts (2)
38-39: Always default timezone to a valid stringGuard against undefined and send a consistent value.
- 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone ?? 'UTC',
47-55: Propagate timezone to subscriptions (connectionParams)Resolvers reached via WS won’t see request headers. Pass timezone (and sessioncode) via connectionParams for parity.
export const wsLink = new GraphQLWsLink( createClient({ url: `${REACT_APP_API_URL}/graphql`, + connectionParams: () => ({ + sessioncode: sessionStorage.getItem('sessioncode') || '', + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone ?? 'UTC', + }), retryAttempts: 1000,Please confirm the server’s WS context reads connectionParams (graphql-ws) and that CORS/WS configs allow these fields.
backend/plugins/operation_api/src/modules/project/utils/charUtils.ts (1)
2-2: Avoid double timezone conversion; add safe defaultUsing toZonedTime here can double-shift if callers already zoned the base date. Prefer formatting directly in the target TZ and default timezone to 'UTC' to keep old call sites safe.
-import { toZonedTime } from 'date-fns-tz'; +import { formatInTimeZone } from 'date-fns-tz'; @@ -export const fillMissingDays = ( +export const fillMissingDays = ( data: { date: string; started: number; completed: number }[], baseDate: Date, totalDays = 7, - timezone: string, + timezone: string = 'UTC', ) => { @@ - const key = format(toZonedTime(date, timezone), 'yyyy-MM-dd'); + const key = formatInTimeZone(date, timezone, 'yyyy-MM-dd');Also applies to: 30-31, 38-38
backend/plugins/operation_api/src/modules/cycle/graphql/resolvers/queries/cycle.ts (1)
83-86: Defensive default when passing timezone to utilsEnsure stable behavior even if context lacks timezone.
- return getCycleProgressChart(_id, assigneeId, models, timezone); + return getCycleProgressChart(_id, assigneeId, models, timezone ?? 'UTC');Also confirm IContext.requestInfo.timezone is typed and always set in HTTP and WS contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/erxes-api-shared/src/utils/apollo/utils.ts(3 hunks)backend/erxes-api-shared/src/utils/headers/index.ts(1 hunks)backend/erxes-api-shared/src/utils/headers/timezone.ts(1 hunks)backend/plugins/operation_api/src/modules/cycle/graphql/resolvers/queries/cycle.ts(2 hunks)backend/plugins/operation_api/src/modules/cycle/utils.ts(5 hunks)backend/plugins/operation_api/src/modules/project/utils/charUtils.ts(2 hunks)frontend/core-ui/src/providers/apollo-provider/apolloClient.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
backend/erxes-api-shared/src/utils/headers/timezone.tsbackend/plugins/operation_api/src/modules/cycle/utils.tsbackend/erxes-api-shared/src/utils/apollo/utils.tsfrontend/core-ui/src/providers/apollo-provider/apolloClient.tsbackend/plugins/operation_api/src/modules/cycle/graphql/resolvers/queries/cycle.tsbackend/erxes-api-shared/src/utils/headers/index.tsbackend/plugins/operation_api/src/modules/project/utils/charUtils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
backend/erxes-api-shared/src/utils/headers/timezone.tsbackend/plugins/operation_api/src/modules/cycle/utils.tsbackend/erxes-api-shared/src/utils/apollo/utils.tsfrontend/core-ui/src/providers/apollo-provider/apolloClient.tsbackend/plugins/operation_api/src/modules/cycle/graphql/resolvers/queries/cycle.tsbackend/erxes-api-shared/src/utils/headers/index.tsbackend/plugins/operation_api/src/modules/project/utils/charUtils.ts
🧬 Code graph analysis (3)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
backend/plugins/operation_api/src/modules/project/utils/charUtils.ts (1)
fillMissingDays(26-58)
backend/erxes-api-shared/src/utils/apollo/utils.ts (1)
backend/erxes-api-shared/src/utils/headers/timezone.ts (1)
getTimezone(3-5)
backend/plugins/operation_api/src/modules/cycle/graphql/resolvers/queries/cycle.ts (1)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
getCycleProgressChart(115-256)
⏰ 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). (4)
- GitHub Check: Sourcery review
- GitHub Check: core-ui
- GitHub Check: frontline_api-ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
171-220: Validate timezone upstream to prevent Mongo aggregation errors$dateToString’s timezone throws on invalid values. Ensure getTimezone validation is in place (see shared util suggestion).
backend/erxes-api-shared/src/utils/headers/index.ts (1)
4-5: Barrel export OK — one consumer found; verify no circular importsFound: backend/erxes-api-shared/src/utils/apollo/utils.ts imports { extractUserFromHeader, getTimezone } from '../headers'.
Confirm backend/erxes-api-shared/src/utils/headers/{timezone.ts,user.ts} do not import from apollo or from modules that depend on apollo. Run:
rg -nP "from '\.\./headers'" backend --type=ts -C2
rg -n "apollo" backend --type=ts -C2
| import { IMainContext } from '../../core-types'; | ||
| import { extractUserFromHeader, getTimezone } from '../headers'; | ||
| import { getSubdomain } from '../utils'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Type timezone strictly; avoid any
getTimezone returns a string after validation; carry that type into context.
- const user: any = extractUserFromHeader(req.headers);
- const timezone: any = getTimezone(req.headers);
+ const user: any = extractUserFromHeader(req.headers);
+ const timezone: string = getTimezone(req.headers);Please verify IContext['requestInfo'] includes timezone: string.
Also applies to: 25-26, 41-44
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\binterface\s+IContext\b' --typescript
rg -nP -C2 'requestInfo\s*:\s*{[^}]*}' --typescriptLength of output: 873
🏁 Script executed:
#!/bin/bash
set -e
# Display first part of the utils.ts file to confirm context usage
sed -n '1,200p' backend/erxes-api-shared/src/utils/apollo/utils.ts
# Search for interface definitions of IContext or IMainContext in .ts files
rg -nP '\binterface\s+(IContext|IMainContext)\b' -g '*.ts' -C2
# Locate IMainContext definition in core-types directory
rg -nP 'export\s+interface\s+IMainContext' -g 'backend/erxes-api-shared/src/core-types/**/*.ts'Length of output: 7065
🏁 Script executed:
#!/bin/bash
set -e
echo "== core-types/common.ts =="
sed -n '1,240p' backend/erxes-api-shared/src/core-types/common.ts || true
echo
echo "== utils/headers.ts =="
sed -n '1,240p' backend/erxes-api-shared/src/utils/headers.ts || true
echo
echo "== rg getTimezone matches =="
rg -n "getTimezone" -S || true
echo
echo "== rg extractUserFromHeader matches =="
rg -n "extractUserFromHeader" -S || true
echo
echo "== rg requestInfo matches =="
rg -n "requestInfo" -S || true
echo
echo "== gateway/connectionResolver.ts =="
sed -n '1,240p' backend/gateway/src/connectionResolver.ts || trueLength of output: 6772
🏁 Script executed:
#!/bin/bash
set -e
echo "== headers directory =="
ls -la backend/erxes-api-shared/src/utils/headers || true
echo
echo "== timezone.ts =="
sed -n '1,240p' backend/erxes-api-shared/src/utils/headers/timezone.ts || true
echo
echo "== user.ts =="
sed -n '1,240p' backend/erxes-api-shared/src/utils/headers/user.ts || true
echo
echo "== headers index (if present) =="
sed -n '1,240p' backend/erxes-api-shared/src/utils/headers/index.ts || trueLength of output: 1595
Enforce timezone as string across context
getTimezone can return string|string[]; coerce it to a string, declare its return type, and propagate strict typing into the context and core types.
- backend/erxes-api-shared/src/utils/headers/timezone.ts — change to return string (handle Array by taking first element) and add a return type.
- backend/erxes-api-shared/src/utils/apollo/utils.ts — change the local type:
- const timezone: any = getTimezone(req.headers);
+ const timezone: string = getTimezone(req.headers);- backend/erxes-api-shared/src/core-types/common.ts — replace requestInfo: any with a typed shape that includes timezone: string (and the used fields like secure, cookies, headers).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/erxes-api-shared/src/utils/apollo/utils.ts around lines 4 to 7,
ensure timezone is treated as a string throughout: change
backend/erxes-api-shared/src/utils/headers/timezone.ts to explicitly return
string (if getTimezone receives an array, take the first element) and add the
return type annotation; update backend/erxes-api-shared/src/core-types/common.ts
to replace requestInfo: any with a concrete typed shape that includes timezone:
string plus the used fields (secure, cookies, headers); then update utils.ts to
call the newly typed getTimezone (or coerce its result with String(...) / take
first element) and propagate the strict timezone:string into the
IMainContext/requestInfo usage so all types align.
| export const getTimezone = (req: IncomingHttpHeaders) => { | ||
| return req['x-timezone'] || req['timezone'] || 'UTC'; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sanitize, normalize, and validate timezone header
IncomingHttpHeaders values can be string | string[] | undefined. Also validate IANA timezones to avoid Mongo $dateToString failures on bad input.
-export const getTimezone = (req: IncomingHttpHeaders) => {
- return req['x-timezone'] || req['timezone'] || 'UTC';
-};
+export const getTimezone = (headers: IncomingHttpHeaders): string => {
+ const raw = headers['x-timezone'] ?? headers['timezone'];
+ const value = Array.isArray(raw) ? raw[0] : raw;
+ if (typeof value !== 'string') return 'UTC';
+ try {
+ // Intl throws RangeError for invalid time zones
+ new Intl.DateTimeFormat('en-US', { timeZone: value }).format();
+ return value;
+ } catch {
+ return 'UTC';
+ }
+};📝 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.
| export const getTimezone = (req: IncomingHttpHeaders) => { | |
| return req['x-timezone'] || req['timezone'] || 'UTC'; | |
| }; | |
| export const getTimezone = (headers: IncomingHttpHeaders): string => { | |
| const raw = headers['x-timezone'] ?? headers['timezone']; | |
| const value = Array.isArray(raw) ? raw[0] : raw; | |
| if (typeof value !== 'string') return 'UTC'; | |
| try { | |
| // Intl throws RangeError for invalid time zones | |
| new Intl.DateTimeFormat('en-US', { timeZone: value }).format(); | |
| return value; | |
| } catch { | |
| return 'UTC'; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In backend/erxes-api-shared/src/utils/headers/timezone.ts around lines 3 to 5,
the current getter returns raw header values which may be string | string[] |
undefined and may contain invalid timezones; update it to normalize and
validate: if header is an array, pick the first element; coerce to string, trim
and uppercase/lowercase as appropriate; validate the candidate timezone using a
reliable IANA check (e.g. attempt to construct an Intl.DateTimeFormat with the
timeZone option and catch RangeError) and only return it when valid; otherwise
fall back to 'UTC'. Ensure the function returns a string and handles undefined
safely.
| const start = toZonedTime(cycle.startDate, timezone); | ||
| const end = toZonedTime(cycle.endDate, timezone); | ||
|
|
||
| const days = differenceInCalendarDays(end, start) + 1; | ||
|
|
||
| chartData.chartData = fillMissingDays( | ||
| chartDataAggregation, | ||
| cycle.startDate, | ||
| start, | ||
| days, | ||
| timezone, | ||
| ); |
There was a problem hiding this comment.
Fix double timezone conversion when filling gaps
You zone start/end, then fillMissingDays zones again, shifting dates twice. Pass an unzoned base into fillMissingDays and keep days computed from zoned bounds.
- const start = toZonedTime(cycle.startDate, timezone);
- const end = toZonedTime(cycle.endDate, timezone);
-
- const days = differenceInCalendarDays(end, start) + 1;
-
- chartData.chartData = fillMissingDays(
- chartDataAggregation,
- start,
- days,
- timezone,
- );
+ const startZoned = toZonedTime(cycle.startDate, timezone);
+ const endZoned = toZonedTime(cycle.endDate, timezone);
+
+ const days = differenceInCalendarDays(endZoned, startZoned) + 1;
+
+ chartData.chartData = fillMissingDays(
+ chartDataAggregation,
+ cycle.startDate, // unzoned base; formatting happens in TZ inside fillMissingDays
+ days,
+ timezone,
+ );📝 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.
| const start = toZonedTime(cycle.startDate, timezone); | |
| const end = toZonedTime(cycle.endDate, timezone); | |
| const days = differenceInCalendarDays(end, start) + 1; | |
| chartData.chartData = fillMissingDays( | |
| chartDataAggregation, | |
| cycle.startDate, | |
| start, | |
| days, | |
| timezone, | |
| ); | |
| const startZoned = toZonedTime(cycle.startDate, timezone); | |
| const endZoned = toZonedTime(cycle.endDate, timezone); | |
| const days = differenceInCalendarDays(endZoned, startZoned) + 1; | |
| chartData.chartData = fillMissingDays( | |
| chartDataAggregation, | |
| cycle.startDate, // unzoned base; formatting happens in TZ inside fillMissingDays | |
| days, | |
| timezone, | |
| ); |
🤖 Prompt for AI Agents
In backend/plugins/operation_api/src/modules/cycle/utils.ts around lines 243 to
253, the code first converts cycle.startDate and cycle.endDate to the timezone
with toZonedTime and then passes those zoned dates into fillMissingDays which
also applies timezone conversion, causing dates to be shifted twice; instead
pass the unzoned base start (use the original cycle.startDate) into
fillMissingDays while keeping the days count computed from the already-zoned
start/end, so compute startZoned = toZonedTime(...), endZoned =
toZonedTime(...), days = differenceInCalendarDays(endZoned, startZoned) + 1, and
call fillMissingDays with the original unzoned start date and the computed days
and timezone.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1edb77a in 31 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx:110
- Draft comment:
Inline activeDot config: consider memoizing this object to avoid recreating it on each render if performance becomes an issue. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx:121
- Draft comment:
Inline activeDot config: consider memoizing this object to avoid recreating it on each render if performance becomes an issue. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_WQ7MX4Zk8gXz0RGJ
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx (3)
110-110: Active dots: nice UX improvement; add a subtle ring for contrastOn light/dark themes and over translucent areas, a small stroke improves visibility and accessibility of the active dot.
Apply this diff:
- activeDot={{ fill: STATUS_COLORS.started, r: 4 }} + activeDot={{ fill: STATUS_COLORS.started, r: 4, stroke: 'hsl(var(--background))', strokeWidth: 1.5 }} … - activeDot={{ fill: STATUS_COLORS.completed, r: 4 }} + activeDot={{ fill: STATUS_COLORS.completed, r: 4, stroke: 'hsl(var(--background))', strokeWidth: 1.5 }}Also applies to: 121-121
75-77: Avoid gradient ID collisions when multiple charts render
id="startedGradient"/completedGradient"(and fill variants) are global in the DOM. Rendering two charts on a page will collide and produce incorrect gradients. Use a unique suffix viauseId()and pass it to both the defs and theurl(#...)references.Apply this diff:
import { CHART_CONFIG, STATUS_COLORS } from '@/cycle/constants'; +import { useId } from 'react'; … export const CycleProgressChart = ({ … }) => { + const gradientId = useId(); … - <ProgressChartGradients data={chartData} /> + <ProgressChartGradients data={chartData} idSuffix={gradientId} /> … - <Area + <Area dataKey="started" type="monotone" - stroke="url(#startedGradient)" - fill="url(#startedFillGradient)" + stroke={`url(#startedGradient-${gradientId})`} + fill={`url(#startedFillGradient-${gradientId})`} … - <Area + <Area dataKey="completed" type="monotone" - stroke="url(#completedGradient)" - fill="url(#completedFillGradient)" + stroke={`url(#completedGradient-${gradientId})`} + fill={`url(#completedFillGradient-${gradientId})`} … -export const ProgressChartGradients = ({ - data = [], -}: { - data: { date: string; started: number; completed: number; future: boolean }[]; -}) => { +export const ProgressChartGradients = ({ + data = [], + idSuffix, +}: { + data: { date: string; started: number; completed: number; future: boolean }[]; + idSuffix: string; +}) => { … - <linearGradient id={`${line}Gradient`} x1="0" y1="0" x2="1" y2="0"> + <linearGradient id={`${line}Gradient-${idSuffix}`} x1="0" y1="0" x2="1" y2="0"> … - <linearGradient id={`${line}FillGradient`} x1="0" y1="0" x2="1" y2="0"> + <linearGradient id={`${line}FillGradient-${idSuffix}`} x1="0" y1="0" x2="1" y2="0">Also applies to: 104-106, 115-117, 139-154, 156-172
60-62: Normalize the baseline date format to match aggregated dataBackend now emits day keys via
$dateToString(date-only). Prepending an ISO timestamp can introduce subtle parsing/formatting inconsistencies. Use a date-only string for the baseline row.- chartData.unshift({ - date: yesterdayEnd.toISOString(), + chartData.unshift({ + date: format(yesterdayEnd, 'yyyy-MM-dd'),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
Files:
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.
Files:
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx
**/*.{webp,tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Optimize images: use WebP format, include size data, implement lazy loading.
Files:
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx
🧬 Code graph analysis (1)
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx (1)
frontend/plugins/operation_ui/src/modules/cycle/constants.ts (1)
STATUS_COLORS(3-7)
⏰ 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). (6)
- GitHub Check: payment_api-ci
- GitHub Check: core-api-ci
- GitHub Check: Sourcery review
- GitHub Check: sales_api-ci
- GitHub Check: core-ui
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b1a6190 in 38 seconds. Click for details.
- Reviewed
76lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. backend/erxes-api-shared/src/utils/apollo/utils.ts:25
- Draft comment:
Good explicit typing: using 'string' for timezone improves type safety. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. backend/erxes-api-shared/src/utils/headers/timezone.ts:3
- Draft comment:
Nice handling of possible array values in timezone headers. Consider validating the timezone string format if needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. backend/plugins/operation_api/src/modules/cycle/utils.ts:248
- Draft comment:
Removed the timezone parameter in the call to fillMissingDays. Ensure that the baseDate has already been converted to the correct timezone. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. backend/plugins/operation_api/src/modules/project/utils/charUtils.ts:36
- Draft comment:
Removed timezone conversion in fillMissingDays (using toZonedTime) to rely on caller conversion. Ensure this aligns with overall timezone handling. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_9K5YpLDbUHAcXICn
You can customize
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 (3)
backend/erxes-api-shared/src/utils/apollo/utils.ts(3 hunks)backend/erxes-api-shared/src/utils/headers/timezone.ts(1 hunks)backend/plugins/operation_api/src/modules/cycle/utils.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/erxes-api-shared/src/utils/apollo/utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
backend/plugins/operation_api/src/modules/cycle/utils.tsbackend/erxes-api-shared/src/utils/headers/timezone.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
backend/plugins/operation_api/src/modules/cycle/utils.tsbackend/erxes-api-shared/src/utils/headers/timezone.ts
🧬 Code graph analysis (1)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
backend/plugins/operation_api/src/modules/project/utils/charUtils.ts (1)
fillMissingDays(25-56)
⏰ 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). (3)
- GitHub Check: Sourcery review
- GitHub Check: core-ui
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
backend/plugins/operation_api/src/modules/cycle/utils.ts (1)
182-186: Using $dateToString with timezone is correct for TZ-aware bucketing. Verify upstream TZ is validated.This looks good, but if an invalid timezone string slips through, the aggregation will error. Ensure getTimezone() validation is in place before reaching here.
backend/erxes-api-shared/src/utils/headers/timezone.ts (2)
3-11: Harden timezone extraction (validate IANA, handle string[], trim; align with style guide).Current code can pass an invalid timezone into Mongo’s $dateToString and crash requests; it also doesn’t sanitize string[]/whitespace. Switch to a validated IANA timezone with UTC fallback and use a function declaration per guidelines.
-import { IncomingHttpHeaders } from 'http'; - -export const getTimezone = (req: IncomingHttpHeaders): string => { - const timezone = req['x-timezone'] || req['timezone'] || 'UTC'; - - if (Array.isArray(timezone)) { - return timezone[0]; - } - - return timezone; -}; +import { IncomingHttpHeaders } from 'http'; + +export function getTimezone(headers: IncomingHttpHeaders): string { + const raw = headers['x-timezone'] ?? headers['timezone']; + const candidate = Array.isArray(raw) ? raw[0] : raw; + if (typeof candidate !== 'string') return 'UTC'; + const tz = candidate.trim(); + if (!tz) return 'UTC'; + try { + // Throws RangeError for invalid time zones + new Intl.DateTimeFormat('en-US', { timeZone: tz }).format(); + return tz; + } catch { + return 'UTC'; + } +}
3-11: Re-export and Apollo context wiring verified — no action required.backend/erxes-api-shared/src/utils/headers/index.ts exports getTimezone; backend/erxes-api-shared/src/utils/apollo/utils.ts calls getTimezone(req.headers) and sets requestInfo.timezone; generateApolloContext is used in backend/erxes-api-shared/src/utils/start-plugin.ts and backend/core-api/src/apollo/apolloServer.ts, so downstream resolvers receive the timezone.
| timezone: string, | ||
| ) => { |
There was a problem hiding this comment.
🧩 Analysis chain
Provide a safe default for the timezone parameter.
Guard callers that haven’t been migrated by defaulting to 'UTC'.
-export const getCycleProgressChart = async (
+export const getCycleProgressChart = async (
cycleId: string,
assigneeId: string | undefined,
models: IModels,
- timezone: string,
+ timezone: string = 'UTC',
) => {Run this to find any remaining call sites not passing a TZ:
🏁 Script executed:
#!/bin/bash
rg -n --type=ts 'getCycleProgressChart\(' -C2Length of output: 2668
Default timezone parameter to 'UTC' in getCycleProgressChart
Default timezone to 'UTC' so callers that haven't been migrated don't break — resolver already passes timezone, but there is at least one internal call that omits it.
-export const getCycleProgressChart = async (
+export const getCycleProgressChart = async (
cycleId: string,
assigneeId: string | undefined,
models: IModels,
- timezone: string,
+ timezone: string = 'UTC',
) => {Remaining caller without TZ: backend/plugins/operation_api/src/modules/cycle/db/models/Cycle.ts:124 (const chartData = await getCycleProgressChart(_id, undefined, models);)
📝 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.
| timezone: string, | |
| ) => { | |
| export const getCycleProgressChart = async ( | |
| cycleId: string, | |
| assigneeId: string | undefined, | |
| models: IModels, | |
| timezone: string = 'UTC', | |
| ) => { |
🤖 Prompt for AI Agents
In backend/plugins/operation_api/src/modules/cycle/utils.ts around lines 119 to
120, the getCycleProgressChart function's timezone parameter is currently
required which breaks internal callers that omit it; update the function
signature to default timezone to 'UTC' (e.g., timezone = 'UTC') so callers that
don't pass a timezone continue to behave correctly, and ensure any internal
calls relying on undefined still work without modification.
| const start = toZonedTime(cycle.startDate, timezone); | ||
| const end = toZonedTime(cycle.endDate, timezone); | ||
|
|
||
| const days = differenceInCalendarDays(end, start) + 1; | ||
|
|
||
| chartData.chartData = fillMissingDays( | ||
| chartDataAggregation, | ||
| cycle.startDate, | ||
| days, | ||
| ); | ||
| chartData.chartData = fillMissingDays(chartDataAggregation, start, days); | ||
|
|
There was a problem hiding this comment.
Avoid double TZ conversion and DST drift; compute days on zoned midnights and pass an unzoned base into fillMissingDays (TZ-aware).
- Normalize start/end to midnight in the target TZ to prevent off‑by‑one around DST.
- If fillMissingDays formats dates in TZ, pass the original (unzoned) base plus the timezone; otherwise you risk double conversion or server-TZ formatting.
-import { differenceInCalendarDays } from 'date-fns';
+import { differenceInCalendarDays, startOfDay } from 'date-fns';
import { toZonedTime } from 'date-fns-tz';
@@
- const start = toZonedTime(cycle.startDate, timezone);
- const end = toZonedTime(cycle.endDate, timezone);
-
- const days = differenceInCalendarDays(end, start) + 1;
-
- chartData.chartData = fillMissingDays(chartDataAggregation, start, days);
+ const startZoned = startOfDay(toZonedTime(cycle.startDate, timezone));
+ const endZoned = startOfDay(toZonedTime(cycle.endDate, timezone));
+
+ const days = differenceInCalendarDays(endZoned, startZoned) + 1;
+
+ // Pass unzoned base and timezone to avoid double-shifting inside fillMissingDays
+ chartData.chartData = fillMissingDays(
+ chartDataAggregation,
+ cycle.startDate,
+ days,
+ timezone,
+ );If fillMissingDays isn’t TZ-aware yet, update it like below (outside this file):
// backend/plugins/operation_api/src/modules/project/utils/charUtils.ts
import { addDays } from 'date-fns';
import { formatInTimeZone } from 'date-fns-tz';
export function fillMissingDays(
data: { date: string; started: number; completed: number }[],
baseDate: Date,
totalDays = 7,
timezone = 'UTC',
) {
const filled: typeof data = [];
const index = new Map(data.map(d => [d.date, d]));
for (let i = 0; i < totalDays; i++) {
const date = addDays(baseDate, i);
const key = formatInTimeZone(date, timezone, 'yyyy-MM-dd');
const item = index.get(key) ?? filled[filled.length - 1] ?? { started: 0, completed: 0, date: key };
filled.push(item.date === key ? item : { date: key, started: item.started, completed: item.completed });
}
return filled;
}
Summary by Sourcery
Propagate client timezone through GraphQL headers to cycle utilities and introduce timezone-aware date handling for cycle progress charts
New Features:
Enhancements:
Chores:
Summary by CodeRabbit
Important
Propagate client timezone through headers for timezone-aware cycle progress charts in both backend and frontend.
getTimezoneintimezone.ts.generateApolloContextinutils.ts.getCycleProgressChartincycle.tsandutils.tsto use timezone in MongoDB aggregations and date conversions.x-timezoneheader in Apollo client requests inapolloClient.ts.CycleProgressChart.tsxto show active data-point markers for started and completed series.index.ts.This description was created by
for b1a6190. You can customize this summary. It will automatically update as commits are pushed.