refactor: datetime to ant [ENG-175] by speaker-ender · Pull Request #7059 · ethyca/fides

speaker-ender · 2025-12-03T15:07:04Z

Description Of Changes

Updates the Configure Monitor form to use the ant DatePicker input.
This also updates the form to use the ant Form component, useForm hook, and all of the other ant input components.

Code Changes

Refactors the ConfigureMonitorForm component to use the ant DatePicker input.

Steps to Confirm

Visit the integrations screen
Select a monitor and navigate to the Data Discovery tab
Click the Add Monitor button
Confirm that the new components have the correct styles and functions correctly
Fill out the form successfully
Click the Edit button next to your newly created monitor
Confirm that you can still edit the monitor

Pre-Merge Checklist

vercel · 2025-12-03T15:07:09Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
fides-plus-nightly	Ready	Preview	Comment	Dec 11, 2025 4:54pm

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
fides-privacy-center	Ignored			Dec 11, 2025 4:54pm

greptile-apps · 2025-12-03T15:44:22Z

Greptile Overview

Greptile Summary

This PR refactors the ConfigureMonitorForm component to migrate from the legacy Formik/Chakra form implementation to Ant Design's Form component and DatePicker.

Key changes:

Migrated from Formik to Ant Form with useForm hook and native Ant validation
Replaced CustomDateTimeInput with Ant DatePicker component
Updated date handling from date-fns to dayjs library
Replaced custom form components with Ant equivalents (Input, Select, Switch)
Updated Cypress test to match new datetime format output

Issues found:

Critical bug: dayjs(undefined) on line 188 creates an invalid date when creating a new monitor (when monitor?.execution_start_date is undefined)
Architectural inconsistency: mixing FormikSharedConfigSelect (a Formik component) within an Ant Form, creating mixed form management patterns

Confidence Score: 2/5

This PR has critical bugs that will cause issues when creating new monitors
Score reflects a critical logical error with date initialization that will break the form when creating new monitors (undefined date value), plus architectural inconsistency mixing Formik and Ant Form patterns
Pay close attention to ConfigureMonitorForm.tsx - the date initialization bug must be fixed before merging

Important Files Changed

File Analysis

Filename	Score	Overview
CHANGELOG.md	5/5	Added changelog entry for DatePicker migration - straightforward documentation update
clients/admin-ui/cypress/e2e/integration-management.cy.ts	5/5	Updated regex pattern to match new datetime format from Ant DatePicker
clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx	2/5	Refactored from Formik to Ant Form but has critical issues: invalid date initialization when creating new monitors, and mixing Formik components with Ant Form
clients/admin-ui/src/features/integrations/configure-monitor/ConfigureWebsiteMonitorForm.tsx	5/5	Exported constant for reuse - minimal, safe change

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx

speaker-ender · 2025-12-03T16:10:58Z

clients/admin-ui/cypress/e2e/integration-management.cy.ts

        cy.getByTestId("input-execution_start_date")
          .should("have.prop", "value")
-          .should("match", /2024-06-04T[0-9][0-9]:11/);
+          .should("match", /2024-06-04 [0-9][0-9]:[0-9][0-9]:11/);


Format change of internal value used by ant component.
This does not represent change in value submitted to api.
Changing the format prop would also change the value displayed to the user.

speaker-ender · 2025-12-03T16:12:10Z

clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx

+  useEffect(() => {
+    form
+      .validateFields({ validateOnly: true })
+      .then(() => setSubmittable(true))
+      .catch(() => setSubmittable(false));
+  }, [form, formValues]);


Pattern used in ant documentation, however I think this could be improved in the future.

lucanovera

Nice update, thanks for updating the Form too!

@speaker-ender It looks like there's an issue where the date string isn't being loaded correctly when editing monitors created before this refactor.

Captura de pantalla 2025-12-04 a la(s) 2 14 31 p m

I am comparing the preview branch with nightly, and I see that after the refactor we're using milliseconds, I would try to keep the strings the same for consistency and preventing bugs:
Preview branch after refactor: 2025-12-04T17:16:45.163000Z
Nightly: 2025-12-04T17:17:00Z

Reference:

Preview: https://fides-plus-nightly-git-refactor-datetime-to-ant-eng-175-ethyca.vercel.app/integrations/bq_connection_1764828859518#data-discovery
-Nighly: https://fides-nightly-build.fides-staging.ethyca.com/integrations/bq_connection_1764828859518#data-discovery

speaker-ender · 2025-12-05T17:07:09Z

@speaker-ender It looks like there's an issue where the date string isn't being loaded correctly when editing monitors created before this refactor.

I think this has to do with null values being returned. Fixed by resolving to undefined

I am comparing the preview branch with nightly, and I see that after the refactor we're using milliseconds, I would try to keep the strings the same for consistency and preventing bugs: Preview branch after refactor: 2025-12-04T17:16:45.163000Z Nightly: 2025-12-04T17:17:00Z

Good catch! This should now send the value without milliseconds

lucanovera

@speaker-ender Thanks for the updates! I no longer see the issue editing old monitors.
I found one more small issue though. After creating the monitor if I go to edit I see the date has moved back by 3 hours (I live in a UTC-3 timezone).
Creating:

Editing:

I did some initial debugging and I think the issue is that when the monitor is created the datetime is passed exactly as it was selected, but it really should be converted to UTC:
In this case I selected 13:08, it should save as 16:08 in utc.

The behavior on edit looks right to me, it's loading the utc value from the server and then applying the local timezone, so shifting it by 3 hours.

ps: sorry for the delay in my re-review. I missed you previous message.

chore: wip chore: more wip feat: nearly done chore: fixes fix: show time chore: update changelog fix: test id fix: inline date format

speaker-ender · 2025-12-11T16:53:11Z

I did some initial debugging and I think the issue is that when the monitor is created the datetime is passed exactly as it was selected, but it really should be converted to UTC: In this case I selected 13:08, it should save as 16:08 in utc.
The behavior on edit looks right to me, it's loading the utc value from the server and then applying the local timezone, so shifting it by 3 hours.

@lucanovera Good catch! I think this should convert to utc correctly now.

lucanovera

Looks and works great now! Code changes look good, approved!

A small comment on date formats: While I prefer the standardized date format used here, usually we use 12hs because that's more natural for customer in the US.
But anyways that should be something that we configure at the fidesui level, not in the implementation.

Captura de pantalla 2025-12-11 a la(s) 2 23 57 p m

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from cc9bb0a to 31ba8ce Compare December 3, 2025 15:08

vercel bot deployed to Preview – fides-plus-nightly December 3, 2025 15:13 View deployment

vercel bot deployed to Preview – fides-plus-nightly December 3, 2025 15:37 View deployment

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 456081e to 498762f Compare December 3, 2025 15:40

speaker-ender marked this pull request as ready for review December 3, 2025 15:41

speaker-ender requested a review from a team as a code owner December 3, 2025 15:41

speaker-ender requested review from lucanovera and removed request for a team December 3, 2025 15:41

greptile-apps bot reviewed Dec 3, 2025

View reviewed changes

clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx Outdated Show resolved Hide resolved

clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorForm.tsx Show resolved Hide resolved

vercel bot deployed to Preview – fides-plus-nightly December 3, 2025 15:45 View deployment

speaker-ender commented Dec 3, 2025

View reviewed changes

lucanovera requested changes Dec 4, 2025

View reviewed changes

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 498762f to ae78ff8 Compare December 5, 2025 16:44

vercel bot deployed to Preview – fides-plus-nightly December 5, 2025 16:49 View deployment

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from ae78ff8 to 10fadfa Compare December 5, 2025 17:07

vercel bot deployed to Preview – fides-plus-nightly December 5, 2025 17:12 View deployment

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 10fadfa to 7354da9 Compare December 5, 2025 17:40

vercel bot deployed to Preview – fides-plus-nightly December 5, 2025 17:44 View deployment

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 7354da9 to 13e947b Compare December 9, 2025 15:10

vercel bot deployed to Preview – fides-plus-nightly December 9, 2025 15:15 View deployment

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 13e947b to 87b2481 Compare December 10, 2025 20:27

speaker-ender requested a review from lucanovera December 10, 2025 20:27

speaker-ender enabled auto-merge December 10, 2025 20:28

vercel bot deployed to Preview – fides-plus-nightly December 10, 2025 20:31 View deployment

lucanovera requested changes Dec 11, 2025

View reviewed changes

refactor: datetime to ant [ENG-175]

f3be803

chore: wip chore: more wip feat: nearly done chore: fixes fix: show time chore: update changelog fix: test id fix: inline date format

speaker-ender added 2 commits December 11, 2025 11:50

fix: handling null date values and matching previous formatting

df4df1e

fix: convert to utc

00c0ed7

speaker-ender force-pushed the refactor/datetime-to-ant--ENG-175 branch from 87b2481 to 00c0ed7 Compare December 11, 2025 16:50

speaker-ender requested a review from lucanovera December 11, 2025 16:53

vercel bot deployed to Preview – fides-plus-nightly December 11, 2025 16:54 View deployment

lucanovera approved these changes Dec 11, 2025

View reviewed changes

speaker-ender added this pull request to the merge queue Dec 11, 2025

Merged via the queue into main with commit 3bc11c8 Dec 11, 2025
47 checks passed

speaker-ender deleted the refactor/datetime-to-ant--ENG-175 branch December 11, 2025 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: datetime to ant [ENG-175]#7059

refactor: datetime to ant [ENG-175]#7059
speaker-ender merged 3 commits intomainfrom
refactor/datetime-to-ant--ENG-175

speaker-ender commented Dec 3, 2025 •

edited

Loading

Uh oh!

vercel bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

greptile-apps bot commented Dec 3, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

Uh oh!

speaker-ender Dec 3, 2025

Uh oh!

speaker-ender Dec 3, 2025

Uh oh!

lucanovera left a comment

Uh oh!

speaker-ender commented Dec 5, 2025

Uh oh!

lucanovera left a comment

Uh oh!

speaker-ender commented Dec 11, 2025

Uh oh!

lucanovera left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

speaker-ender commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Dec 3, 2025

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

speaker-ender Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

speaker-ender Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

lucanovera left a comment

Choose a reason for hiding this comment

Uh oh!

speaker-ender commented Dec 5, 2025

Uh oh!

lucanovera left a comment

Choose a reason for hiding this comment

Uh oh!

speaker-ender commented Dec 11, 2025

Uh oh!

lucanovera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

speaker-ender commented Dec 3, 2025 •

edited

Loading

vercel bot commented Dec 3, 2025 •

edited

Loading