Skip to content

Conversation

@JadeCara
Copy link
Contributor

@JadeCara JadeCara commented Nov 24, 2025

Ticket ENG-1290

Depends on #6984 to be merged before this one. It is testable though 😄

Description Of Changes

🎯 As a user, I want to specify conditions that must be true to create manual tasks, which check the request data, so that I can create tasks conditionally based on data in the request.

This PR adds some location based convenience fields. Fides uses a location hierarchy that is stored in a yaml. This means we can use the same data to improve the location customer experience with convenience columns.

Examples:

  • US-CA → location_country: "us", location_groups: ["us"], location_regulations: ["ccpa"]
  • US-CO → location_country: "us", location_groups: ["us"], location_regulations: ["cpa"]
  • FR → location_country: None, location_groups: ["eea"], location_regulations: ["gdpr"]

Code Changes

  • added location convenience fields to src/fides/api/task/conditional_dependencies/privacy_request/convenience_fields.py
  • added the data extraction for these new convenience fields to src/fides/api/task/conditional_dependencies/privacy_request/privacy_request_data.py
  • Updated the schema for allowable convenience fields to include the new location ones in src/fides/api/task/conditional_dependencies/privacy_request/schemas.py
  • data extractiont tests added to tests/api/task/conditional_dependencies/privacy_request/test_privacy_request_data.py
  • manual task integration tetsts added totests/api/task/manual/test_manual_task_conditional_evaluation.py

Steps to Confirm

  1. Run FidesPlus pointed at this branch
  2. Create a manual task (you can assign users/ add regular conditions as you like)
  3. Using the API (Front end is in the works) - add some privacy request conditions.
    For a full list of available conditions you can hit the GET /api/v1/plus/connection/{connection_key}/manual-task/dependency-conditions/privacy-request-fields endpoint. (available in https://github.com/ethyca/fidesplus/pull/2814/)

Example - This includes my existing data set condition and adds two privacy request conditions:

[
    {
      "logical_operator": "and",
      "conditions": [
        {
          "field_address": "bigquery_example_test_dataset:customer_039e6fc2:email",
          "operator": "not_exists",
          "value": null
        },
        {
          "field_address": "privacy_request.location_country",
          "operator": "eq",
          "value": "us"
        }, 
        {
          "field_address": "privacy_request.location_groups",
          "operator": "list_contains",
          "value": "us"
        },
        {
          "field_address": "privacy_request.location_regulations",
          "operator": "not_in_list",
          "value": "gdpr"
        }
      ]
    }
  ]

The new conditions will show up on the integration once added.
Screenshot 2025-11-24 at 12 57 37 PM

  1. Create a privacy request that meets all the conditions - verify it creates the ManualTask line
Screenshot 2025-11-24 at 11 59 39 AM
  1. Create a privacy request that does not meet all the conditions - verify it creates a skipped Manual task and the logs list the conditions as expected.
Screenshot 2025-11-24 at 1 14 13 PM

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

Jade Wibbels and others added 30 commits November 17, 2025 10:24
…il.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

👍 looks good to me! just a few of nits/ideas, but nothing blocking!


# If location has parent groups, the first one is typically the country
# (e.g., us_ca belongs to [us])
# For countries themselves (e.g., fr belongs to [eea]), location_country stays None
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: doesn't this comment line make a bit more sense above the next block, since it has to do with populating location_country?

"location_country": None,
"location_groups": [],
"location_regulations": [],
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason you didn't want to extract these a bit more formally, e.g. as an enum or perhaps as a typeddict, or even just some constant vars?

basically, my feeling (without understanding the overall code structure super well, so take this with a grain of salt) is that this could be a bit DRYer alongside the same field names declared in L80 of privacy_request_data.py. obviously 3 field names isn't too intractable, but just formalizing that structure a bit can help to promote consistency and improve readability.

from a quick glance, i suppose that comment could hold true for many of the other field classes. perhaps you've already given it some thought.

anyway, not a blocker by any means, curious to hear what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating to use the convenience field schema values.

Comment on lines 80 to 83
location_convenience_field_names = [
"location_country",
"location_groups",
"location_regulations",
Copy link
Contributor

Choose a reason for hiding this comment

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

just to tie this to the above comment - this is the other area where i think it could be helpful to be referencing some shared code, rather than redefining these literals!

Comment on lines 126 to 129
# Normalize location to match locations.yml format (lowercase with underscores)
# ISO 3166: "US-CA" -> locations.yml: "us_ca"
location_normalized = location.lower().replace("-", "_")
location_data = locations_by_id.get(location_normalized)
Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw, this may be useful as a utility in the locations module - just generally, it's not great that we've got different conventions coming in on the FE than we store on the BE (nothing you've caused), but i think it could help to have that conversion standardized a bit on the BE? i haven't looked at this area of the codebase in a long time, so i could be missing something :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a get location function that normalizes as well to src/fides/api/models/location_regulation_selections.py and updated the the conveniences to use it.

Base automatically changed from ENG-1290-privacy-request-fields to main December 1, 2025 21:34
@JadeCara JadeCara added this pull request to the merge queue Dec 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2025
Jade Wibbels added 2 commits December 1, 2025 16:52
…yca/fides into ENG-1290-convenience-fields-location
@JadeCara JadeCara added this pull request to the merge queue Dec 2, 2025
Merged via the queue into main with commit 2c36bf6 Dec 2, 2025
69 checks passed
@JadeCara JadeCara deleted the ENG-1290-convenience-fields-location branch December 2, 2025 01:08
jjdaurora pushed a commit that referenced this pull request Dec 5, 2025
Co-authored-by: Jade Wibbels <jade@ethyca.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
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.

3 participants