diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5fdd3fa027..7931b10506 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ repos: description: Detect hardcoded secrets pre-commit using Gitleaks entry: gitleaks protect --verbose --no-banner --redact --staged language: system - stages: [pre-commit] + stages: [commit] - id: gitleaks-pre-push # To use this pre-push hook, run @@ -20,4 +20,4 @@ repos: description: Detect hardcoded secrets pre-push using Gitleaks entry: gitleaks detect --verbose --no-banner --redact --log-opts "origin..HEAD" language: system - stages: [pre-push] + stages: [push] diff --git a/CHANGELOG.md b/CHANGELOG.md index c8d8ddaba8..325bba29cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,10 +28,14 @@ The types of changes are: - Added new `has_next` parameter for the `link` pagination strategy [#5596](https://github.com/ethyca/fides/pull/5596) - Added a `DBCache` model for database-backed caching [#5613](https://github.com/ethyca/fides/pull/5613) - Adds "reclassify" button to discovery result tables [#5574](https://github.com/ethyca/fides/pull/5574) +- Added support for exporting datamaps with column renaming, reordering and visibility options [#5543](https://github.com/ethyca/fides/pull/5543) ### Changed - Adjusted Ant's Select component colors and icon [#5594](https://github.com/ethyca/fides/pull/5594) - Replaced taxonomies page with new UI based on an interactive tree visualization [#5602](https://github.com/ethyca/fides/pull/5602) +- Adjusted functionality around updating taxonomy active field, includes data migration to re-activate taxonomy nodes [#5617](https://github.com/ethyca/fides/pull/5617) +- Migrated breadcrumbs to Ant Design [#5610](https://github.com/ethyca/fides/pull/5610) +- Updated `CustomReportConfig` to be more intuitive on its contents [#5543](https://github.com/ethyca/fides/pull/5543) ### Fixed - Fixing quickstart.py script [#5585](https://github.com/ethyca/fides/pull/5585) diff --git a/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx b/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx index 1025669100..0b0164ce12 100644 --- a/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx +++ b/clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx @@ -65,7 +65,7 @@ import { getDatamapReportColumns, getDefaultColumn, } from "./DatamapReportTableColumns"; -import { getGrouping, getPrefixColumns } from "./utils"; +import { getColumnOrder, getGrouping, getPrefixColumns } from "./utils"; const emptyMinimalDatamapReportResponse: Page_DatamapReport_ = { items: [], @@ -223,6 +223,14 @@ export const DatamapReportTable = () => { ], ); + useEffect(() => { + if (datamapReport?.items?.length) { + const columnIDs = Object.keys(datamapReport.items[0]); + setColumnOrder(getColumnOrder(groupBy, columnIDs)); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [groupBy, datamapReport]); + const { isOpen: isColumnSettingsOpen, onOpen: onColumnSettingsOpen, diff --git a/clients/admin-ui/src/features/datamap/reporting/datamap-report-context.tsx b/clients/admin-ui/src/features/datamap/reporting/datamap-report-context.tsx index 65aa83b333..3051bfd243 100644 --- a/clients/admin-ui/src/features/datamap/reporting/datamap-report-context.tsx +++ b/clients/admin-ui/src/features/datamap/reporting/datamap-report-context.tsx @@ -11,7 +11,6 @@ import { DATAMAP_GROUPING } from "~/types/api"; import { DatamapReportFilterSelections } from "../types"; import { COLUMN_IDS, DATAMAP_LOCAL_STORAGE_KEYS } from "./constants"; -import { getColumnOrder } from "./utils"; interface DatamapReportContextProps { savedCustomReportId: string; @@ -61,7 +60,7 @@ export const DatamapReportProvider = ({ const [columnOrder, setColumnOrder] = useLocalStorage( DATAMAP_LOCAL_STORAGE_KEYS.COLUMN_ORDER, - getColumnOrder(groupBy), + [], ); const [columnVisibility, setColumnVisibility] = useLocalStorage< diff --git a/clients/admin-ui/src/features/datamap/reporting/utils.ts b/clients/admin-ui/src/features/datamap/reporting/utils.ts index 405a7fb222..6c7b910134 100644 --- a/clients/admin-ui/src/features/datamap/reporting/utils.ts +++ b/clients/admin-ui/src/features/datamap/reporting/utils.ts @@ -12,24 +12,23 @@ export const getGrouping = (groupBy?: DATAMAP_GROUPING) => { } }; -export const getColumnOrder = (groupBy: DATAMAP_GROUPING) => { +export const getColumnOrder = ( + groupBy: DATAMAP_GROUPING, + columnIDs: string[], +) => { let columnOrder: string[] = []; if (DATAMAP_GROUPING.SYSTEM_DATA_USE === groupBy) { - columnOrder = [ - COLUMN_IDS.SYSTEM_NAME, - COLUMN_IDS.DATA_USE, - COLUMN_IDS.DATA_CATEGORY, - COLUMN_IDS.DATA_SUBJECT, - ]; + columnOrder = [COLUMN_IDS.SYSTEM_NAME, COLUMN_IDS.DATA_USE]; } if (DATAMAP_GROUPING.DATA_USE_SYSTEM === groupBy) { - columnOrder = [ - COLUMN_IDS.DATA_USE, - COLUMN_IDS.SYSTEM_NAME, - COLUMN_IDS.DATA_CATEGORY, - COLUMN_IDS.DATA_SUBJECT, - ]; + columnOrder = [COLUMN_IDS.DATA_USE, COLUMN_IDS.SYSTEM_NAME]; } + columnOrder = columnOrder.concat( + columnIDs.filter( + (columnID) => + columnID !== COLUMN_IDS.SYSTEM_NAME && columnID !== COLUMN_IDS.DATA_USE, + ), + ); return columnOrder; }; diff --git a/clients/admin-ui/src/features/taxonomy/components/TaxonomyInteractiveTree.tsx b/clients/admin-ui/src/features/taxonomy/components/TaxonomyInteractiveTree.tsx index 23e503a4e6..d43e8f282d 100644 --- a/clients/admin-ui/src/features/taxonomy/components/TaxonomyInteractiveTree.tsx +++ b/clients/admin-ui/src/features/taxonomy/components/TaxonomyInteractiveTree.tsx @@ -53,9 +53,9 @@ const TaxonomyInteractiveTree = ({ // Reset the zoom level and center the view when the taxonomy type changes useEffect(() => { - // Timeout is needed because fitView doesn't work if it's + // A small delay is needed because fitView doesn't work if it's // called before the nodes are rendered - setTimeout(() => fitView(), 0); + setTimeout(() => fitView(), 150); }, [fitView, taxonomyType]); // Root node (the taxonomy type) diff --git a/clients/admin-ui/src/pages/taxonomy/index.tsx b/clients/admin-ui/src/pages/taxonomy/index.tsx index 7b8f7f9156..bbb88deb3e 100644 --- a/clients/admin-ui/src/pages/taxonomy/index.tsx +++ b/clients/admin-ui/src/pages/taxonomy/index.tsx @@ -21,7 +21,10 @@ import PageHeader from "~/features/common/PageHeader"; import { errorToastParams, successToastParams } from "~/features/common/toast"; import TaxonomyEditDrawer from "~/features/taxonomy/components/TaxonomyEditDrawer"; import TaxonomyInteractiveTree from "~/features/taxonomy/components/TaxonomyInteractiveTree"; -import { CoreTaxonomiesEnum } from "~/features/taxonomy/constants"; +import { + CoreTaxonomiesEnum, + TAXONOMY_ROOT_NODE_ID, +} from "~/features/taxonomy/constants"; import useTaxonomySlices from "~/features/taxonomy/hooks/useTaxonomySlices"; import { TaxonomyEntity } from "~/features/taxonomy/types"; @@ -67,9 +70,11 @@ const TaxonomyPage: NextPage = () => { return; } + const isChildOfRoot = draftNewItem?.parent_key === TAXONOMY_ROOT_NODE_ID; const newItem = { ...draftNewItem, name: labelName, + parent_key: isChildOfRoot ? null : draftNewItem.parent_key, }; const result = await createTrigger(newItem); @@ -79,7 +84,7 @@ const TaxonomyPage: NextPage = () => { } setLastCreatedItemKey(result.data.fides_key); toast(successToastParams("New label successfully created")); - setTimeout(() => setDraftNewItem(null)); + setDraftNewItem(null); }, [createTrigger, draftNewItem, toast], ); diff --git a/src/fides/api/alembic/migrations/versions/10c6b7709be3_adding_foreign_key_on_fides_key_for_.py b/src/fides/api/alembic/migrations/versions/10c6b7709be3_adding_foreign_key_on_fides_key_for_.py new file mode 100644 index 0000000000..3762552843 --- /dev/null +++ b/src/fides/api/alembic/migrations/versions/10c6b7709be3_adding_foreign_key_on_fides_key_for_.py @@ -0,0 +1,48 @@ +"""adding foreign key on fides key for taxonomy + +Revision ID: 10c6b7709be3 +Revises: b63ecb007556 +Create Date: 2024-12-17 14:54:02.325442 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "10c6b7709be3" +down_revision = "b63ecb007556" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_foreign_key( + "ctl_data_categories_parent_key_fkey", + "ctl_data_categories", + "ctl_data_categories", + ["parent_key"], + ["fides_key"], + ondelete="RESTRICT", + ) + op.create_foreign_key( + "ctl_data_uses_parent_key_fkey", + "ctl_data_uses", + "ctl_data_uses", + ["parent_key"], + ["fides_key"], + ondelete="RESTRICT", + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint( + "ctl_data_categories_parent_key_fkey", "ctl_data_categories", type_="foreignkey" + ) + op.drop_constraint( + "ctl_data_uses_parent_key_fkey", "ctl_data_uses", type_="foreignkey" + ) + # ### end Alembic commands ### diff --git a/src/fides/api/alembic/migrations/versions/ae65da77c468_add_data_migration_to_reactivate_.py b/src/fides/api/alembic/migrations/versions/ae65da77c468_add_data_migration_to_reactivate_.py new file mode 100644 index 0000000000..e571f11f75 --- /dev/null +++ b/src/fides/api/alembic/migrations/versions/ae65da77c468_add_data_migration_to_reactivate_.py @@ -0,0 +1,110 @@ +"""Add data migration to reactivate taxonomy nodes + +Revision ID: ae65da77c468 +Revises: 10c6b7709be3 +Create Date: 2024-12-18 12:27:47.239489 + +""" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import text +from sqlalchemy.engine import Connection +from sqlalchemy.sql.elements import TextClause + +# revision identifiers, used by Alembic. +revision = "ae65da77c468" +down_revision = "10c6b7709be3" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """ + This is a data migration to activate taxonomy nodes if it is de-active and has any children that are active + + E.g. + Taxonomy Tree: A----B----C + \ + ----D + Current Active Fields: A (false), B (false), C (true), D (false) + Upgrade Active Fields: A (true), B (true), C (true), D (false) + """ + bind: Connection = op.get_bind() + + reactivate_data_categories_query: TextClause = text( + """ + WITH RECURSIVE leaf_nodes AS ( + SELECT DISTINCT dc.fides_key + FROM ctl_data_categories dc + WHERE dc.fides_key NOT IN ( + SELECT DISTINCT parent_key + FROM ctl_data_categories + WHERE parent_key IS NOT NULL + ) + AND dc.active = true + ), + parent_hierarchy AS ( + SELECT dc.fides_key, dc.parent_key + FROM ctl_data_categories dc + INNER JOIN leaf_nodes ln ON dc.fides_key = ln.fides_key + + UNION + + SELECT dc.fides_key, dc.parent_key + FROM ctl_data_categories dc + INNER JOIN parent_hierarchy ph ON dc.fides_key = ph.parent_key + ) + UPDATE ctl_data_categories + SET active = true + WHERE fides_key IN ( + SELECT fides_key FROM parent_hierarchy + ) + AND active = false; + """ + ) + + reactivate_data_uses_query: TextClause = text( + """ + WITH RECURSIVE leaf_nodes AS ( + SELECT DISTINCT dc.fides_key + FROM ctl_data_uses dc + WHERE dc.fides_key NOT IN ( + SELECT DISTINCT parent_key + FROM ctl_data_uses + WHERE parent_key IS NOT NULL + ) + AND dc.active = true + ), + parent_hierarchy AS ( + SELECT dc.fides_key, dc.parent_key + FROM ctl_data_uses dc + INNER JOIN leaf_nodes ln ON dc.fides_key = ln.fides_key + + UNION + + SELECT dc.fides_key, dc.parent_key + FROM ctl_data_uses dc + INNER JOIN parent_hierarchy ph ON dc.fides_key = ph.parent_key + ) + UPDATE ctl_data_uses + SET active = true + WHERE fides_key IN ( + SELECT fides_key FROM parent_hierarchy + ) + AND active = false; + """ + ) + + # Update ctl_data_categories + bind.execute(reactivate_data_categories_query) + + # Update ctl_data_uses + bind.execute(reactivate_data_uses_query) + + +def downgrade() -> None: + """ + This migration does not support downgrades. + """ + pass diff --git a/src/fides/api/api/v1/endpoints/generic_overrides.py b/src/fides/api/api/v1/endpoints/generic_overrides.py index e52d20bd5d..20123ede9b 100644 --- a/src/fides/api/api/v1/endpoints/generic_overrides.py +++ b/src/fides/api/api/v1/endpoints/generic_overrides.py @@ -9,7 +9,7 @@ from sqlalchemy.orm import Session from sqlalchemy.sql.expression import select from starlette import status -from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY +from starlette.status import HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY from fides.api.api.deps import get_db from fides.api.common_exceptions import KeyOrNameAlreadyExists @@ -22,17 +22,20 @@ from fides.api.schemas.filter_params import FilterParams from fides.api.schemas.taxonomy_extensions import ( DataCategory, - DataCategoryCreate, + DataCategoryCreateOrUpdate, DataSubject, - DataSubjectCreate, + DataSubjectCreateOrUpdate, DataUse, - DataUseCreate, + DataUseCreateOrUpdate, ) +from fides.api.util.errors import FidesError, ForbiddenIsDefaultTaxonomyError from fides.api.util.filter_utils import apply_filters_to_query from fides.common.api.scope_registry import ( DATA_CATEGORY_CREATE, + DATA_CATEGORY_UPDATE, DATA_SUBJECT_CREATE, DATA_USE_CREATE, + DATA_USE_UPDATE, DATASET_READ, ) from fides.common.api.v1.urn_registry import V1_URL_PREFIX @@ -42,6 +45,7 @@ DataCategory as DataCategoryDbModel, DataSubject as DataSubjectDbModel, DataUse as DataUseDbModel, + ModelWithDefaultField, ) # We create routers to override specific methods in those defined in generic.py @@ -110,17 +114,60 @@ async def list_dataset_paginated( return await async_paginate(db, filtered_query, pagination_params) +def activate_taxonomy_parents( + resource: Union[DataCategoryDbModel, DataUseDbModel, DataSubjectDbModel], + db: Session, +) -> None: + """ + Activates parents to match newly-active taxonomy node. + """ + parent = resource.parent + if parent: + parent.active = True + db.commit() + + activate_taxonomy_parents(parent, db) + + +def deactivate_taxonomy_node_and_descendants( + resource: Union[DataCategoryDbModel, DataUseDbModel, DataSubjectDbModel], + db: Session, +) -> None: + """ + Recursively de-activates all descendants of a given taxonomy node. + """ + resource.active = False + db.commit() + children = resource.children + + for child in children: + # Deactivate current child + child.active = False + db.commit() + + # Recursively deactivate all descendants of this child + deactivate_taxonomy_node_and_descendants(child, db) + + def validate_and_create_taxonomy( db: Session, model: Union[ Type[DataCategoryDbModel], Type[DataUseDbModel], Type[DataSubjectDbModel] ], validation_schema: type, - data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], + data: Union[ + DataCategoryCreateOrUpdate, DataUseCreateOrUpdate, DataSubjectCreateOrUpdate + ], ) -> Dict: """ Validate and create a taxonomy element. """ + if not data.fides_key: + raise FidesError(f"Fides key is required to create a {model.__name__} resource") + if isinstance(model, ModelWithDefaultField) and data.is_default: + raise ForbiddenIsDefaultTaxonomyError( + model.__name__, data.fides_key, action="create" + ) validated_taxonomy = validation_schema(**data.model_dump(mode="json")) return model.create(db=db, data=validated_taxonomy.model_dump(mode="json")) @@ -129,25 +176,38 @@ def validate_and_update_taxonomy( db: Session, resource: Union[DataCategoryDbModel, DataUseDbModel, DataSubjectDbModel], validation_schema: type, - data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], + data: Union[ + DataCategoryCreateOrUpdate, DataUseCreateOrUpdate, DataSubjectCreateOrUpdate + ], ) -> Dict: """ Validate and update a taxonomy element. """ + # If active field is being updated, cascade change either up or down + if hasattr(data, "active"): + if data.active: + activate_taxonomy_parents(resource, db) + else: + # Cascade down - deactivate current node and children to match newly-deactivated taxonomy item + deactivate_taxonomy_node_and_descendants(resource, db) + validated_taxonomy = validation_schema(**data.model_dump(mode="json")) return resource.update(db=db, data=validated_taxonomy.model_dump(mode="json")) def create_or_update_taxonomy( db: Session, - data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], + data: Union[ + DataCategoryCreateOrUpdate, DataUseCreateOrUpdate, DataSubjectCreateOrUpdate + ], model: Union[ Type[DataCategoryDbModel], Type[DataUseDbModel], Type[DataSubjectDbModel] ], validation_schema: type, ) -> Dict: """ - Create or update a taxonomy element. If the element is disabled, it will be updated and re-enabled. + Create or update a taxonomy element. + If the element is deactivated, it will be updated and re-activated, along with its parents. """ if data.fides_key is None: disabled_resource_with_name = ( @@ -162,9 +222,11 @@ def create_or_update_taxonomy( {"key": data.fides_key, "name": data.name}, validation_schema.__name__ ) if data.parent_key if hasattr(data, "parent_key") else None: + # Updates fides_key if it is not the root level taxonomy node data.fides_key = f"{data.parent_key}.{data.fides_key}" # type: ignore[union-attr] if disabled_resource_with_name: data.active = True + activate_taxonomy_parents(disabled_resource_with_name, db) return validate_and_update_taxonomy( db, disabled_resource_with_name, validation_schema, data ) @@ -181,7 +243,7 @@ def create_or_update_taxonomy( name="Create", ) async def create_data_use( - data_use: DataUseCreate, + data_use: DataUseCreateOrUpdate, db: Session = Depends(get_db), ) -> Dict: """ @@ -194,10 +256,10 @@ async def create_data_use( status_code=HTTP_422_UNPROCESSABLE_ENTITY, detail=f"Data use with key {data_use.fides_key} or name {data_use.name} already exists.", ) - except Exception as e: + except Exception: raise HTTPException( status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Error creating data use: {e}", + detail="Error creating data use. Try a different name or key", ) @@ -209,7 +271,7 @@ async def create_data_use( name="Create", ) async def create_data_category( - data_category: DataCategoryCreate, + data_category: DataCategoryCreateOrUpdate, db: Session = Depends(get_db), ) -> Dict: """ @@ -225,10 +287,10 @@ async def create_data_category( status_code=HTTP_422_UNPROCESSABLE_ENTITY, detail=f"Data category with key {data_category.fides_key} or name {data_category.name} already exists.", ) - except Exception as e: + except Exception: raise HTTPException( status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Error creating data category: {e}", + detail="Error creating data category. Try a different name or key.", ) @@ -240,7 +302,7 @@ async def create_data_category( name="Create", ) async def create_data_subject( - data_subject: DataSubjectCreate, + data_subject: DataSubjectCreateOrUpdate, db: Session = Depends(get_db), ) -> Dict: """ @@ -256,10 +318,72 @@ async def create_data_subject( status_code=HTTP_422_UNPROCESSABLE_ENTITY, detail=f"Data subject with key {data_subject.fides_key} or name {data_subject.name} already exists.", ) - except Exception as e: + except Exception: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail="Error creating data subject. Try a different name or key.", + ) + + +@data_use_router.put( + "/data_use", + dependencies=[Security(verify_oauth_client, scopes=[DATA_USE_UPDATE])], + response_model=DataUse, + status_code=status.HTTP_200_OK, + name="Update", +) +async def update_data_use( + data_use: DataUseCreateOrUpdate, + db: Session = Depends(get_db), +) -> Dict: + """ + Update a data use. Ensures updates to "active" are appropriately cascaded. + """ + + resource = DataUseDbModel.get_by(db, field="fides_key", value=data_use.fides_key) + if not resource: + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Data use not found with key: {data_use.fides_key}", + ) + try: + return validate_and_update_taxonomy(db, resource, DataUse, data_use) + except Exception: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail="Error updating data use", + ) + + +@data_category_router.put( + "/data_category", + dependencies=[Security(verify_oauth_client, scopes=[DATA_CATEGORY_UPDATE])], + response_model=DataCategory, + status_code=status.HTTP_200_OK, + name="Update", +) +async def update_data_category( + data_category: DataCategoryCreateOrUpdate, + db: Session = Depends(get_db), +) -> Dict: + """ + Update a data category. Ensures updates to "active" are appropriately cascaded. + """ + + resource = DataCategoryDbModel.get_by( + db, field="fides_key", value=data_category.fides_key + ) + if not resource: + raise HTTPException( + status_code=HTTP_404_NOT_FOUND, + detail=f"Data category not found with key: {data_category.fides_key}", + ) + try: + return validate_and_update_taxonomy(db, resource, DataCategory, data_category) + except Exception: raise HTTPException( status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Error creating data subject: {e}", + detail="Error updating data category", ) diff --git a/src/fides/api/models/sql_models.py b/src/fides/api/models/sql_models.py index 2132ffe597..c89e90c526 100644 --- a/src/fides/api/models/sql_models.py +++ b/src/fides/api/models/sql_models.py @@ -36,7 +36,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession, async_object_session from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm import Session, relationship +from sqlalchemy.orm import RelationshipProperty, Session, relationship from sqlalchemy.sql import Select from sqlalchemy.sql.elements import Case from sqlalchemy.sql.sqltypes import DateTime @@ -171,8 +171,11 @@ class DataCategory(Base, FidesBase): """ __tablename__ = "ctl_data_categories" + fides_key = Column(String, primary_key=True, index=True, unique=True) - parent_key = Column(Text) + parent_key = Column( + Text, ForeignKey("ctl_data_categories.fides_key", ondelete="RESTRICT") + ) active = Column(BOOLEAN, default=True, nullable=False) # Default Fields @@ -181,6 +184,19 @@ class DataCategory(Base, FidesBase): version_deprecated = Column(Text) replaced_by = Column(Text) + children: "RelationshipProperty[List[DataCategory]]" = relationship( + "DataCategory", + back_populates="parent", + cascade="save-update, merge, refresh-expire", # intentionally do not cascade deletes + passive_deletes="all", + ) + + parent: "RelationshipProperty[Optional[DataCategory]]" = relationship( + "DataCategory", + back_populates="children", + remote_side=[fides_key], + ) + @classmethod def from_fideslang_obj( cls, data_category: FideslangDataCategory @@ -219,8 +235,11 @@ class DataUse(Base, FidesBase): """ __tablename__ = "ctl_data_uses" + fides_key = Column(String, primary_key=True, index=True, unique=True) - parent_key = Column(Text) + parent_key = Column( + Text, ForeignKey("ctl_data_uses.fides_key", ondelete="RESTRICT") + ) active = Column(BOOLEAN, default=True, nullable=False) # Default Fields @@ -229,6 +248,19 @@ class DataUse(Base, FidesBase): version_deprecated = Column(Text) replaced_by = Column(Text) + children: "RelationshipProperty[List[DataUse]]" = relationship( + "DataUse", + back_populates="parent", + cascade="save-update, merge, refresh-expire", # intentionally do not cascade deletes + passive_deletes="all", + ) + + parent: "RelationshipProperty[Optional[DataUse]]" = relationship( + "DataUse", + back_populates="children", + remote_side=[fides_key], + ) + @staticmethod def get_parent_uses_from_key(data_use_key: str) -> Set[str]: """ diff --git a/src/fides/api/schemas/taxonomy_extensions.py b/src/fides/api/schemas/taxonomy_extensions.py index cb9c956a40..0da281c025 100644 --- a/src/fides/api/schemas/taxonomy_extensions.py +++ b/src/fides/api/schemas/taxonomy_extensions.py @@ -29,23 +29,23 @@ class DataSubject(BaseDataSubject): active: bool = active_field -class TaxonomyCreateBase(DefaultModel, BaseModel): +class TaxonomyCreateOrUpdateBase(DefaultModel, BaseModel): name: Optional[str] = None - description: str - active: bool = True + description: Optional[str] + active: Optional[bool] = True fides_key: Optional[FidesKey] = None tags: Optional[List[str]] = None organization_fides_key: Optional[FidesKey] = "default_organization" -class DataUseCreate(TaxonomyCreateBase): +class DataUseCreateOrUpdate(TaxonomyCreateOrUpdateBase): parent_key: Optional[FidesKey] = None -class DataCategoryCreate(TaxonomyCreateBase): +class DataCategoryCreateOrUpdate(TaxonomyCreateOrUpdateBase): parent_key: Optional[FidesKey] = None -class DataSubjectCreate(TaxonomyCreateBase): +class DataSubjectCreateOrUpdate(TaxonomyCreateOrUpdateBase): rights: Optional[DataSubjectRights] = None automated_decisions_or_profiling: Optional[bool] = None diff --git a/tests/ctl/core/test_taxonomy_models.py b/tests/ctl/core/test_taxonomy_models.py new file mode 100644 index 0000000000..37418a3659 --- /dev/null +++ b/tests/ctl/core/test_taxonomy_models.py @@ -0,0 +1,199 @@ +from typing import Generator + +import pytest +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from fides.api.models.sql_models import DataUse + + +class TestHierarchicalTaxonomy: + + @pytest.fixture(scope="function") + def child_data_use_b(self, db: Session) -> Generator: + payload_b = { + "name": "Data Use B", + "fides_key": "data_use_a.data_use_b", + "parent_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use B", + } + data_use_b = DataUse.create(db=db, data=payload_b) + + yield data_use_b + + data_use_b.delete(db) + + @pytest.fixture(scope="function") + def child_data_use_c(self, db: Session) -> Generator: + payload_c = { + "name": "Data Use C", + "fides_key": "data_use_a.data_use_c", + "parent_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use C", + } + data_use_c = DataUse.create(db=db, data=payload_c) + + yield data_use_c + + data_use_c.delete(db) + + @pytest.fixture(scope="function") + def parent_data_use_a(self, db) -> Generator: + + payload_a = { + "name": "Data Use A", + "fides_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use A", + } + + data_use_a = DataUse.create(db=db, data=payload_a) + + yield data_use_a + + data_use_a.delete(db) + + def test_create_with_invalid_parent_key(self, db): + with pytest.raises(IntegrityError) as exc: + payload = { + "name": "Data Use Testing", + "fides_key": "data_use_testing", + "parent_key": "invalid", + "active": True, + "is_default": False, + "description": "Data Use Test", + } + + DataUse.create(db=db, data=payload) + assert "violates foreign key" in str(exc) + + def test_update_with_invalid_parent_key( + self, db, parent_data_use_a, child_data_use_b, child_data_use_c + ): + with pytest.raises(IntegrityError) as exc: + payload = { + "name": "Data Use update", + "fides_key": child_data_use_b.fides_key, + "parent_key": "invalid", + "description": "Data Use Update", + } + + child_data_use_b.update(db=db, data=payload) + assert "violates foreign key" in str(exc) + + def test_create_with_parent_key( + self, db, parent_data_use_a, child_data_use_b, child_data_use_c + ): + new_key = "new_data_use_with_b_parent" + payload = { + "name": "New data use b parent", + "fides_key": new_key, + "parent_key": child_data_use_b.fides_key, + "active": True, + "is_default": False, + "description": "Something", + } + + data_use = DataUse.create(db=db, data=payload) + assert data_use.fides_key == new_key + + # clean up + db.delete(data_use) + db.commit() + + def test_update_with_parent_key( + self, db, parent_data_use_a, child_data_use_b, child_data_use_c + ): + """ + Tree: A----B + \ + ----C + """ + payload = { + "name": "Data Use Test Update With Parent", + "fides_key": child_data_use_b.fides_key, + "parent_key": child_data_use_b.parent_key, + "active": True, + "is_default": False, + "description": "updating this", + } + + child_data_use_b.update(db=db, data=payload) + db.commit() + assert child_data_use_b.description == "updating this" + + def test_update_no_parent_key( + self, db, parent_data_use_a, child_data_use_b, child_data_use_c + ): + """ + Tree: A----B + \ + ----C + """ + payload = { + "name": "Data Use Test No Parent Key", + "fides_key": child_data_use_b.fides_key, + "active": True, + "is_default": False, + "description": "updating this", + } + + child_data_use_b.update(db=db, data=payload) + db.commit() + assert child_data_use_b.description == "updating this" + + @pytest.mark.skip( + reason="We never update the parent key from the FE, but we should evaluate what we want to do here" + ) + def test_update_new_parent_key( + self, db, parent_data_use_a, child_data_use_b, child_data_use_c + ): + pass + + def test_delete_child_data_use(self, db, parent_data_use_a, child_data_use_b): + """ + Tree: A----B + \ + ----C + """ + # Manually create data use c so that we can delete it safely outside the fixture + payload_c = { + "name": "Data Use C", + "fides_key": "data_use_a.data_use_c", + "parent_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use C", + } + child_data_use_c = DataUse.create(db=db, data=payload_c) + assert len(parent_data_use_a.children) == 2 + assert child_data_use_b.parent.fides_key == parent_data_use_a.fides_key + assert child_data_use_c.parent.fides_key == parent_data_use_a.fides_key + + child_data_use_c.delete(db) + + # verify data use A is removed from the parent + db.refresh(parent_data_use_a) + assert len(parent_data_use_a.children) == 1 + assert parent_data_use_a.children[0].fides_key == child_data_use_b.fides_key + + def test_cannot_delete_parent_notice_with_children( + self, db, parent_data_use_a, child_data_use_c + ): + """ + Tree: A----C + """ + assert len(parent_data_use_a.children) == 1 + assert child_data_use_c.parent.fides_key == parent_data_use_a.fides_key + + with pytest.raises(IntegrityError) as exc: + parent_data_use_a.delete(db) + assert "violates foreign key" in str(exc) + + assert DataUse.get_by(db, field="fides_key", value=parent_data_use_a.fides_key) + assert DataUse.get_by(db, field="fides_key", value=child_data_use_c.fides_key) diff --git a/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py b/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py index 07c9fb0ac5..2fff36660c 100644 --- a/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py +++ b/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py @@ -6,7 +6,12 @@ from fides.api.models.client import ClientDetail from fides.api.models.sql_models import DataUse -from fides.common.api.scope_registry import DATA_USE, DATA_USE_CREATE, STORAGE_READ +from fides.common.api.scope_registry import ( + DATA_USE, + DATA_USE_CREATE, + DATA_USE_UPDATE, + STORAGE_READ, +) from fides.common.api.v1.urn_registry import V1_URL_PREFIX @@ -24,7 +29,7 @@ def payload(self): } @pytest.fixture(scope="function") - def disabled_data_use(self, db: Session): + def deactive_data_use(self, db: Session): payload = { "name": "test data use", "fides_key": "test_data_use", @@ -37,7 +42,7 @@ def disabled_data_use(self, db: Session): dataUse.delete(db) @pytest.fixture(scope="function") - def enabled_data_use(self, db: Session): + def active_data_use(self, db: Session): payload = { "name": "test data use", "fides_key": "test_data_use", @@ -147,7 +152,7 @@ def test_create_data_use_with_conflicting_key( api_client: TestClient, payload, url, - enabled_data_use, + active_data_use, generate_auth_header, ): auth_header = generate_auth_header([DATA_USE_CREATE]) @@ -161,7 +166,7 @@ def test_create_data_use_with_disabled_matching_name( api_client: TestClient, payload, url, - disabled_data_use, + deactive_data_use, generate_auth_header, ): auth_header = generate_auth_header([DATA_USE_CREATE]) @@ -174,3 +179,251 @@ def test_create_data_use_with_disabled_matching_name( assert response_body["description"] == "this is a test data use" data_use = db.query(DataUse).filter_by(fides_key="test_data_use")[0] data_use.delete(db) + + +class TestUpdateDataUse: + @pytest.fixture(scope="function") + def url(self, oauth_client: ClientDetail) -> str: + return V1_URL_PREFIX + "/" + DATA_USE + + @pytest.fixture(scope="function") + def payload(self): + return { + "fides_key": "test_data_use", + "name": "test data use", + "description": "this is a test data use", + "is_default": False, + } + + @pytest.fixture(scope="function") + def data_use(self, db: Session): + payload = { + "name": "test data use", + "fides_key": "test_data_use", + "active": False, + "is_default": False, + "description": "De-active data use", + } + dataUse = DataUse.create(db=db, data=payload) + yield dataUse + dataUse.delete(db) + + def test_update_data_use_not_authenticated( + self, + api_client: TestClient, + data_use, + payload, + url, + ): + response = api_client.put(url, headers={}, json=payload) + assert 401 == response.status_code + + def test_update_data_use_incorrect_scope( + self, + api_client: TestClient, + payload, + data_use, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([STORAGE_READ]) + response = api_client.put(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_update_data_use_not_found( + self, + db: Session, + api_client: TestClient, + payload, + data_use, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([DATA_USE_UPDATE]) + payload["fides_key"] = "does_not_exist" + response = api_client.put(url, headers=auth_header, json=payload) + + assert 404 == response.status_code + + def test_update_data_use_name_and_description( + self, + api_client: TestClient, + payload, + data_use, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([DATA_USE_UPDATE]) + payload["name"] = "New name" + payload["description"] = "New description" + response = api_client.put(url, headers=auth_header, json=payload) + + assert 200 == response.status_code + response_body = json.loads(response.text) + + assert response_body["name"] == "New name" + assert response_body["description"] == "New description" + + def test_update_data_use_activate_propagates_up( + self, + db: Session, + api_client: TestClient, + url, + generate_auth_header, + ): + """ + Tree: A----B----C + \ + ----D + Current Active Fields: A (false), B (false), C (false), D (false) + Payload: C: active=True + Result Active Fields: A (true), B (true), C (true), D (false) + """ + # Set up Current Taxonomy Tree + payload_a = { + "name": "Data Use A", + "fides_key": "data_use_a", + "active": False, + "is_default": False, + "description": "Data Use A", + } + payload_b = { + "name": "Data Use B", + "fides_key": "data_use_a.data_use_b", + "parent_key": "data_use_a", + "active": False, + "is_default": False, + "description": "Data Use B", + } + payload_c = { + "name": "Data Use C", + "fides_key": "data_use_a.data_use_b.data_use_c", + "parent_key": "data_use_a.data_use_b", + "active": False, + "is_default": False, + "description": "Data Use C", + } + payload_d = { + "name": "Data Use D", + "fides_key": "data_use_a.data_use_b.data_use_d", + "parent_key": "data_use_a.data_use_b", + "active": False, + "is_default": False, + "description": "Data Use D", + } + data_use_a = DataUse.create(db=db, data=payload_a) + data_use_b = DataUse.create(db=db, data=payload_b) + data_use_c = DataUse.create(db=db, data=payload_c) + data_use_d = DataUse.create(db=db, data=payload_d) + + # Run Update + auth_header = generate_auth_header([DATA_USE_UPDATE]) + payload = { + "name": "Data Use C", + "fides_key": "data_use_a.data_use_b.data_use_c", + "parent_key": "data_use_a.data_use_b", + "active": True, + "description": "Data Use C", + } + response = api_client.put(url, headers=auth_header, json=payload) + assert 200 == response.status_code + + # Assert + a_result = DataUse.get_by(db, field="fides_key", value=data_use_a.fides_key) + assert a_result.active is True + b_result = DataUse.get_by(db, field="fides_key", value=data_use_b.fides_key) + assert b_result.active is True + c_result = DataUse.get_by(db, field="fides_key", value=data_use_c.fides_key) + assert c_result.active is True + d_result = DataUse.get_by(db, field="fides_key", value=data_use_d.fides_key) + assert d_result.active is False + + # Clean up + d_result.delete(db) + c_result.delete(db) + b_result.delete(db) + a_result.delete(db) + + def test_update_data_use_deactivate_propagates_down( + self, + db: Session, + api_client: TestClient, + url, + generate_auth_header, + ): + """ + Tree: A----B----C + \ + ----D + Current Active Fields: A (true), B (true), C (true), D (true) + Payload: B: active=False + Result Active Fields: A (true), B (false), C (false), D (false) + """ + # Set up Current Taxonomy Tree + payload_a = { + "name": "Data Use A", + "fides_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use A", + } + payload_b = { + "name": "Data Use B", + "fides_key": "data_use_a.data_use_b", + "parent_key": "data_use_a", + "active": True, + "is_default": False, + "description": "Data Use B", + } + payload_c = { + "name": "Data Use C", + "fides_key": "data_use_a.data_use_b.data_use_c", + "parent_key": "data_use_a.data_use_b", + "active": True, + "is_default": False, + "description": "Data Use C", + } + payload_d = { + "name": "Data Use D", + "fides_key": "data_use_a.data_use_b.data_use_d", + "parent_key": "data_use_a.data_use_b", + "active": True, + "is_default": False, + "description": "Data Use D", + } + data_use_a = DataUse.create(db=db, data=payload_a) + data_use_b = DataUse.create(db=db, data=payload_b) + data_use_c = DataUse.create(db=db, data=payload_c) + data_use_d = DataUse.create(db=db, data=payload_d) + + # Run Update + auth_header = generate_auth_header([DATA_USE_UPDATE]) + payload = { + "name": "Data Use B", + "fides_key": "data_use_a.data_use_b", + "parent_key": "data_use_a", + "active": False, + "description": "Data Use B", + } + response = api_client.put(url, headers=auth_header, json=payload) + assert 200 == response.status_code + db.refresh(data_use_a) + db.refresh(data_use_b) + db.refresh(data_use_c) + db.refresh(data_use_d) + + # Assert + a_result = DataUse.get_by(db, field="fides_key", value=data_use_a.fides_key) + assert a_result.active is True + b_result = DataUse.get_by(db, field="fides_key", value=data_use_b.fides_key) + assert b_result.active is False + c_result = DataUse.get_by(db, field="fides_key", value=data_use_c.fides_key) + assert c_result.active is False + d_result = DataUse.get_by(db, field="fides_key", value=data_use_d.fides_key) + assert d_result.active is False + + # Clean up + d_result.delete(db) + c_result.delete(db) + b_result.delete(db) + a_result.delete(db)