[Bug] - Greeting message uses incorrect timezone (Issue #8580)#8735
[Bug] - Greeting message uses incorrect timezone (Issue #8580)#8735Ivanq50 wants to merge 2 commits intomakeplane:previewfrom
Conversation
… of user's local timezone. We now use the timezone on the OS/Browser when it is defaulted to UTC.
📝 WalkthroughWalkthroughThe changes introduce a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect “Good morning/afternoon/evening” greetings by computing the current hour/date using an appropriate timezone (preferring a real user profile timezone and otherwise falling back to the browser/OS timezone). Also adjusts how Space/Admin base paths are derived from env vars.
Changes:
- Update
UserGreetingsViewto derive atimeZoneand apply it consistently to hour/date/weekday/time formatting. - Replace
joinUrlPathusage in Space/Admin Vite + React Router configs with a localnormalizeBasePathhelper for consistent leading/trailing slashes. - Remove now-unused
joinUrlPathimports from the affected config files.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/core/components/home/user-greetings.tsx | Applies a derived timezone to greeting/date/time formatting to avoid UTC-default greetings. |
| apps/space/vite.config.ts | Normalizes VITE_SPACE_BASE_PATH via a local helper before passing to Vite base. |
| apps/space/react-router.config.ts | Normalizes VITE_SPACE_BASE_PATH before passing to React Router basename. |
| apps/admin/vite.config.ts | Normalizes VITE_ADMIN_BASE_PATH via a local helper before passing to Vite base. |
| apps/admin/react-router.config.ts | Normalizes VITE_ADMIN_BASE_PATH before passing to React Router basename. |
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | ||
| }; |
There was a problem hiding this comment.
normalizeBasePath is duplicated in both the router config and Vite config for this app. Consider importing it from a shared helper to ensure base and basename stay consistent if the normalization logic changes.
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | ||
| }; |
There was a problem hiding this comment.
normalizeBasePath is duplicated here and in apps/admin/react-router.config.ts (and also duplicated in the space app). Consider extracting this into a shared helper to avoid future drift in base path normalization.
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | ||
| }; |
There was a problem hiding this comment.
The base path normalization logic is duplicated between this file and apps/admin/vite.config.ts (and similarly in apps/space). Consider moving normalizeBasePath to a shared helper module and importing it so config files remain consistent.
| // Use profile timezone when set to a real zone; treat UTC/Etc/UTC as unset (common | ||
| // backend default) so we show local time. Otherwise use browser/OS timezone. | ||
| const profileZone = user?.user_timezone?.trim(); | ||
| const isUtcDefault = | ||
| !profileZone || profileZone === "UTC" || profileZone === "Etc/UTC" || profileZone.toLowerCase() === "utc"; | ||
| const timeZone = isUtcDefault | ||
| ? typeof Intl !== "undefined" && Intl.DateTimeFormat | ||
| ? Intl.DateTimeFormat().resolvedOptions().timeZone | ||
| : undefined |
There was a problem hiding this comment.
isUtcDefault currently has redundant checks (profileZone === "UTC" and profileZone.toLowerCase() === "utc" overlap). Consider normalizing once (e.g., const normalized = profileZone?.trim().toUpperCase()) and comparing against a small set to keep the intent clear and avoid repeated string ops.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | ||
| }; |
There was a problem hiding this comment.
normalizeBasePath is now duplicated here and in the corresponding react-router.config.ts (and similarly in apps/admin). Consider extracting this helper into a small shared module (e.g., apps/space/config/base-path.ts or a utilities package) so the normalization rules don’t diverge over time.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin/react-router.config.ts`:
- Around line 3-6: normalizeBasePath currently preserves accidental internal
'//' from VITE_ADMIN_BASE_PATH which can change router basename; update
normalizeBasePath to canonicalize repeated slashes by collapsing any consecutive
slashes into a single slash (e.g., apply a global replace like replace(/\/+/g,
"/")) after trimming, then ensure the result has a single leading and single
trailing slash (still returning "/" for empty/root). Modify the
normalizeBasePath function so it both trims and normalizes internal duplicate
slashes before returning the final basename.
In `@apps/admin/vite.config.ts`:
- Around line 7-10: normalizeBasePath currently preserves internal duplicate
slashes (e.g., "admin//preview") causing wrong Vite base; update
normalizeBasePath to collapse consecutive "/" into a single "/" before
stripping/adding edges so internal "//" becomes "/". Specifically, in
normalizeBasePath (and where VITE_ADMIN_BASE_PATH is used) perform a global
collapse of repeated separators (e.g., via a replace of /\/+/g with "/"), then
remove leading/trailing slashes and wrap with a single leading and trailing "/"
as the final return value.
In `@apps/space/react-router.config.ts`:
- Around line 3-6: normalizeBasePath currently only trims ends and preserves
internal duplicate slashes; update normalizeBasePath to collapse repeated
slashes inside the path before trimming and assembling the final basename (e.g.,
run a global collapse like replacing /{2,} with / on the value or trimmed
value), then strip leading/trailing slashes and return "/" for empty root;
locate the function normalizeBasePath to apply this change so inputs like
"space//beta" become "/space/beta/" consistently.
In `@apps/space/vite.config.ts`:
- Around line 7-10: The normalizeBasePath function currently preserves internal
double slashes (e.g., "space//beta") which causes Vite base to emit
"/space//beta/"; update normalizeBasePath to collapse consecutive slashes into a
single slash after trimming so the output aligns with the router basename: in
the normalizeBasePath function (and any related callers) replace sequences of
multiple "/" within the path (e.g., via a regex that squeezes /{2,} to "/")
before adding the leading/trailing "/" so inputs like "space//beta" become
"/space/beta/".
In `@apps/web/core/components/home/user-greetings.tsx`:
- Around line 27-34: Normalize the profileZone before comparing so "Etc/UTC" is
matched case-insensitively: compute a trimmed, lowercased variant of
user?.user_timezone (e.g., profileZoneNormalized =
user?.user_timezone?.trim().toLowerCase()) and update isUtcDefault to check
against normalized values ("utc" and "etc/utc") instead of the current
case-sensitive checks; keep the rest of the timeZone logic the same and
reference the existing identifiers profileZone, isUtcDefault, and timeZone when
making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c90e344d-6dc7-4571-9870-17f605759614
📒 Files selected for processing (5)
apps/admin/react-router.config.tsapps/admin/vite.config.tsapps/space/react-router.config.tsapps/space/vite.config.tsapps/web/core/components/home/user-greetings.tsx
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; |
There was a problem hiding this comment.
Keep basename canonical here as well.
normalizeBasePath now preserves accidental internal //, so the admin router basename changes for env values that the old path joiner would have normalized. That leaves routing behavior dependent on slash typos in VITE_ADMIN_BASE_PATH.
Possible fix
const normalizeBasePath = (value: string): string => {
const trimmed = value.trim();
if (!trimmed || trimmed === "/") return "/";
- return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`;
+ return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`;
};📝 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 normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | |
| const normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/react-router.config.ts` around lines 3 - 6, normalizeBasePath
currently preserves accidental internal '//' from VITE_ADMIN_BASE_PATH which can
change router basename; update normalizeBasePath to canonicalize repeated
slashes by collapsing any consecutive slashes into a single slash (e.g., apply a
global replace like replace(/\/+/g, "/")) after trimming, then ensure the result
has a single leading and single trailing slash (still returning "/" for
empty/root). Modify the normalizeBasePath function so it both trims and
normalizes internal duplicate slashes before returning the final basename.
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; |
There was a problem hiding this comment.
Normalize duplicate separators before setting Vite base.
This version preserves internal //, so VITE_ADMIN_BASE_PATH="admin//preview" now produces /admin//preview/. That regresses the old joinUrlPath behavior and can send the admin bundle to the wrong asset prefix.
Possible fix
const normalizeBasePath = (value: string): string => {
const trimmed = value.trim();
if (!trimmed || trimmed === "/") return "/";
- return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`;
+ return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`;
};📝 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 normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; | |
| const normalizeBasePath = (value: string): string => { | |
| const trimmed = value.trim(); | |
| if (!trimmed || trimmed === "/") return "/"; | |
| return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/vite.config.ts` around lines 7 - 10, normalizeBasePath currently
preserves internal duplicate slashes (e.g., "admin//preview") causing wrong Vite
base; update normalizeBasePath to collapse consecutive "/" into a single "/"
before stripping/adding edges so internal "//" becomes "/". Specifically, in
normalizeBasePath (and where VITE_ADMIN_BASE_PATH is used) perform a global
collapse of repeated separators (e.g., via a replace of /\/+/g with "/"), then
remove leading/trailing slashes and wrap with a single leading and trailing "/"
as the final return value.
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; |
There was a problem hiding this comment.
Collapse repeated slashes before assigning basename.
normalizeBasePath only trims the ends, so VITE_SPACE_BASE_PATH="space//beta" now becomes /space//beta/. The previous joinUrlPath flow normalized duplicate separators, and React Router will mount under a different basename for the same env value.
Possible fix
const normalizeBasePath = (value: string): string => {
const trimmed = value.trim();
if (!trimmed || trimmed === "/") return "/";
- return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`;
+ return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/space/react-router.config.ts` around lines 3 - 6, normalizeBasePath
currently only trims ends and preserves internal duplicate slashes; update
normalizeBasePath to collapse repeated slashes inside the path before trimming
and assembling the final basename (e.g., run a global collapse like replacing
/{2,} with / on the value or trimmed value), then strip leading/trailing slashes
and return "/" for empty root; locate the function normalizeBasePath to apply
this change so inputs like "space//beta" become "/space/beta/" consistently.
| const normalizeBasePath = (value: string): string => { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed || trimmed === "/") return "/"; | ||
| return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`; |
There was a problem hiding this comment.
Preserve the old path normalization for Vite base.
This helper now leaves internal // intact. A value like VITE_SPACE_BASE_PATH="space//beta" will emit assets under /space//beta/, which is a behavior change from the old joinUrlPath flow. Please normalize repeated separators here so base stays aligned with the router basename.
Possible fix
const normalizeBasePath = (value: string): string => {
const trimmed = value.trim();
if (!trimmed || trimmed === "/") return "/";
- return `/${trimmed.replace(/^\/+|\/+$/g, "")}/`;
+ return `/${trimmed.replace(/\/+/g, "/").replace(/^\/+|\/+$/g, "")}/`;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/space/vite.config.ts` around lines 7 - 10, The normalizeBasePath
function currently preserves internal double slashes (e.g., "space//beta") which
causes Vite base to emit "/space//beta/"; update normalizeBasePath to collapse
consecutive slashes into a single slash after trimming so the output aligns with
the router basename: in the normalizeBasePath function (and any related callers)
replace sequences of multiple "/" within the path (e.g., via a regex that
squeezes /{2,} to "/") before adding the leading/trailing "/" so inputs like
"space//beta" become "/space/beta/".
| const profileZone = user?.user_timezone?.trim(); | ||
| const isUtcDefault = | ||
| !profileZone || profileZone === "UTC" || profileZone === "Etc/UTC" || profileZone.toLowerCase() === "utc"; | ||
| const timeZone = isUtcDefault | ||
| ? typeof Intl !== "undefined" && Intl.DateTimeFormat | ||
| ? Intl.DateTimeFormat().resolvedOptions().timeZone | ||
| : undefined | ||
| : profileZone; |
There was a problem hiding this comment.
Etc/UTC is still case-sensitive here.
The fallback only matches the exact "Etc/UTC" spelling. Values like "etc/utc" or "ETC/UTC" — which the PR description calls out as defaults too — will still be treated as real profile zones, so those users keep seeing UTC instead of their local time.
Possible fix
const profileZone = user?.user_timezone?.trim();
- const isUtcDefault =
- !profileZone || profileZone === "UTC" || profileZone === "Etc/UTC" || profileZone.toLowerCase() === "utc";
+ const isUtcDefault = !profileZone || /^(?:etc\/)?utc$/i.test(profileZone);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/core/components/home/user-greetings.tsx` around lines 27 - 34,
Normalize the profileZone before comparing so "Etc/UTC" is matched
case-insensitively: compute a trimmed, lowercased variant of user?.user_timezone
(e.g., profileZoneNormalized = user?.user_timezone?.trim().toLowerCase()) and
update isUtcDefault to check against normalized values ("utc" and "etc/utc")
instead of the current case-sensitive checks; keep the rest of the timeZone
logic the same and reference the existing identifiers profileZone, isUtcDefault,
and timeZone when making the change.
Description
Added logic to select the correct timezone for the user instead of defaulting to UTC. Reads the profile timezone and treats values like UTC, etc/UTC, any variance of UTC, or missing/empty values as "unset/default". If it is unset/default, it falls back to browser/OS timezone, otherwise, it uses the profile timezone.
Type of Change
Screenshots and Media (if applicable)
Correct time: 2:33PM PST
Correct time: 2:22PM PST (24 hour)
Test Scenarios
References
https://github.com/makeplane/plane/issues/8580