feat: normalize all date and time rendering to YYYY-MM-DD / HH:mm#950
feat: normalize all date and time rendering to YYYY-MM-DD / HH:mm#950bassner wants to merge 3 commits into
Conversation
Replace the mix of locale-dependent US (MM/DD/YYYY), German (DD.MM.YYYY) and long-form (April 21, 2026) date formats with a single unified ISO 8601-aligned layout everywhere a date is shown to a user — web UI, emails, PDFs, anonymization records. - Client: replace formatDate helper with three explicit helpers (formatDate / formatTime / formatDateTime), no options flag. Build output from getFullYear/getMonth/getDate with zero-padding, fully locale-independent. Guard invalid Date inputs. - Migrate every formatDate call site plus direct toLocale* / dayjs renderers; classify per site as date-only vs date+time. - Add valueFormat props to all Mantine DateInput and DateTimePicker instances so pickers display and parse ISO-style values. - Server: update DataFormatter, PDFBuilder, ThesisAnonymizationService, UserDeletionService formatter patterns. Email DataFormatter uses "yyyy-MM-dd HH:mm (zzz)" pinned to Locale.ENGLISH so German JVMs don't render MEZ/MESZ. PDFs use "yyyy-MM-dd HH:mm" with no zone suffix. - ThesisService assessment PDF: render presentation scheduledAt as date+time (previously dropped the time). - New Liquibase changeset 40 rewrites the inline Thymeleaf #temporals.format pattern in the rejection-reminder email template row. Migration 14 source left untouched to preserve checksum. - DataFormatterTest gains a locale-independence test that forces a German default locale and asserts CET (English) output.
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 48 minutes and 15 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
WalkthroughThis pull request standardizes date and time formatting across the application by replacing locale-based formatting (e.g., Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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: 3
🧹 Nitpick comments (4)
server/src/main/resources/db/changelog/changes/40_normalize_date_format_in_rejection_reminder.sql (1)
4-9: Restrict the migration to rows that contain the legacy pattern.As written, every
email_templatesrow is updated/locked even whenbody_htmldoes not contain the old formatter. Add aWHEREclause to avoid unnecessary writes and trigger/audit side effects.♻️ Proposed SQL refinement
UPDATE email_templates SET body_html = REPLACE( body_html, '#temporals.format(app.rejectionDate, ''dd.MM.yyyy'')', '#temporals.format(app.rejectionDate, ''yyyy-MM-dd'')' -); +) +WHERE body_html LIKE '%#temporals.format(app.rejectionDate, ''dd.MM.yyyy'')%';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/main/resources/db/changelog/changes/40_normalize_date_format_in_rejection_reminder.sql` around lines 4 - 9, The UPDATE currently writes every row in the email_templates table; restrict it to only rows that contain the legacy formatter by adding a WHERE clause that checks body_html for the exact legacy pattern (e.g., using LIKE '%#temporals.format(app.rejectionDate, ''dd.MM.yyyy'')%') so only rows with that substring are updated and avoid unnecessary writes/triggers/audit entries.server/src/main/java/de/tum/cit/aet/thesis/utility/DataFormatter.java (1)
23-50: Optional: cache formatters as static constants.
formatDateandformatDateTimeallocate a freshDateTimeFormatteron every call. SinceDateTimeFormatteris thread-safe and immutable, hoisting both intoprivate static finalfields is a cheap win on hot paths (PDF/email rendering loops).♻️ Proposed refactor
+ private static final DateTimeFormatter DATE_FORMATTER = + DateTimeFormatter.ofPattern("yyyy-MM-dd").withZone(ZoneId.systemDefault()); + + private static final DateTimeFormatter DATE_TIME_FORMATTER = + DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm (zzz)") + .withLocale(Locale.ENGLISH) + .withZone(ZoneId.systemDefault()); + public static String formatDate(Object time) { if (!(time instanceof Instant)) { return ""; } - - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd") - .withZone(ZoneId.systemDefault()); - - return formatter.format((Instant) time); + return DATE_FORMATTER.format((Instant) time); }Note: caching locks
ZoneId.systemDefault()at class-load time. If the server's default zone can change at runtime (unusual), keep the current per-call construction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/main/java/de/tum/cit/aet/thesis/utility/DataFormatter.java` around lines 23 - 50, formatDate and formatDateTime currently create a new DateTimeFormatter per call; hoist these into private static final constants (e.g., DATE_FORMATTER and DATETIME_FORMATTER) and reuse them in the methods to avoid repeated allocations, ensuring you initialize them with the same patterns, Locale.ENGLISH for the datetime formatter, and withZone(ZoneId.systemDefault()) at class load time (leave per-call construction if your runtime may change default ZoneId). Update formatDate and formatDateTime to call DATE_FORMATTER.format((Instant) time) and DATETIME_FORMATTER.format((Instant) time) respectively.client/src/pages/PresentationOverviewPage/PresentationOverviewPage.tsx (1)
260-260: Render the normalized date key directly.
dateis already created asYYYY-MM-DD, so the extra formatting pass is redundant and can be avoided.♻️ Proposed simplification
- <Title order={5}>{dayjs(date).format('YYYY-MM-DD')}</Title> + <Title order={5}>{date}</Title> ... - <Divider label={dayjs(date).format('YYYY-MM-DD')} data-date={date} /> + <Divider label={date} data-date={date} />Also applies to: 315-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/pages/PresentationOverviewPage/PresentationOverviewPage.tsx` at line 260, The Title rendering is re-formatting a date string that's already normalized to "YYYY-MM-DD"; change the JSX in PresentationOverviewPage where <Title order={5}>{dayjs(date).format('YYYY-MM-DD')}</Title> (and the similar occurrence around the 315 region) to render the normalized date variable directly as <Title order={5}>{date}</Title>, removing the redundant dayjs formatting call.client/src/pages/InterviewTopicOverviewPage/components/CollapsibleDateCard.tsx (1)
299-299: UseformatDatehelper for consistency with the rest of the PR.This is the lone holdout still routed through
dayjs(...).format('YYYY-MM-DD'). The whole point of this PR is to centralize date rendering viaformatDateso future format tweaks land in one place. Might as well let this one join the party.♻️ Proposed change
-import dayjs from 'dayjs' +import { formatDate } from '../../../utils/format' @@ - <Title order={6}>{dayjs(date).format('YYYY-MM-DD')}</Title> + <Title order={6}>{formatDate(date)}</Title>(Drop the
dayjsimport if no longer used elsewhere in the file.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/pages/InterviewTopicOverviewPage/components/CollapsibleDateCard.tsx` at line 299, Replace the direct dayjs formatting in the CollapsibleDateCard component — specifically the Title rendering currently using dayjs(date).format('YYYY-MM-DD') — with the shared formatDate helper (e.g., Title order={6}>{formatDate(date)}</Title>) to centralize date rendering; after changing the Title, remove the dayjs import from the file if no other code references it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/utils/format.ts`:
- Around line 17-50: The toValidDate routine is susceptible to timezone shifts
for date-only strings like "YYYY-MM-DD"; update toValidDate to detect an ISO
date-only pattern (e.g., /^\d{4}-\d{2}-\d{2}$/), parse year/month/day by
splitting the string, create a local Date via new Date() then setFullYear(year,
monthIndex, day) and setHours(0,0,0,0) so it represents midnight local time,
validate that the constructed Date's year/month/day match the parsed values and
return null on mismatch or invalid input; leave handling of other string formats
and Date instances unchanged so formatDate, formatTime and formatDateTime
continue to use toValidDate safely.
In `@server/src/main/java/de/tum/cit/aet/thesis/service/ThesisService.java`:
- Line 767: Replace the timezone-inclusive call in ThesisService
(builder.addOverviewItem("Presentation Date",
DataFormatter.formatDateTime(presentation.getScheduledAt()))) with a PDF-style
no-zone formatter: either add a new public helper on DataFormatter (e.g.,
DataFormatter.formatDateTimeForPdf or similar) that formats using the pattern
"yyyy-MM-dd HH:mm" and call that from ThesisService, or apply
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm") directly to
presentation.getScheduledAt() in ThesisService to match the formatting used by
PDFBuilder; ensure the new method (if added) is public and used for all PDF
output to keep behavior consistent.
In `@server/src/test/java/de/tum/cit/aet/thesis/utility/DataFormatterTest.java`:
- Around line 89-108: The test formatDateTime_IsLocaleIndependent mutates
JVM-global Locale/TimeZone which breaks test isolation; update the test to avoid
changing globals by either invoking DataFormatter.formatDateTime in a way that
supplies a deterministic locale/time-zone (e.g., use an overload or helper on
DataFormatter that accepts a Locale/ZoneId or Clock) or annotate the test with
`@ResourceLock`(Resources.LOCALE) and `@ResourceLock`(Resources.TIME_ZONE) to
serialize access; also replace JUnit assertions (assertTrue/assertFalse) with
AssertJ assertions (assertThat(...).endsWith("(CET)") and
assertThat(...).doesNotContain("MEZ")) and keep restoring any originals if you
must touch globals.
---
Nitpick comments:
In
`@client/src/pages/InterviewTopicOverviewPage/components/CollapsibleDateCard.tsx`:
- Line 299: Replace the direct dayjs formatting in the CollapsibleDateCard
component — specifically the Title rendering currently using
dayjs(date).format('YYYY-MM-DD') — with the shared formatDate helper (e.g.,
Title order={6}>{formatDate(date)}</Title>) to centralize date rendering; after
changing the Title, remove the dayjs import from the file if no other code
references it.
In `@client/src/pages/PresentationOverviewPage/PresentationOverviewPage.tsx`:
- Line 260: The Title rendering is re-formatting a date string that's already
normalized to "YYYY-MM-DD"; change the JSX in PresentationOverviewPage where
<Title order={5}>{dayjs(date).format('YYYY-MM-DD')}</Title> (and the similar
occurrence around the 315 region) to render the normalized date variable
directly as <Title order={5}>{date}</Title>, removing the redundant dayjs
formatting call.
In `@server/src/main/java/de/tum/cit/aet/thesis/utility/DataFormatter.java`:
- Around line 23-50: formatDate and formatDateTime currently create a new
DateTimeFormatter per call; hoist these into private static final constants
(e.g., DATE_FORMATTER and DATETIME_FORMATTER) and reuse them in the methods to
avoid repeated allocations, ensuring you initialize them with the same patterns,
Locale.ENGLISH for the datetime formatter, and withZone(ZoneId.systemDefault())
at class load time (leave per-call construction if your runtime may change
default ZoneId). Update formatDate and formatDateTime to call
DATE_FORMATTER.format((Instant) time) and DATETIME_FORMATTER.format((Instant)
time) respectively.
In
`@server/src/main/resources/db/changelog/changes/40_normalize_date_format_in_rejection_reminder.sql`:
- Around line 4-9: The UPDATE currently writes every row in the email_templates
table; restrict it to only rows that contain the legacy formatter by adding a
WHERE clause that checks body_html for the exact legacy pattern (e.g., using
LIKE '%#temporals.format(app.rejectionDate, ''dd.MM.yyyy'')%') so only rows with
that substring are updated and avoid unnecessary writes/triggers/audit entries.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c763bb56-1075-468b-88f7-e95015ce0d16
📒 Files selected for processing (41)
client/src/components/ApplicationData/ApplicationData.tsxclient/src/components/ApplicationReviewForm/ApplicationReviewForm.tsxclient/src/components/ApplicationsTable/ApplicationsTable.tsxclient/src/components/GanttChart/components/GanttChartRangeSlider/GanttChartRangeSlider.tsxclient/src/components/InterviewSlotInformation/InterviewSlotInformation.tsxclient/src/components/PresentationsTable/PresentationsTable.tsxclient/src/components/PresentationsTable/components/ReplacePresentationModal/ReplacePresentationModal.tsxclient/src/components/ThesesGanttChart/ThesesGanttChart.tsxclient/src/components/ThesesTable/ThesesTable.tsxclient/src/components/ThesisCommentsList/components/ThesisCommentElement.tsxclient/src/components/ThesisData/ThesisData.tsxclient/src/components/TopicData/TopicData.tsxclient/src/components/TopicsTable/TopicsTable.tsxclient/src/pages/InterviewBookingPage/InterviewBookingPage.tsxclient/src/pages/InterviewTopicOverviewPage/components/CollapsibleDateCard.tsxclient/src/pages/InterviewTopicOverviewPage/components/SlotItem.tsxclient/src/pages/ManageTopicsPage/components/ReplaceTopicModal/ReplaceTopicModal.tsxclient/src/pages/PresentationOverviewPage/PresentationOverviewPage.tsxclient/src/pages/PresentationPage/PresentationPage.tsxclient/src/pages/ReplaceApplicationPage/components/MotivationStep/MotivationStep.tsxclient/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsxclient/src/pages/SettingsPage/components/AccountDeletion/AccountDeletion.tsxclient/src/pages/SettingsPage/components/DataExport/DataExport.tsxclient/src/pages/ThesisPage/components/FileHistoryTable/FileHistoryTable.tsxclient/src/pages/ThesisPage/components/ThesisConfigSection/ThesisConfigSection.tsxclient/src/pages/ThesisPage/components/ThesisFeedbackOverview/ThesisFeedbackOverview.tsxclient/src/pages/ThesisPage/components/ThesisPresentationSection/components/PresentationCard.tsxclient/src/pages/ThesisPage/components/ThesisWritingSection/ThesisWritingSection.tsxclient/src/pages/TopicPage/components/DateItemAdditonalInformation.tsxclient/src/utils/format.tsserver/src/main/java/de/tum/cit/aet/thesis/mailvariables/MailApplication.javaserver/src/main/java/de/tum/cit/aet/thesis/mailvariables/MailInterviewSlot.javaserver/src/main/java/de/tum/cit/aet/thesis/mailvariables/MailThesisPresentation.javaserver/src/main/java/de/tum/cit/aet/thesis/service/ThesisAnonymizationService.javaserver/src/main/java/de/tum/cit/aet/thesis/service/ThesisService.javaserver/src/main/java/de/tum/cit/aet/thesis/service/UserDeletionService.javaserver/src/main/java/de/tum/cit/aet/thesis/utility/DataFormatter.javaserver/src/main/java/de/tum/cit/aet/thesis/utility/PDFBuilder.javaserver/src/main/resources/db/changelog/changes/40_normalize_date_format_in_rejection_reminder.sqlserver/src/main/resources/db/changelog/db.changelog-master.xmlserver/src/test/java/de/tum/cit/aet/thesis/utility/DataFormatterTest.java
- client format.ts: parse YYYY-MM-DD date-only strings as local midnight, not UTC midnight, to avoid off-by-one day shifts in negative UTC-offset browsers. - DataFormatter: add formatDateTimeWithoutZone() for PDF/internal contexts; ThesisService PDF presentation date now uses it so the assessment PDF stays zone-free and matches PDFBuilder's convention. - Liquibase changeset 40: add WHERE clause so only rows that contain the legacy pattern are updated; avoids unnecessary writes and audit noise. - PresentationOverviewPage: render the pre-normalized YYYY-MM-DD map key directly instead of re-formatting with dayjs. - CollapsibleDateCard: replace the last dayjs-based date render with the shared formatDate helper; drop the now-unused dayjs import. - DataFormatterTest: use AssertJ assertions and annotate the locale-independence test with @ResourceLock(LOCALE / TIME_ZONE) so parallel test execution cannot bleed JVM-global state.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@bassner Clean, thorough normalization across the full stack. The timezone-safe parsing for date-only strings and the locale-pinned zone abbreviations are nice touches. Good call adding the Liquibase migration for the stored template rather than editing the original changeset.
# Conflicts: # client/src/components/PresentationsTable/PresentationsTable.tsx # client/src/components/TopicsTable/TopicsTable.tsx # client/src/pages/InterviewBookingPage/InterviewBookingPage.tsx # client/src/pages/InterviewTopicOverviewPage/components/CollapsibleDateCard.tsx # client/src/utils/format.ts
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@bassner Re-reviewing after the develop merge. The date normalization itself is unchanged from what I approved and still looks great, and CodeRabbit's earlier points are all addressed. The blocker is purely a merge artifact: the merge pulled in the #966 guard test (client/test/format.test.mjs) whose formatDate — issue #827 block pins the old dateStyle: 'medium' implementation and now reds run-tests / build — see inline. Minor aside: ApplicationListItem.tsx ('Submission Date', createdAt) and ThesisPage.tsx (anonymizedAt) weren't touched, so they silently dropped their time when formatDate's default flipped from withTime: true to date-only — the former now disagrees with ApplicationData.tsx, which shows the same field with time. Worth a quick look if that divergence isn't intended.
| return item | ||
| } | ||
|
|
||
| export function formatDate(date: string | Date | null | undefined): string { |
There was a problem hiding this comment.
@bassner [high] The develop merge dragged in client/test/format.test.mjs (the #966 guard test), and its formatDate — issue #827 describe block pins the old implementation — it reads this very source file and asserts dateStyle: 'medium' is present plus the withTime ? { timeStyle: 'short' } : {} gate. Since you replaced all of that with the manual ISO builder, the node --test test/*.test.mjs step fails and reds run-tests / build. That whole describe block needs rewriting for the new format (the ensureAbsoluteLinkHref block below it is fine and should stay).
🤖 Prompt for AI agents
In client/test/format.test.mjs, the describe('formatDate — issue #827 ...') block (roughly lines 14-99) pins the obsolete dateStyle: 'medium' implementation and fails because client/src/utils/format.ts no longer uses toLocaleString. Rewrite that block to match the new helpers: assert formatDate(...) yields YYYY-MM-DD, formatTime(...) yields HH:mm, and formatDateTime(...) yields YYYY-MM-DD HH:mm, and replace the source-pinning assertions (/dateStyle:\s*'medium'/, the withTime ternary regex) with checks for the new pad/getFullYear-based implementation. Leave the ensureAbsoluteLinkHref describe block untouched. Verify with cd client && node --test test/*.test.mjs.
Summary
Replace the inconsistent mix of locale-dependent date formats (US
MM/DD/YYYY, GermanDD.MM.YYYY, long-formApril 21, 2026, raw ISOInstant.toString()) with a single, locale-neutral, unambiguous layout everywhere a date is shown to a user:YYYY-MM-DDHH:mm(24h)YYYY-MM-DD HH:mmYYYY-MM-DD HH:mm (ZONE)— zone suffix added because recipients may not share the server's timezoneTimezone handling is unchanged (server:
ZoneId.systemDefault(), client: browser local). Only display format changes. API payloads, iCalendar (.ics), and persisted data are untouched.What changed
Client (31 files)
client/src/utils/format.ts: replacedformatDatewith three explicit helpers (formatDate/formatTime/formatDateTime), no options flag. Output is built fromgetFullYear/getMonth/getDate/getHours/getMinuteswith zero-padding — fully locale-independent. InvalidDateinputs → empty string; years <1000 are zero-padded to 4 digits.formatDate(...)call site migrated per intent (date-only vs date+time).toLocaleDateString/toLocaleTimeString/toLocaleStringanddayjs().format(...)renderers migrated to the new helpers.DateInputandDateTimePickerinstances now setvalueFormat="YYYY-MM-DD"or"YYYY-MM-DD HH:mm"so pickers display (and parse typed input in) the new format.toDateString()/toISOString()used as map keys) are intentionally left alone.Server (10 files, 1 new)
DataFormatter.formatDate→yyyy-MM-dd.DataFormatter.formatDateTime→yyyy-MM-dd HH:mm (zzz)pinned toLocale.ENGLISHso German JVMs don't renderMEZ/MESZ.PDFBuilder.DATETIME_FORMATTER→yyyy-MM-dd HH:mm(no zone suffix — internal document).ThesisAnonymizationService.DATE_FORMATTER,UserDeletionService.DATE_FORMATTER→yyyy-MM-dd.ThesisServiceassessment PDF:presentation.scheduledAtnow renders as date+time (previously dropped the time entirely).MailInterviewSlot/MailThesisPresentation/MailApplicationtemplateVariables()updated so the variable-picker UI hints match actual output.40_normalize_date_format_in_rejection_reminder.sqlrewrites the inline Thymeleaf#temporals.format(app.rejectionDate, 'dd.MM.yyyy')pattern stored in theemail_templatestable row seeded by migration 14. Migration 14's source is left untouched so Liquibase checksums on existing deployments remain valid.DataFormatterTestgains a locale-independence test that forcesLocale.GERMAN+Europe/Berlinand asserts the output ends with(CET)and does not containMEZ— this will fail if someone removes.withLocale(Locale.ENGLISH)from production.Why
Locale-dependent rendering has led to real misunderstandings (e.g.
04/05/2026interpreted as April 5 vs May 4). A single structurally-unambiguous format removes the ambiguity without requiring per-user locale preferences.Test plan
cd client && npx tsc --noEmit— cleancd client && npx eslint src/— cleancd server && ./gradlew spotlessApply test— 752 tests, 751 passed, 1 skipped (pre-existing), 0 failedYYYY-MM-DD/YYYY-MM-DD HH:mmYYYY-MM-DDandYYYY-MM-DD HH:mminput works in each migratedDateInput/DateTimePicker2026-04-21 14:30 (CEST)(or local zone equivalent)yyyy-MM-ddand the presentation date renders asyyyy-MM-dd HH:mmwith no zone suffixNotes
Research-group-customized email templates where someone hand-typed
dd.MM.yyyyinto a#temporals.format(...)call will not be auto-migrated — those would need manual re-editing. Flag in release notes if relevant.Summary by CodeRabbit
Release Notes
Refactor
YYYY-MM-DDfor dates,YYYY-MM-DD HH:mmfor timestamps) for improved consistency and clarity.Chores