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

HJ-86 system endpoint new lifecycle table #5484

Merged
merged 14 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""add 'hidden' column to systems

Revision ID: abc53a00755b
Revises: 5a4859f74832
Create Date: 2024-11-11 20:18:27.770078

"""

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "abc53a00755b"
down_revision = "5a4859f74832"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column(
"ctl_systems",
sa.Column("hidden", sa.BOOLEAN(), server_default="f", nullable=False),
)
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column("ctl_systems", "hidden")
# ### end Alembic commands ###
73 changes: 49 additions & 24 deletions src/fides/api/api/v1/endpoints/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,37 +389,62 @@ async def ls( # pylint: disable=invalid-name
data_uses: Optional[List[FidesKey]] = Query(None),
data_categories: Optional[List[FidesKey]] = Query(None),
data_subjects: Optional[List[FidesKey]] = Query(None),
dnd_relevant: Optional[bool] = Query(None),
show_hidden: Optional[bool] = Query(False),
) -> List:
"""Get a list of all of the Systems.
If any pagination parameters (size or page) are provided, then the response will be paginated
& provided filters (search, taxonomy fields) will be applied.
If any parameters or filters are provided the response will be paginated and/or filtered.
erosselli marked this conversation as resolved.
Show resolved Hide resolved
Otherwise all Systems will be returned (this may be a slow operation if there are many systems,
so using the pagination parameters is recommended).
"""
if size or page:
pagination_params = Params(page=page or 1, size=size or 50)
# Need to join with PrivacyDeclaration in order to be able to filter
# by data use, data category, and data subject
query = select(System).outerjoin(
PrivacyDeclaration, System.id == PrivacyDeclaration.system_id
)
filter_params = FilterParams(
search=search,
data_uses=data_uses,
data_categories=data_categories,
data_subjects=data_subjects,
)
filtered_query = apply_filters_to_query(
query=query,
filter_params=filter_params,
search_model=System,
taxonomy_model=PrivacyDeclaration,
if not (
size
or page
or search
or data_uses
or data_categories
or data_subjects
or dnd_relevant
or show_hidden
):
return await list_resource(System, db)

pagination_params = Params(page=page or 1, size=size or 50)
# Need to join with PrivacyDeclaration in order to be able to filter
# by data use, data category, and data subject
query = select(System).outerjoin(
PrivacyDeclaration, System.id == PrivacyDeclaration.system_id
)

# Fetch any system that is relevant for Detection and Discovery, ie any of the following:
# - has connection configurations (has some integration for DnD or SaaS)
# - has dataset references
if dnd_relevant:
query = query.filter(
(System.connection_configs != None)
| (System.dataset_references.any()) # noqa: E712
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we use != None for one but .any() for the other condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dataset_references is a column on ctl_systems, so we just check for anything in that column. System.connection_configs is a derived relationship, Systems have a property connection_configs but it's attached via the connectionconfig table having a system_id, so the SQL that gets generated has a join and works better with != None

)
# Add a distinct so we only get one row per system
duplicates_removed = filtered_query.distinct(System.id)
return await async_paginate(db, duplicates_removed, pagination_params)

return await list_resource(System, db)
# Filter out any hidden systems, unless explicilty asked for
if not show_hidden:
query = query.filter(System.hidden == False) # noqa: E712

filter_params = FilterParams(
search=search,
data_uses=data_uses,
data_categories=data_categories,
data_subjects=data_subjects,
)
filtered_query = apply_filters_to_query(
query=query,
filter_params=filter_params,
search_model=System,
taxonomy_model=PrivacyDeclaration,
)

# Add a distinct so we only get one row per system
duplicates_removed = filtered_query.distinct(System.id)
return await async_paginate(db, duplicates_removed, pagination_params)


@SYSTEM_ROUTER.get(
Expand Down
1 change: 1 addition & 0 deletions src/fides/api/models/sql_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ class System(Base, FidesBase):
)
legitimate_interest_disclosure_url = Column(String)
user_id = Column(String, nullable=True)
hidden = Column(BOOLEAN(), default=False, server_default="f", nullable=False)

privacy_declarations = relationship(
"PrivacyDeclaration",
Expand Down
30 changes: 30 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,25 @@ def system(db: Session) -> System:
return system


@pytest.fixture(scope="function")
def system_hidden(db: Session) -> Generator[System, None, None]:
system = System.create(
db=db,
data={
"fides_key": f"system_key-f{uuid4()}",
"name": f"system-{uuid4()}",
"description": "fixture-made-system set as hidden",
"organization_fides_key": "default_organization",
"system_type": "Service",
"hidden": True,
},
)

db.refresh(system)
yield system
db.delete(system)


@pytest.fixture(scope="function")
def system_with_cleanup(db: Session) -> Generator[System, None, None]:
system = System.create(
Expand Down Expand Up @@ -1308,6 +1327,17 @@ def system_with_cleanup(db: Session) -> Generator[System, None, None]:
check_name=False,
)

ConnectionConfig.create(
db=db,
data={
"system_id": system.id,
"connection_type": "bigquery",
"name": "test_connection",
"secrets": {"password": "test_password"},
"access": "write",
},
)

db.refresh(system)
yield system
db.delete(system)
Expand Down
70 changes: 70 additions & 0 deletions tests/ctl/core/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,76 @@ def test_list_with_pagination_and_multiple_filters(

assert result_json["items"][0]["fides_key"] == tcf_system.fides_key

def test_list_with_dnd_filter(
self,
test_config,
system_with_cleanup, # one that has a connection config
system_third_party_sharing, # one that doesn't have a connection config
):
result = _api.ls(
url=test_config.cli.server_url,
headers=test_config.user.auth_header,
resource_type="system",
query_params={
"page": 1,
"size": 5,
"dnd_relevant": "true",
},
)

assert result.status_code == 200
result_json = result.json()
assert result_json["total"] == 1
assert len(result_json["items"]) == 1

# only "system_with_cleanup" has a connection config attached to it in fixtures
assert result_json["items"][0]["fides_key"] == system_with_cleanup.fides_key

def test_list_with_show_hidden(
erosselli marked this conversation as resolved.
Show resolved Hide resolved
self,
test_config,
system_hidden,
system_with_cleanup,
):

result = _api.ls(
url=test_config.cli.server_url,
headers=test_config.user.auth_header,
resource_type="system",
query_params={
"page": 1,
"size": 5,
"show_hidden": "true",
},
)

assert result.status_code == 200
result_json = result.json()
assert result_json["total"] == 2
assert len(result_json["items"]) == 2

actual_keys = [item["fides_key"] for item in result_json["items"]]
assert system_hidden.fides_key in actual_keys
assert system_with_cleanup.fides_key in actual_keys

result = _api.ls(
url=test_config.cli.server_url,
headers=test_config.user.auth_header,
resource_type="system",
query_params={
"page": 1,
"size": 5,
"show_hidden": "false",
},
)

assert result.status_code == 200
result_json = result.json()
assert result_json["total"] == 1
assert len(result_json["items"]) == 1

assert result_json["items"][0]["fides_key"] == system_with_cleanup.fides_key

@pytest.mark.skip("Until we re-visit filter implementation")
def test_list_with_pagination_and_multiple_filters_2(
self,
Expand Down
Loading