Changes to support bidirectional consent #5118

galvana · 2024-07-22T17:54:14Z

Closes PROD-2343

Description Of Changes

Core changes to support the PUT and GET /consentable-items endpoints in Fidesplus.

Code Changes

Migration script to add the plus_consent_automation and plus_consentable_item tables
Adding new SaaSRequestTypes to support new bidirectional consent actions (get_consentable_items, update_consent, process_consent_webhook)

Steps to Confirm

Follow the steps outlined in the corresponding Fidesplus PR https://github.com/ethyca/fidesplus/pull/1524

Pre-Merge Checklist

vercel · 2024-07-22T17:54:17Z

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

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 31, 2024 4:44pm

cypress · 2024-07-22T18:05:35Z

Test summary

4 • 0 • 0 • 0

Run details


Project	fides
Status	Passed
Commit	ec44f723f2 ℹ️
Started	Jul 31, 2024 4:53 PM
Ended	Jul 31, 2024 4:54 PM
Duration	00:35 💡
OS	Linux Ubuntu -
Browser	Electron 106

View run in Cypress Cloud ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

codecov · 2024-07-22T18:21:04Z

Codecov Report

Attention: Patch coverage is 82.31293% with 26 lines in your changes missing coverage. Please review.

Project coverage is 86.52%. Comparing base (c3eae69) to head (622f8d8).

Files	Patch %	Lines
...vice/saas_request/saas_request_override_factory.py	52.63%	6 Missing and 3 partials ⚠️
src/fides/api/service/connectors/saas_connector.py	42.85%	5 Missing and 3 partials ⚠️
src/fides/api/schemas/consentable_item.py	86.48%	3 Missing and 2 partials ⚠️
src/fides/api/models/consent_automation.py	94.36%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5118      +/-   ##
==========================================
- Coverage   86.55%   86.52%   -0.03%     
==========================================
  Files         357      359       +2     
  Lines       22340    22470     +130     
  Branches     2957     2978      +21     
==========================================
+ Hits        19336    19443     +107     
- Misses       2480     2494      +14     
- Partials      524      533       +9

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

…onsent

src/fides/api/alembic/migrations/versions/d69cf8f82a58_add_consent_automation_tables.py

src/fides/api/service/connectors/saas_connector.py

src/fides/api/service/saas_request/saas_request_override_factory.py

…onsent

…hema to prevent calling from_orm during payload validation

eastandwestwind

Overall a very clean implementation @galvana , well done! Overall the functionality works as expected, but just a couple items to clarify and some additional tests for some of the DB functionality. Thanks!

eastandwestwind · 2024-07-30T16:16:49Z

src/fides/api/alembic/migrations/versions/d69cf8f82a58_add_consent_automation_tables.py

+            ["privacynotice.id"],
+        ),
+        sa.ForeignKeyConstraint(
+            ["parent_id"], ["plus_consentable_item.id"], ondelete="CASCADE"


Can we add a test for this cascade functionality, too?

eastandwestwind · 2024-07-30T16:17:21Z

src/fides/api/alembic/migrations/versions/d69cf8f82a58_add_consent_automation_tables.py

+        sa.ForeignKeyConstraint(
+            ["consent_automation_id"],
+            ["plus_consent_automation.id"],
+            ondelete="CASCADE",


Can we add a test for this cascade functionality?

eastandwestwind · 2024-07-30T16:20:18Z

src/fides/api/models/consent_automation.py

+                    notice_id=item_data.get("notice_id"),
+                )
+                db.add(item)
+                # flush to the DB so we can get the auto-generated ID for this item


can you explain how flush works? I haven't had to use this manually before

flush() lets you push changes to the database without committing the transaction. In this case, we just called db.add(item) to add a new ConsentableItem to this current DB session, but if we want to get the ID for that new item, we can call flush() and this would be executed in the current transaction

2024-07-30 11:09:21 2024-07-30 18:09:21.081 UTC [148] LOG: statement: INSERT INTO plus_consentable_item (id, consent_automation_id, external_id, parent_id, notice_id, type, name) VALUES ('plu_16815018-1e85-45f8-beb0-8c1a7998412f', 'plu_6f56003c-610e-4196-8e21-ed6815be9bc8', '1', 'plu_fc600d19-7101-4db4-b148-1849c667fae3', NULL, 'Message type', 'Weekly Ads')

So it's enough for us to get an ID without us having to commit the current transaction. If any exceptions occur during later in the code (after flush()), we can still roll everything back as if nothing happened.

eastandwestwind · 2024-07-30T16:20:46Z

src/fides/api/models/consent_automation.py

+    parent: "RelationshipProperty[Optional[ConsentableItem]]" = relationship(
+        "ConsentableItem",
+        back_populates="children",
+        remote_side=[id],


what is remote_side?

It's needed for self-referential relationships like what we're doing here with ConsentableItem. There's an in-depth explanation here but the way I was able to reason about it was that it's telling SQLAlchemy which side of the relationship is the "parent" or "one" side of a one-to-many relationship. As an example, one ConsentableItem can have an id of 1, but many ConsentableItems can have their parent_id be 1, so id would be the "remote_side".

src/fides/api/service/saas_request/saas_request_override_factory.py

…onsent

eastandwestwind · 2024-07-31T14:03:36Z

tests/ops/models/test_consent_automation.py

+            .filter(ConsentableItem.consent_automation_id == consent_automation.id)
+            .all()
+        )
+        assert len(consentable_items) == 0


awesome, thanks!

eastandwestwind

thanks for adding those tests @galvana !

eastandwestwind · 2024-07-31T14:04:37Z

Oh, and can we add a CHANGELOG entry here too?

…onsent

cypress · 2024-07-31T17:42:06Z

Test summary

4 • 0 • 0 • 0

Run details


Project	fides
Status	Passed
Commit	`681b39c`
Started	Jul 31, 2024 5:41 PM
Ended	Jul 31, 2024 5:42 PM
Duration	00:35 💡
OS	Linux Ubuntu -
Browser	Electron 106

View run in Cypress Cloud ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

Changes to support bidirectional consent

ca5f113

galvana added 6 commits July 22, 2024 15:10

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

4715cbf

…onsent

Adding consent automation table

45ea791

Refactoring consentable items to use a dedicated database table

c6817de

Updating create or update for consentable items

78b7da0

Moving ConsentableItem schema to Fides

f93333a

Changing unmapped flag to default to false

9abb34f

galvana requested a review from eastandwestwind July 28, 2024 00:24

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

e23a7cc

…onsent

galvana marked this pull request as ready for review July 28, 2024 00:24

vercel bot deployed to Preview July 28, 2024 00:27 View deployment

galvana commented Jul 28, 2024

View reviewed changes

galvana added 4 commits July 29, 2024 09:18

Updating supported actions logic

1e26d2a

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

f2312e9

…onsent

Misc fixes

fc76450

Changing ConsentableItem to inherit from BaseModel instead of FidesSc…

61f9df4

…hema to prevent calling from_orm during payload validation

eastandwestwind reviewed Jul 30, 2024

View reviewed changes

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

db52e9e

…onsent

vercel bot deployed to Preview July 30, 2024 17:48 View deployment

Changes based on PR feedback

79fc16c

galvana requested a review from eastandwestwind July 30, 2024 22:29

eastandwestwind reviewed Jul 31, 2024

View reviewed changes

eastandwestwind approved these changes Jul 31, 2024

View reviewed changes

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

13ec623

…onsent

vercel bot deployed to Preview July 31, 2024 15:30 View deployment

Misc fixes and change log entry

3cf4ec6

Merge branch 'main' into PROD-2343-changes-to-support-bidirectional-c…

622f8d8

…onsent

vercel bot deployed to Preview July 31, 2024 16:44 View deployment

galvana merged commit 681b39c into main Jul 31, 2024
45 of 47 checks passed

galvana deleted the PROD-2343-changes-to-support-bidirectional-consent branch July 31, 2024 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to support bidirectional consent #5118

Changes to support bidirectional consent #5118

galvana commented Jul 22, 2024 •

edited by jira bot

Loading

vercel bot commented Jul 22, 2024 •

edited

Loading

cypress bot commented Jul 22, 2024 •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading

eastandwestwind left a comment

eastandwestwind Jul 30, 2024

eastandwestwind Jul 30, 2024

eastandwestwind Jul 30, 2024

galvana Jul 30, 2024

eastandwestwind Jul 30, 2024

galvana Jul 30, 2024

eastandwestwind Jul 31, 2024

eastandwestwind left a comment

eastandwestwind commented Jul 31, 2024

cypress bot commented Jul 31, 2024

Changes to support bidirectional consent #5118

Changes to support bidirectional consent #5118

Conversation

galvana commented Jul 22, 2024 • edited by jira bot Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Jul 22, 2024 • edited Loading

cypress bot commented Jul 22, 2024 • edited Loading

Test summary

Run details

codecov bot commented Jul 22, 2024 • edited Loading

Codecov Report

eastandwestwind left a comment

Choose a reason for hiding this comment

eastandwestwind Jul 30, 2024

Choose a reason for hiding this comment

eastandwestwind Jul 30, 2024

Choose a reason for hiding this comment

eastandwestwind Jul 30, 2024

Choose a reason for hiding this comment

galvana Jul 30, 2024

Choose a reason for hiding this comment

eastandwestwind Jul 30, 2024

Choose a reason for hiding this comment

galvana Jul 30, 2024

Choose a reason for hiding this comment

eastandwestwind Jul 31, 2024

Choose a reason for hiding this comment

eastandwestwind left a comment

Choose a reason for hiding this comment

eastandwestwind commented Jul 31, 2024

cypress bot commented Jul 31, 2024

Test summary

Run details

galvana commented Jul 22, 2024 •

edited by jira bot

Loading

vercel bot commented Jul 22, 2024 •

edited

Loading

cypress bot commented Jul 22, 2024 •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading