Improved privacy request error handling #5500

galvana · 2024-11-15T19:57:06Z

Closes LA-102 and LA-68

Description Of Changes

This PR contains miscellaneous improvements to the privacy request flow.

Code Changes

Main change

Fixed issue where database sessions were no longer available after long-running access, erasure, or consent requests

Bug fixes

Fixed bug where datasets from disabled integrations were still used in the traversal
Fixed bug where privacy request did not transition from approved to in progress
Fixed bug where execution log timestamps were incorrect

Misc improvements

Updates the privacy request detail page to poll for the latest results as the request is processing
Adds a new Dataset traversal timeline event before the actual request starts
Removed the extra click between the activity timeline to the event logs
Adds the privacy request ID to all privacy request logs

Steps to Confirm

Set the following values in fides.toml

[execution]
use_dsr_3_0 = true

[celery]
task_always_eager = false

Start the test environment with nox -s dev -- postgres, if this fails you might need to run nox -s build
Connect to the postgres_example database and run this SQL script postgres_sample.txt
Create this dataset
postgres_dataset.txt
Create a new system with a Postgres integration to the postgres_example database. After saving attach the dataset from the previous step

host: host.docker.internal
port: 6432
username: postgres
password: postgres
database: postgres_example

Submit a privacy request via the Privacy Center for jane@example.com
Go the privacy request detail page for this request and approve it. You should see the page auto-update as the request processes
Go back to the integration and disable it, then submit another request. The Postgres dataset should not appear in the activity timeline

Pre-Merge Checklist

vercel · 2024-11-15T19:57:09Z

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		Nov 20, 2024 5:33pm

cypress · 2024-11-15T20:13:09Z

fides Run #11114

Run Properties: Passed #11114 • 514567e1b2 ℹ️: Merge 9baf5e4931813ad690114a9e4fc8501efd380d9e into 0a7a2b2feb03717bbc5297eb0bc9...

Project	`fides`
Branch Review	`refs/pull/5500/merge`
Run status	`Passed #11114`
Run duration	`00m 39s`
Commit	`514567e1b2 ℹ️: Merge 9baf5e4931813ad690114a9e4fc8501efd380d9e into 0a7a2b2feb03717bbc5297eb0bc9...`
Committer	`Adrian Galvan`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
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 ↗︎

codecov · 2024-11-15T20:29:51Z

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.19%. Comparing base (1c8b2d7) to head (2a1c2cc).

Files with missing lines	Patch %	Lines
.../service/privacy_request/request_runner_service.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5500      +/-   ##
==========================================
+ Coverage   85.18%   85.19%   +0.01%     
==========================================
  Files         387      387              
  Lines       24265    24285      +20     
  Branches     2646     2649       +3     
==========================================
+ Hits        20669    20689      +20     
  Misses       3042     3042              
  Partials      554      554

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

Consistent privacy request logging New traversal entry in activity timeline

eastandwestwind

Nice work so far @galvana ! I like how you've fortified our logging by adding additional args into @log_context decorator.

Can you explain more about how you envision future devs should use this decorator in combination with manual log statements?

clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx

eastandwestwind · 2024-11-18T10:42:43Z

clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx


 type ActivityTimelineProps = {
  subjectRequest: PrivacyRequestEntity;
-  setEventDetails: (d: EventData) => void;
 };

 const hasUnresolvedError = (entries: ExecutionLog[]) => {


I know this method existed before- but mind adding a short docstring explaining what this does? Specifically- what an "unresolved" error means vs just an error?

clients/admin-ui/src/features/privacy-requests/events-and-logs/EventLog.tsx

...api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py

src/fides/api/models/privacy_request.py

src/fides/api/service/privacy_request/request_runner_service.py

src/fides/api/task/create_request_tasks.py

src/fides/api/task/execute_request_tasks.py

src/fides/api/util/logger_context_utils.py

clients/admin-ui/src/features/common/RequestStatusBadge.tsx

clients/admin-ui/src/features/privacy-requests/PrivacyRequest.tsx

galvana · 2024-11-18T04:48:03Z

clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx

I broke out the TimelineEntry and LogDrawer into separate components. I also removed the EventDetails component since it added unnecessary click before opening the LogDrawer

galvana · 2024-11-18T04:52:30Z

...api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py

+
+def upgrade():
+    op.alter_column(
+        "executionlog", "created_at", server_default=text("clock_timestamp()")


The previous server_default for the created_at and updated_at columns of ExecutionLog were set to now(). This meant that when the execution log entry was written to the database it would use the timestamp from when the database session was opened instead of when the ExecutionLog entry was created. Changing this to clock_timestamp() makes it so we don't used the cached timestamp and use the actual event creation time. This value is still calculated on the database so there's no issues with application time inconsistencies in the case of multiple webservers or workers.

This makes sense!

src/fides/api/graph/traversal.py

src/fides/api/models/privacy_request.py

src/fides/api/service/privacy_request/request_runner_service.py

src/fides/api/task/execute_request_tasks.py

src/fides/api/util/logger_context_utils.py

galvana · 2024-11-19T03:57:09Z

clients/admin-ui/src/features/privacy-requests/events-and-logs/helpers.ts

I simplified this so it's easier to understand

eastandwestwind

Nice work @galvana !

cypress · 2024-11-20T17:53:06Z

fides Run #11115

Run Properties: Passed #11115 • 97edac0ea8: Improved privacy request error handling (#5500)

Project	`fides`
Branch Review	`main`
Run status	`Passed #11115`
Run duration	`00m 39s`
Commit	`97edac0ea8: Improved privacy request error handling (#5500)`
Committer	`Adrian Galvan`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
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 ↗︎

Improved privacy request error handling

2a1c2cc

Privacy request detail page improvements

3cbd165

Consistent privacy request logging New traversal entry in activity timeline

vercel bot had a problem deploying to Preview November 18, 2024 04:41 Failure

galvana requested a review from eastandwestwind November 18, 2024 05:26

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

60b7fa3

vercel bot had a problem deploying to Preview November 18, 2024 05:27 Failure

eastandwestwind reviewed Nov 18, 2024

View reviewed changes

galvana commented Nov 18, 2024

View reviewed changes

Minor front-end refactor + unit tests

58ec278

vercel bot had a problem deploying to Preview November 18, 2024 18:52 Failure

galvana added 2 commits November 18, 2024 15:32

Adding tests

3a84196

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

3cf9d92

vercel bot deployed to Preview November 18, 2024 23:40 View deployment

Logging additional context + log_context tests

6ea292a

galvana marked this pull request as ready for review November 19, 2024 01:33

galvana added 3 commits November 18, 2024 17:34

Mypy fix

de7c753

Fixing tests

8ca8513

Fixing tests and types

fc4ae85

vercel bot deployed to Preview November 19, 2024 03:55 View deployment

galvana commented Nov 19, 2024

View reviewed changes

Fixing tests

3229589

galvana added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Nov 19, 2024

galvana added 2 commits November 18, 2024 22:59

Fixing tests

99fd2f2

Disabling hanging tests

5b87348

galvana requested a review from eastandwestwind November 19, 2024 08:42

galvana added 2 commits November 19, 2024 08:09

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

2771579

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

cd8d1ee

vercel bot deployed to Preview November 19, 2024 23:48 View deployment

eastandwestwind approved these changes Nov 20, 2024

View reviewed changes

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

8c6cdc3

vercel bot deployed to Preview November 20, 2024 16:54 View deployment

galvana added 2 commits November 20, 2024 09:26

Merge branch 'main' into LA-102-improve-privacy-request-error-handling

64e1676

Updating change log

9baf5e4

galvana merged commit 97edac0 into main Nov 20, 2024
20 checks passed

galvana deleted the LA-102-improve-privacy-request-error-handling branch November 20, 2024 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved privacy request error handling #5500

Improved privacy request error handling #5500

galvana commented Nov 15, 2024 •

edited by jira bot

Loading

vercel bot commented Nov 15, 2024 •

edited

Loading

cypress bot commented Nov 15, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024

eastandwestwind left a comment

eastandwestwind Nov 18, 2024

galvana Nov 18, 2024

galvana Nov 18, 2024

eastandwestwind Nov 18, 2024

galvana Nov 19, 2024

eastandwestwind left a comment

cypress bot commented Nov 20, 2024

Improved privacy request error handling #5500

Improved privacy request error handling #5500

Conversation

galvana commented Nov 15, 2024 • edited by jira bot Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Nov 15, 2024 • edited Loading

cypress bot commented Nov 15, 2024 • edited Loading

fides Run #11114

codecov bot commented Nov 15, 2024

Codecov Report

eastandwestwind left a comment

Choose a reason for hiding this comment

eastandwestwind Nov 18, 2024

Choose a reason for hiding this comment

galvana Nov 18, 2024

Choose a reason for hiding this comment

galvana Nov 18, 2024

Choose a reason for hiding this comment

eastandwestwind Nov 18, 2024

Choose a reason for hiding this comment

galvana Nov 19, 2024

Choose a reason for hiding this comment

eastandwestwind left a comment

Choose a reason for hiding this comment

cypress bot commented Nov 20, 2024

fides Run #11115

galvana commented Nov 15, 2024 •

edited by jira bot

Loading

vercel bot commented Nov 15, 2024 •

edited

Loading

cypress bot commented Nov 15, 2024 •

edited

Loading