Skip to content

ENG-3447: Add att_exempt field to PrivacyNotice#8029

Merged
thabofletcher merged 12 commits into
mainfrom
ENG-3447/att-notice-exempt-field
May 6, 2026
Merged

ENG-3447: Add att_exempt field to PrivacyNotice#8029
thabofletcher merged 12 commits into
mainfrom
ENG-3447/att-notice-exempt-field

Conversation

@thabofletcher
Copy link
Copy Markdown
Contributor

@thabofletcher thabofletcher commented Apr 24, 2026

Ticket ENG-3447

Description Of Changes

Adds an att_exempt boolean field to the PrivacyNotice, PrivacyNoticeHistory, and PrivacyNoticeTemplate models (via PrivacyNoticeBase).

Behaviour: By default all privacy notices are ATT-controlled (att_exempt: false) — when a user denies Apple's App Tracking Transparency prompt, the notice is automatically disabled and locked. Setting att_exempt: true marks the notice as exempt from ATT, so it remains user-toggleable regardless of the ATT decision.

Includes an Alembic migration to add the column to all three tables with server_default=false.

Code Changes

  • Added att_exempt Boolean column to PrivacyNoticeBase SQLAlchemy model (flows to privacynotice, privacynoticehistory, privacynoticetemplate)
  • Added att_exempt: bool = False to PrivacyNoticeHistorySchema
  • Added Alembic migration e3f4a5b6c7d8 adding the column with server_default=false

Steps to Confirm

  1. Run alembic upgrade head — confirm column appears on all three tables
  2. POST /api/v1/privacy-notice without att_exempt — confirm response has att_exempt: false
  3. POST with att_exempt: true — confirm it round-trips correctly
  4. PATCH to toggle the value — confirm history table also reflects the new value

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:
    • 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
    • No migrations
  • Documentation:
    • No documentation updates required

Adds a boolean att_exempt column to privacynotice, privacynoticehistory,
and privacynoticetemplate. When false (default), the notice is controlled
by Apple's ATT prompt — automatically disabled and locked when the user
denies tracking. When true, the notice is exempt and remains
user-toggleable regardless of ATT.

Includes Alembic migration and schema update.
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 24, 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 May 6, 2026 3:10pm
fides-privacy-center Ignored Ignored May 6, 2026 3:10pm

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.19%. Comparing base (aaf933d) to head (36a8783).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8029   +/-   ##
=======================================
  Coverage   85.19%   85.19%           
=======================================
  Files         638      638           
  Lines       42008    42010    +2     
  Branches     4937     4937           
=======================================
+ Hits        35788    35790    +2     
  Misses       5111     5111           
  Partials     1109     1109           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.6% (3007/45498) 5.93% (1554/26197) 4.63% (622/13431)
fides-js Coverage: 78%
79.43% (2013/2534) 66.13% (1244/1881) 73.09% (345/472)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

@thabofletcher thabofletcher marked this pull request as ready for review May 5, 2026 21:28
@thabofletcher thabofletcher requested a review from a team as a code owner May 5, 2026 21:28
@thabofletcher thabofletcher requested review from vcruces and removed request for a team May 5, 2026 21:28
@thabofletcher thabofletcher removed the request for review from vcruces May 6, 2026 00:59
Comment thread CHANGELOG.md Outdated
sa.Column(
"att_exempt",
sa.Boolean(),
nullable=False,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd argue maybe this should be nullable=True, since some privacy notices (notice-only) cannot be affected by ATT right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Notice-only handling isn't a DB concern; it's application logic (filterAttDeniedFromDraft keeps notice_only notices regardless of att_exempt). Making it nullable would introduce ambiguity: what does null mean - unset? exempt? not exempt? — and you'd need null-checks everywhere - null only makes sense from the persepctive of a notice only row, but how it treast this filed is already clear

I'd like to avoid this change for now unless it has some real value, since it adds code risk and change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's fine- if not at the BE level, we do need to handle the beginning and end of the flow:

  1. When a user sets ATT exempt for a notice-only notice, can we at least have some info / help text that says notice-only notices are not affected or something?
  2. At the end of the flow (Mobile SDK) let's remember to add tests around what we expect for this use case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. I'll add this into this ticket for when we create the UI
  2. I will definitely add this test coverage in the final PR #3467

@thabofletcher thabofletcher added this pull request to the merge queue May 6, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 6, 2026
@thabofletcher thabofletcher added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit e638d16 May 6, 2026
69 checks passed
@thabofletcher thabofletcher deleted the ENG-3447/att-notice-exempt-field branch May 6, 2026 15:31
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