fix: normalize ISO-8601 date strings for Safari compatibility (Closes #642)#677
Merged
ritesh-1918 merged 2 commits intoMay 31, 2026
Merged
Conversation
- test_duplicate_service.py: 7 tests covering check_duplicate * Empty store, custom threshold, default threshold * Degraded mode, find duplicates, best match, load on demand - test_auto_close.py: 12 tests covering AutoCloseService * System settings fallback, missing/partial data * Ticket close success/error, disabled toggle * Full run flow with multiple companies * Fatal error handling, singleton, test_query Closes ritesh-1918#297 and ritesh-1918#279
…itesh-1918#642) Safari fails to parse date strings in 'YYYY-MM-DDTHH:MM:SS' format (without timezone indicator). Added normalizeDateString() helper that appends 'Z' when no timezone is present, ensuring cross-browser compatibility. Changes: - Frontend/src/utils/dateUtils.js - Added normalizeDateString() helper - formatTimelineDate() now uses normalized dates - Graceful fallback for invalid/corrupt dates
|
@espcris05-commits is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces three independent changes: a Safari-compatible date normalization fix in the frontend date utilities, a new test suite for AutoCloseService, and a new test suite for DuplicateService. Combined, the changes strengthen client-side date handling and backend service reliability through comprehensive test coverage. ChangesSafari Date Parsing Fix
AutoCloseService Test Suite
DuplicateService Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 Warning |
![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
🧹 Nitpick comments (2)
backend/tests/test_duplicate_service.py (2)
21-25: 💤 Low valueRedundant import and an incorrect submodule mock.
Line 21 re-imports
MagicMock(already imported on Line 13). Line 25 registerssentence_transformers.util.cos_simas a fake module, butcos_simis a function, not a submodule — this entry is never resolved by an import and is dead. Sincesentence_transformersis already aMagicMock(Line 22),util.cos_simis auto-stubbed anyway.♻️ Proposed cleanup
# Mock heavy dependencies before any import -from unittest.mock import MagicMock sys.modules['sentence_transformers'] = MagicMock() sys.modules['sentence_transformers.SentenceTransformer'] = MagicMock() sys.modules['sentence_transformers.util'] = MagicMock() -sys.modules['sentence_transformers.util.cos_sim'] = MagicMock()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_duplicate_service.py` around lines 21 - 25, Remove the redundant MagicMock import and the incorrect sys.modules entry for sentence_transformers.util.cos_sim; since MagicMock is already imported earlier and sys.modules['sentence_transformers'] = MagicMock() will auto-stub attributes like util.cos_sim, delete the duplicate "from unittest.mock import MagicMock" and the line registering "sentence_transformers.util.cos_sim" as a module, leaving only sys.modules['sentence_transformers'] = MagicMock() (and the optional sys.modules['sentence_transformers.SentenceTransformer'] if you need a specific attribute stub) to simplify the test setup.
89-115: 💤 Low valueUse the imported
SIMILARITY_THRESHOLDinstead of hardcoded values.
SIMILARITY_THRESHOLDis imported on Line 27 but never used; this test instead hardcodes the assumption that the default is0.70(Line 108). If the default changes, the comment goes stale and the0.85/0.50cases may no longer straddle the boundary, weakening the test. Deriving the values from the constant keeps the test self-validating.♻️ Example
- # Default threshold is 0.70 - with patch.object(svc_module.util, 'cos_sim', return_value=MockCosSim(0.85)): + above = SIMILARITY_THRESHOLD + 0.1 + below = SIMILARITY_THRESHOLD - 0.1 + with patch.object(svc_module.util, 'cos_sim', return_value=MockCosSim(above)): result = service.check_duplicate("Some text") assert result["is_duplicate"] is True - with patch.object(svc_module.util, 'cos_sim', return_value=MockCosSim(0.50)): + with patch.object(svc_module.util, 'cos_sim', return_value=MockCosSim(below)): result = service.check_duplicate("Some text") assert result["is_duplicate"] is False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_duplicate_service.py` around lines 89 - 115, The test test_check_duplicate_uses_default_threshold currently hardcodes a presumed default (0.70) and uses fixed cos_sim return values; instead import and use the actual SIMILARITY_THRESHOLD constant (already imported at top) to compute values just above and just below the threshold so the test remains correct if the default changes—update the two patch.object calls in the test to return values derived from SIMILARITY_THRESHOLD (e.g., SIMILARITY_THRESHOLD + delta and SIMILARITY_THRESHOLD - delta) and assert using service.check_duplicate as before; reference the test function name and the SIMILARITY_THRESHOLD symbol to locate and change the assertions and patched return values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/test_auto_close.py`:
- Around line 309-317: The test resets the module-global _instance (in
backend.services.auto_close_service) and never restores it, causing state
leakage; modify the test around the load() assertions to save the original value
(orig = mod._instance) before setting mod._instance = None and restore it after
the assertions (in a finally block or teardown) so the module-global is returned
to its prior state; ensure this restoration happens even if the assertions fail
and keep the existing patch of create_client intact.
- Around line 20-24: The test currently sets sys.modules['supabase'] =
supabase_mock at module import time which pollutes sys.modules for other tests;
change this to a scoped mock so the original module is preserved: either use
unittest.mock.patch.dict(sys.modules, {'supabase': supabase_mock}) around the
import of AutoCloseService inside the test or save original =
sys.modules.get('supabase') before setting and restore it after import (or in
teardown); update the code that creates supabase_mock and the import of
AutoCloseService to perform the mocked insertion and restoration within the
test/fixture scope rather than at module scope.
In `@Frontend/src/utils/dateUtils.js`:
- Line 33: The current guard returns the string 'Invalid Date' when
date.getTime() is NaN, but the issue requires defaulting to the current local
timestamp; update the invalid-date branch (the check using
isNaN(date.getTime())) to create/use a new Date() (e.g., const fallback = new
Date()) and return/format that fallback timestamp instead of the literal
'Invalid Date' so downstream code receives the current local time; ensure you
preserve the existing output format/serialization logic around the original
variable `date`.
- Around line 16-22: The current branch in dateUtils.js incorrectly only checks
for '+' when detecting timezone offsets on dateStr, causing negative offsets
(e.g., "2026-05-29T12:00:00-05:00") to be treated as lacking a TZ and producing
malformed strings; update the condition that tests for timezone characters to
also check for '-' (or better, use a regex that detects an ISO-8601 offset after
the time part) so that the block which appends 'Z' (the logic around
dateStr.includes('T') && !dateStr.includes('Z') && !dateStr.includes('+'))
correctly recognizes both '+' and '-' offsets and skips appending 'Z' when a
negative offset is present.
- Around line 12-26: Add unit tests and test runner configuration to cover edge
cases for normalizeDateString and formatTimelineDate: configure a test runner in
Frontend/package.json (e.g., jest or vitest) and add specs that assert behavior
for date-only strings (should append "T00:00:00Z" treating as UTC), inputs
already containing "Z" or positive offsets like "+hh:mm" (should be returned
unchanged or preserve timezone), negative offsets like "-hh:mm" (should NOT get
a trailing "Z"), passing a Date object to formatTimelineDate (ensure it formats
correctly), and malformed/empty inputs (null, undefined, or "Invalid Date" ->
expected null or handled value). Reference normalizeDateString and
formatTimelineDate when writing tests and assert exact returned strings or null
behavior to ensure all branches in normalizeDateString are exercised.
---
Nitpick comments:
In `@backend/tests/test_duplicate_service.py`:
- Around line 21-25: Remove the redundant MagicMock import and the incorrect
sys.modules entry for sentence_transformers.util.cos_sim; since MagicMock is
already imported earlier and sys.modules['sentence_transformers'] = MagicMock()
will auto-stub attributes like util.cos_sim, delete the duplicate "from
unittest.mock import MagicMock" and the line registering
"sentence_transformers.util.cos_sim" as a module, leaving only
sys.modules['sentence_transformers'] = MagicMock() (and the optional
sys.modules['sentence_transformers.SentenceTransformer'] if you need a specific
attribute stub) to simplify the test setup.
- Around line 89-115: The test test_check_duplicate_uses_default_threshold
currently hardcodes a presumed default (0.70) and uses fixed cos_sim return
values; instead import and use the actual SIMILARITY_THRESHOLD constant (already
imported at top) to compute values just above and just below the threshold so
the test remains correct if the default changes—update the two patch.object
calls in the test to return values derived from SIMILARITY_THRESHOLD (e.g.,
SIMILARITY_THRESHOLD + delta and SIMILARITY_THRESHOLD - delta) and assert using
service.check_duplicate as before; reference the test function name and the
SIMILARITY_THRESHOLD symbol to locate and change the assertions and patched
return values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89172806-8c77-4b39-ae6b-a734e9b350ef
📒 Files selected for processing (3)
Frontend/src/utils/dateUtils.jsbackend/tests/test_auto_close.pybackend/tests/test_duplicate_service.py
Comment on lines
+20
to
+24
| # Mock supabase before import | ||
| supabase_mock = MagicMock() | ||
| sys.modules['supabase'] = supabase_mock | ||
|
|
||
| from backend.services.auto_close_service import AutoCloseService |
There was a problem hiding this comment.
Avoid global sys.modules pollution during import mocking.
sys.modules['supabase'] is overridden at module scope and never restored, which can affect unrelated tests in the same run.
Suggested fix
+import importlib
+from unittest.mock import patch
+
# Mock supabase before import
-supabase_mock = MagicMock()
-sys.modules['supabase'] = supabase_mock
-
-from backend.services.auto_close_service import AutoCloseService
+with patch.dict(sys.modules, {"supabase": MagicMock()}):
+ auto_close_module = importlib.import_module("backend.services.auto_close_service")
+AutoCloseService = auto_close_module.AutoCloseService🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_auto_close.py` around lines 20 - 24, The test currently
sets sys.modules['supabase'] = supabase_mock at module import time which
pollutes sys.modules for other tests; change this to a scoped mock so the
original module is preserved: either use unittest.mock.patch.dict(sys.modules,
{'supabase': supabase_mock}) around the import of AutoCloseService inside the
test or save original = sys.modules.get('supabase') before setting and restore
it after import (or in teardown); update the code that creates supabase_mock and
the import of AutoCloseService to perform the mocked insertion and restoration
within the test/fixture scope rather than at module scope.
Comment on lines
+309
to
+317
| # Reset singleton | ||
| import backend.services.auto_close_service as mod | ||
| mod._instance = None | ||
|
|
||
| with patch('backend.services.auto_close_service.create_client', return_value=MagicMock()): | ||
| instance1 = load() | ||
| instance2 = load() | ||
| assert instance1 is instance2 | ||
|
|
There was a problem hiding this comment.
Restore singleton state after the test to keep test isolation.
Line 311 mutates module-global _instance and leaves it changed after the test, which can create order-dependent failures.
Suggested fix
import backend.services.auto_close_service as mod
- mod._instance = None
+ previous_instance = mod._instance
+ mod._instance = None
- with patch('backend.services.auto_close_service.create_client', return_value=MagicMock()):
- instance1 = load()
- instance2 = load()
- assert instance1 is instance2
+ try:
+ with patch('backend.services.auto_close_service.create_client', return_value=MagicMock()):
+ instance1 = load()
+ instance2 = load()
+ assert instance1 is instance2
+ finally:
+ mod._instance = previous_instance📝 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.
Suggested change
| # Reset singleton | |
| import backend.services.auto_close_service as mod | |
| mod._instance = None | |
| with patch('backend.services.auto_close_service.create_client', return_value=MagicMock()): | |
| instance1 = load() | |
| instance2 = load() | |
| assert instance1 is instance2 | |
| # Reset singleton | |
| import backend.services.auto_close_service as mod | |
| previous_instance = mod._instance | |
| mod._instance = None | |
| try: | |
| with patch('backend.services.auto_close_service.create_client', return_value=MagicMock()): | |
| instance1 = load() | |
| instance2 = load() | |
| assert instance1 is instance2 | |
| finally: | |
| mod._instance = previous_instance |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_auto_close.py` around lines 309 - 317, The test resets the
module-global _instance (in backend.services.auto_close_service) and never
restores it, causing state leakage; modify the test around the load() assertions
to save the original value (orig = mod._instance) before setting mod._instance =
None and restore it after the assertions (in a finally block or teardown) so the
module-global is returned to its prior state; ensure this restoration happens
even if the assertions fail and keep the existing patch of create_client intact.
Comment on lines
+12
to
+26
| const normalizeDateString = (dateStr) => { | ||
| if (!dateStr) return null; | ||
|
|
||
| // If it already has timezone info, return as-is | ||
| if (dateStr.includes('Z') || dateStr.includes('+') || dateStr.includes('T')) { | ||
| // Replace "YYYY-MM-DDTHH:MM:SS" with "YYYY-MM-DDTHH:MM:SSZ" if no TZ | ||
| if (dateStr.includes('T') && !dateStr.includes('Z') && !dateStr.includes('+')) { | ||
| return dateStr + 'Z'; | ||
| } | ||
| return dateStr; | ||
| } | ||
|
|
||
| // Raw date without time - assume UTC | ||
| return dateStr + 'T00:00:00Z'; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing dateUtils tests
fd -i 'dateUtils' --extension test.js --extension spec.js
rg -l 'normalizeDateString|formatTimelineDate' --glob '*test*' --glob '*spec*'Repository: ritesh-1918/HELPDESK.AI
Length of output: 49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate dateUtils implementation
echo "== dateUtils.js =="
fd -a "dateUtils.js" . || true
fd -a "dateUtils" Frontend . || true
# Find any test/spec files in the repo
echo "== test/spec files =="
fd -t f --hidden --exclude-dir node_modules \
-g '*.{test,spec}.{js,jsx,ts,tsx}' . \
| head -n 200
# Search for test files referencing dateUtils / specific functions
echo "== test files referencing dateUtils/normalizeDateString/formatTimelineDate =="
rg -l --hidden --no-ignore-vcs \
"dateUtils|normalizeDateString|formatTimelineDate" \
--glob '*test*' --glob '*spec*' . || true
# Search for any imports of dateUtils from test files
echo "== test files importing Frontend/src/utils/dateUtils =="
rg -l --hidden --no-ignore-vcs \
"from ['\"].*dateUtils['\"]|require\\(['\"].*dateUtils['\"]\\)" \
--glob '*test*' --glob '*spec*' . || true
# Search for normalizeDateString usage in non-test code (to see if tests might be elsewhere)
echo "== normalizeDateString usage (non-test) =="
rg -n --hidden --no-ignore-vcs \
"normalizeDateString\\(" \
--glob '!**/*test*' --glob '!**/*spec*' . | head -n 50 || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 513
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dateUtils.js =="
fd -a "dateUtils.js" . || true
echo "== __tests__ / test directories =="
fd -t d --hidden --exclude node_modules \
-g '*__tests__*' -g '*tests*' -g '*test*' . | head -n 200 || true
echo "== test/spec files (common extensions) =="
fd -t f --hidden --exclude node_modules \
-g '*.{test,spec}.{js,jsx,ts,tsx}' . \
| head -n 200 || true
echo "== test/spec files referencing dateUtils/normalizeDateString/formatTimelineDate =="
rg -l --hidden --no-ignore-vcs \
"dateUtils|normalizeDateString|formatTimelineDate" \
--glob '*test*' --glob '*spec*' . || true
echo "== imports of Frontend/src/utils/dateUtils from test/spec files =="
rg -l --hidden --no-ignore-vcs \
"(from ['\"].*dateUtils['\"]|require\\(['\"].*dateUtils['\"]\\))" \
--glob '*test*' --glob '*spec*' . || true
echo "== any occurrences of normalizeDateString outside test/spec =="
rg -n --hidden --no-ignore-vcs \
"normalizeDateString\\(" \
--glob '!**/*test*' --glob '!**/*spec*' . \
| head -n 50 || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 640
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dateUtils implementation =="
fd -a "dateUtils.js" .
echo "== directories that look like tests =="
# Use name globbing via filename patterns (not multiple -g in fd)
find . -type d \( -name "__tests__" -o -name "tests" -o -name "test" -o -name "__test__" -o -name "spec" \) -not -path "*/node_modules/*" | head -n 200 || true
echo "== common test framework config files =="
fd -t f --hidden --exclude node_modules \
-g '*jest*.{js,ts,mjs,cjs}' \
-g '*vitest*.{js,ts,mjs,cjs}' \
-g '*mocha*.{js,ts,mjs,cjs}' \
-g '*ava*.{js,ts,mjs,cjs}' \
-g '*enzyme*.{js,ts,mjs,cjs}' \
-g '*karma*.{js,ts,mjs,cjs}' . \
| head -n 200 || true
echo "== any test/spec files anywhere (broad) =="
find . -type f -not -path "*/node_modules/*" \
\( -name "*test*.js" -o -name "*test*.ts" -o -name "*test*.tsx" -o -name "*test*.jsx" \
-o -name "*spec*.js" -o -name "*spec*.ts" -o -name "*spec*.tsx" -o -name "*spec*.jsx" \
-o -name "*.test.js" -o -name "*.test.ts" -o -name "*.spec.js" -o -name "*.spec.ts" \) \
| head -n 200 || true
echo "== files referencing describe()/it()/test() =="
rg -l --hidden --no-ignore-vcs \
"(^|\\W)(describe|it|test)\\s*\\(" \
--glob '*.{js,jsx,ts,tsx,mjs,cjs}' . \
--glob '!*node_modules/*' \
| head -n 200 || true
echo "== any occurrences of normalizeDateString in non-implementation files =="
rg -n --hidden --no-ignore-vcs \
"normalizeDateString\\(" \
--glob '!**/dateUtils.js' \
--glob '!**/*test*' \
--glob '!**/*spec*' . || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 1001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dateUtils.js exports/usages =="
sed -n '1,120p' Frontend/src/utils/dateUtils.js
echo "== any references to dateUtils.js in repo (esp. tests) =="
rg -l --hidden --no-ignore-vcs \
"Frontend/src/utils/dateUtils\\.js|\\bdateUtils\\b" \
--glob '*test*' --glob '*spec*' --glob 'backend/tests/**' --glob 'scratch/**' . \
|| true
echo "== any references to normalizeDateString / formatTimelineDate anywhere =="
rg -n --hidden --no-ignore-vcs \
"normalizeDateString\\b|formatTimelineDate\\b" \
. || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 4042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/tests (top 200 files) =="
find backend/tests -type f -maxdepth 5 -print | head -n 200 || true
echo "== dateUtils usage inside backend/tests =="
rg -n --hidden --no-ignore-vcs \
"dateUtils\\.js|normalizeDateString\\b|formatTimelineDate\\b|getTimeZoneAbbr\\b" \
backend/tests || true
echo "== Frontend package.json (test scripts/config) =="
find Frontend -maxdepth 2 -name package.json -print -o -name pnpm-lock.yaml -print -o -name yarn.lock -print | head -n 50
if [ -f Frontend/package.json ]; then
rg -n "test|jest|vitest|mocha|ava" Frontend/package.json || true
fi
echo "== repo-wide test framework config files =="
find . -maxdepth 4 -type f \
\( -name "jest.config.*" -o -name "vitest.config.*" -o -name "mocha.config.*" -o -name ".mocharc.*" \
-o -name "karma.conf.*" -o -name "ava.config.*" \) \
! -path "*/node_modules/*" | head -n 200
echo "== scratch/test_*.js references to dateUtils =="
rg -n --hidden --no-ignore-vcs \
"dateUtils\\.js|normalizeDateString\\b|formatTimelineDate\\b|getTimeZoneAbbr\\b" \
scratch/test_*.js 2>/dev/null || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Frontend/package.json (scripts/deps excerpts) =="
cat -n Frontend/package.json | sed -n '1,220p' | sed -n '1,120p'
echo "----"
cat -n Frontend/package.json | sed -n '120,240p' | sed -n '1,120p'
echo "---- scripts search ----"
rg -n "scripts|jest|vitest|mocha|test" Frontend/package.json || true
echo "---- dependencies search ----"
rg -n "jest|vitest|mocha" Frontend/package.json || true
echo "== Frontend test files (search) =="
find Frontend -type d \
\( -name "__tests__" -o -name "tests" -o -name "test" -o -name "spec" \) \
-not -path "*/node_modules/*" | head -n 200 || true
echo "== Frontend test/spec files (common patterns) =="
find Frontend -type f -not -path "*/node_modules/*" \
\( -name "*test*.js" -o -name "*test*.ts" -o -name "*spec*.js" -o -name "*spec*.ts" \) | head -n 200 || true
echo "== repo-wide test framework configs =="
find . -type f -not -path "*/node_modules/*" \
\( -name "jest.config.*" -o -name "vitest.config.*" -o -name "mocha.config.*" \
-o -name ".mocharc.*" -o -name "karma.conf.*" -o -name "ava.config.*" \) | head -n 200 || true
echo "== repo-wide jest/vitest/mocha keywords =="
rg -n --hidden --no-ignore-vcs \
"jest|vitest|mocha" . | head -n 200 || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 13141
Add unit tests for Frontend/src/utils/dateUtils.js edge cases (timezone/date inputs)
- No Frontend unit tests/spec coverage found for
formatTimelineDate/normalizeDateString, andFrontend/package.jsonhas no test runner configured. - Add tests for: date-only strings (UTC behavior), strings with
Zand with positive offsets (+hh:mm), negative offset strings (-hh:mm, should not get a trailingZ), passing aDateobject (currently used viaformatTimelineDate(new Date())), and malformed/empty inputs (null /Invalid Date).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/utils/dateUtils.js` around lines 12 - 26, Add unit tests and
test runner configuration to cover edge cases for normalizeDateString and
formatTimelineDate: configure a test runner in Frontend/package.json (e.g., jest
or vitest) and add specs that assert behavior for date-only strings (should
append "T00:00:00Z" treating as UTC), inputs already containing "Z" or positive
offsets like "+hh:mm" (should be returned unchanged or preserve timezone),
negative offsets like "-hh:mm" (should NOT get a trailing "Z"), passing a Date
object to formatTimelineDate (ensure it formats correctly), and malformed/empty
inputs (null, undefined, or "Invalid Date" -> expected null or handled value).
Reference normalizeDateString and formatTimelineDate when writing tests and
assert exact returned strings or null behavior to ensure all branches in
normalizeDateString are exercised.
Comment on lines
+16
to
+22
| if (dateStr.includes('Z') || dateStr.includes('+') || dateStr.includes('T')) { | ||
| // Replace "YYYY-MM-DDTHH:MM:SS" with "YYYY-MM-DDTHH:MM:SSZ" if no TZ | ||
| if (dateStr.includes('T') && !dateStr.includes('Z') && !dateStr.includes('+')) { | ||
| return dateStr + 'Z'; | ||
| } | ||
| return dateStr; | ||
| } |
There was a problem hiding this comment.
Missing check for negative timezone offsets corrupts dates.
The condition checks for + but not - in the timestamp portion. ISO-8601 offsets can be negative (e.g., 2026-05-29T12:00:00-05:00 for US Eastern). This input would incorrectly match line 18 and produce 2026-05-29T12:00:00-05:00Z, a malformed string.
🐛 Proposed fix to detect negative offsets
const normalizeDateString = (dateStr) => {
if (!dateStr) return null;
+
+ // Convert Date objects to ISO string
+ if (dateStr instanceof Date) {
+ return dateStr.toISOString();
+ }
- // If it already has timezone info, return as-is
- if (dateStr.includes('Z') || dateStr.includes('+') || dateStr.includes('T')) {
- // Replace "YYYY-MM-DDTHH:MM:SS" with "YYYY-MM-DDTHH:MM:SSZ" if no TZ
- if (dateStr.includes('T') && !dateStr.includes('Z') && !dateStr.includes('+')) {
- return dateStr + 'Z';
- }
- return dateStr;
+ // Check if string already has timezone info (Z, +HH:MM, or -HH:MM after T)
+ if (dateStr.includes('Z')) return dateStr;
+
+ if (dateStr.includes('T')) {
+ const timePart = dateStr.split('T')[1] || '';
+ // Check for offset in time portion (not the date hyphens)
+ if (timePart.includes('+') || timePart.includes('-')) {
+ return dateStr;
+ }
+ // No timezone, append Z
+ return dateStr + 'Z';
}
// Raw date without time - assume UTC
return dateStr + 'T00:00:00Z';
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/utils/dateUtils.js` around lines 16 - 22, The current branch in
dateUtils.js incorrectly only checks for '+' when detecting timezone offsets on
dateStr, causing negative offsets (e.g., "2026-05-29T12:00:00-05:00") to be
treated as lacking a TZ and producing malformed strings; update the condition
that tests for timezone characters to also check for '-' (or better, use a regex
that detects an ISO-8601 offset after the time part) so that the block which
appends 'Z' (the logic around dateStr.includes('T') && !dateStr.includes('Z') &&
!dateStr.includes('+')) correctly recognizes both '+' and '-' offsets and skips
appending 'Z' when a negative offset is present.
| hour12: true | ||
| }); | ||
| const date = new Date(normalized); | ||
| if (isNaN(date.getTime())) return 'Invalid Date'; |
There was a problem hiding this comment.
Fallback behavior differs from stated objective.
The linked issue #642 specifies that corrupt dates should "default to the current local timestamp." The implementation returns the string 'Invalid Date' instead. If the current behavior is intentional (arguably more transparent to users), consider updating the issue/PR description to reflect the design choice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/utils/dateUtils.js` at line 33, The current guard returns the
string 'Invalid Date' when date.getTime() is NaN, but the issue requires
defaulting to the current local timestamp; update the invalid-date branch (the
check using isNaN(date.getTime())) to create/use a new Date() (e.g., const
fallback = new Date()) and return/format that fallback timestamp instead of the
literal 'Invalid Date' so downstream code receives the current local time;
ensure you preserve the existing output format/serialization logic around the
original variable `date`.
ritesh-1918
added
gssoc
Owner
|
Hi @espcris05-commits! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Fix: Safari Date Parsing
Closes #642
Problem
Safari fails to parse ISO-8601 timestamps without timezone indicator (e.g.,
2026-05-29T12:00:00), showing "Invalid Date" on Ticket Timeline.Solution
Added
normalizeDateString()helper that appends "Z" (UTC) when no timezone is present, ensuring cross-browser compatibility.Changes
Frontend/src/utils/dateUtils.jsnormalizeDateString()helperformatTimelineDate()now uses normalized datesTesting
Payment: USDC Stellar
GCR377MUJ75YKFLNZKT6XPWNDUIEDZDUHUWY5E2LHOBBSSJDU7MJRZXFSummary by CodeRabbit
Bug Fixes
Tests