Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Adds AuditLog and ExecutionLogs to seed_test_data command #1097

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Conversation

seanpreston
Copy link
Contributor

@seanpreston seanpreston commented Aug 16, 2022

Purpose

This PR adds test AuditLog and ExecutionLog data to the seed_test_data command.

Screenshot 2022-08-16 at 17 33 45

Changes

  • Create AuditLog and ExecutionLogs for created PrivacyRequest objects of every permutation of action and status.

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

@seanpreston seanpreston changed the title add auditlog + executionlogs to seed_test_data command Adds AuditLog and ExecutionLogs to seed_test_data command Aug 16, 2022
@seanpreston seanpreston marked this pull request as ready for review August 16, 2022 21:33
"message": f"Execution log for request id {pr.id} status {status} and action_type {action_type}",
},
)
# run_privacy_request.apply(args=[pr.id]) # Run the request synchronously
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented-out code

data={
"dataset_name": "dummy_dataset",
"collection_name": "dummy_collection",
"fields_affected": ["dummy_field_1", "dummy_field_2"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the correct format for fields_affected - it looks more like:

[{
        "path": "dataset_name:collection_name:field_name",
        "field_name": "field_name",
        "data_categories": ["data_category_1", "data_category_2"]
    }]

Comment on lines 204 to 206
AuditLogAction.approved,
AuditLogAction.denied,
AuditLogAction.finished,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know this is dummy data, but maybe switching the order of approved and denied might be useful, so in a demo it doesn't look like it was denied and then it finished

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good improvement to our test data!

"path": "dummy_dataset:dummy_collection:dummy_field_1",
"field_name": "dummy_field",
"data_categories": [
"data_category_1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a fine for now, but at some point we may be using this data and have validation where Pydantic expects them to be proper data categories.

@pattisdr pattisdr merged commit cef5901 into main Aug 17, 2022
@pattisdr pattisdr deleted the test-log-data branch August 17, 2022 22:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants