Skip to content

Conversation

@jjdaurora
Copy link
Contributor

@jjdaurora jjdaurora commented Dec 12, 2025

Ticket [ENG-2224]

Description Of Changes

Custom alpha image for client. Includes updates in 2.76.1 plus Banner Resurfacing.

Code Changes

Steps to Confirm

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

@jjdaurora jjdaurora requested a review from a team as a code owner December 12, 2025 15:09
@jjdaurora jjdaurora requested review from speaker-ender and removed request for a team December 12, 2025 15:09
@vercel
Copy link

vercel bot commented Dec 12, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ignored Ignored Dec 12, 2025 3:09pm
fides-privacy-center Ignored Ignored Dec 12, 2025 3:09pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 12, 2025

Greptile Overview

Greptile Summary

This PR modifies the shouldResurfaceBanner function to resurface the consent banner for TCF (Transparency and Consent Framework) experiences when users have previously dismissed or rejected the banner.

  • Added a check for ConsentMethod.DISMISS and ConsentMethod.REJECT in the TCF-specific branch of shouldResurfaceBanner, causing the banner to reappear when users have previously dismissed or rejected consent
  • Updated the corresponding test expectation from false to true for the TCF dismiss case
  • This change only affects TCF experiences; non-TCF experiences still do not resurface the banner after dismiss

Confidence Score: 4/5

  • This PR is safe to merge - it's a focused behavioral change with corresponding test updates.
  • The code change is straightforward and the logic is sound. The test was updated to reflect the new behavior. Minor deduction for the mismatched test label and missing test coverage for the REJECT case.
  • The test file has a label mismatch that should be addressed for clarity.

Important Files Changed

File Analysis

Filename Score Overview
clients/fides-js/src/lib/consent-utils.ts 5/5 Added condition to resurface TCF banner when consent method is DISMISS or REJECT, ensuring users can reconsider their choices.
clients/fides-js/tests/lib/consent-utils.test.ts 4/5 Updated test expectation for TCF DISMISS case to expect true, but the test label still says "returns false" which is misleading.

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.

Additional Comments (1)

  1. clients/fides-js/__tests__/lib/consent-utils.test.ts, line 340 (link)

    style: Test label says "returns false" but the expected value is now true. Update the label to match the new behavior.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 345 to +348
},
savedConsent: mockSavedConsent,
options: {},
expected: false,
expected: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding a test case for ConsentMethod.REJECT in TCF experiences to ensure both DISMISS and REJECT behaviors are covered by tests.

@jjdaurora jjdaurora added the do not merge Please don't merge yet, bad things will happen if you do label Dec 12, 2025
@jjdaurora jjdaurora changed the title Resurface banner on reject; Custom Alpha Image - Resurface Banner on Reject - v2 Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Please don't merge yet, bad things will happen if you do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants