Updates for consent signal processing #5200
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
refs/pull/5200/merge
|
| Run status |
|
| Run duration | 00m 37s |
| Commit |
|
| Committer | Adrian Galvan |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5200 +/- ##
==========================================
- Coverage 86.41% 86.41% -0.01%
==========================================
Files 362 362
Lines 22781 22792 +11
Branches 3058 3060 +2
==========================================
+ Hits 19687 19695 +8
- Misses 2537 2538 +1
- Partials 557 559 +2 ☔ View full report in Codecov by Sentry. |
…for-consent-signal-processing
…for-consent-signal-processing
…for-consent-signal-processing
...s/admin-ui/src/features/datastore-connections/system_portal_config/ConsentAutomationForm.tsx
Show resolved
Hide resolved
src/fides/api/alembic/migrations/versions/d9064e71f69d_add_connections_to_client.py
Show resolved
Hide resolved
| @@ -654,21 +656,23 @@ def run_consent_request( | |||
| identity_data: Dict[str, Any], | |||
| session: Session, | |||
| ) -> bool: | |||
| """Execute a consent request. Return whether the consent request to the third party succeeded. | |||
| # pylint: disable=too-many-branches, too-many-statements | |||
There was a problem hiding this comment.
There's a lot happening in run_consent_request now, a followup ticket could try to break some of this logic into separate functions -
There was a problem hiding this comment.
Agreed! I thought about doing it during this pass but a follow up ticket sound better https://ethyca.atlassian.net/browse/PROD-2659
| consent_propagation_status = ( | ||
| ConsentPropagationStatus.missing_data | ||
| ) |
There was a problem hiding this comment.
This seems net new - if I'm following this will now cause an exception to be raised on L826 when before it was just ignored? Is this intentional?
There was a problem hiding this comment.
We previously set fired = False at the start, and if fired remained False by the end, we assumed data was missing and raised a SkippingConsentPropagation exception with the message "Missing needed values to propagate request". Now, we're setting this explicitly if we get a ValueError when building a consent request and skip_missing_param_values is set to True
except ValueError as exc:
if consent_request.skip_missing_param_values:
Same behavior as before but now it's more explicit
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 35s |
| Commit |
|
| Committer | Adrian Galvan |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Closes PROD-2601
Description Of Changes
Fides changes to support consent signal processing in Fidesplus https://github.com/ethyca/fidesplus/pull/1563/
Code Changes
ClientDetailmodel to supportconnectionsin a similar pattern as the existingsystemsfieldtoken_duration_overridekwarg to theextract_token_and_load_clientfunction to be able to specify a longer TTL for consent webhook tokens (see the fidesplus PR for more details)booltoConsentPropagationStatusto provide a more granular response to the caller instead of just a fired/not-fired booleanrun_consent_requestfunction in theSaaSConnectorclassSteps to Confirm
Pre-Merge Checklist
CHANGELOG.mdmaindowngrade()migration is correct and works