Skip to content

ENG-3516: BE: Submission-time resolution of display_condition rules#8027

Merged
mikeGarifullin merged 1 commit into
mainfrom
ENG-3516-1
May 5, 2026
Merged

ENG-3516: BE: Submission-time resolution of display_condition rules#8027
mikeGarifullin merged 1 commit into
mainfrom
ENG-3516-1

Conversation

@mikeGarifullin
Copy link
Copy Markdown
Contributor

@mikeGarifullin mikeGarifullin commented Apr 24, 2026

Ticket ENG-3516

Description Of Changes

Add submission-time resolution and enforcement of display_condition rules on custom Privacy Center fields. Builds on DisplayConditionValidator (PR #8026, save-time): once a config is known to be well-formed, this PR resolves which fields are applicable given the submitted values and enforces the contract on incoming privacy request payloads.

Two enforced rules:

  • a field gated off by its display_condition must not be submitted;
  • a required field that is applicable must have a non-empty value.

Wired into PrivacyRequestService.create_privacy_request so both public and authenticated creation paths reject violating payloads with 422 (via PrivacyRequestError).

Code Changes

  • New src/fides/api/schemas/custom_field_display_evaluator.py:
    • DisplayConditionViolation(ValueError) — raised on first contract breach; callers translate to layer-appropriate error.
    • evaluate_submission(fields, submitted, condition_evaluator) — single public entry point.
    • _resolve_applicable runs a fixed-point: keep filtering the applicable set until it stops shrinking (handles chained conditions where field B's visibility depends on field A's, and A is itself gated off).
    • _extract_submitted_value normalises raw JSON dicts, CustomPrivacyRequestField Pydantic instances, and bare values into a single comparable shape.
    • Evaluator errors fail closed — field treated as not applicable when applicability can't be proven.
  • src/fides/service/privacy_request/privacy_request_service.py:
    • New _validate_field_visibility resolves config + action for the request, filters out LocationCustomPrivacyRequestField entries (location handled separately), and calls evaluate_submission with a ConditionEvaluator(self.db).
    • Hooked into create_privacy_request next to the existing required-location validation.
    • DisplayConditionViolation is translated to PrivacyRequestError.

Steps to Confirm

  1. With a Privacy Center action that has a custom field gated by display_condition, POST a privacy request submitting that field with a value while the gating condition is false — expect 422 ("gated off … must not be submitted").
  2. With a required custom field whose display_condition evaluates true, POST a privacy request omitting it — expect 422 ("Required field … is missing").
  3. POST a valid request where the gated field is omitted while the gating condition is false — expect 200.
  4. POST a valid request where the gated field is submitted while the gating condition is true — expect 200.
  5. Submit nested chained conditions (B depends on A, A is gated off) — confirm B is treated as not applicable (fixed-point).
  6. Confirm LocationCustomPrivacyRequestField entries are still validated by the existing required-location path and not double-checked here.

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 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 5, 2026 9:53am
fides-privacy-center Ignored Ignored May 5, 2026 9:53am

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.15%. Comparing base (3ab7f3f) to head (b9f8a72).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8027      +/-   ##
==========================================
+ Coverage   83.91%   85.15%   +1.23%     
==========================================
  Files         636      637       +1     
  Lines       41864    41934      +70     
  Branches     4914     4927      +13     
==========================================
+ Hits        35130    35707     +577     
+ Misses       5611     5120     -491     
+ Partials     1123     1107      -16     

☔ 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.

@mikeGarifullin mikeGarifullin force-pushed the ENG-3516-1 branch 2 times, most recently from 39f61ff to b8eadb4 Compare April 24, 2026 13:25
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516-1 branch 3 times, most recently from 9120296 to f7ef420 Compare April 24, 2026 14:32
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516 branch 2 times, most recently from a222fd9 to 6b280f9 Compare April 24, 2026 15:37
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516-1 branch 8 times, most recently from 94b22a8 to 8a18b78 Compare April 27, 2026 12:31
@mikeGarifullin mikeGarifullin marked this pull request as ready for review April 27, 2026 12:36
@mikeGarifullin mikeGarifullin requested a review from a team as a code owner April 27, 2026 12:36
@mikeGarifullin mikeGarifullin requested review from vcruces and removed request for a team April 27, 2026 12:36
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516 branch 5 times, most recently from 8ea0480 to 8c4f2db Compare April 28, 2026 11:56
Copy link
Copy Markdown
Contributor

@vcruces vcruces left a comment

Choose a reason for hiding this comment

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

This looks great overall. I left a few comments, mostly questions or minor details. Approving!

Comment thread src/fides/service/privacy_request/privacy_request_service.py Outdated
Comment thread src/fides/service/privacy_request/privacy_request_service.py
Comment thread src/fides/api/schemas/custom_field_display_evaluator.py Outdated
Comment thread tests/service/privacy_request/test_privacy_request_service.py Outdated
Comment thread src/fides/service/privacy_request/custom_field_display_evaluator.py
Comment thread tests/service/privacy_request/test_privacy_request_service.py
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516-1 branch 2 times, most recently from d388813 to fc878d5 Compare April 30, 2026 11:07
@mikeGarifullin mikeGarifullin force-pushed the ENG-3516-1 branch 2 times, most recently from 0f35c91 to a2d3fe3 Compare May 5, 2026 09:36
Base automatically changed from ENG-3516 to main May 5, 2026 09:45
@mikeGarifullin mikeGarifullin enabled auto-merge May 5, 2026 09:53
@mikeGarifullin mikeGarifullin added this pull request to the merge queue May 5, 2026
Merged via the queue into main with commit 74abe1d May 5, 2026
69 checks passed
@mikeGarifullin mikeGarifullin deleted the ENG-3516-1 branch May 5, 2026 10:11
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