T1 Bounty: TimeAgo component (package-lock fixed)#627
T1 Bounty: TimeAgo component (package-lock fixed)#627chronoeth-creator merged 8 commits intoSolFoundry:mainfrom
Conversation
- Create reusable TimeAgo component with auto-update - Add comprehensive test suite - Apply to bounty cards, list view, timeline, and activity feed - Supports multiple formats: just now, Xm ago, Xh ago, Xd ago, date - Includes tooltip with full datetime on hover
- Fix 7-day boundary logic (< 7 to <= 7) - Add invalid date handling with safe fallbacks - Fix test expectations for 60-minute boundary - Add tests for invalid date inputs Fixes critical issues from multi-LLM review (score: 6.4/10)
- Fix 'just now' test with deterministic timing - Add act() wrapper for auto-update test - Import act from testing-library/react
|
Wallet: EDFxPF6yAQNod3nFzwV7z1qwSjt42WDYzmdT6b6YHDh7 Ready for review! All tests passing. |
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a reusable Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/bounties/BountyListView.tsx`:
- Around line 61-63: BountyListView currently renders the TimeAgo component
unconditionally which can display "just now" when b.createdAt is undefined;
update the JSX around TimeAgo in BountyListView (the TimeAgo usage that
references b.createdAt) to match BountyCard's behavior by conditionally
rendering TimeAgo only when b.createdAt is present (e.g., render the <TimeAgo
... /> element only if b.createdAt truthy), preserving the same className and
layout when omitted.
In `@frontend/src/components/common/__tests__/TimeAgo.test.tsx`:
- Around line 120-130: Add a complementary test in TimeAgo.test.tsx that
verifies no auto-updates occur for dates older than 7 days: render the TimeAgo
component (symbol: TimeAgo) with a date more than 7 days in the past and an
updateInterval (e.g., 60000), capture the initial rendered text (e.g., "8d ago"
or the expected label), then wrap vi.advanceTimersByTime(...) in act(...) to
advance several intervals and assert the displayed text remains the same; ensure
the test uses fake timers (vi.useFakeTimers()) and cleans up timers after to
mirror the existing interval test structure.
In `@frontend/src/components/common/TimeAgo.tsx`:
- Around line 103-104: The component currently masks invalid dates by setting
safeDate = isValidDate(date) ? date : new Date().toISOString(); — instead,
detect invalid dates via isValidDate(date) and in that branch (where safeDate
would be replaced) log a warning in development (check process.env.NODE_ENV ===
'development') including the original date value and the component name TimeAgo,
and change rendering to show a distinct indicator (e.g., "—" or the raw date
string) rather than using new Date().toISOString(); update any
variables/functions that consume safeDate accordingly so the invalid path
returns the indicator and does not produce a misleading "just now" UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58345631-b277-43b3-9220-0b544e7def68
📒 Files selected for processing (6)
frontend/src/components/BountyDetailPage.tsxfrontend/src/components/BountyTimeline.tsxfrontend/src/components/bounties/BountyCard.tsxfrontend/src/components/bounties/BountyListView.tsxfrontend/src/components/common/TimeAgo.tsxfrontend/src/components/common/__tests__/TimeAgo.test.tsx
| // Handle invalid dates gracefully | ||
| const safeDate = isValidDate(date) ? date : new Date().toISOString(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent fallback for invalid dates may mask upstream issues.
When an invalid date is passed, the component falls back to new Date().toISOString(), displaying "just now" instead of indicating an error. While this prevents UI breakage, it could hide bugs where invalid data flows through the system.
Consider logging a warning in development mode or rendering a distinct visual indicator (like "—" or the raw value) so issues are discoverable:
Alternative approach
export function TimeAgo({
date,
className = '',
updateInterval = 60000,
}: TimeAgoProps) {
// Handle invalid dates gracefully
- const safeDate = isValidDate(date) ? date : new Date().toISOString();
+ const isValid = isValidDate(date);
+ if (!isValid && process.env.NODE_ENV === 'development') {
+ console.warn('TimeAgo: received invalid date', date);
+ }
+ const safeDate = isValid ? date : new Date().toISOString();📝 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.
| // Handle invalid dates gracefully | |
| const safeDate = isValidDate(date) ? date : new Date().toISOString(); | |
| // Handle invalid dates gracefully | |
| const isValid = isValidDate(date); | |
| if (!isValid && process.env.NODE_ENV === 'development') { | |
| console.warn('TimeAgo: received invalid date', date); | |
| } | |
| const safeDate = isValid ? date : new Date().toISOString(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/common/TimeAgo.tsx` around lines 103 - 104, The
component currently masks invalid dates by setting safeDate = isValidDate(date)
? date : new Date().toISOString(); — instead, detect invalid dates via
isValidDate(date) and in that branch (where safeDate would be replaced) log a
warning in development (check process.env.NODE_ENV === 'development') including
the original date value and the component name TimeAgo, and change rendering to
show a distinct indicator (e.g., "—" or the raw date string) rather than using
new Date().toISOString(); update any variables/functions that consume safeDate
accordingly so the invalid path returns the indicator and does not produce a
misleading "just now" UI.
- Consistent with BountyCard.tsx handling - Shows '-' when createdAt is undefined - Addresses CodeRabbit review feedback
|
Fixed! Added conditional check for missing createdAt in BountyListView to match BountyCard.tsx pattern. Shows '-' when undefined. |
- Verifies that TimeAgo does not set interval for old dates - Complements existing auto-update test for recent dates - Addresses CodeRabbit review feedback
|
✅ All 3 issues fixed:
All 17 tests passing! Ready for re-review. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/common/__tests__/TimeAgo.test.tsx`:
- Around line 84-155: Add tests to cover invalid date prop handling in the
TimeAgo component: render TimeAgo with an invalid string (e.g., "not-a-date")
and with an invalid numeric timestamp (e.g., NaN or -1), assert the component
renders the fallback output used by the helper (no crash), confirm it does NOT
have datetime or title attributes, and verify it does not auto-update when
updateInterval is provided; target the TimeAgo component tests in
TimeAgo.test.tsx and reference the TimeAgo component render calls and
updateInterval behavior to locate where to add these cases.
- Around line 132-144: The test for TimeAgo should assert that no update
interval is created for dates older than 7 days; add a spy on global.setInterval
(or use vi.spyOn(global, 'setInterval')) before rendering
TimeAgo(date="2026-03-12T12:00:00Z", updateInterval=60000) and assert it was not
called, then advance timers and assert the output remains "Mar 12" to ensure no
interval-driven re-render; update the test in TimeAgo.test.tsx to install and
restore the spy around the render/assert steps and reference the TimeAgo
component and its updateInterval prop when making the assertions.
- Around line 54-58: The tests fail intermittently due to locale timezone
differences; update the date formatting in TimeAgo.tsx (functions like
formatTimeAgo / any toLocaleDateString/toLocaleString calls) to pass a fixed
timeZone: 'UTC' option to toLocaleDateString(...) and toLocaleString(...) so the
rendered strings (e.g., 'Mar 14', 'Feb 22', 'Dec 25') are deterministic across
environments, or alternatively pin the test runner timezone by setting TZ=UTC in
the test config/script if you prefer that approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96432643-c7c0-4379-9b4c-bb92b0fe02b3
📒 Files selected for processing (1)
frontend/src/components/common/__tests__/TimeAgo.test.tsx
- Add UTC timezone for deterministic date formatting - Add comprehensive invalid date handling tests - Add setInterval spy tests for >7 days and invalid dates - Fix component to skip interval for invalid dates - Fix NaN handling in isValidDate All 21 tests passing!
|
✅ All CodeRabbit issues fixed (batch 2):
All 21 tests passing! Ready for final review. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/src/components/common/TimeAgo.tsx (2)
54-59:⚠️ Potential issue | 🟠 MajorFuture >7d formatting is timezone-dependent and can render the wrong calendar day.
Line [58] uses
toLocaleDateString('en-US', { month, day })withouttimeZone, while Line [74] explicitly uses UTC. This inconsistency can shift displayed dates across environments for the future-date fallback path.As per coding guidelines, "frontend/**: React/TypeScript frontend. Check: Component structure and state management" and "Error/loading/empty state handling".
Also applies to: 73-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/TimeAgo.tsx` around lines 54 - 59, The future >7d branch in the TimeAgo.tsx component (in the function that formats relative times, e.g., TimeAgo/formatRelativeTime) uses toLocaleDateString('en-US', { month: 'short', day: 'numeric' }) without specifying a timeZone, causing timezone-dependent shifts; make this consistent with the other UTC formatting path by adding timeZone: 'UTC' (or using the same UTC-based formatting helper used around line 74) so the fallback calendar date for future dates is rendered in UTC rather than local time.
110-116:⚠️ Potential issue | 🟡 MinorInvalid-date UI is internally inconsistent between text and tooltip metadata.
For invalid input, visible text is
'Invalid date'(Line [114]/Line [146]), buttitleis derived from fallback current time viasafeDate(Line [111], Line [115], Line [143]). This produces contradictory UI semantics and can mask upstream data issues.As per coding guidelines, "frontend/**: React/TypeScript frontend. Check: Error/loading/empty state handling".
Also applies to: 142-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/common/TimeAgo.tsx` around lines 110 - 116, The component shows "Invalid date" in the visible text (formatTimeAgo(date)) but uses safeDate (new Date().toISOString()) for the tooltip via formatFullDate(safeDate), causing inconsistent UI; change the logic so both displayed text and tooltip derive from the same validity check: if date is invalid then set timeAgo and fullDate to the same "Invalid date" (or a localized invalid-message) instead of falling back to current time, by updating the initialization of timeAgo (useState(() => isValid ? formatTimeAgo(date) : "Invalid date")), the useMemo for fullDate (useMemo(() => isValid ? formatFullDate(date) : "Invalid date", [date, isValid])), and any places that use safeDate so they use date when valid and the explicit invalid placeholder when not.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/common/__tests__/TimeAgo.test.tsx`:
- Around line 60-65: The tests in TimeAgo.test.tsx currently exercise future
dates only up to 7 days and miss the codepath for future dates >7 days (the
date-string fallback) and its interval suppression behavior; add a unit test for
formatTimeAgo that passes a future date more than 7 days ahead (e.g., now + 8+
days) asserting the returned value uses the long date-string format (not "in
Xd") and verify any interval/auto-update suppression behavior (no periodic
short-form updates) by asserting stable output across a simulated tick/update;
locate and update the existing TimeAgo.test.tsx cases around the "handles future
dates" block and mirror the same checks near the later tests referenced (lines
~182-194) to cover regressions.
In `@frontend/src/components/common/TimeAgo.tsx`:
- Around line 127-132: The check using diffDays allows negative values for
future timestamps, so TimeAgo still sets an interval for far-future dates;
update the condition around then/diffDays in the TimeAgo component to ensure the
target time is not in the future before scheduling updates (e.g., require
diffDays >= 0 && diffDays <= 7 or check then <= Date.now()), then only call
setInterval when that stricter condition is met and keep clearing the interval
in the existing cleanup.
---
Duplicate comments:
In `@frontend/src/components/common/TimeAgo.tsx`:
- Around line 54-59: The future >7d branch in the TimeAgo.tsx component (in the
function that formats relative times, e.g., TimeAgo/formatRelativeTime) uses
toLocaleDateString('en-US', { month: 'short', day: 'numeric' }) without
specifying a timeZone, causing timezone-dependent shifts; make this consistent
with the other UTC formatting path by adding timeZone: 'UTC' (or using the same
UTC-based formatting helper used around line 74) so the fallback calendar date
for future dates is rendered in UTC rather than local time.
- Around line 110-116: The component shows "Invalid date" in the visible text
(formatTimeAgo(date)) but uses safeDate (new Date().toISOString()) for the
tooltip via formatFullDate(safeDate), causing inconsistent UI; change the logic
so both displayed text and tooltip derive from the same validity check: if date
is invalid then set timeAgo and fullDate to the same "Invalid date" (or a
localized invalid-message) instead of falling back to current time, by updating
the initialization of timeAgo (useState(() => isValid ? formatTimeAgo(date) :
"Invalid date")), the useMemo for fullDate (useMemo(() => isValid ?
formatFullDate(date) : "Invalid date", [date, isValid])), and any places that
use safeDate so they use date when valid and the explicit invalid placeholder
when not.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1fc1215-ac17-44be-bb12-b56701f3b977
📒 Files selected for processing (2)
frontend/src/components/common/TimeAgo.tsxfrontend/src/components/common/__tests__/TimeAgo.test.tsx
| it('handles future dates', () => { | ||
| expect(formatTimeAgo(new Date('2026-03-22T12:00:05Z'))).toBe('just now'); | ||
| expect(formatTimeAgo(new Date('2026-03-22T12:05:00Z'))).toBe('in 5m'); | ||
| expect(formatTimeAgo(new Date('2026-03-22T14:00:00Z'))).toBe('in 2h'); | ||
| expect(formatTimeAgo(new Date('2026-03-25T12:00:00Z'))).toBe('in 3d'); | ||
| }); |
There was a problem hiding this comment.
Missing regression coverage for future dates older than 7 days.
Line [60]-Line [65] tests future values only within 7 days (in Xm/in Xh/in Xd). There is no explicit test for future values beyond 7 days (date-string fallback path), nor for interval suppression in that branch. This leaves a gap where future >7d behavior can regress undetected.
As per coding guidelines, "frontend/**: React/TypeScript frontend. Check: Component structure and state management" and "Error/loading/empty state handling".
Also applies to: 182-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/common/__tests__/TimeAgo.test.tsx` around lines 60 -
65, The tests in TimeAgo.test.tsx currently exercise future dates only up to 7
days and miss the codepath for future dates >7 days (the date-string fallback)
and its interval suppression behavior; add a unit test for formatTimeAgo that
passes a future date more than 7 days ahead (e.g., now + 8+ days) asserting the
returned value uses the long date-string format (not "in Xd") and verify any
interval/auto-update suppression behavior (no periodic short-form updates) by
asserting stable output across a simulated tick/update; locate and update the
existing TimeAgo.test.tsx cases around the "handles future dates" block and
mirror the same checks near the later tests referenced (lines ~182-194) to cover
regressions.
| const then = new Date(safeDate).getTime(); | ||
| const diffDays = (Date.now() - then) / 86400000; | ||
|
|
||
| // Only auto-update if 7 days or less old | ||
| if (diffDays <= 7) { | ||
| const interval = setInterval(() => { |
There was a problem hiding this comment.
Far-future timestamps still schedule minute intervals due to signed day-diff check.
Line [128]-Line [132] computes a signed diffDays; any future date produces a negative value, which always satisfies <= 7. That means even far-future items keep an active interval, increasing timer pressure in list views without user-visible benefit.
As per coding guidelines, "frontend/**: React/TypeScript frontend. Check: Component structure and state management".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/common/TimeAgo.tsx` around lines 127 - 132, The check
using diffDays allows negative values for future timestamps, so TimeAgo still
sets an interval for far-future dates; update the condition around then/diffDays
in the TimeAgo component to ensure the target time is not in the future before
scheduling updates (e.g., require diffDays >= 0 && diffDays <= 7 or check then
<= Date.now()), then only call setInterval when that stricter condition is met
and keep clearing the interval in the existing cleanup.
- Replace custom formatRelativeTime with TimeAgo component - Maintains auto-update functionality via TimeAgo's built-in interval
- Add TimeAgo display for most recent PR submission timestamp - Shows 'Last PR submitted' with relative time in profile stats
|
✅ Completeness issues FIXED:
TimeAgo tests: 21/21 passing ✅ Note: Other repo tests have pre-existing failures (jest.clearAllMocks issue), not related to this PR. |
1 participant
Fixes #486
Summary
Reusable TimeAgo component with relative timestamps, auto-updates, and hover tooltips.
Changes
Fixes from review
Note
Previous PR #593 was auto-closed due to package-lock.json - this branch has clean commits without dependency files.
Wallet: EDFxPF6yAQNod3nFzwV7z1qwSjt42WDYzmdT6b6YHDh7