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

Support for consent management report #4452

Merged
merged 11 commits into from
Dec 1, 2023

Conversation

galvana
Copy link
Contributor

@galvana galvana commented Nov 22, 2023

Description Of Changes

Adds a GIN index on ctl_systems.name and a new /purposes endpoint to support the new consent management report table

Code Changes

  • Migration step to add an index to ctl_systems
  • New /purposes endpoint that returns a map of purpose and special purpose IDs to mapped purposes (purpose info + data uses)

Steps to Confirm

  • Navigate to https://localhost:8080/docs
  • Authenticate
  • Test out the GET /purposes endpoint

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Update CHANGELOG.md

Copy link

vercel bot commented Nov 22, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Dec 1, 2023 0:05am



def upgrade():
op.execute("CREATE EXTENSION IF NOT EXISTS pg_trgm;")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pg_trgm extension has been available since Postgres 8.3 so we should be ok to enable it

Comment on lines +20 to +24
op.create_index(
"ix_ctl_systems_name",
"ctl_systems",
[sa.text("name gin_trgm_ops")],
postgresql_using="gin",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adds a GIN index to be able to do a partial search on system name (%name%)


@router.get(
"/purposes",
dependencies=[Security(verify_oauth_client)],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not tied to any scope, this is available for any authenticated user

@@ -112,6 +112,9 @@
"/privacy-request/transfer/{privacy_request_id}/{rule_key}"
)

# Purpose URLs
PURPOSES = "/purposes"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it's best to make this its own URL or if it should go under an existing path

Copy link

cypress bot commented Nov 22, 2023

Passing run #5338 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 79487f6 into 533cc27...
Project: fides Commit: 9e51c22c3f ℹ️
Status: Passed Duration: 00:35 💡
Started: Nov 22, 2023 4:07 PM Ended: Nov 22, 2023 4:08 PM

Review all test suite changes for PR #4452 ↗︎

from fides.common.api.v1.urn_registry import PURPOSES, V1_URL_PREFIX


class TestGetPurposes:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple test that just checks access based on auth header

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (533cc27) 87.10% compared to head (79487f6) 87.11%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4452      +/-   ##
==========================================
+ Coverage   87.10%   87.11%   +0.01%     
==========================================
  Files         329      330       +1     
  Lines       20366    20390      +24     
  Branches     2623     2625       +2     
==========================================
+ Hits        17739    17762      +23     
- Misses       2163     2164       +1     
  Partials      464      464              

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

Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

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

🚀

@galvana galvana merged commit f008dcf into main Dec 1, 2023
14 checks passed
@galvana galvana deleted the PROD-1335-create-new-system-aggregate-endpoint branch December 1, 2023 00:06
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