Skip to content

Privacy request flow bugfixes#7544

Merged
jpople merged 3 commits intomainfrom
jpople/eng-2867/privacy-center-bug
Mar 3, 2026
Merged

Privacy request flow bugfixes#7544
jpople merged 3 commits intomainfrom
jpople/eng-2867/privacy-center-bug

Conversation

@jpople
Copy link
Contributor

@jpople jpople commented Mar 3, 2026

Ticket ENG-2867

Description Of Changes

Fixes some bugs with the privacy request flow.

  • Fixes an uncaught error when a ValidationError occurred when submitting a request
  • Fixes ValidationErrors occurring after clearing or not filling out the "Location" field due to "" being submitted
  • Fixes "submit" button loading state ending and button being re-enabled before submission processing is finished

Steps to Confirm

  1. Go to the privacy center
  2. Click "erase your data"; if the location field is populated, refresh so it's empty
  3. Fill in the required fields and submit
  4. Request should succeed, should be redirected to success page
  5. "Submit" button should enter loading state and not leave

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • Changelog:
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • No UX review needed
  • Followup issues:
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

@jpople jpople requested a review from a team as a code owner March 3, 2026 00:53
@jpople jpople requested review from lucanovera and removed request for a team March 3, 2026 00:53
@vercel
Copy link
Contributor

vercel bot commented Mar 3, 2026

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

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Mar 3, 2026 0:55am
fides-privacy-center Ignored Ignored Mar 3, 2026 0:55am

Request Review

description: errorMessage,
...ErrorToastOptions,
});
onExit();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made sense when the form was in a modal but it's weird to be redirected back to the homepage for an error IMO.

@jpople jpople enabled auto-merge March 3, 2026 00:55
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR fixes three bugs in the privacy request submission flow in usePrivacyRequestForm.ts: it adds an isSubmitPending state to keep the submit button in a loading state until all async processing finishes, prevents empty-string location values from being sent to the API (replacing "" with undefined), and fixes an uncaught error when a ValidationError is returned by narrowing the error type from any to unknown and guarding the toast description.

  • Submit button loading state: A new isSubmitPending flag is introduced and ORed with formik.isSubmitting in the returned object, correctly extending the loading state past Formik's own submit lifecycle.
  • Empty location fix: location: values?.location ? values.location : undefined prevents the empty string "" from being submitted when the field is blank.
  • onExit() removed from error handler: The modal no longer closes on error, allowing the user to correct their input and resubmit — this is the correct behaviour.
  • Error description: The error type is correctly narrowed from any to unknown, but the short-circuit expression typeof error === "string" && error produces false (not undefined) when no string error is present, which may render the literal string "false" as the toast description. A ternary should be used instead.

Confidence Score: 3/5

  • Mostly safe to merge with one small logic bug that could show "false" as a toast description in error cases.
  • Three of the four fixes are clearly correct. The remaining issue — typeof error === "string" && error producing false instead of undefined — is a small but user-visible bug that is easy to fix with a ternary before merging.
  • clients/privacy-center/components/modals/privacy-request-modal/usePrivacyRequestForm.ts — specifically line 193 where the && short-circuit yields false instead of undefined.

Important Files Changed

Filename Overview
clients/privacy-center/components/modals/privacy-request-modal/usePrivacyRequestForm.ts Bugfixes for privacy request flow: adds isSubmitPending state to keep the submit button disabled through async processing, avoids submitting empty string for location, narrows error type from any to unknown, and removes the inadvertent onExit() call on errors — but uses && short-circuit that produces false instead of undefined for the toast description when no string error is present.

Last reviewed commit: 7172c01

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +193 to 198
const errorMessage = typeof error === "string" && error;
toast({
title,
description: error,
description: errorMessage,
...ErrorToastOptions,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description: false passed to toast

When error is undefined or a non-string (e.g. a Pydantic validation error object), typeof error === "string" && error evaluates to false (not undefined). This means description: false is passed to the toast component, which may render the string "false" as the toast description text — a user-visible bug.

Use a ternary instead so the description is undefined (omitted) when there is no string error:

Suggested change
const errorMessage = typeof error === "string" && error;
toast({
title,
description: error,
description: errorMessage,
...ErrorToastOptions,
});
setIsSubmitPending(false);
const errorMessage = typeof error === "string" ? error : undefined;
toast({
title,
description: errorMessage,
...ErrorToastOptions,
});

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed steps to reproduces and everything worked as expected. Code changes look good. Approved!

@jpople jpople added this pull request to the merge queue Mar 3, 2026
Merged via the queue into main with commit 464dc35 Mar 3, 2026
41 of 42 checks passed
@jpople jpople deleted the jpople/eng-2867/privacy-center-bug branch March 3, 2026 14:30
jpople added a commit that referenced this pull request Mar 3, 2026
@greptile-apps greptile-apps bot mentioned this pull request Mar 3, 2026
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants