Skip to content

Conversation

@JadeCara
Copy link
Contributor

@JadeCara JadeCara commented Dec 12, 2025

Ticket ENG-2185

Description Of Changes

This ticket is asking for consent manual tasks. We are going forward with a work around #7102 because consent task graphs and access/erasure task graphs are different and have different capabilities - especially around conditional dependencies.

In order to make conditional dependencies work for consent tasks and access/erasure tasks we need to be able to assign conditions to specific manual task instances/submissions rather than to the whole task. (There was another request to be able to assign users the same way).

In addition the original update model where conditions would be updated individually was replaced by a full delete replace on conditions making individual condition search/retrieval unnecessary. The logging being used by the front end is almost entirely the general audit logs so the manual task log table and utilities are now redundant.

This PR is the first step to getting manual tasks and conditions into shape for the above requirements. It is primarily clean up based on the first few rounds of customer use and learning actual usage patterns.

  1. single row for conditional dependencies:
  • Migration to move all rows for conditional dependencies under into a JSONB in a single row.
    • This will save time when reading, writing, deleting etc.
    • This also sets us up nicely for future work on verifying that conditions do not negate each other (for policies coming up)
    • Right now the structure is limited to a single leaf or AND -> [leaves] The migration can handle this very easily.
    • Right now there are a couple customers using this but they don't have a LOT of conditions created so it should be easily run as a migration.
  • Update to the ORM removing the old parent/child column and adding the condition tree column
  • Updates to all tests

Code Changes

  • .fides/db_dataset.yml - update new column structure
  • src/fides/api/alembic/migrations/versions/xx_2025_12_12_1313_c37e8fde5a71_migrate_to_one_row_conditional_.py New migration, does a data migration from multi row to single row structure
  • src/fides/api/models/conditional_dependency/conditional_dependency_base.py updated to use new JsonB column removing old parent col etc
  • src/fides/api/models/digest/conditional_dependencies.py updated to use new inherited base
  • src/fides/api/models/manual_task/conditional_dependency.py updated to use new inherited base
  • src/fides/api/task/manual/manual_task_conditional_evaluation.py updated to pull field addresses from tree rather than rows
  • src/fides/api/task/manual/manual_task_utils.py removed the enum for the old row based conditional dependencies and added field address extraction function.
  • Updated all tests using the new model

Steps to Confirm

  1. There should be NO change in functionality.
  2. Manual tasks, conditional dependencies etc should continue to function as before.
  3. Please test with FidesPlus 2918
  4. Run FidesPlus branch above pointed at this branch.
  5. Create a ManualTask with several submissions required and several conditions, both dataset and privacy request based.
  6. Create privacy requests that meet and do not meet the requirements. Verify that they create/do not create as expected.
  7. Verify that you can complete the manual tasks with no errors.
  8. Verify that you can see the skipped logs and the manual task and manual task actions appear on the DSR activity log.
  9. Verify that you receive the access package with all expected manual task inclusions (attachments and text)
  10. Create a digest - The conditions are currently hardcoded for this so it should send as normal/expected. (Tip: you can adjust the rate for sending using the API so you dont have to wait until Monday at 9am to receive the digest)

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

vercel bot commented Dec 12, 2025

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 Dec 15, 2025 3:05pm
fides-privacy-center Ignored Ignored Dec 15, 2025 3:05pm

@codecov
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 86.27451% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.26%. Comparing base (ae2e461) to head (0baca7c).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/fides/api/task/manual/manual_task_utils.py 77.77% 1 Missing and 3 partials ⚠️
.../task/manual/manual_task_conditional_evaluation.py 84.21% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7117      +/-   ##
==========================================
- Coverage   87.30%   87.26%   -0.05%     
==========================================
  Files         533      533              
  Lines       35088    34977     -111     
  Branches     4067     4066       -1     
==========================================
- Hits        30633    30522     -111     
+ Misses       3570     3567       -3     
- Partials      885      888       +3     

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

@JadeCara JadeCara changed the base branch from main to ENG-2185-migration-remove-redundant-logging December 12, 2025 18:28
…185-migration-dependency-model-simplification
@JadeCara JadeCara mentioned this pull request Dec 15, 2025
18 tasks
…185-migration-dependency-model-simplification
@JadeCara
Copy link
Contributor Author

Splitting changes into smaller PRs

@JadeCara JadeCara closed this Dec 16, 2025
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