-
Notifications
You must be signed in to change notification settings - Fork 3
Timezone aware #372
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
base: main
Are you sure you want to change the base?
Timezone aware #372
Conversation
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.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Reviewer's GuideThis PR makes cycle checks and progress charts fully timezone-aware, introduces a timezone selector in general settings, updates routing and layout for the settings page, tweaks the chart styling, and adjusts the job scheduler frequency. Class diagram for updated general settings form and schemaclassDiagram
class TGeneralSettingsProps {
+string TIMEZONE
...
}
class generalSettingsSchema {
+TIMEZONE: string
...
}
class SelectTimezone {
+render()
}
TGeneralSettingsProps <|-- generalSettingsSchema
SelectTimezone --> TGeneralSettingsProps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe PR introduces timezone-aware handling across cycle utilities and workers, updates scheduling to hourly checks, enables the General settings route with a new timezone field, adjusts project chart utilities to use Moment, and adds minor UI tweaks to a cycle progress chart. It also adds a new SelectTimezone component. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Cron as Scheduler (cron: 0 * * * *)
participant W as Worker
participant Org as Organizations
participant Cfg as Config (TIMEZONE)
participant Q as Queue (checkCycle)
participant Job as Worker: checkCycle
participant DB as DB (Cycles)
Cron->>W: Trigger operations-daily-cycles-check (hourly)
W->>Org: Fetch organizations
alt SaaS
loop per org
W->>Q: Enqueue checkCycle { subdomain, timezone }
end
else Self-hosted
W->>Cfg: Get TIMEZONE
W->>Q: Enqueue checkCycle { subdomain: "os", timezone }
end
Q->>Job: Start job
Job->>Job: tzToday = now.tz(timezone)
Job-->>Q: Exit if tzToday.hour != 0
Job->>Job: utcStart/utcEnd from tzToday
par End cycles
Job->>DB: Find cycles endDate ∈ [utcStart, utcEnd], isActive, !isCompleted
and Start cycles
Job->>DB: Find cycles startDate ∈ [utcStart, utcEnd], !isActive, !isCompleted
end
Job-->>Q: Complete
sequenceDiagram
autonumber
participant CU as Cycle Utils
participant Cfg as Config (TIMEZONE via TRPC)
participant DB as DB (Aggregation)
CU->>Cfg: sendTRPCMessage(get TIMEZONE)
Cfg-->>CU: timezone
CU->>DB: Aggregate with $dateToString (timezone) for daily buckets
DB-->>CU: Aggregates grouped by YYYY-MM-DD (tz)
CU->>CU: Fill missing days using Moment baseDate & totalDays
CU-->>Caller: Chart data with tz-aligned dates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
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.
Hey there - I've reviewed your changes - here's some feedback:
- getCycleProgressChart fetches the TIMEZONE config on every call—consider caching it to avoid repeated RPC overhead.
- You’re mixing date-fns and moment-timezone throughout; it may simplify maintenance to standardize on a single date library.
- Running the daily cycle check hourly for all tenants adds load—think about dynamic scheduling per timezone or a more targeted trigger instead of every hour.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- getCycleProgressChart fetches the TIMEZONE config on every call—consider caching it to avoid repeated RPC overhead.
- You’re mixing date-fns and moment-timezone throughout; it may simplify maintenance to standardize on a single date library.
- Running the daily cycle check hourly for all tenants adds load—think about dynamic scheduling per timezone or a more targeted trigger instead of every hour.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Important
Looks good to me! 👍
Reviewed everything up to 1db30ac in 1 minute and 45 seconds. Click for details.
- Reviewed
500lines of code in13files - Skipped
0files when reviewing. - Skipped posting
9draft 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/plugins/operation_api/src/modules/cycle/utils.ts:177
- Draft comment:
Refactored aggregation to use $dateToString with timezone. Ensure the 'timezone' config is validated and in the expected format to avoid incorrect date conversion. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. backend/plugins/operation_api/src/modules/cycle/utils.ts:253
- Draft comment:
Using moment-timezone for 'start' and 'end' date conversion is a good approach. Verify that cycle.startDate and cycle.endDate are in a compatible format for proper timezone adjustments. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. backend/plugins/operation_api/src/worker/index.ts:13
- Draft comment:
The cron pattern was changed to '0 * * * *', which schedules jobs hourly. Since checkCycle only proceeds when tzToday.hour() is zero, consider using a daily cron pattern to avoid needless job invocations. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment makes a specific claim about checkCycle's behavior that would justify keeping the daily pattern, but I cannot verify this claim. The function implementation is not shown. Without seeing the actual implementation of checkCycle, I cannot be certain that the comment's reasoning is correct. The name 'daily-cycles-check' suggests daily checks, but names can be misleading. I might be too hesitant - the queue name and worker name both strongly suggest daily operations. Also, changing from daily to hourly is a significant change that could impact system resources. While the names suggest daily operations, without seeing the actual implementation of checkCycle, I cannot verify the comment's specific claim about tzToday.hour(). Names and assumptions are not strong enough evidence. The comment should be deleted because we lack strong evidence to verify its claim about checkCycle's implementation, following the principle of requiring strong evidence to keep comments.
4. backend/plugins/operation_api/src/worker/dailyCheckCycles.ts:50
- Draft comment:
The checkCycle function returns early if the current hour isn't 0. Combined with an hourly trigger, this may lead to unnecessary job invocations. Consider refining the scheduling so the job only runs at midnight. - 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% The comment makes a valid point about efficiency - if this job runs hourly but only does work at hour 0, that's 23 wasted invocations per day. However, we don't actually know the current scheduling setup. The comment is speculative about there being "hourly triggers". The timezone handling suggests this is intentionally checking frequently to handle different timezones. I don't have visibility into how this job is actually scheduled. The hour=0 check might be intentional to handle timezone edge cases. While the comment raises a valid efficiency concern, the current implementation might be intentionally designed this way to properly handle timezone transitions and edge cases. The comment is speculative without knowing the scheduling setup, and the current implementation might be intentional for timezone handling. We should not keep speculative comments.
5. frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx:110
- Draft comment:
The addition of activeDot properties for the 'started' and 'completed' areas enhances chart interactivity. Confirm that these visual tweaks align with the overall design guidelines. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that the visual changes align with design guidelines, which is against the rules. It doesn't provide a specific suggestion or point out a potential issue with the code itself.
6. frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx:127
- Draft comment:
GeneralSettings route has been re-enabled. Ensure that its inclusion does not conflict with other settings routes and that the component loads as expected. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that the inclusion of the GeneralSettings route does not conflict with other settings routes and that the component loads as expected. This falls under the category of asking the author to double-check things and ensure behavior, which is against the rules.
7. frontend/core-ui/src/modules/settings/constants/data.ts:125
- Draft comment:
A new 'General' navigation item has been added. Verify that its icon and routing path correctly match the design requirements. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
8. frontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx:26
- Draft comment:
When merging existing configs with new settings using the spread operator, ensure that no critical or sensitive configurations are inadvertently overwritten. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that no critical or sensitive configurations are overwritten, which is a form of asking them to double-check their work. This violates the rule against asking the author to ensure behavior is intended or to double-check things.
9. frontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsx:18
- Draft comment:
Default TIMEZONE is set using detectTimeZone. Please confirm that detectTimeZone returns the expected timezone string format across different environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm the behavior of the detectTimeZone function across different environments. This falls under the rule of not asking the author to confirm or ensure behavior, which is not allowed.
Workflow ID: wflow_1189hUN0F6Jouxwb
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx (1)
17-17: Replaceanytype with a proper interface.The
statisticsprop is typed asany, which bypasses TypeScript's type checking and violates the coding guideline to "Use TypeScript for all code; prefer interfaces over types."Based on the usage at line 25 where
statistics.chartDatais cast toIGetCycleProgressChart, define a proper interface:+interface ICycleStatistics { + chartData?: IGetCycleProgressChart; +} + export const CycleProgressChart = ({ cycleId, isCompleted, statistics, }: { cycleId: string; isCompleted: boolean; - statistics: any; + statistics: ICycleStatistics; }) => {As per coding guidelines.
🧹 Nitpick comments (3)
frontend/core-ui/src/modules/settings/general/schema/index.ts (1)
13-13: Consider adding timezone format validation.The TIMEZONE field accepts any string without validating against the IANA timezone format (e.g., 'America/New_York'). While the SelectTimezone component constrains user input to valid IANA timezones, adding schema-level validation would provide an additional safety layer against invalid data from other sources.
Apply this diff to add format validation:
- TIMEZONE: z.string(), + TIMEZONE: z.string().regex(/^[A-Za-z]+\/[A-Za-z_]+$/, 'Invalid timezone format'),Alternatively, if you have a list of valid IANA timezones, use
z.enum()for stricter validation.backend/plugins/operation_api/src/modules/project/utils/charUtils.ts (1)
36-37: Consider migrating other date utility functions to Moment for consistency.The
fillMissingDaysfunction now uses Moment, but other functions in this file (fillUntilTargetDate,fillFromLastDate,fillMissingDaysWithStartDate) still use date-fns. This creates inconsistency in the codebase. Consider migrating all date utilities to use Moment consistently for timezone-aware operations.frontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsx (1)
5-25: Add TypeScript return type annotation.The component follows the coding guidelines for functional components but lacks an explicit return type. Adding a return type improves type safety and makes the component's contract clearer.
Apply this diff to add the return type:
-export function SelectTimezone() { +export function SelectTimezone(): JSX.Element { const form = useFormContext<TGeneralSettingsProps>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/plugins/operation_api/src/modules/cycle/utils.ts(5 hunks)backend/plugins/operation_api/src/modules/project/utils/charUtils.ts(2 hunks)backend/plugins/operation_api/src/worker/dailyCheckCycles.ts(3 hunks)backend/plugins/operation_api/src/worker/index.ts(1 hunks)frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx(2 hunks)frontend/core-ui/src/modules/settings/constants/data.ts(2 hunks)frontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx(4 hunks)frontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsx(1 hunks)frontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsx(2 hunks)frontend/core-ui/src/modules/settings/general/schema/index.ts(1 hunks)frontend/core-ui/src/modules/types/paths/SettingsPath.ts(1 hunks)frontend/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsx(1 hunks)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/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsxfrontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsxfrontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsxfrontend/core-ui/src/modules/app/components/SettingsRoutes.tsxfrontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsxfrontend/core-ui/src/modules/types/paths/SettingsPath.tsbackend/plugins/operation_api/src/modules/cycle/utils.tsfrontend/core-ui/src/modules/settings/general/schema/index.tsbackend/plugins/operation_api/src/worker/index.tsbackend/plugins/operation_api/src/modules/project/utils/charUtils.tsfrontend/core-ui/src/modules/settings/constants/data.tsbackend/plugins/operation_api/src/worker/dailyCheckCycles.tsfrontend/core-ui/src/modules/settings/general/components/GeneralSettings.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/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsxfrontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsxfrontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsxfrontend/core-ui/src/modules/app/components/SettingsRoutes.tsxfrontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsxfrontend/core-ui/src/modules/types/paths/SettingsPath.tsbackend/plugins/operation_api/src/modules/cycle/utils.tsfrontend/core-ui/src/modules/settings/general/schema/index.tsbackend/plugins/operation_api/src/worker/index.tsbackend/plugins/operation_api/src/modules/project/utils/charUtils.tsfrontend/core-ui/src/modules/settings/constants/data.tsbackend/plugins/operation_api/src/worker/dailyCheckCycles.tsfrontend/core-ui/src/modules/settings/general/components/GeneralSettings.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/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsxfrontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsxfrontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsxfrontend/core-ui/src/modules/app/components/SettingsRoutes.tsxfrontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsxfrontend/core-ui/src/modules/settings/general/components/GeneralSettings.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/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsxfrontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsxfrontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsxfrontend/core-ui/src/modules/app/components/SettingsRoutes.tsxfrontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsxfrontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx
**/*.{webp,tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Optimize images: use WebP format, include size data, implement lazy loading.
Files:
frontend/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsxfrontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsxfrontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsxfrontend/core-ui/src/modules/app/components/SettingsRoutes.tsxfrontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsxfrontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx
🧬 Code graph analysis (7)
frontend/core-ui/src/pages/settings/workspace/GeneralSettingsPage.tsx (2)
frontend/core-ui/src/modules/settings/components/SettingsBreadcrumbs.tsx (1)
SettingsBreadcrumbs(7-27)frontend/libs/erxes-ui/src/components/scroll-area.tsx (1)
ScrollArea(77-81)
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)
frontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsx (1)
frontend/libs/erxes-ui/src/utils/localization/detectTimeZone.ts (1)
detectTimeZone(5-6)
frontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsx (2)
frontend/libs/erxes-ui/src/components/form.tsx (1)
Form(166-174)frontend/libs/erxes-ui/src/modules/inputs/components/TimezoneSelect.tsx (1)
TimezoneSelect(7-46)
backend/plugins/operation_api/src/modules/cycle/utils.ts (2)
backend/erxes-api-shared/src/utils/trpc/index.ts (1)
sendTRPCMessage(42-68)backend/plugins/operation_api/src/modules/project/utils/charUtils.ts (1)
fillMissingDays(26-57)
backend/plugins/operation_api/src/worker/dailyCheckCycles.ts (2)
backend/erxes-api-shared/src/utils/mq-worker.ts (1)
sendWorkerQueue(47-50)backend/erxes-api-shared/src/utils/trpc/index.ts (1)
sendTRPCMessage(42-68)
frontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx (1)
frontend/core-ui/src/modules/settings/general/components/SelectTimezone.tsx (1)
SelectTimezone(5-25)
🪛 ESLint
frontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx
[error] 65-65: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
⏰ 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 (6)
frontend/plugins/operation_ui/src/modules/cycle/components/detail/CycleProgressChart.tsx (1)
110-110: LGTM! Consistent active dot styling.The
activeDotprops enhance interactivity by displaying visible indicators on hover/selection for the "started" and "completed" series. The fill colors correctly referenceSTATUS_COLORS, and the radius of 4 provides good visibility without overwhelming the chart.Also applies to: 121-121
frontend/core-ui/src/modules/types/paths/SettingsPath.ts (1)
11-11: LGTM!The General settings path is now properly enabled and follows the existing pattern for other workspace settings paths.
frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (2)
35-39: LGTM!The lazy loading implementation follows the established pattern and coding guidelines for dynamic loading of non-critical components.
126-129: LGTM!The route registration is properly configured and follows the existing pattern for other workspace settings routes.
frontend/core-ui/src/modules/settings/general/hooks/useGeneralSettingsForms.tsx (1)
18-18: LGTM!Using
detectTimeZone()provides a sensible default that automatically detects the user's local timezone, improving user experience by pre-populating the field with their current timezone.frontend/core-ui/src/modules/settings/constants/data.ts (1)
124-128: LGTM!The General settings navigation item is properly configured and follows the established pattern for other workspace settings navigation entries.
| import { sendTRPCMessage } from 'erxes-api-shared/utils'; | ||
| import { tz } from 'moment-timezone'; |
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.
Fix Moment Timezone import to avoid compilation failure
moment-timezone doesn’t expose tz as a named export, so the build breaks (TypeScript reports “Module 'moment-timezone' has no exported member 'tz'”). Switch to the default moment import and call moment.tz(...) instead on Line 253 and Line 254.
Apply this diff:
-import { sendTRPCMessage } from 'erxes-api-shared/utils';
-import { tz } from 'moment-timezone';
+import { sendTRPCMessage } from 'erxes-api-shared/utils';
+import moment from 'moment-timezone';
@@
- const start = tz(new Date(cycle.startDate), timezone);
- const end = tz(new Date(cycle.endDate), timezone);
+ const start = moment.tz(new Date(cycle.startDate), timezone);
+ const end = moment.tz(new Date(cycle.endDate), timezone);Also applies to: 253-259
🤖 Prompt for AI Agents
In backend/plugins/operation_api/src/modules/cycle/utils.ts around lines 3-4 and
253-259, the file currently imports tz as a named export from moment-timezone
which TypeScript rejects; replace that import with the default moment import
(e.g., import moment from 'moment-timezone') and update all occurrences where
tz(...) is used on lines 253-259 to call moment.tz(...), ensuring any type
annotations/import adjustments are updated accordingly.
| export const fillMissingDays = ( | ||
| data: { date: string; started: number; completed: number }[], | ||
| baseDate: Date, | ||
| baseDate: Moment, |
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.
🧩 Analysis chain
Verify all callers of fillMissingDays have been updated.
The function signature changed from baseDate: Date to baseDate: Moment, which is a breaking change. Ensure that all call sites have been updated to pass a Moment instance instead of a Date object.
Run the following script to find all usages of fillMissingDays:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of fillMissingDays to verify they pass Moment instances
# Search for function calls with context
rg -n -C5 'fillMissingDays\s*\(' --type=tsLength of output: 6253
Convert baseDate/startDate to Moment in fillMissingDays calls
In project/graphql/resolvers/queries/project.ts (e.g., lines 600, 614, 642, 661), you’re still passing raw Date objects (baseDate/startDate) into fillMissingDays(baseDate: Moment, …). Wrap those dates with moment() or tz() so they match the updated signature.
🤖 Prompt for AI Agents
In backend/plugins/operation_api/src/modules/project/utils/charUtils.ts around
line 28: the helper signature now expects Moment for baseDate but callers in
project/graphql/resolvers/queries/project.ts (around lines 600, 614, 642, 661)
still pass raw Date objects; update those calls to wrap the Date values with
moment() (or moment.tz(...) if timezone context is required) so you pass a
Moment instance into fillMissingDays; ensure any existing timezone handling is
preserved when converting.
| sendTRPCMessage, | ||
| sendWorkerQueue, | ||
| } from 'erxes-api-shared/utils'; | ||
| import { tz } from 'moment-timezone'; | ||
| import { generateModels } from '~/connectionResolvers'; |
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.
Import moment-timezone correctly in the worker
moment-timezone lacks a named tz export, so this file won’t compile. Use the default moment import and call moment.tz(...) on Line 48. Update downstream references accordingly (Lines 56-57).
Apply this diff:
-import {
- getEnv,
- getSaasOrganizations,
- sendTRPCMessage,
- sendWorkerQueue,
-} from 'erxes-api-shared/utils';
-import { tz } from 'moment-timezone';
+import {
+ getEnv,
+ getSaasOrganizations,
+ sendTRPCMessage,
+ sendWorkerQueue,
+} from 'erxes-api-shared/utils';
+import moment from 'moment-timezone';
@@
- const tzToday = tz(new Date(), timezone);
+ const tzToday = moment.tz(new Date(), timezone);
@@
- const utcStart = tzToday.startOf('day').toDate();
- const utcEnd = tzToday.endOf('day').toDate();
+ const utcStart = tzToday.clone().startOf('day').toDate();
+ const utcEnd = tzToday.clone().endOf('day').toDate();Also applies to: 48-58
🤖 Prompt for AI Agents
In backend/plugins/operation_api/src/worker/dailyCheckCycles.ts around lines 5
to 9 and 48 to 58, the file imports tz as a named export from moment-timezone
which does not exist; replace the named import with a default moment import
(import moment from 'moment-timezone') and update code that calls tz(...) to use
moment.tz(...), adjusting any downstream variables or references on lines 56-57
to use the values returned by moment.tz(...) accordingly.
| pattern: '0 * * * *', | ||
| tz: 'UTC', |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the worker has timezone-aware guards to prevent excessive executions.
The cron schedule changed from daily at midnight (0 0 * * *) to hourly (0 * * * *), a 24x frequency increase. Per the PR objectives, the worker should "run only at midnight in the configured timezone." Ensure the dailyCheckCycles worker implementation includes proper guards to execute only once per day per organization based on their configured timezone, preventing unnecessary executions during the other 23 hours.
Run the following script to examine the worker implementation:
🏁 Script executed:
#!/bin/bash
# Description: Verify dailyCheckCycles has timezone-aware execution guards
# Find the dailyCheckCycles implementation
ast-grep --pattern $'export $_ dailyCheckCycles = $_'
# Search for timezone checking logic
rg -n -C10 'dailyCheckCycles' backend/plugins/operation_api/src/worker/dailyCheckCycles.tsLength of output: 649
Add timezone-based execution guard in dailyCheckCycles
dailyCheckCycles is now scheduled hourly but currently enqueues jobs for every org each run and never uses the imported tz. Refactor to use moment-timezone to check tz(Date.now(), org.timezone).hour() === 0 (and minute === 0) so jobs only enqueue at local midnight.
backend/plugins/operation_api/src/worker/dailyCheckCycles.ts:14–21
| const timezone = configs?.find((data: any) => data.code === 'TIMEZONE'); | ||
|
|
||
| methods.setValue('dealCurrency', currencies?.value); | ||
| methods.setValue('mainCurrency', mainCurrency?.value); | ||
|
|
||
| timezone && methods.setValue('TIMEZONE', timezone?.value); | ||
| } |
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.
Resolve ESLint failure when hydrating TIMEZONE
Linting fails on Line 65 (@typescript-eslint/no-unused-expressions). Replace the short-circuit expression with an explicit conditional.
Apply this diff:
- const timezone = configs?.find((data: any) => data.code === 'TIMEZONE');
-
- methods.setValue('dealCurrency', currencies?.value);
- methods.setValue('mainCurrency', mainCurrency?.value);
-
- timezone && methods.setValue('TIMEZONE', timezone?.value);
+ const timezone = configs?.find((data: any) => data.code === 'TIMEZONE');
+
+ methods.setValue('dealCurrency', currencies?.value);
+ methods.setValue('mainCurrency', mainCurrency?.value);
+
+ if (timezone) {
+ methods.setValue('TIMEZONE', timezone.value);
+ }📝 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 timezone = configs?.find((data: any) => data.code === 'TIMEZONE'); | |
| methods.setValue('dealCurrency', currencies?.value); | |
| methods.setValue('mainCurrency', mainCurrency?.value); | |
| timezone && methods.setValue('TIMEZONE', timezone?.value); | |
| } | |
| const timezone = configs?.find((data: any) => data.code === 'TIMEZONE'); | |
| methods.setValue('dealCurrency', currencies?.value); | |
| methods.setValue('mainCurrency', mainCurrency?.value); | |
| if (timezone) { | |
| methods.setValue('TIMEZONE', timezone.value); | |
| } | |
| } |
🧰 Tools
🪛 ESLint
[error] 65-65: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/general/components/GeneralSettings.tsx
around lines 60 to 66, the short-circuit expression "timezone &&
methods.setValue('TIMEZONE', timezone?.value)" triggers an ESLint
no-unused-expressions error; replace it with an explicit conditional that checks
for timezone and then calls methods.setValue with timezone.value (i.e., use an
if (timezone) { methods.setValue('TIMEZONE', timezone.value); } block) to
eliminate the unused-expression pattern and ensure proper typing.
Summary by Sourcery
Introduce comprehensive timezone awareness across backend workers, cycle utilities, progress charts, and frontend settings by propagating timezone configs, converting dates with moment-timezone, and adding a user-selectable timezone field in the general settings.
New Features:
Enhancements:
Build:
Summary by CodeRabbit