Skip to content

Fix duplicate PATCH requests on banner dismissal#7252

Merged
gilluminate merged 1 commit intomainfrom
gill/ENG-2426/duplicate-call-being-made-to-patch-api-v1-privacy
Jan 27, 2026
Merged

Fix duplicate PATCH requests on banner dismissal#7252
gilluminate merged 1 commit intomainfrom
gill/ENG-2426/duplicate-call-being-made-to-patch-api-v1-privacy

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Jan 27, 2026

Ticket ENG-2426

Description Of Changes

Fixed duplicate PATCH requests to /api/v1/privacy-preferences when dismissing the consent banner via the X close button. The issue was caused by a redundant onClose() call in the CloseButton click handler that triggered the dismissal flow twice.

When PR #7227 added onDismiss() to the banner's a11y-dialog onClose callback to fix TCF dismissal with GPC-enabled custom notices, it created an unintended side effect. The CloseButton was already calling both closeButton.onClick() (which triggers the a11y-dialog's onClose callback) and an explicit onClose() prop, resulting in handleDismiss() being called twice and sending duplicate API requests.

Code Changes

  • Removed redundant onClose() call from ConsentBanner CloseButton click handler
  • The a11y-dialog's built-in close mechanism now handles the full dismissal flow once

Steps to Confirm

  1. Navigate to a non-TCF experience with banner + modal (e.g., http://localhost:3001/fides-js-demo.html?geolocation=ca-qu)
  2. Open browser DevTools Network tab and filter for /api/v1/privacy-preferences
  3. Click the X close button on the consent banner
  4. Verify only ONE PATCH request is made (previously two requests were sent)
  5. Verify banner closes properly
  6. Verify consent is recorded correctly in the cookie
  7. Test action buttons (Accept All, Reject All) still work correctly and close the banner

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Jan 27, 2026

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

2 Skipped Deployments
Project Deployment Review Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Jan 27, 2026 0:39am
fides-privacy-center Ignored Ignored Jan 27, 2026 0:39am

Request Review

@gilluminate gilluminate force-pushed the gill/ENG-2426/duplicate-call-being-made-to-patch-api-v1-privacy branch from 1e84212 to 61b6de4 Compare January 27, 2026 00:28
@gilluminate gilluminate force-pushed the gill/ENG-2426/duplicate-call-being-made-to-patch-api-v1-privacy branch from 61b6de4 to b2c7379 Compare January 27, 2026 00:39
@gilluminate gilluminate marked this pull request as ready for review January 27, 2026 00:54
@gilluminate gilluminate requested a review from a team as a code owner January 27, 2026 00:54
@gilluminate gilluminate requested review from lucanovera and removed request for a team January 27, 2026 00:54
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

This PR fixes duplicate PATCH requests to /api/v1/privacy-preferences when dismissing the consent banner via the X close button.

Root Cause Analysis:
When PR #7227 added onDismiss() to the a11y-dialog's onClose callback in Overlay.tsx:177, it inadvertently created a duplicate call path. The ConsentBanner CloseButton was calling both:

  1. closeButton.onClick() - which triggers the a11y-dialog's onClose callback (lines 50-56 in a11y-dialog.tsx)
  2. An explicit onClose() prop passed from the parent

This resulted in handleDismiss() being invoked twice, sending duplicate API requests.

The Fix:
The PR correctly removes the redundant direct onClose() call and the onClose prop from ConsentBanner. Now the dismissal flow follows a single clean path:

  • User clicks CloseButton → closeButton.onClick() → a11y-dialog's handleClose()onClose callback in Overlay.tsxonDismiss()handleDismiss()

Impact:

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The change is a simple, well-reasoned fix that removes redundant code. The PR description provides excellent context including the root cause analysis and clear testing steps. The fix correctly addresses the duplicate API call issue by removing the redundant onClose() invocation while preserving the single correct call path through the a11y-dialog mechanism. No regressions are expected.
  • No files require special attention

Important Files Changed

Filename Overview
clients/fides-js/src/components/ConsentBanner.tsx Removed redundant onClose() call from CloseButton handler - fix correctly addresses duplicate PATCH requests

Copy link
Copy Markdown
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.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Contributor

@nrxsmith nrxsmith left a comment

Choose a reason for hiding this comment

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

Makes sense to me 👍

@gilluminate gilluminate removed the request for review from lucanovera January 27, 2026 16:58
@gilluminate gilluminate added this pull request to the merge queue Jan 27, 2026
Merged via the queue into main with commit 137da32 Jan 27, 2026
41 of 42 checks passed
@gilluminate gilluminate deleted the gill/ENG-2426/duplicate-call-being-made-to-patch-api-v1-privacy branch January 27, 2026 17:10
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