Fix: format timestamps to show date and time consistently in UI#206
Conversation
…sue 195), format timestamps to show date and time consistently in UI
|
@shaleenjain28 is attempting to deploy a commit to the AMPM Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughStandardizes date rendering using date-fns Changes
Sequence Diagram(s)(Skipped — changes are UI/formatting and a new form element; no multi-component sequential flow requiring visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/common/FrontendExcelUploadDialog.tsx (1)
128-151: Parser does not actually supportDD/MM/YYYYas stated in error message and guidance.The error message (line 151) and format guidance (lines 576-577) tell users to enter
DD/MM/YYYY, butnew Date(value)does not reliably parse this format. TheDateconstructor primarily accepts ISO 8601 (YYYY-MM-DD), US format (MM/DD/YYYY), or named months—not DD/MM/YYYY. Inputs like25/12/2024will fail validation.Implement explicit DD/MM/YYYY parsing with regex or remove the format from guidance/error message.
💡 Suggested fix (explicit DD/MM/YYYY parsing)
if (typeof value === "string") { - const date = new Date(value); - if (!isNaN(date.getTime())) { - return { - isValid: true, - processedValue: date.toISOString().split("T")[0], - }; - } + const trimmed = value.trim(); + const match = trimmed.match(/^(\d{2})\/(\d{2})\/(\d{4})$/); + if (match) { + const [, dd, mm, yyyy] = match; + const date = new Date(Date.UTC(Number(yyyy), Number(mm) - 1, Number(dd))); + if (!Number.isNaN(date.getTime())) { + return { + isValid: true, + processedValue: date.toISOString().split("T")[0], + }; + } + } + + const date = new Date(trimmed); + if (!Number.isNaN(date.getTime())) { + return { + isValid: true, + processedValue: date.toISOString().split("T")[0], + }; + } }Also applies to: 576-577
🤖 Fix all issues with AI agents
In `@apps/web/app/`(dashboards)/hod/kpi-management/[id]/page.tsx:
- Around line 1-4: The code currently calls format(review.at) which can throw if
review.at is missing or invalid; add a small reusable helper named
formatTimestamp(value: string | number | Date | undefined): string that
validates the input (e.g., check for falsy or invalid Date via new Date(value)
and isNaN(date.getTime())), returns a formatted string using format(date, 'Pp')
when valid, and a safe fallback like '-' when invalid; then replace direct calls
to format(review.at) in both the coordinator review history and QC review
history sections with formatTimestamp(review.at) so both areas use the safe
formatter.
In `@apps/web/components/common/ReviewPanel.tsx`:
- Around line 1-4: The component's call to format(review.at) in ReviewPanel can
throw when review.at is a malformed/empty string; update ReviewPanel to first
coerce review.at to a Date (e.g. const parsed = review.at instanceof Date ?
review.at : new Date(review.at)) and validate it (use date-fns isValid(parsed)
or check !isNaN(parsed.getTime())) before calling format; if invalid, render a
safe fallback string like "Unknown date" (or an empty string) instead of calling
format so the component cannot crash.
In `@apps/web/components/formbuilder/table-rendered.tsx`:
- Around line 1418-1428: The visual-validation booleans (isStartInvalid and
isEndInvalid) only run when attributes.minDate is present, so invalid states are
missed when only attributes.maxDate is configured; update the logic for
isStartInvalid and isEndInvalid to not gate on attributes.minDate and instead
check start/end exist and then compare against attributes.minDate (if set) and
attributes.maxDate (if set) — keep isRangeInvalid as-is — so the red-border
visual validation matches the existing form validation that checks both minDate
and maxDate.
In `@apps/web/components/qc/readonly-form-table.tsx`:
- Around line 1-4: The code calls format(new Date(comment.reviewed_at), "...")
directly in places that can receive invalid date strings (used alongside the
Value component which already guards dates); add a helper function
formatDateSafe(dateInput: string | number | Date | null | undefined, fmt:
string) that constructs a Date, checks Number.isNaN(d.getTime()) and returns an
empty string (or a safe fallback) when invalid, otherwise returns format(d,
fmt); replace the direct calls format(new Date(comment.reviewed_at),
"dd/MM/yyyy") with formatDateSafe(comment.reviewed_at, "dd/MM/yyyy") in the
locations where reviewed_at is formatted and reuse the same helper in the Value
component so both paths share the same validation logic.
🧹 Nitpick comments (2)
apps/web/components/formbuilder/element-settings.tsx (1)
132-151: Add simple min/max constraints between the two date inputs.This prevents invalid ranges (min > max) at the source and improves UX.
♻️ Suggested tweak
<Input id="minDate" type="date" value={attributes.minDate || ""} + max={attributes.maxDate || undefined} onChange={(e) => handleChange("minDate", e.target.value)} /> ... <Input id="maxDate" type="date" value={attributes.maxDate || ""} + min={attributes.minDate || undefined} onChange={(e) => handleChange("maxDate", e.target.value)} />apps/web/components/formbuilder/table-rendered.tsx (1)
1148-1151: Consistent date formatting applied.Consider adding a fallback in case
reviewed_atis missing or invalid, though this appears to be within a block wherecommentis already validated.Optional defensive enhancement
- {format( - new Date(comment.reviewed_at), - "dd/MM/yyyy", - )} + {comment.reviewed_at + ? format( + new Date(comment.reviewed_at), + "dd/MM/yyyy", + ) + : "—"}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/components/formbuilder/table-rendered.tsx`:
- Around line 602-643: The required-check currently uses a falsy check on
entry[element.id] (value) which treats 0/0.0 and false as missing; update the
logic in the block that reads const value = entry[element.id] and the "Required
check" (where element.attributes.required is evaluated) to use an explicit empty
test — e.g., consider value missing only when value === undefined || value ===
null || value === '' — and special-case checkbox/boolean fields (element.type
=== "checkbox" or typeof value === "boolean") so that false is accepted as
present; keep the existing date-range presence checks but only coerce/split
String(value) when value is not empty per the new test and continue using
invalidRows for marking invalid entries.
- Line 3: The code calls format(new Date(...)) directly and can throw for
invalid/malformed reviewed_at values; add a small safe formatter (e.g.,
safeFormat or safeFormatReviewedAt) that: 1) parses the input into a Date, 2)
uses date-fns isValid (or checks !Number.isNaN(date.getTime())) before calling
format, and 3) returns a fallback string (empty or '-') when invalid; replace
the direct format(new Date(...)) usages in table-rendered.tsx (both call sites
that render reviewed_at) with this helper and keep the existing format string.
♻️ Duplicate comments (1)
apps/web/components/qc/readonly-form-table.tsx (1)
3-3: Guard date-fns formatting against invalidreviewed_atvalues.
format(new Date(...))will throw on invalid dates. The Value component already guards; apply the same safety here to avoid runtime crashes whenreviewed_atis malformed or empty. This mirrors a prior review note.✅ Proposed fix: safe helper used by both call sites
import React, { useState } from "react"; import { format } from "date-fns"; import { FormElementInstance } from "@/lib/types"; +const formatDateSafe = ( + value: string | number | Date | null | undefined, + pattern: string, +) => { + const d = new Date(value ?? ""); + return Number.isNaN(d.getTime()) ? "—" : format(d, pattern); +}; + ... - {format(new Date(comment.reviewed_at), "dd/MM/yyyy")} + {formatDateSafe(comment.reviewed_at, "dd/MM/yyyy")} ... - {format(new Date(comment.reviewed_at), "dd/MM/yyyy")} + {formatDateSafe(comment.reviewed_at, "dd/MM/yyyy")}In date-fns v4, does format(new Date("invalid"), "dd/MM/yyyy") throw? What is the recommended guard pattern for invalid dates?Also applies to: 442-446, 528-532
… date in the file
#195
Fix: remove hardcoded date formats and rely on native date picker (Issue 195), format timestamps to show date and time consistently in UI
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.