Refactor dateFormatter to use ISO date strings as timestamps#3857
Refactor dateFormatter to use ISO date strings as timestamps#3857kasya merged 33 commits intoOWASP:mainfrom
Conversation
…date is numbers now , not strings
|
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:
WalkthroughThis pull request unifies timestamp handling across the application by transitioning from mixed formats (numeric Unix timestamps and various string representations) to standardized ISO 8601 strings. Changes affect type definitions, utility functions, test fixtures, and both frontend and backend components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: This PR spans 80+ files across frontend and backend with changes to type definitions, utility function signatures, test data normalization, and model/resolver implementations. While individual changes follow consistent patterns (timestamp format updates, type narrowing), the breadth of affected areas (types, utilities, components, tests, backend models/resolvers) and the need to verify consistency across multiple layers demand thorough, context-aware review. The refactoring touches core date handling infrastructure, requiring attention to backward compatibility and data flow across the full stack. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
frontend/__tests__/unit/components/SingleModuleCard.test.tsx (2)
53-55:⚠️ Potential issue | 🟡 MinorMock
formatDateproduces incorrect dates with numeric timestamps.The mock uses
new Date(date)which interprets the numeric Unix timestamp (seconds) as milliseconds, yielding dates in 1970 instead of 2024. The realformatDatedoesnew Date(input * 1000). While current tests don't assert on specific formatted date strings, this will silently produce wrong output.Proposed fix
jest.mock('utils/dateFormatter', () => ({ - formatDate: jest.fn((date: string) => new Date(date).toLocaleDateString()), + formatDate: jest.fn((date: number) => new Date(date * 1000).toLocaleDateString()), }))
62-69:⚠️ Potential issue | 🟡 MinorMock
getSimpleDurationhas the same seconds-vs-milliseconds issue.
new Date(start)on a Unix-seconds value gives a 1970 date. The diff calculation will be wrong. Should multiply by 1000.Proposed fix
jest.mock('components/ModuleCard', () => ({ - getSimpleDuration: jest.fn((start: string, end: string) => { - const startDate = new Date(start) - const endDate = new Date(end) + getSimpleDuration: jest.fn((start: number, end: number) => { + const startDate = new Date(start * 1000) + const endDate = new Date(end * 1000) const diffTime = Math.abs(endDate.getTime() - startDate.getTime()) const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24)) return `${diffDays} days` }), }))frontend/src/components/ModuleCard.tsx (1)
189-203:⚠️ Potential issue | 🟡 Minor
getSimpleDurationshould accept onlynumberparameters to match theModuletype.The
Moduletype definesstartedAtandendedAtasnumber, yetgetSimpleDurationacceptsstring | number. All production callers pass numeric values fromModuleobjects, making the string-handling branch dead code.♻️ Suggested change
-export const getSimpleDuration = (start: string | number, end: string | number): string => { +export const getSimpleDuration = (start: number, end: number): string => { if (!start || !end) return 'N/A' - const startDate = typeof start === 'number' ? new Date(start * 1000) : new Date(start) - const endDate = typeof end === 'number' ? new Date(end * 1000) : new Date(end) + const startDate = new Date(start * 1000) + const endDate = new Date(end * 1000)frontend/__tests__/unit/utils/getIcsFileUrl.test.ts (1)
40-78:⚠️ Potential issue | 🟡 MinorDuplicate test: Lines 40–58 and 60–78 are identical in setup, input, and assertions.
Both tests spread
mockEventwith the samestartDate/endDatevalues it already contains, mockcreateEventidentically, and assert the samestart/endarrays. One of these should be removed to avoid dead test weight.frontend/src/utils/getGoogleCalendarUrl.ts (1)
6-11:⚠️ Potential issue | 🔴 CriticalBug:
formatLocalDateuses local-timezone accessors on UTC-midnight timestamps, producing wrong dates in non-UTC timezones.
formatLocalDatecallsgetFullYear(),getMonth(),getDate()— all local-time methods. When a user's browser is in a timezone behind UTC (e.g., UTC−5), a midnight-UTC timestamp like1764547200(2025-12-01T00:00:00Z) resolves to Nov 30 locally, so the all-day event date will be off by a day.Since these timestamps are defined as UTC (line 23–24 comment confirms this), the all-day branch should use UTC accessors, consistent with
formatUTCDateTime.Proposed fix
function formatLocalDate(date: Date) { - const year = date.getFullYear() - const month = pad(date.getMonth() + 1) - const day = pad(date.getDate()) + const year = date.getUTCFullYear() + const month = pad(date.getUTCMonth() + 1) + const day = pad(date.getUTCDate()) return `${year}${month}${day}` }You may also want to rename it to
formatUTCDatefor clarity.Also applies to: 44-48
frontend/__tests__/unit/components/Milestones.test.tsx (1)
126-127:⚠️ Potential issue | 🟡 MinorThe test mock User values use milliseconds instead of seconds.
The values
1640995200000increateMockUserare milliseconds (Jan 1, 2022), but given that this PR standardizes on Unix timestamps in seconds, these should be1640995200. This matters becauseuser.createdAtis used withformatDatein production code (e.g.,frontend/src/app/members/[memberKey]/page.tsx:114), which expects seconds. The test currently passes only because the Milestones component doesn't displayauthor.createdAt, but the mock data is incorrect and should match the pattern inmockUserDetails.ts, which correctly uses seconds.
🤖 Fix all issues with AI agents
In `@frontend/__tests__/mockData/mockRepositoryData.ts`:
- Line 58: The createdAt timestamp in mockRepositoryData (createdAt: 1740826800)
is off by one hour relative to the comment and other mocks; update the createdAt
value in mockRepositoryData to 1740823200 so it matches 2025-03-01T10:00:00Z and
stays consistent with mockUserDetails and e2e/mockHomeData entries.
In `@frontend/__tests__/unit/components/CardDetailsPage.test.tsx`:
- Line 555: The test uses numeric epoch seconds for Milestone.createdAt but the
createMilestones helper still sets createdAt to a string via new
Date(...).toISOString(); update the createMilestones function to return a
numeric timestamp (matching Milestone.createdAt?: number) — e.g. replace the
toISOString() usage with a numeric epoch seconds value (compute from Date using
getTime()/1000 or Math.floor) so createdAt is a number like 1672531200.
In
`@frontend/src/app/my/mentorship/programs/`[programKey]/modules/[moduleKey]/page.tsx:
- Around line 52-56: The current code double-converts timestamps by calling new
Date(...).getTime()/1000 before passing to formatDate; instead pass the
Unix-second timestamp directly from mentorshipModule.startedAt and
mentorshipModule.endedAt to formatDate (or coerce them to number if they may be
strings) because formatDate expects seconds and performs its own new Date(input
* 1000); update the objects where label: 'Start Date' and 'End Date' are
constructed to use mentorshipModule.startedAt and mentorshipModule.endedAt
directly.
In `@frontend/src/app/page.tsx`:
- Line 190: The call passes label={formatDateRange(event.startDate as number,
event.endDate as number)} which hides a real type mismatch: event.startDate is
already number but event.endDate is number | undefined, and the unnecessary "as
number" casts suppress the compiler; remove the casts and either (A) update the
Event type to make endDate required, or (B) change formatDateRange (or the call
site) to accept/handle endDate?: number (e.g., provide a fallback or overload)
so the parameter types align—locate formatDateRange, the Event type, and this
usage (event.startDate/event.endDate) and make one of those two fixes so
TypeScript type checking is preserved.
In `@frontend/src/components/CardDetailsPage.tsx`:
- Around line 463-465: The code double-converts milestone.createdAt before
calling formatDate, causing incorrect dates; instead pass the numeric Unix
seconds directly. In CardDetailsPage replace the expression formatDate(new
Date(milestone.createdAt).getTime() / 1000) with formatDate(milestone.createdAt)
(use the milestone.createdAt value as-is) so formatDate receives seconds as
expected.
In `@frontend/src/components/MenteeIssues.tsx`:
- Around line 86-89: In MenteeIssues.tsx the timestamps are being
double-converted before formatting: remove the unnecessary new
Date(...).getTime() / 1000 wrapper and pass issue.createdAt and issue.updatedAt
(when present) directly to formatDate, e.g. replace formatDate(new
Date(issue.createdAt).getTime() / 1000) with formatDate(issue.createdAt); keep
the conditional for issue.updatedAt as-is and mirror the pattern used in
RecentIssues.tsx so formatDate receives the original Unix seconds number.
In `@frontend/src/components/ModuleCard.tsx`:
- Around line 106-110: The displayed start date is wrong because ModuleCard's
TextInfoItem passes formatDate(new Date(module.startedAt).getTime() / 1000)
which treats an already-second Unix timestamp as milliseconds; update the call
to pass the seconds value directly (use formatDate(module.startedAt)) in the
ModuleCard component where TextInfoItem is rendered so formatDate (which expects
seconds) receives the correct input; ensure you adjust only the expression
around module.startedAt and leave TextInfoItem and formatDate implementations
unchanged.
In `@frontend/src/components/RecentPullRequests.tsx`:
- Line 32: RecentPullRequests.tsx is double-converting createdAt which breaks
numeric Unix timestamps; update the span rendering to check typeof
item.createdAt: if it's a number pass item.createdAt directly to formatDate
(since formatDate expects seconds), otherwise convert the string via new
Date(item.createdAt).getTime() / 1000 and pass that; reference the JSX span
using item.createdAt and the formatDate call so you only divide by 1000 for
string dates and avoid double-conversion for numeric timestamps.
In `@frontend/src/components/SingleModuleCard.tsx`:
- Around line 45-46: The current code double-converts timestamps by calling new
Date(module.startedAt).getTime()/1000 and new
Date(module.endedAt).getTime()/1000 before passing to formatDate; instead, pass
the Unix-second values directly (module.startedAt and module.endedAt) to
formatDate. Locate the entries where the labels 'Start Date' and 'End Date' are
formed in the SingleModuleCard component and replace the new
Date(...).getTime()/1000 expressions with module.startedAt and module.endedAt
respectively so formatDate receives the correct seconds-based timestamps.
🧹 Nitpick comments (5)
frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
364-364: Using0as a sentinel for "no date" relies on JS falsiness.This works because
0is falsy, soif (startedAt)guards in the component correctly fall through to "No dates set". However,0is technically a valid Unix timestamp (epoch). If the type werenumber | null, intent would be clearer and less prone to accidental misuse in arithmetic paths. This is a broader design choice for the PR though, not specific to these tests.Also applies to: 378-378, 393-393
frontend/src/components/Card.tsx (1)
86-91: Conversion is correct here, butCardProps.timelinetypes are stillstring.Unlike the
Modulecase,timeline.startandtimeline.endare still typed asstringinCardProps(seefrontend/src/types/card.ts, lines 33-34), so thenew Date(string).getTime() / 1000conversion is valid. However, for consistency with the PR's goal of enforcing numeric timestamps everywhere, consider updatingCardProps.timelineto usenumberas well and moving the conversion upstream to the call sites.frontend/src/utils/dateFormatter.ts (1)
62-69: Parameter namedateStris now misleading — it's a number, not a string.A small rename to
timestampordatewould improve clarity.Suggested rename
-export const formatDateForInput = (dateStr: number) => { - if (!dateStr) return '' - const date = new Date(dateStr * 1000) +export const formatDateForInput = (timestamp: number) => { + if (!timestamp) return '' + const date = new Date(timestamp * 1000)frontend/__tests__/unit/utils/dateFormatter.test.ts (1)
3-23: Consider adding edge-case tests for the0and invalid inputs.The
formatDateimplementation has two distinct code paths not covered here:formatDate(0)returns''(falsy guard), andformatDate(NaN)throws'Invalid date'. Adding a test for at least the0→''case would improve coverage of the guard clause, and a test for the error path would document the contract.frontend/__tests__/unit/components/Milestones.test.tsx (1)
14-31: The mock duplicates the realformatDateimplementation.This mock reimplements the exact same logic as the production
formatDate(multiply by 1000,toLocaleDateStringwith identical options). If the real formatter's behavior changes, this mock won't detect regressions. Consider either:
- Using a trivial mock (
jest.fn(() => 'Jan 1, 2023')) and asserting the mock was called with the expected timestamp, or- Not mocking
dateFormatterat all, letting the real implementation run.♻️ Simpler mock approach
jest.mock('utils/dateFormatter', () => { - const mockFormatDate = jest.fn((timestamp: number) => { - if (!timestamp) return '' - const date = new Date(timestamp * 1000) - return date.toLocaleDateString('en-US', { - year: 'numeric', - month: 'short', - day: 'numeric', - timeZone: 'UTC', - }) - }) + const mockFormatDate = jest.fn((timestamp: number) => + timestamp ? `formatted-${timestamp}` : '' + ) return { __esModule: true, formatDate: mockFormatDate, default: mockFormatDate, } })Then update assertions to match on the stub output (e.g.,
'formatted-1672531200') or verify.toHaveBeenCalledWith(1672531200).
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
16 issues found across 52 files
Confidence score: 3/5
- Several date fields are double-converted to epoch seconds (e.g.,
formatDatefednew Date(...).getTime()/1000on values already in seconds) causing incorrect dates (often around 1970) infrontend/src/components/MenteeIssues.tsx,frontend/src/components/SingleModuleCard.tsx,frontend/src/components/ModuleCard.tsx,frontend/src/components/CardDetailsPage.tsx, andfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx. frontend/src/app/page.tsxcasts optionalevent.endDatetonumber, which can yield an Invalid date at runtime when the value is missing.- These are user-facing date display issues with concrete impact, so there’s some merge risk despite the fixes being straightforward.
- Pay close attention to
frontend/src/components/MenteeIssues.tsx,frontend/src/components/SingleModuleCard.tsx,frontend/src/components/ModuleCard.tsx,frontend/src/components/CardDetailsPage.tsx,frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx,frontend/src/app/page.tsx- ensure epoch seconds aren’t scaled twice and optional dates are guarded.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/__tests__/mockData/mockRepositoryData.ts">
<violation number="1" location="frontend/__tests__/mockData/mockRepositoryData.ts:58">
P2: The Unix timestamp does not match the ISO comment; `2025-03-01T10:00:00Z` should be 1740823200, not 1740826800. This makes the mock data inconsistent and can cause time-based tests to drift by an hour.</violation>
</file>
<file name="frontend/src/components/MenteeIssues.tsx">
<violation number="1" location="frontend/src/components/MenteeIssues.tsx:86">
P2: `formatDate` already expects a Unix-seconds number. Wrapping `issue.createdAt` in `new Date(...).getTime() / 1000` will shrink already-seconds values by 1000 and render incorrect dates. Use the numeric timestamp directly.</violation>
<violation number="2" location="frontend/src/components/MenteeIssues.tsx:88">
P2: Same issue for `updatedAt`: `formatDate` expects a Unix-seconds number, so the extra `new Date(...).getTime() / 1000` will mis-scale seconds-based values.</violation>
</file>
<file name="frontend/src/components/SingleModuleCard.tsx">
<violation number="1" location="frontend/src/components/SingleModuleCard.tsx:45">
P2: `module.startedAt` is already a Unix-seconds number; converting it through `new Date(...).getTime()/1000` divides it by 1000 and produces an incorrect timestamp. Pass the seconds value directly to `formatDate` to avoid shifting dates to 1970.</violation>
<violation number="2" location="frontend/src/components/SingleModuleCard.tsx:46">
P2: `module.endedAt` is already a Unix-seconds number; converting it via `new Date(...).getTime()/1000` shrinks the timestamp and produces wrong dates. Pass the seconds value directly to `formatDate`.</violation>
</file>
<file name="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx">
<violation number="1" location="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx:54">
P2: `formatDate` already expects a seconds timestamp; wrapping `mentorshipModule.startedAt` in `new Date(...).getTime() / 1000` will turn a seconds value into a much smaller timestamp (dates around 1970). Pass the numeric timestamp directly.</violation>
<violation number="2" location="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx:56">
P2: `formatDate` already expects a seconds timestamp; wrapping `mentorshipModule.endedAt` in `new Date(...).getTime() / 1000` will turn a seconds value into a much smaller timestamp (dates around 1970). Pass the numeric timestamp directly.</violation>
</file>
<file name="frontend/src/app/page.tsx">
<violation number="1" location="frontend/src/app/page.tsx:190">
P2: Casting `event.endDate` to `number` hides the fact that `endDate` is optional and can be `undefined`, which can trigger an Invalid date error at runtime. Guard or fall back when `endDate` is missing instead of asserting.</violation>
</file>
<file name="frontend/src/components/ModuleCard.tsx">
<violation number="1" location="frontend/src/components/ModuleCard.tsx:109">
P2: Module.startedAt is already a numeric Unix timestamp (seconds). Wrapping it in `new Date(...).getTime() / 1000` converts seconds as if they were milliseconds and then divides again, yielding a timestamp 1000× too small. This will render incorrect start dates. Pass the seconds value directly to `formatDate` (or explicitly multiply by 1000 only if `startedAt` is in ms).</violation>
</file>
<file name="frontend/src/components/RecentPullRequests.tsx">
<violation number="1" location="frontend/src/components/RecentPullRequests.tsx:32">
P2: This conversion double-scales numeric Unix seconds. Since `formatDate` already expects seconds, only convert ISO strings and pass numeric timestamps through unchanged to avoid incorrect dates.</violation>
</file>
<file name="frontend/src/components/CardDetailsPage.tsx">
<violation number="1" location="frontend/src/components/CardDetailsPage.tsx:464">
P2: `milestone.createdAt` is already a numeric Unix timestamp; wrapping it in `new Date(...).getTime() / 1000` scales it down by 1000, yielding incorrect dates. Pass the numeric value directly (and optionally guard for undefined) instead of re-converting it.</violation>
</file>
<file name="frontend/src/utils/getIcsFileUrl.ts">
<violation number="1" location="frontend/src/utils/getIcsFileUrl.ts:13">
P2: DateArray values are now derived with UTC getters, but `createEvent` treats DateArray inputs as local time by default. This can shift the calendar date in non‑UTC timezones. Use local getters or set `startInputType/endInputType` to `utc` when passing UTC timestamps.</violation>
</file>
<file name="frontend/__tests__/mockData/mockHomeData.ts">
<violation number="1" location="frontend/__tests__/mockData/mockHomeData.ts:4">
P2: The recentProjects createdAt timestamps don’t match the original ISO dates. These should be converted to the correct epoch for 2024-12-06T20:46:54Z (1733518014) to avoid changing the mock timeline.</violation>
<violation number="2" location="frontend/__tests__/mockData/mockHomeData.ts:97">
P2: The publishedAt timestamp doesn’t match the original ISO value; it’s off by ~40 minutes. Convert 2024-12-13T14:43:46Z to its correct epoch value.</violation>
<violation number="3" location="frontend/__tests__/mockData/mockHomeData.ts:128">
P2: endDate conversion doesn’t match the original date-only value; use the midnight UTC epoch for 2025-02-28 to keep the mock date consistent.</violation>
<violation number="4" location="frontend/__tests__/mockData/mockHomeData.ts:130">
P2: startDate conversion doesn’t match the original date-only value; use the midnight UTC epoch for 2025-02-27 to keep the mock date consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
closing the PR because of conversions errors |
Please don't create multiple PRs if you need to improve your code. It's okay to close it if you won't create new PRs related to this issue. |
|
Thanks for the feedback, I'll reopen the PR when I address the bot comments |
88532b8 to
fb768d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/utils/getIcsFileUrl.ts (1)
16-23:⚠️ Potential issue | 🟠 MajorSame local-vs-UTC issue in
getEndDate.
getEndDateconstructs anew Date(y, m - 1, d + 1)which interprets values in local time and then extracts components with local-time getters. IfparseDateis fixed to return UTC components, this function should also useDate.UTC(...)and UTC getters to stay consistent; otherwise the "+1 day" rollover logic may still drift across timezone boundaries.Proposed fix
const getEndDate = (start: DateArray, end: DateArray): DateArray => { if (start.join('-') === end.join('-')) { const [y, m, d] = end - const nextDay = new Date(y, m - 1, d + 1) - return [nextDay.getFullYear(), nextDay.getMonth() + 1, nextDay.getDate()] + const nextDay = new Date(Date.UTC(y, m - 1, d + 1)) + return [nextDay.getUTCFullYear(), nextDay.getUTCMonth() + 1, nextDay.getUTCDate()] } return end }frontend/__tests__/a11y/components/RecentPullRequests.a11y.test.tsx (1)
34-34:⚠️ Potential issue | 🟠 MajorConvert
mockUser.createdAtfrom milliseconds to seconds.The
formatDatefunction expects Unix timestamps in seconds and multiplies by 1000 to convert to milliseconds (line 6 offrontend/src/utils/dateFormatter.ts). The value1577836800000is in milliseconds; it should be1577836800in seconds. This is inconsistent with all other test mocks in the codebase (mockUserDetails, mockRepositoryData, mockOrganizationData, etc.), which correctly use seconds. Without this fix, the component will render an incorrect date.Proposed fix
- createdAt: 1577836800000, + createdAt: 1577836800,
🤖 Fix all issues with AI agents
In `@frontend/src/utils/getIcsFileUrl.ts`:
- Around line 10-14: The parseDate function uses local-time getters which can
shift the calendar day across timezones; change parseDate to use UTC getters:
create the Date from the timestamp*1000 as before, then use d.getUTCFullYear(),
d.getUTCMonth() + 1, and d.getUTCDate() to populate the DateArray so the
date-only value reflects the absolute UTC calendar day (keep the month +1
adjustment).
|
@coderabbitai review changes |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
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 (5)
frontend/__tests__/a11y/components/Release.a11y.test.tsx (1)
6-19:⚠️ Potential issue | 🟡 MinorUpdate
publishedAtto ISO string in test fixture.The
Release.publishedAttype is defined asstringinfrontend/src/types/release.ts, but the test fixture usesDate.now()which returns a number. This creates a type mismatch. Line 8 should be updated to match the correctedauthor.createdAtformat on line 19.🔧 Proposed fix
const release: ReleaseType = { name: 'v1.0 The First Release', - publishedAt: Date.now(), + publishedAt: new Date().toISOString(), repositoryName: 'our-awesome-project',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/a11y/components/Release.a11y.test.tsx` around lines 6 - 19, The test fixture uses a numeric timestamp for Release.publishedAt but the ReleaseType defines publishedAt as a string; update the publishedAt field in the Release fixture (in the Release.a11y.test.tsx file) to an ISO string (e.g. new Date().toISOString() or a hard-coded ISO date) so it matches the string type like author.createdAt and satisfies the ReleaseType; ensure the field name publishedAt in the ReleaseType fixture is the only change.frontend/__tests__/unit/components/Release.test.tsx (1)
71-76:⚠️ Potential issue | 🟠 MajorType mismatch:
publishedAtshould be a string, not a number.
Date.now()returns a number, butRelease.publishedAtis now typed asstringper the changes infrontend/src/types/release.ts. This will cause TypeScript compilation errors.🐛 Proposed fix
-const now = Date.now() +const now = new Date().toISOString() const mockReleases: ReleaseType[] = [ { id: 'release-test-1', name: 'v1.0 The First Release', publishedAt: now,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/components/Release.test.tsx` around lines 71 - 76, The test uses Date.now() (a number) for publishedAt but Release.publishedAt is typed as string; update the mock in Release.test.tsx by changing the now variable or the publishedAt value in the mockReleases entry (referencing the mockReleases array and the now constant) to a string representation (e.g., ISO string or String(now)) so the publishedAt field matches the ReleaseType string type.backend/tests/apps/github/api/internal/nodes/user_test.py (2)
80-87:⚠️ Potential issue | 🟠 MajorTest expects float but implementation returns string.
Same issue as
test_created_at_field: the test setsmock_user.idx_updated_at = 1234567890.0and usesmath.isclose(), but the resolver now returns a string.Proposed fix
def test_updated_at_field(self): - """Test updated_at field resolution.""" + """Test updated_at field resolution returns ISO string.""" mock_user = Mock() - mock_user.idx_updated_at = 1234567890.0 + mock_user.idx_updated_at = "2009-02-13T23:31:30+00:00" field = self._get_field_by_name("updated_at", UserNode) result = field.base_resolver.wrapped_func(None, mock_user) - assert math.isclose(result, 1234567890.0) + assert result == "2009-02-13T23:31:30+00:00"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/apps/github/api/internal/nodes/user_test.py` around lines 80 - 87, The test fails because UserNode's updated_at resolver (accessed in the test via field = self._get_field_by_name("updated_at", UserNode) and field.base_resolver.wrapped_func) is returning a string instead of a numeric float; update the resolver implementation for updated_at to return the original numeric value (cast/parse idx_updated_at to float if necessary) so that calling field.base_resolver.wrapped_func(None, mock_user) yields a float comparable with math.isclose instead of a string.
53-60:⚠️ Potential issue | 🟠 MajorTest expects float but implementation returns string.
The test sets
mock_user.idx_created_at = 1234567890.0and asserts withmath.isclose(result, 1234567890.0). However, thecreated_atresolver now returnsroot.idx_created_atwhich is expected to be an ISO format string (based on changes inUserIndexMixin.idx_created_at).This test will fail because the resolver's return type is now
str, notfloat.Proposed fix
def test_created_at_field(self): - """Test created_at field resolution.""" + """Test created_at field resolution returns ISO string.""" mock_user = Mock() - mock_user.idx_created_at = 1234567890.0 + mock_user.idx_created_at = "2009-02-13T23:31:30+00:00" field = self._get_field_by_name("created_at", UserNode) result = field.base_resolver.wrapped_func(None, mock_user) - assert math.isclose(result, 1234567890.0) + assert result == "2009-02-13T23:31:30+00:00"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/apps/github/api/internal/nodes/user_test.py` around lines 53 - 60, Test assumes created_at is a float but the resolver now returns whatever root.idx_created_at holds (an ISO string from UserIndexMixin.idx_created_at); update test_created_at_field to set mock_user.idx_created_at to the ISO-format string you expect and assert equality (e.g., result == mock_user.idx_created_at) instead of using math.isclose, locating the field via _get_field_by_name("created_at", UserNode) and calling field.base_resolver.wrapped_func as before.frontend/__tests__/mockData/mockHomeData.ts (1)
40-40:⚠️ Potential issue | 🟡 MinorThe
typefield change removes test coverage for the 'Other' project type.The
typefield forowasp-project-5was changed from'Other'to an empty string''. While the empty string is handled by thegetProjectIcon()default case (which returns the file code icon), this change affects the UI display: with an empty type, no label text is shown (only the icon); with'Other', both the icon and "Other" label are displayed.Since project type is rendered on the home page and the
'Other'type is a valid option in the codebase, restoring it to'Other'would maintain test coverage of that project type variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/mockData/mockHomeData.ts` at line 40, Restore the project type for the mock entry with id 'owasp-project-5' by changing its type field back from an empty string to 'Other' so the UI test covers the 'Other' project type variant; update the object in mockHomeData.ts (the entry containing id 'owasp-project-5' and the property type) so getProjectIcon() still falls through to the correct icon while the label text "Other" is rendered in tests.
♻️ Duplicate comments (2)
frontend/__tests__/unit/components/CardDetailsPage.test.tsx (1)
1905-1922:⚠️ Potential issue | 🟡 MinorInconsistent timestamp format in
createMilestoneshelper.The
mockRecentMilestonesfixture at line 714 uses an ISO string forcreatedAt, but thiscreateMilestoneshelper generates a numeric Unix timestamp. IfMilestone.createdAtis being migrated tostring, this helper should produce ISO strings for consistency.🔧 Proposed fix
milestones.push({ author: mockUser, body: `Milestone description ${i + 1}`, closedIssuesCount: 5, - createdAt: Math.floor((Date.now() - 10000000) / 1000), + createdAt: new Date(Date.now() - 10000000).toISOString(), openIssuesCount: 2,#!/bin/bash # Verify the Milestone.createdAt type definition rg -n "createdAt" frontend/src/types/milestone.ts -A2 -B2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/components/CardDetailsPage.test.tsx` around lines 1905 - 1922, The createMilestones helper generates numeric Unix timestamps for Milestone.createdAt but other fixtures like mockRecentMilestones use ISO strings; update createMilestones to produce ISO-formatted strings (e.g., new Date(...).toISOString()) for createdAt so the type matches Milestone.createdAt and tests stay consistent; locate the helper function createMilestones in the test file and replace the numeric timestamp generation with ISO string generation, ensuring all createdAt usages across tests (including mockRecentMilestones) are consistent.frontend/src/utils/getIcsFileUrl.ts (1)
10-14:⚠️ Potential issue | 🟠 MajorICS date extraction is timezone-unsafe and missing UTC input type flags.
parseDateandgetEndDateuse local-time APIs, which can shift calendar days. Also,icsshould be told these arrays are UTC to avoid reinterpretation as local time.💡 Proposed fix
- const parseDate = (date: string): DateArray => { - // ISO date string - const d = new Date(date) - return [d.getFullYear(), d.getMonth() + 1, d.getDate()] - } + const parseDate = (date: string): DateArray => { + const d = new Date(date) + return [d.getUTCFullYear(), d.getUTCMonth() + 1, d.getUTCDate()] + } @@ - const nextDay = new Date(y, m - 1, d + 1) - return [nextDay.getFullYear(), nextDay.getMonth() + 1, nextDay.getDate()] + const nextDay = new Date(Date.UTC(y, m - 1, d + 1)) + return [nextDay.getUTCFullYear(), nextDay.getUTCMonth() + 1, nextDay.getUTCDate()] @@ const eventAttributes: EventAttributes = { start: startArray, + startInputType: 'utc', end: finalEndArray, + endInputType: 'utc', title: event.title,#!/bin/bash # Verify timezone-sensitive APIs and UTC input flags in ICS utility rg -n -C2 "parseDate|getFullYear|getMonth|getDate|getUTCFullYear|getUTCMonth|getUTCDate|startInputType|endInputType" frontend/src/utils/getIcsFileUrl.tsBased on learnings, in
frontend/src/utils/getIcsFileUrl.tsUTC getters plusstartInputType: 'utc'andendInputType: 'utc'are required to preserve correct UTC dates with theicslibrary.Also applies to: 16-21, 29-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/getIcsFileUrl.ts` around lines 10 - 14, parseDate and getEndDate currently use local-time getters which can shift calendar days; update parseDate to use UTC getters (getUTCFullYear, getUTCMonth + 1, getUTCDate) and adjust getEndDate similarly so both build DateArray in UTC, and when creating the ics event ensure you pass startInputType: 'utc' and endInputType: 'utc' so the ics library treats those arrays as UTC. Locate the functions parseDate and getEndDate and the place where the ics event/options are constructed and apply these changes.
🧹 Nitpick comments (5)
backend/apps/owasp/models/mixins/committee.py (2)
12-15: Consider adding type annotations for consistency.The implementation correctly returns ISO strings. For consistency with other mixins (e.g.,
ProjectIndexMixin.idx_updated_atwhich has-> str), consider adding type annotations:♻️ Optional: Add type hints
`@property` - def idx_created_at(self): + def idx_created_at(self) -> str | None: """Return created at for indexing.""" return self.created_at.isoformat() if self.created_at else None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/models/mixins/committee.py` around lines 12 - 15, The idx_created_at property lacks a type annotation; update the signature of idx_created_at to include a return type (use -> Optional[str]) and ensure Optional is imported from typing if not already present so the method signature reads idx_created_at(self) -> Optional[str]; keep the implementation returning self.created_at.isoformat() if present else None and leave the docstring as-is.
32-35: Implementation correctly returns ISO string.Same suggestion for type annotation consistency:
♻️ Optional: Add type hint
`@property` - def idx_updated_at(self): + def idx_updated_at(self) -> str | None: """Return updated at for indexing.""" return self.updated_at.isoformat() if self.updated_at else None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/models/mixins/committee.py` around lines 32 - 35, Add a return type annotation to the idx_updated_at property so its signature explicitly indicates it returns an optional string (e.g., Optional[str] or str | None) and import Optional from typing if not already present; update the property signature def idx_updated_at(self) -> Optional[str]: and keep the implementation using self.updated_at.isoformat() if self.updated_at else None, referencing idx_updated_at and updated_at to locate the change.backend/apps/github/models/mixins/user.py (1)
225-241: Date format inconsistency in nested fields.
idx_issuesreturnscreated_atas a Unix timestamp (.timestamp()), whileidx_created_atandidx_updated_atreturn ISO strings (.isoformat()). The same pattern appears inidx_releaseswithpublished_at. Consider standardizing nested date fields to ISO format for consistency across the indexing layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/github/models/mixins/user.py` around lines 225 - 241, The nested date in the issues index is rendered as a Unix timestamp (created_at.timestamp()) causing inconsistency with other indexers that use ISO strings; update the code in the idx_issues logic (the list comprehension returning dicts for self.issues) to use ISO format (created_at.isoformat()) and mirror this standard for nested date fields elsewhere (e.g., idx_releases published_at) so that idx_issues, idx_created_at, idx_updated_at and idx_releases consistently emit ISO8601 strings.frontend/__tests__/unit/utils/getIcsFileUrl.test.ts (2)
42-45: Keep test fixtures aligned withCalendarEventrequired fields.These fixtures omit
title, which weakens contract-level confidence forgetIcsFileUrl(event: CalendarEvent).💡 Minimal fixture alignment
const eventWithTimestamps = { + title: 'Event', startDate: '2025-01-01T00:00:00Z', endDate: '2025-01-03T00:00:00Z', } @@ const singleDayEvent = { + title: 'Single Day Event', startDate: '2025-01-01T00:00:00Z', endDate: '2025-01-01T00:00:00Z', }Also applies to: 61-64, 80-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/utils/getIcsFileUrl.test.ts` around lines 42 - 45, The test fixtures for getIcsFileUrl(event: CalendarEvent) are missing the required title field, weakening the contract coverage; update each fixture object (e.g., eventWithTimestamps and the other event* variables used in this test file) to include a valid title string so they conform to CalendarEvent shape and exercise real inputs for getIcsFileUrl.
39-75: Two UTC/date-array tests are duplicates.Both tests use identical payloads and assertions, so the second one doesn’t add coverage. Consider merging into one case or changing one input to exercise a distinct timezone scenario.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/utils/getIcsFileUrl.test.ts` around lines 39 - 75, The two tests in frontend/__tests__/unit/utils/getIcsFileUrl.test.ts are duplicates; update the tests that call getIcsFileUrl (and mock createEvent) so they cover distinct scenarios: either remove the second identical "should correctly format Unix timestamps into DateArray (UTC conversion)" test and keep a single UTC-case, or change the second test's eventWithTimestamps to a non-UTC/timezone-offset input (e.g., '2025-01-01T00:00:00-05:00') and adjust the expected createEvent DateArray values accordingly; locate the tests by the it blocks that call getIcsFileUrl and reference the mocked createEvent to implement the change.
🤖 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/__tests__/unit/utils/getGoogleCalendarUrl.test.ts`:
- Around line 26-31: The test case name in the getGoogleCalendarUrl unit test is
misleading—it's labeled "handles Unix timestamps correctly" but passes ISO
string inputs; rename the test description to reflect ISO string handling (e.g.,
"handles ISO timestamp strings correctly" or similar) so it matches the fixture
values used in the getGoogleCalendarUrl test; locate the it(...) block inside
the 'timestamp handling' describe and update the string only.
In `@frontend/src/app/board/`[year]/candidates/page.tsx:
- Around line 36-37: The createdAt/firstOwaspContributionAt handling is
inconsistent with formatDate(string|null): change both field types to string |
null (update the interface/type that declares createdAt and
firstOwaspContributionAt) and remove the runtime numeric-branch (typeof ... ===
'number' and any *1000 logic) from the rendering; instead ensure any incoming
numeric timestamps are converted to ISO strings at the data parsing layer so
render code simply calls formatDate(createdAt) and
formatDate(firstOwaspContributionAt) without number guards.
In `@frontend/src/app/organizations/`[organizationKey]/page.tsx:
- Around line 121-126: The mapped recentReleases currently force-casts
release.publishedAt to string which hides a real GraphQL type (DateTime | null);
update the mapping in the recentReleases transform to explicitly handle
DateTime|null by converting a non-null value to an ISO string (e.g., via new
Date(release.publishedAt).toISOString()) and providing a safe fallback (e.g.,
empty string or null) so the Release type and runtime values align, or
alternatively adjust the Release type to accept DateTime | null and propagate
that change where Release is consumed.
In `@frontend/src/utils/getGoogleCalendarUrl.ts`:
- Around line 23-29: isAllDayEvent uses UTC checks but formatLocalDate uses
local getters, causing date shifts for UTC midnight timestamps; rename
formatLocalDate to formatUTCDate and update its implementation (and all call
sites in this file) to use getUTCFullYear(), getUTCMonth(), and getUTCDate() so
formatting is done in UTC and matches isAllDayEvent (ensure getUTC* getters
replace local getters wherever formatLocalDate was used).
---
Outside diff comments:
In `@backend/tests/apps/github/api/internal/nodes/user_test.py`:
- Around line 80-87: The test fails because UserNode's updated_at resolver
(accessed in the test via field = self._get_field_by_name("updated_at",
UserNode) and field.base_resolver.wrapped_func) is returning a string instead of
a numeric float; update the resolver implementation for updated_at to return the
original numeric value (cast/parse idx_updated_at to float if necessary) so that
calling field.base_resolver.wrapped_func(None, mock_user) yields a float
comparable with math.isclose instead of a string.
- Around line 53-60: Test assumes created_at is a float but the resolver now
returns whatever root.idx_created_at holds (an ISO string from
UserIndexMixin.idx_created_at); update test_created_at_field to set
mock_user.idx_created_at to the ISO-format string you expect and assert equality
(e.g., result == mock_user.idx_created_at) instead of using math.isclose,
locating the field via _get_field_by_name("created_at", UserNode) and calling
field.base_resolver.wrapped_func as before.
In `@frontend/__tests__/a11y/components/Release.a11y.test.tsx`:
- Around line 6-19: The test fixture uses a numeric timestamp for
Release.publishedAt but the ReleaseType defines publishedAt as a string; update
the publishedAt field in the Release fixture (in the Release.a11y.test.tsx file)
to an ISO string (e.g. new Date().toISOString() or a hard-coded ISO date) so it
matches the string type like author.createdAt and satisfies the ReleaseType;
ensure the field name publishedAt in the ReleaseType fixture is the only change.
In `@frontend/__tests__/mockData/mockHomeData.ts`:
- Line 40: Restore the project type for the mock entry with id 'owasp-project-5'
by changing its type field back from an empty string to 'Other' so the UI test
covers the 'Other' project type variant; update the object in mockHomeData.ts
(the entry containing id 'owasp-project-5' and the property type) so
getProjectIcon() still falls through to the correct icon while the label text
"Other" is rendered in tests.
In `@frontend/__tests__/unit/components/Release.test.tsx`:
- Around line 71-76: The test uses Date.now() (a number) for publishedAt but
Release.publishedAt is typed as string; update the mock in Release.test.tsx by
changing the now variable or the publishedAt value in the mockReleases entry
(referencing the mockReleases array and the now constant) to a string
representation (e.g., ISO string or String(now)) so the publishedAt field
matches the ReleaseType string type.
---
Duplicate comments:
In `@frontend/__tests__/unit/components/CardDetailsPage.test.tsx`:
- Around line 1905-1922: The createMilestones helper generates numeric Unix
timestamps for Milestone.createdAt but other fixtures like mockRecentMilestones
use ISO strings; update createMilestones to produce ISO-formatted strings (e.g.,
new Date(...).toISOString()) for createdAt so the type matches
Milestone.createdAt and tests stay consistent; locate the helper function
createMilestones in the test file and replace the numeric timestamp generation
with ISO string generation, ensuring all createdAt usages across tests
(including mockRecentMilestones) are consistent.
In `@frontend/src/utils/getIcsFileUrl.ts`:
- Around line 10-14: parseDate and getEndDate currently use local-time getters
which can shift calendar days; update parseDate to use UTC getters
(getUTCFullYear, getUTCMonth + 1, getUTCDate) and adjust getEndDate similarly so
both build DateArray in UTC, and when creating the ics event ensure you pass
startInputType: 'utc' and endInputType: 'utc' so the ics library treats those
arrays as UTC. Locate the functions parseDate and getEndDate and the place where
the ics event/options are constructed and apply these changes.
---
Nitpick comments:
In `@backend/apps/github/models/mixins/user.py`:
- Around line 225-241: The nested date in the issues index is rendered as a Unix
timestamp (created_at.timestamp()) causing inconsistency with other indexers
that use ISO strings; update the code in the idx_issues logic (the list
comprehension returning dicts for self.issues) to use ISO format
(created_at.isoformat()) and mirror this standard for nested date fields
elsewhere (e.g., idx_releases published_at) so that idx_issues, idx_created_at,
idx_updated_at and idx_releases consistently emit ISO8601 strings.
In `@backend/apps/owasp/models/mixins/committee.py`:
- Around line 12-15: The idx_created_at property lacks a type annotation; update
the signature of idx_created_at to include a return type (use -> Optional[str])
and ensure Optional is imported from typing if not already present so the method
signature reads idx_created_at(self) -> Optional[str]; keep the implementation
returning self.created_at.isoformat() if present else None and leave the
docstring as-is.
- Around line 32-35: Add a return type annotation to the idx_updated_at property
so its signature explicitly indicates it returns an optional string (e.g.,
Optional[str] or str | None) and import Optional from typing if not already
present; update the property signature def idx_updated_at(self) ->
Optional[str]: and keep the implementation using self.updated_at.isoformat() if
self.updated_at else None, referencing idx_updated_at and updated_at to locate
the change.
In `@frontend/__tests__/unit/utils/getIcsFileUrl.test.ts`:
- Around line 42-45: The test fixtures for getIcsFileUrl(event: CalendarEvent)
are missing the required title field, weakening the contract coverage; update
each fixture object (e.g., eventWithTimestamps and the other event* variables
used in this test file) to include a valid title string so they conform to
CalendarEvent shape and exercise real inputs for getIcsFileUrl.
- Around line 39-75: The two tests in
frontend/__tests__/unit/utils/getIcsFileUrl.test.ts are duplicates; update the
tests that call getIcsFileUrl (and mock createEvent) so they cover distinct
scenarios: either remove the second identical "should correctly format Unix
timestamps into DateArray (UTC conversion)" test and keep a single UTC-case, or
change the second test's eventWithTimestamps to a non-UTC/timezone-offset input
(e.g., '2025-01-01T00:00:00-05:00') and adjust the expected createEvent
DateArray values accordingly; locate the tests by the it blocks that call
getIcsFileUrl and reference the mocked createEvent to implement the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
frontend/src/types/__generated__/boardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/chapterQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/committeeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/homeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/snapshotQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/userQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (72)
backend/apps/github/api/internal/nodes/user.pybackend/apps/github/models/mixins/user.pybackend/apps/owasp/api/internal/nodes/chapter.pybackend/apps/owasp/api/internal/nodes/committee.pybackend/apps/owasp/api/internal/nodes/common.pybackend/apps/owasp/models/mixins/chapter.pybackend/apps/owasp/models/mixins/committee.pybackend/apps/owasp/models/mixins/project.pybackend/tests/apps/github/api/internal/nodes/user_test.pybackend/tests/apps/github/models/mixins/user_test.pybackend/tests/apps/owasp/models/mixins/chapter_test.pybackend/tests/apps/owasp/models/mixins/committee_test.pybackend/tests/apps/owasp/models/mixins/project_test.pyfrontend/__tests__/a11y/components/CalendarButton.a11y.test.tsxfrontend/__tests__/a11y/components/ItemCardList.a11y.test.tsxfrontend/__tests__/a11y/components/Milestones.a11y.test.tsxfrontend/__tests__/a11y/components/RecentIssues.a11y.test.tsxfrontend/__tests__/a11y/components/RecentPullRequests.a11y.test.tsxfrontend/__tests__/a11y/components/RecentRelease.a11y.test.tsxfrontend/__tests__/a11y/components/Release.a11y.test.tsxfrontend/__tests__/a11y/components/SnapshotCard.a11y.test.tsxfrontend/__tests__/e2e/data/mockHomeData.tsfrontend/__tests__/mockData/mockChapterData.tsfrontend/__tests__/mockData/mockChapterDetailsData.tsfrontend/__tests__/mockData/mockCommitteeData.tsfrontend/__tests__/mockData/mockCommitteeDetailsData.tsfrontend/__tests__/mockData/mockContributeData.tsfrontend/__tests__/mockData/mockHomeData.tsfrontend/__tests__/mockData/mockOrganizationData.tsfrontend/__tests__/mockData/mockProgramData.tsfrontend/__tests__/mockData/mockProjectDetailsData.tsfrontend/__tests__/mockData/mockRepositoryData.tsfrontend/__tests__/mockData/mockSnapshotData.tsfrontend/__tests__/mockData/mockUserDetails.tsfrontend/__tests__/unit/components/CalendarButton.test.tsxfrontend/__tests__/unit/components/CardDetailsPage.test.tsxfrontend/__tests__/unit/components/ItemCardList.test.tsxfrontend/__tests__/unit/components/Milestones.test.tsxfrontend/__tests__/unit/components/MultiSearch.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/RecentIssues.test.tsxfrontend/__tests__/unit/components/RecentPullRequests.test.tsxfrontend/__tests__/unit/components/RecentRelease.test.tsxfrontend/__tests__/unit/components/Release.test.tsxfrontend/__tests__/unit/components/SingleModuleCard.test.tsxfrontend/__tests__/unit/components/SnapshotCard.test.tsxfrontend/__tests__/unit/pages/EditModule.test.tsxfrontend/__tests__/unit/pages/EditProgram.test.tsxfrontend/__tests__/unit/pages/MyMentorship.test.tsxfrontend/__tests__/unit/pages/Organization.test.tsxfrontend/__tests__/unit/pages/Snapshots.test.tsxfrontend/__tests__/unit/utils/dateFormatter.test.tsfrontend/__tests__/unit/utils/getGoogleCalendarUrl.test.tsfrontend/__tests__/unit/utils/getIcsFileUrl.test.tsfrontend/src/app/board/[year]/candidates/page.tsxfrontend/src/app/organizations/[organizationKey]/page.tsxfrontend/src/components/ItemCardList.tsxfrontend/src/components/Milestones.tsxfrontend/src/types/calendar.tsfrontend/src/types/chapter.tsfrontend/src/types/committee.tsfrontend/src/types/issue.tsfrontend/src/types/mentorship.tsfrontend/src/types/milestone.tsfrontend/src/types/project.tsfrontend/src/types/pullRequest.tsfrontend/src/types/release.tsfrontend/src/types/user.tsfrontend/src/utils/dateFormatter.tsfrontend/src/utils/getGoogleCalendarUrl.tsfrontend/src/utils/getIcsFileUrl.tsfrontend/src/utils/utility.ts
💤 Files with no reviewable changes (1)
- frontend/src/types/milestone.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/tests/unit/components/RecentRelease.test.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- frontend/tests/unit/pages/MyMentorship.test.tsx
- frontend/src/components/ItemCardList.tsx
- frontend/tests/a11y/components/RecentPullRequests.a11y.test.tsx
- frontend/src/components/Milestones.tsx
- frontend/tests/unit/components/SnapshotCard.test.tsx
- frontend/tests/unit/components/SingleModuleCard.test.tsx
- frontend/tests/unit/pages/EditProgram.test.tsx
- frontend/tests/unit/components/MultiSearch.test.tsx
- frontend/tests/unit/components/ItemCardList.test.tsx
- frontend/tests/mockData/mockSnapshotData.ts
- frontend/tests/a11y/components/Milestones.a11y.test.tsx
- frontend/src/types/issue.ts
- frontend/src/utils/dateFormatter.ts
- frontend/src/types/calendar.ts
- frontend/tests/mockData/mockProgramData.ts
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/tests/mockData/mockUserDetails.ts
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/board/[year]/candidates/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/page.tsx:216">
P2: Repository list is no longer limited to 5 items; `.slice(1)` now renders all remaining repos while the header still says "Top 5 Active Repositories".</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@kasya PR is ready for review Changes
|
kasya
left a comment
There was a problem hiding this comment.
@Mr-Rahul-Paul Hi! Thank you so much for working on this! Great job 👍🏼
I pushed a small update to resolve a conflict in one of the test files.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3857 +/- ##
==========================================
- Coverage 99.43% 99.43% -0.01%
==========================================
Files 519 519
Lines 16145 16146 +1
Branches 2206 2207 +1
==========================================
Hits 16054 16054
Misses 69 69
- Partials 22 23 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|



Resolves #3371
Proposed change
Refactored the dateFormatter utility to exclusively accept numeric Unix timestamps (in seconds) instead of the previous
string | numberunion type. This PR also fixes the affected 50+ files which are other utilities, methods, jest tests and e2e tests.My approach to fix the issue
dateFormatter.tsto take numbers onlycd frontend && npx tscto get a list of affected filesmake check-testworks, All backend ,jest and e2e tests pass and warnings addressed.Checklist
make check-testlocally: all warnings addressed, tests passed