Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LA-199: Enhanced functionality around handling taxonomy active / de-active nodes #5617

Merged
merged 30 commits into from
Dec 19, 2024

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Dec 18, 2024

❗ Contains 2 migrations ❗

Closes https://ethyca.atlassian.net/browse/LA-199

Description Of Changes

This PR does 2 things:

  1. Adjusts functionality when the active field of taxonomy items are updated. If deactivating a node, we cascade deactivate children of a label. If activating a node, we ensure all its' parent nodes are activated.

  2. Adds a data migration such that if any currently de-active node has any number of active children, we enable that node so that it can be shown in the new Taxonomy UI. For this data migration, a downgrade is not possible as we do not track the changed taxonomy updates.

Code Changes

  • Adds 2 migrations, 1 to handle foreign key between parent / child taxonomy relationships, the other to handle the data migration described above
  • Adds tests for the parent-child relationships in the base taxonomy model
  • Adds 2 new generic endpoint overrides, 1 for updating data categories, 1 for updating data uses.
  • Adds tests for the new update functionality where we cascade up/down when we change the active field

Steps to Confirm

  1. Confirm all tests pass
  2. Navigate to Taxonomy tab to view new Taxonomy UI
  3. Attempt to create children, delete (aka de-activate) children, update description / name / etc, see that everything works as it should
  4. Specifically, when you delete a parent, all of its child nodes should also be deleted / removed from the taxonomy UI view

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
  • Followup issues:
    • Followup issues created (include link)
    • 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

Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Dec 19, 2024 6:06pm

Copy link

cypress bot commented Dec 18, 2024

fides    Run #11594

Run Properties:  status check passed Passed #11594  •  git commit 739671997c ℹ️: Merge 51857b59418af94e9ee66e810cd871f229199a5b into 8f8f6ea2ef75f2040c76404b98dd...
Project fides
Branch Review refs/pull/5617/merge
Run status status check passed Passed #11594
Run duration 00m 59s
Commit git commit 739671997c ℹ️: Merge 51857b59418af94e9ee66e810cd871f229199a5b into 8f8f6ea2ef75f2040c76404b98dd...
Committer Catherine Smith
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@eastandwestwind eastandwestwind changed the title LA-199 LA-199: Enhanced functionality around handling taxonomy active / de-active nodes Dec 18, 2024
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 73.77049% with 16 lines in your changes missing coverage. Please review.

Project coverage is 87.13%. Comparing base (8f8f6ea) to head (51857b5).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...rc/fides/api/api/v1/endpoints/generic_overrides.py 65.21% 13 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5617      +/-   ##
==========================================
- Coverage   87.16%   87.13%   -0.04%     
==========================================
  Files         388      388              
  Lines       23940    23988      +48     
  Branches     2586     2594       +8     
==========================================
+ Hits        20868    20902      +34     
- Misses       2514     2525      +11     
- Partials      558      561       +3     

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

@galvana galvana self-requested a review December 19, 2024 17:31
@eastandwestwind eastandwestwind merged commit f319894 into main Dec 19, 2024
38 of 39 checks passed
@eastandwestwind eastandwestwind deleted the LA-199 branch December 19, 2024 18:43
eastandwestwind added a commit that referenced this pull request Dec 19, 2024
…ctive nodes (#5617)

Co-authored-by: Adrian Galvan <adrian@ethyca.com>
Copy link

cypress bot commented Dec 19, 2024

fides    Run #11596

Run Properties:  status check passed Passed #11596  •  git commit f319894c92: LA-199: Enhanced functionality around handling taxonomy active / de-active nodes...
Project fides
Branch Review main
Run status status check passed Passed #11596
Run duration 00m 48s
Commit git commit f319894c92: LA-199: Enhanced functionality around handling taxonomy active / de-active nodes...
Committer Catherine Smith
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

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