Change request/ID verification flow to use separate pages instead of a modal by jpople · Pull Request #7238 · ethyca/fides

jpople · 2026-01-16T08:02:14Z

Description Of Changes

Converted the privacy request flow from a modal-based approach to full-page routes. The form, verification, and success states now use dedicated Next.js pages with a consistent layout. Refactored ExternalAuthLayout to use a shared AuthFormLayout component to reduce code duplication.

Code Changes

Extracted request creation and ID verification to separate pages instead of modals
Refactored ExternalAuthLayout to use AuthFormLayout instead of duplicating layout code
Added e2e tests for the privacy request flow

Steps to Confirm

Navigate to the privacy center homepage and click a privacy request action
Fill out the form and verify request is submitted as expected
Update your privacy center config so requests require identity verification
Confirm navigation to the verification page and ability to enter OTP code
After successful verification, confirm navigation to the success page
Verify external manual task authentication pages still render correctly with the refactored layout

Pre-Merge Checklist

vercel · 2026-01-16T08:02:18Z

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

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	Feb 25, 2026 0:17am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		Feb 25, 2026 0:17am

greptile-apps · 2026-01-16T08:05:04Z

Greptile Summary

Converts the privacy request flow from a modal-based approach to full-page Next.js routes, improving user experience with dedicated pages for form submission, ID verification, and success confirmation. Successfully refactored ExternalAuthLayout to use a new shared AuthFormLayout component, reducing ~80 lines of duplicated code.

Key changes:

Extracted privacy request form, verification, and success states into separate Next.js pages under /privacy-request/[actionKey]/*
Created reusable AuthFormLayout component that both privacy request and external auth flows now share
Updated routing in HomePage to navigate to pages instead of opening modals
Adapted existing form components to work in both modal and page contexts with new callback props
Added comprehensive e2e tests covering the full page-based flow including verification and edge cases
Uses sessionStorage to pass privacy request ID between pages during verification flow

Minor improvements:

The PR uses direct window.sessionStorage access in a couple places where the global context pattern would be more consistent with the codebase standards

Confidence Score: 4/5

This PR is safe to merge with minor style improvements recommended
The refactor is well-executed with proper routing, comprehensive e2e tests, and successful code deduplication. The only concerns are minor style issues around direct window access that don't affect functionality but could be more consistent with codebase patterns.
Pay close attention to PrivacyRequestFormPage.tsx and VerificationPage.tsx for the sessionStorage usage pattern

Important Files Changed

Filename	Overview
clients/privacy-center/components/privacy-request/PrivacyRequestFormPage.tsx	Uses `sessionStorage` API directly instead of accessing through config; otherwise handles routing and state management properly
clients/privacy-center/components/privacy-request/VerificationPage.tsx	Uses `sessionStorage` API directly instead of accessing through config; otherwise handles verification flow correctly with proper redirects
clients/privacy-center/components/common/AuthFormLayout.tsx	Well-structured shared layout component that properly uses Flex components and reduces duplication
clients/privacy-center/features/external-manual-tasks/components/ExternalAuthLayout.tsx	Successfully refactored to use shared `AuthFormLayout`, greatly reducing code duplication

_{Last reviewed commit: 7c0d9df}

greptile-apps

_{13 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-16T08:05:03Z

clients/privacy-center/components/common/AuthFormLayout.tsx

+      }}
+      data-testid={dataTestId}
+    >
+      <div style={{ width: "100%", maxWidth, padding: "48px 24px" }}>


style: uses div instead of semantic components - per coding standards, prefer Ant Design's Flex component or other semantic elements

Suggested change

<div style={{ width: "100%", maxWidth, padding: "48px 24px" }}>

<Flex style={{ width: "100%", maxWidth, padding: "48px 24px" }}>

Context Used: Rule from dashboard - Avoid using div elements when possible. Use semantic HTML elements or component library alternativ... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Not as worried about semantic dom here, but some of the styles could use tailwind instead of being inline

greptile-apps · 2026-01-16T08:05:04Z

clients/privacy-center/components/common/AuthFormLayout.tsx

+          <div
+            style={{
+              backgroundColor: "white",
+              padding: "48px",
+              width: "100%",
+              borderRadius: "4px",
+              boxShadow:
+                "0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px 0 rgba(0, 0, 0, 0.06)",
+            }}


style: uses div for form container - prefer semantic or Flex component

Suggested change

<div

style={{

backgroundColor: "white",

padding: "48px",

width: "100%",

borderRadius: "4px",

boxShadow:

"0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px 0 rgba(0, 0, 0, 0.06)",

}}

<Flex

style={{

backgroundColor: "white",

padding: "48px",

width: "100%",

borderRadius: "4px",

boxShadow:

"0 1px 3px 0 rgba(0, 0, 0, 0.1), 0 1px 2px 0 rgba(0, 0, 0, 0.06)",

}}

>

Context Used: Rule from dashboard - Avoid using div elements when possible. Use semantic HTML elements or component library alternativ... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

speaker-ender

Some minor nits and questions but overall looks good.

speaker-ender · 2026-01-26T14:47:22Z

clients/privacy-center/components/common/AuthFormLayout.tsx

+      }}
+      data-testid={dataTestId}
+    >
+      <div style={{ width: "100%", maxWidth, padding: "48px 24px" }}>


Not as worried about semantic dom here, but some of the styles could use tailwind instead of being inline

speaker-ender · 2026-01-26T14:47:35Z

clients/privacy-center/components/HomePage.tsx

        iconPath={action.icon_path}
        description={action.description}
-        onOpen={onPrivacyModalOpen}
+        onOpen={handlePrivacyRequestOpen}


nit: should we change the onOpen prop to onClick to match the native ant card component since it's not "opening" something?

speaker-ender · 2026-01-26T14:51:48Z

clients/privacy-center/components/modals/privacy-request-modal/usePrivacyRequestForm.ts

+          if (onSuccessWithoutVerification) {
+            onSuccessWithoutVerification();
+          } else {
+            onClose();


nit: can we rename onClose since we aren't closing a modal?

speaker-ender · 2026-01-26T14:54:19Z

clients/privacy-center/components/privacy-request/PrivacyRequestFormPage.tsx

+  }, [getIdVerificationConfigQuery]);
+
+  if (Number.isNaN(parsedActionIndex)) {
+    return null;


Should we re-direct or throw some kind of error message if the route isn't valid instead of displaying nothing?

speaker-ender · 2026-01-26T14:59:58Z

clients/privacy-center/components/privacy-request/PrivacyRequestFormPage.tsx

+  const action = config.actions[parsedActionIndex] as
+    | PrivacyRequestOption
+    | undefined;


Is there a reason to type cast here?

speaker-ender · 2026-01-26T15:06:53Z

clients/privacy-center/app/privacy-request/[actionIndex]/page.tsx

+  params: Promise<{ actionIndex: string }>;
+  searchParams: NextSearchParams;
+}) => {
+  const { actionIndex } = await params;


I like the idea of having a dynamic route for the different actions.
Are we able to/do you think it makes sense to use the action title or some other identifier instead of an index?
Using the index is a little less stable. If someone re-organizes the config, the links to actions would be mixed up.

Good suggestion, I've updated to use the action key instead of the index.

speaker-ender · 2026-01-26T15:08:27Z

clients/privacy-center/components/privacy-request/PrivacyRequestLayout.tsx

+  title,
+}: PrivacyRequestLayoutProps) => {
+  return (
+    <AuthFormLayout


Is this component doing anything other than passing some defaults to the AuthFormLayout component? If so, why not just use the component directly?

greptile-apps

_{16 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-24T08:23:57Z

clients/privacy-center/components/privacy-request/PrivacyRequestFormPage.tsx

+    if (typeof window !== "undefined") {
+      sessionStorage.setItem("privacyRequestId", id);
+    }


accesses window.sessionStorage directly - prefer using global context when available rather than window access

Suggested change

if (typeof window !== "undefined") {

sessionStorage.setItem("privacyRequestId", id);

}

sessionStorage.setItem("privacyRequestId", id);

Context Used: Rule from dashboard - Prefer passing configuration options as props to components instead of accessing global window objec... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-02-24T08:23:58Z

clients/privacy-center/components/privacy-request/VerificationPage.tsx

+    if (typeof window !== "undefined") {
+      const storedId = sessionStorage.getItem("privacyRequestId");
+      if (storedId) {
+        setPrivacyRequestId(storedId);
+      } else {
+        // If no request ID, redirect back to form
+        router.push(`/privacy-request/${encodeURIComponent(policyKey)}`);
+      }
+    }


accesses window.sessionStorage directly - prefer using global context when available rather than window access

Suggested change

if (typeof window !== "undefined") {

const storedId = sessionStorage.getItem("privacyRequestId");

if (storedId) {

setPrivacyRequestId(storedId);

} else {

// If no request ID, redirect back to form

router.push(`/privacy-request/${encodeURIComponent(policyKey)}`);

}

}

const storedId = sessionStorage.getItem("privacyRequestId");

Context Used: Rule from dashboard - Prefer passing configuration options as props to components instead of accessing global window objec... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

jpople added 7 commits January 15, 2026 01:50

robot

f2182ec

remove back button from verification page

216157d

test updates

34e8c91

Merge branch 'main' into jpople/eng-2283/id-verification-flow

c080c96

lint

b9d638d

more lint

94f7b8e

simplify page structure

7bd39f7

jpople requested a review from a team as a code owner January 16, 2026 08:02

jpople requested review from speaker-ender and removed request for a team January 16, 2026 08:02

greptile-apps bot reviewed Jan 16, 2026

View reviewed changes

changelog

302dc23

speaker-ender reviewed Jan 26, 2026

View reviewed changes

jpople added 6 commits January 27, 2026 16:59

code cleanup

5f5db0a

update to use keys instead of indices

1fc5e6f

test updates

cf1d4d6

format

579dc60

Merge branch 'main' into jpople/eng-2283/id-verification-flow

e07388e

test fixes

1e9144e

jpople requested a review from speaker-ender February 10, 2026 00:30

Merge branch 'main' into jpople/eng-2283/id-verification-flow

44a172f

jpople marked this pull request as draft February 18, 2026 17:08

vercel bot deployed to Preview – fides-plus-nightly February 18, 2026 17:12 View deployment

Merge branch 'main' into jpople/eng-2283/id-verification-flow

4124965

vercel bot deployed to Preview – fides-plus-nightly February 24, 2026 06:41 View deployment

jpople added 3 commits February 24, 2026 00:55

test fix

a56d765

test fix?

1c027f4

test fix?

6a0d4a6

jpople added 2 commits February 24, 2026 02:09

test fix???

6e9d9e5

format

7c0d9df

jpople marked this pull request as ready for review February 24, 2026 08:20

greptile-apps bot reviewed Feb 24, 2026

View reviewed changes

Merge branch 'main' into jpople/eng-2283/id-verification-flow

f2eca73

vercel bot deployed to Preview – fides-plus-nightly February 24, 2026 23:54 View deployment

Merge branch 'main' into jpople/eng-2283/id-verification-flow

6751ccf

vercel bot deployed to Preview – fides-plus-nightly February 25, 2026 00:17 View deployment

speaker-ender approved these changes Feb 25, 2026

View reviewed changes

jpople added this pull request to the merge queue Feb 25, 2026

Merged via the queue into main with commit f5dd02e Feb 25, 2026
40 of 41 checks passed

jpople deleted the jpople/eng-2283/id-verification-flow branch February 25, 2026 15:57

JadeCara mentioned this pull request Feb 25, 2026

ENG-2213: Remove deprecated DSR 2.0 code #7438

Merged

8 tasks

	<div style={{ width: "100%", maxWidth, padding: "48px 24px" }}>
	<Flex style={{ width: "100%", maxWidth, padding: "48px 24px" }}>

Conversation

jpople commented Jan 16, 2026 • edited by atlassian bot 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 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

speaker-ender left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jpople commented Jan 16, 2026 •

edited by atlassian bot

Loading

vercel bot commented Jan 16, 2026 •

edited

Loading

greptile-apps bot commented Jan 16, 2026 •

edited

Loading