From 8927dd0d8b6fce9bc8fec046ff008c5f6b9eb731 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 14:28:05 +0000 Subject: [PATCH 01/12] migration --- ...emove_un_used_columns_from_conditional_.py | 242 ++++++++ .../conditional_dependency_base.py | 185 +----- .../models/digest/conditional_dependencies.py | 116 +--- .../manual_task/conditional_dependency.py | 38 +- tests/api/models/digest/conftest.py | 170 +---- .../digest/test_conditional_dependencies.py | 457 ++++---------- ...st_manual_task_conditional_dependencies.py | 581 ++---------------- .../test_conditional_dependency_base.py | 569 +---------------- tests/api/task/manual/conftest.py | 115 +--- 9 files changed, 528 insertions(+), 1945 deletions(-) create mode 100644 src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py diff --git a/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py b/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py new file mode 100644 index 0000000000..5ea9ba4489 --- /dev/null +++ b/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py @@ -0,0 +1,242 @@ +"""remove unused columns from conditional dependencies + +Revision ID: 9cf7bb472a7c +Revises: 85ce2c1c9579 +Create Date: 2025-12-30 14:21:46.184655 + +""" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "9cf7bb472a7c" +down_revision = "85ce2c1c9579" +branch_labels = None +depends_on = None + + +def upgrade(): + # === digest_condition table === + # Drop indexes on columns being removed + op.drop_index("ix_digest_condition_condition_type", table_name="digest_condition") + op.drop_index("ix_digest_condition_parent_id", table_name="digest_condition") + op.drop_index("ix_digest_condition_sort_order", table_name="digest_condition") + + # Replace partial unique index with a full unique constraint + op.drop_index("ix_digest_condition_unique_root_per_type", table_name="digest_condition") + op.create_unique_constraint( + "uq_digest_condition_config_type", + "digest_condition", + ["digest_config_id", "digest_condition_type"], + ) + + # Drop parent_id foreign key constraint and column + op.drop_constraint( + "digest_condition_parent_id_fkey", "digest_condition", type_="foreignkey" + ) + op.drop_column("digest_condition", "parent_id") + + # Drop remaining unused columns + op.drop_column("digest_condition", "sort_order") + op.drop_column("digest_condition", "condition_type") + op.drop_column("digest_condition", "operator") + op.drop_column("digest_condition", "logical_operator") + op.drop_column("digest_condition", "value") + op.drop_column("digest_condition", "field_address") + + # === manual_task_conditional_dependency table === + # Drop indexes on columns being removed + op.drop_index( + "ix_manual_task_conditional_dependency_condition_type", + table_name="manual_task_conditional_dependency", + ) + op.drop_index( + "ix_manual_task_conditional_dependency_parent_id", + table_name="manual_task_conditional_dependency", + ) + op.drop_index( + "ix_manual_task_conditional_dependency_sort_order", + table_name="manual_task_conditional_dependency", + ) + + # Change manual_task_id index to unique and add unique constraint + op.drop_index( + "ix_manual_task_conditional_dependency_manual_task_id", + table_name="manual_task_conditional_dependency", + ) + op.create_index( + "ix_manual_task_conditional_dependency_manual_task_id", + "manual_task_conditional_dependency", + ["manual_task_id"], + unique=True, + ) + op.create_unique_constraint( + "uq_manual_task_conditional_dependency", + "manual_task_conditional_dependency", + ["manual_task_id"], + ) + + # Drop parent_id foreign key constraint and column + op.drop_constraint( + "manual_task_conditional_dependency_parent_id_fkey", + "manual_task_conditional_dependency", + type_="foreignkey", + ) + op.drop_column("manual_task_conditional_dependency", "parent_id") + + # Drop remaining unused columns + op.drop_column("manual_task_conditional_dependency", "sort_order") + op.drop_column("manual_task_conditional_dependency", "condition_type") + op.drop_column("manual_task_conditional_dependency", "operator") + op.drop_column("manual_task_conditional_dependency", "logical_operator") + op.drop_column("manual_task_conditional_dependency", "value") + op.drop_column("manual_task_conditional_dependency", "field_address") + + +def downgrade(): + # === manual_task_conditional_dependency table === + # Re-add columns + op.add_column( + "manual_task_conditional_dependency", + sa.Column("field_address", sa.VARCHAR(), nullable=True), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column( + "value", postgresql.JSONB(astext_type=sa.Text()), nullable=True + ), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column("logical_operator", sa.VARCHAR(), nullable=True), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column("operator", sa.VARCHAR(), nullable=True), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column("condition_type", sa.VARCHAR(), nullable=False), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column("sort_order", sa.INTEGER(), nullable=False), + ) + op.add_column( + "manual_task_conditional_dependency", + sa.Column("parent_id", sa.VARCHAR(), nullable=True), + ) + + # Re-add parent_id foreign key + op.create_foreign_key( + "manual_task_conditional_dependency_parent_id_fkey", + "manual_task_conditional_dependency", + "manual_task_conditional_dependency", + ["parent_id"], + ["id"], + ondelete="CASCADE", + ) + + # Revert unique constraint and index changes + op.drop_constraint( + "uq_manual_task_conditional_dependency", + "manual_task_conditional_dependency", + type_="unique", + ) + op.drop_index( + "ix_manual_task_conditional_dependency_manual_task_id", + table_name="manual_task_conditional_dependency", + ) + op.create_index( + "ix_manual_task_conditional_dependency_manual_task_id", + "manual_task_conditional_dependency", + ["manual_task_id"], + unique=False, + ) + + # Re-add indexes + op.create_index( + "ix_manual_task_conditional_dependency_sort_order", + "manual_task_conditional_dependency", + ["sort_order"], + ) + op.create_index( + "ix_manual_task_conditional_dependency_parent_id", + "manual_task_conditional_dependency", + ["parent_id"], + ) + op.create_index( + "ix_manual_task_conditional_dependency_condition_type", + "manual_task_conditional_dependency", + ["condition_type"], + ) + + # === digest_condition table === + # Re-add columns + op.add_column( + "digest_condition", + sa.Column("field_address", sa.VARCHAR(length=255), nullable=True), + ) + op.add_column( + "digest_condition", + sa.Column( + "value", postgresql.JSONB(astext_type=sa.Text()), nullable=True + ), + ) + op.add_column( + "digest_condition", + sa.Column("logical_operator", sa.VARCHAR(), nullable=True), + ) + op.add_column( + "digest_condition", + sa.Column("operator", sa.VARCHAR(), nullable=True), + ) + op.add_column( + "digest_condition", + sa.Column("condition_type", sa.VARCHAR(), nullable=False), + ) + op.add_column( + "digest_condition", + sa.Column("sort_order", sa.INTEGER(), nullable=False), + ) + op.add_column( + "digest_condition", + sa.Column("parent_id", sa.VARCHAR(length=255), nullable=True), + ) + + # Re-add parent_id foreign key + op.create_foreign_key( + "digest_condition_parent_id_fkey", + "digest_condition", + "digest_condition", + ["parent_id"], + ["id"], + ondelete="CASCADE", + ) + + # Revert unique constraint to partial index + op.drop_constraint( + "uq_digest_condition_config_type", "digest_condition", type_="unique" + ) + op.create_index( + "ix_digest_condition_unique_root_per_type", + "digest_condition", + ["digest_config_id", "digest_condition_type"], + unique=True, + postgresql_where=sa.text("parent_id IS NULL"), + ) + + # Re-add indexes + op.create_index( + "ix_digest_condition_sort_order", "digest_condition", ["sort_order"] + ) + op.create_index( + "ix_digest_condition_parent_id", "digest_condition", ["parent_id"] + ) + op.create_index( + "ix_digest_condition_condition_type", + "digest_condition", + ["condition_type"], + ) diff --git a/src/fides/api/models/conditional_dependency/conditional_dependency_base.py b/src/fides/api/models/conditional_dependency/conditional_dependency_base.py index 6234eda3e2..eb579b9b3c 100644 --- a/src/fides/api/models/conditional_dependency/conditional_dependency_base.py +++ b/src/fides/api/models/conditional_dependency/conditional_dependency_base.py @@ -1,22 +1,12 @@ -from enum import Enum -from typing import TYPE_CHECKING, Any, Optional, Union +from typing import Any, Optional from pydantic import TypeAdapter -from sqlalchemy import Column, Integer, String +from sqlalchemy import Column from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Session from fides.api.db.base_class import Base -from fides.api.db.util import EnumColumn -from fides.api.task.conditional_dependencies.schemas import ( - Condition, - ConditionGroup, - ConditionLeaf, -) - -if TYPE_CHECKING: - from sqlalchemy.orm.relationships import RelationshipProperty - +from fides.api.task.conditional_dependencies.schemas import Condition # TypeAdapter for deserializing JSONB to Condition (handles Union discrimination) ConditionTypeAdapter: TypeAdapter[Condition] = TypeAdapter(Condition) @@ -30,63 +20,49 @@ def __init__(self, message: str): super().__init__(self.message) -class ConditionalDependencyType(str, Enum): - """Shared enum for conditional dependency node types. - - Attributes: - leaf: Individual condition (field_address + operator + value) - group: Collection of conditions with logical operator (AND/OR) - """ - - leaf = "leaf" - group = "group" - - class ConditionalDependencyBase(Base): """Abstract base class for all conditional dependency models. - This class provides a common structure for building hierarchical condition trees - or storing condition trees as a single JSONB object. + This class provides a common structure for storing condition trees as a single JSONB object that can be evaluated to determine when certain actions should be taken. Architecture: - JSONB Storage: Full condition tree stored as a single JSONB object - Pydantic Integration: Uses Condition schema for serialization/deserialization - - Tree Structure: Supports parent-child relationships for complex logic - - Two Node Types: 'leaf' (individual conditions) and 'group' (logical operators) - - Flexible Schema: Uses JSONB for dynamic value storage - - Ordered Evaluation: sort_order ensures predictable condition processing + - Single Row Per Entity: Each parent entity has one row containing its full condition tree Concrete Implementations: - - ManualTaskConditionalDependency: Single-type hierarchy for manual tasks - - Single-type hierarchy means one condition tree per manual task, this condition - may be a nested group of conditions or a single leaf condition. + - ManualTaskConditionalDependency: Single condition tree per manual task - DigestCondition: Multi-type hierarchy with digest_condition_type separation - - Multi-type hierarchy means one digest_config can have multiple independent - condition trees, each with a different digest_condition_type (RECEIVER, CONTENT, PRIORITY) - - Within each tree, all nodes must have the same digest_condition_type - - This enables separate condition logic for different aspects of digest processing + - One row per (digest_config_id, digest_condition_type) combination Usage Pattern: 1. Inherit from this base class 2. Define your table name with @declared_attr - 3. Add foreign key relationships (parent_id, entity_id) + 3. Add foreign key to parent entity 4. Implement get_root_condition() classmethod 5. Add any domain-specific columns - Example Tree Structure: - Root Group (AND) - ├── Leaf: user.role == "admin" - ├── Leaf: request.priority >= 3 - └── Child Group (OR) - ├── Leaf: user.department == "security" - └── Leaf: user.department == "compliance" + Example Tree Structure (stored in condition_tree JSONB): + { + "logical_operator": "and", + "conditions": [ + {"field_address": "user.role", "operator": "eq", "value": "admin"}, + {"field_address": "request.priority", "operator": "gte", "value": 3}, + { + "logical_operator": "or", + "conditions": [ + {"field_address": "user.department", "operator": "eq", "value": "security"}, + {"field_address": "user.department", "operator": "eq", "value": "compliance"} + ] + } + ] + } Note: - This is a SQLAlchemy abstract model (__abstract__ = True) - No database table is created for this base class - Subclasses must implement get_root_condition() - - The 'children' relationship must be defined in concrete subclasses """ __abstract__ = True @@ -96,123 +72,12 @@ class ConditionalDependencyBase(Base): # Format matches Condition schema: either ConditionLeaf or ConditionGroup structure. condition_tree = Column(JSONB, nullable=True) - # Tree structure - parent_id defined in concrete classes for proper foreign keys - condition_type = Column( - EnumColumn(ConditionalDependencyType), nullable=False, index=True - ) - - # Condition details (for leaf nodes) - field_address = Column(String(255), nullable=True) # For leaf conditions - operator = Column(String, nullable=True) # For leaf conditions - value = Column(JSONB, nullable=True) # For leaf conditions - logical_operator = Column(String, nullable=True) # 'and' or 'or' for groups - - # Ordering - sort_order = Column(Integer, nullable=False, default=0, index=True) - - def to_correct_condition_type(self) -> Union[ConditionLeaf, ConditionGroup]: - """Convert this database model to the correct condition type.""" - if self.condition_type == ConditionalDependencyType.leaf: - return self.to_condition_leaf() - return self.to_condition_group() - - def to_condition_leaf(self) -> ConditionLeaf: - """Convert this database model to a ConditionLeaf schema object. - - This method transforms a leaf-type conditional dependency from its database - representation into a structured ConditionLeaf object that can be used for - evaluation and serialization. - - Returns: - ConditionLeaf: Schema object containing field_address, operator, and value - - Raises: - ValueError: If this condition is not a leaf type (i.e., it's a group) - - Example: - >>> condition = SomeConcreteConditionalDependency( - ... condition_type="leaf", - ... field_address="user.role", - ... operator="eq", - ... value="admin" - ... ) - >>> leaf = condition.to_condition_leaf() - >>> print(leaf.field_address) # "user.role" - """ - if self.condition_type != ConditionalDependencyType.leaf: - raise ValueError( - f"Cannot convert {self.condition_type} condition to leaf. " - f"Only conditions with condition_type='leaf' can be converted to ConditionLeaf. " - f"This condition has type '{self.condition_type}' and should be converted using to_condition_group()." - ) - - return ConditionLeaf( - field_address=self.field_address, operator=self.operator, value=self.value - ) - - def to_condition_group(self) -> ConditionGroup: - """Convert this database model to a ConditionGroup schema object. - - This method transforms a group-type conditional dependency from its database - representation into a structured ConditionGroup object. It recursively processes - all child conditions, maintaining the tree structure and sort order. - - Returns: - ConditionGroup: Schema object containing logical_operator and child conditions - - Raises: - ValueError: If this condition is not a group type (i.e., it's a leaf) - AttributeError: If the 'children' relationship is not properly defined - - Example: - >>> # Assume we have a group with two leaf children - >>> group_condition = SomeConcreteConditionalDependency( - ... condition_type="group", - ... logical_operator="and" - ... ) - >>> condition_group = group_condition.to_condition_group() - >>> print(condition_group.logical_operator) # "and" - >>> print(len(condition_group.conditions)) # 2 - """ - if self.condition_type != ConditionalDependencyType.group: - raise ValueError( - f"Cannot convert {self.condition_type} condition to group. " - f"Only conditions with condition_type='group' can be converted to ConditionGroup. " - f"This condition has type '{self.condition_type}' and should be converted using to_condition_leaf()." - ) - - # Recursively build children - note: 'children' must be defined in concrete classes - try: - children_list = [child for child in self.children] # type: ignore[attr-defined] - except AttributeError: - raise AttributeError( - f"The 'children' relationship is not defined on {self.__class__.__name__}. " - f"Concrete subclasses must define a 'children' relationship for group conditions to work properly." - ) - - child_conditions = [] - for child in sorted(children_list, key=lambda x: x.sort_order): - if child.condition_type == ConditionalDependencyType.leaf: - child_conditions.append(child.to_condition_leaf()) - elif child.condition_type == ConditionalDependencyType.group: - child_conditions.append(child.to_condition_group()) - else: - raise ValueError( - f"Unknown condition_type '{child.condition_type}' found in child condition. " - f"Expected '{ConditionalDependencyType.leaf}' or '{ConditionalDependencyType.group}'." - ) - - return ConditionGroup( - logical_operator=self.logical_operator, conditions=child_conditions - ) - @classmethod def get_root_condition(cls, db: Session, **kwargs: Any) -> Optional[Condition]: """Get the condition tree for a parent entity. This abstract method must be implemented by concrete subclasses to define how to retrieve the condition tree for their specific use case. - The root condition represents the top-level node in a condition tree. Args: db: SQLAlchemy database session for querying @@ -232,7 +97,7 @@ def get_root_condition(cls, db: Session, **kwargs: Any) -> Optional[Condition]: """ raise NotImplementedError( f"Subclasses of {cls.__name__} must implement get_root_condition(). " - f"This method should query for the root condition (parent_id=None) " - f"and return it as a Condition schema object, or None if not found. " + f"This method should query for the condition row " + f"and return condition_tree as a Condition schema object, or None if not found. " f"See the docstring for implementation guidelines and examples." ) diff --git a/src/fides/api/models/digest/conditional_dependencies.py b/src/fides/api/models/digest/conditional_dependencies.py index fff57d9287..fdceac9e23 100644 --- a/src/fides/api/models/digest/conditional_dependencies.py +++ b/src/fides/api/models/digest/conditional_dependencies.py @@ -1,7 +1,7 @@ from enum import Enum from typing import TYPE_CHECKING, Any, Optional, Union -from sqlalchemy import Column, ForeignKey, Index, String, text +from sqlalchemy import Column, ForeignKey, String, UniqueConstraint from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.orm import Session, relationship @@ -43,14 +43,12 @@ class DigestConditionType(str, Enum): class DigestCondition(ConditionalDependencyBase): - """Digest conditional dependencies - multi-type hierarchies, stores the condition tree - as JSONB. + """Digest conditional dependencies - stores condition tree as JSONB. Each digest_config can have up to three independent condition trees, one per digest_condition_type (RECEIVER, CONTENT, PRIORITY). This enables separate condition logic for different aspects of digest processing. - Example Tree Structure: DigestConfig (e.g., "Weekly Privacy Digest") ├── RECEIVER condition_tree (who gets the digest) @@ -65,8 +63,6 @@ class DigestCondition(ConditionalDependencyBase): def __tablename__(cls) -> str: return "digest_condition" - # We need to redefine it here so that self-referential relationships - # can properly reference the `id` column instead of the built-in Python function. id = Column(String(255), primary_key=True, default=FidesBase.generate_uuid) # Foreign key to parent digest config @@ -76,12 +72,6 @@ def __tablename__(cls) -> str: nullable=False, index=True, ) - parent_id = Column( - String(255), - ForeignKey("digest_condition.id", ondelete="CASCADE"), - nullable=True, - index=True, - ) # Condition category - determines which aspect of digest this condition controls digest_condition_type = Column( @@ -90,72 +80,16 @@ def __tablename__(cls) -> str: # Relationship to parent config digest_config = relationship("DigestConfig", back_populates="conditions") - parent = relationship( - "DigestCondition", - remote_side=[id], - back_populates="children", - foreign_keys=[parent_id], - ) - children = relationship( - "DigestCondition", - back_populates="parent", - cascade="all, delete-orphan", - foreign_keys=[parent_id], - ) - # Ensure only one root condition per digest_condition_type per digest_config + # Ensure only one condition per (digest_config_id, digest_condition_type) combination __table_args__ = ( - # TODO update to this in next migration to use UniqueConstraint - # UniqueConstraint( - # "digest_config_id", - # "digest_condition_type", - # name="uq_digest_condition_config_type", - # ), - Index( - "ix_digest_condition_unique_root_per_type", + UniqueConstraint( "digest_config_id", "digest_condition_type", - unique=True, - postgresql_where=text("parent_id IS NULL"), + name="uq_digest_condition_config_type", ), ) - @staticmethod - def _validate_condition_type_consistency(db: Session, data: dict[str, Any]) -> None: - """Validate that a condition's digest_condition_type matches its parent's type. - - Since each parent was validated when created, checking against the immediate parent - is sufficient to ensure tree-wide consistency. - - Args: - db: Database session for querying - data: Dictionary containing condition data to validate (must include both parent_id and digest_condition_type) - - Raises: - ValueError: If parent doesn't exist or digest_condition_type doesn't match parent's type - """ - parent_id = data.get("parent_id") - digest_condition_type = data.get("digest_condition_type") - - if not parent_id: - # Root condition - no validation needed - return - - # Get the parent condition - parent = ( - db.query(DigestCondition).filter(DigestCondition.id == parent_id).first() - ) - if not parent: - raise ValueError(f"Parent condition with id '{parent_id}' does not exist") - - # Validate that the new condition matches the parent's digest_condition_type - if parent.digest_condition_type != digest_condition_type: - raise ValueError( - f"Cannot create condition with type '{digest_condition_type}' under parent " - f"with type '{parent.digest_condition_type}'. All conditions in the same tree " - f"must have the same digest_condition_type." - ) - @classmethod def create( cls, @@ -164,52 +98,22 @@ def create( data: dict[str, Any], check_name: bool = True, ) -> "DigestCondition": - """Create a new DigestCondition with validation.""" - # Validate condition type consistency - cls._validate_condition_type_consistency(db, data) + """Create a new DigestCondition.""" try: return super().create(db=db, data=data, check_name=check_name) except Exception as e: raise ConditionalDependencyError(str(e)) - def update(self, db: Session, *, data: dict[str, Any]) -> "DigestCondition": - """Update DigestCondition with validation.""" - # Ensure validation data includes current values for fields not being updated - validation_data = { - "parent_id": data.get("parent_id", self.parent_id), - "digest_condition_type": data.get( - "digest_condition_type", self.digest_condition_type - ), - } - - # Validate before updating - self._validate_condition_type_consistency(db, validation_data) - return super().update(db=db, data=data) # type: ignore[return-value] - - def save(self, db: Session) -> "DigestCondition": - """Save DigestCondition with validation.""" - # Extract current object data for validation - data = { - "parent_id": self.parent_id, - "digest_condition_type": self.digest_condition_type, - } - - # Validate before saving (only if this has a parent) - if self.parent_id: - self._validate_condition_type_consistency(db, data) - return super().save(db=db) # type: ignore[return-value] - @classmethod def get_root_condition( cls, db: Session, **kwargs: Any, ) -> Optional[Union[ConditionLeaf, ConditionGroup]]: - """Get the root condition tree for a specific digest condition type. + """Get the condition tree for a specific digest condition type. - Implementation of the abstract base method for DigestCondition's multi-type hierarchy. Each digest_config can have separate condition trees for RECEIVER, CONTENT, and PRIORITY - types. This method retrieves the root of one specific tree. + types. This method retrieves the condition tree for a specific type. Args: db: SQLAlchemy database session for querying @@ -219,7 +123,7 @@ def get_root_condition( Must be one of: RECEIVER, CONTENT, PRIORITY Returns: - Optional[Union[ConditionLeaf, ConditionGroup]]: Condition tree for the specified + Optional[Union[ConditionLeaf, ConditionGroup]]: Condition tree for the specified type Raises: ValueError: If required parameters are missing @@ -244,13 +148,11 @@ def get_root_condition( "digest_config_id and digest_condition_type are required keyword arguments" ) - # Filter for root condition (parent_id IS NULL) since child rows don't have condition_tree condition_row = ( db.query(cls) .filter( cls.digest_config_id == digest_config_id, cls.digest_condition_type == digest_condition_type, - cls.parent_id.is_(None), ) .one_or_none() ) diff --git a/src/fides/api/models/manual_task/conditional_dependency.py b/src/fides/api/models/manual_task/conditional_dependency.py index 3f3c8f4644..c88a641290 100644 --- a/src/fides/api/models/manual_task/conditional_dependency.py +++ b/src/fides/api/models/manual_task/conditional_dependency.py @@ -1,6 +1,6 @@ from typing import TYPE_CHECKING, Any, Optional, Union -from sqlalchemy import Column, ForeignKey, String +from sqlalchemy import Column, ForeignKey, String, UniqueConstraint from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.orm import Session, relationship @@ -29,38 +29,24 @@ class ManualTaskConditionalDependency(ConditionalDependencyBase): def __tablename__(cls) -> str: return "manual_task_conditional_dependency" - # We need to redefine it here so that self-referential relationships - # can properly reference the `id` column instead of the built-in Python function. id = Column(String(255), primary_key=True, default=FidesBase.generate_uuid) - # Foreign key to parent manual task + + # Foreign key to parent manual task - one condition tree per task manual_task_id = Column( String, ForeignKey("manual_task.id", ondelete="CASCADE"), nullable=False, + unique=True, index=True, ) - parent_id = Column( - String, - ForeignKey("manual_task_conditional_dependency.id", ondelete="CASCADE"), - nullable=True, - index=True, - # TODO update to this in next migration to use UniqueConstraint - # unique=True, - ) # Relationship to parent manual task task = relationship("ManualTask", back_populates="conditional_dependencies") - parent = relationship( - "ManualTaskConditionalDependency", - remote_side=[id], - back_populates="children", - foreign_keys=[parent_id], - ) - children = relationship( - "ManualTaskConditionalDependency", - back_populates="parent", - cascade="all, delete-orphan", - foreign_keys=[parent_id], + + __table_args__ = ( + UniqueConstraint( + "manual_task_id", name="uq_manual_task_conditional_dependency" + ), ) @classmethod @@ -85,11 +71,9 @@ def get_root_condition( if not manual_task_id: raise ValueError("manual_task_id is required as a keyword argument") - # Filter for root condition (parent_id IS NULL) since child rows don't have condition_tree + condition_row = ( - db.query(cls) - .filter(cls.manual_task_id == manual_task_id, cls.parent_id.is_(None)) - .first() + db.query(cls).filter(cls.manual_task_id == manual_task_id).first() ) if not condition_row or condition_row.condition_tree is None: diff --git a/tests/api/models/digest/conftest.py b/tests/api/models/digest/conftest.py index 7655516579..02b88aed70 100644 --- a/tests/api/models/digest/conftest.py +++ b/tests/api/models/digest/conftest.py @@ -5,9 +5,6 @@ import pytest from sqlalchemy.orm import Session -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.digest import DigestConfig, DigestType from fides.api.models.digest.conditional_dependencies import ( DigestCondition, @@ -51,44 +48,6 @@ def priority_condition(digest_config: DigestConfig) -> dict[str, Any]: } -@pytest.fixture -def group_condition_or() -> dict[str, Any]: - """Basis for a group condition with OR logical operator.""" - return { - "condition_type": ConditionalDependencyType.group, - "logical_operator": GroupOperator.or_, - } - - -@pytest.fixture -def group_condition_and() -> dict[str, Any]: - """Basis for a group condition with AND logical operator.""" - return { - "condition_type": ConditionalDependencyType.group, - "logical_operator": GroupOperator.and_, - } - - -@pytest.fixture -def receiver_group( - db: Session, - group_condition_and: dict[str, Any], - digest_config: DigestConfig, -): - """Create a receiver group condition using separate digest config.""" - group = DigestCondition.create( - db=db, - data={ - **group_condition_and, - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - "sort_order": 1, - }, - ) - yield group - group.delete(db) - - # ============================================================================ # Base Digest Config Fixtures # ============================================================================ @@ -172,38 +131,29 @@ def sample_conditions( ): """Create sample conditions for all types.""" - # Receiver condition + # Receiver condition with condition_tree receiver_cond = DigestCondition.create( db=db, data={ **receiver_condition, - **receiver_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": receiver_condition_leaf.model_dump(), }, ) - # Content condition - with condition_tree for JSONB storage + # Content condition with condition_tree content_cond = DigestCondition.create( db=db, data={ **content_condition, - **content_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": content_condition_leaf.model_dump(), }, ) - # Priority condition - with condition_tree for JSONB storage + # Priority condition with condition_tree priority_cond = DigestCondition.create( db=db, data={ **priority_condition, - **priority_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": priority_condition_leaf.model_dump(), }, ) @@ -216,15 +166,12 @@ def receiver_digest_condition_leaf( digest_config: DigestConfig, receiver_condition_leaf: ConditionLeaf, ) -> DigestCondition: - """Create a receiver condition in the database using separate digest config.""" + """Create a receiver condition in the database.""" condition = DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, - **receiver_condition_leaf.model_dump(), - "sort_order": 1, "condition_tree": receiver_condition_leaf.model_dump(), }, ) @@ -238,15 +185,12 @@ def content_digest_condition_leaf( digest_config: DigestConfig, content_condition_leaf: ConditionLeaf, ) -> DigestCondition: - """Create a content condition in the database using separate digest config.""" + """Create a content condition in the database.""" condition = DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, "digest_condition_type": DigestConditionType.CONTENT, - **content_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": content_condition_leaf.model_dump(), }, ) @@ -260,15 +204,12 @@ def priority_digest_condition_leaf( digest_config: DigestConfig, priority_condition_leaf: ConditionLeaf, ) -> DigestCondition: - """Create a priority condition in the database using separate digest config.""" + """Create a priority condition in the database.""" condition = DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, "digest_condition_type": DigestConditionType.PRIORITY, - **priority_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": priority_condition_leaf.model_dump(), }, ) @@ -280,40 +221,38 @@ def priority_digest_condition_leaf( def complex_condition_tree( db: Session, content_condition: dict[str, Any], - group_condition_or: dict[str, Any], - group_condition_and: dict[str, Any], ): """Create a complex condition tree for testing.""" # Build full condition_tree for JSONB storage: (A AND B) OR (C AND D) condition_tree = { - "logical_operator": "or", + "logical_operator": GroupOperator.or_, "conditions": [ { - "logical_operator": "and", + "logical_operator": GroupOperator.and_, "conditions": [ { "field_address": "task.assignee", - "operator": "eq", + "operator": Operator.eq, "value": "user123", }, { "field_address": "task.due_date", - "operator": "lte", + "operator": Operator.lte, "value": "2024-01-01", }, ], }, { - "logical_operator": "and", + "logical_operator": GroupOperator.and_, "conditions": [ { "field_address": "task.category", - "operator": "eq", + "operator": Operator.eq, "value": "urgent", }, { "field_address": "task.created_at", - "operator": "gte", + "operator": Operator.gte, "value": "2024-01-01T00:00:00Z", }, ], @@ -321,96 +260,17 @@ def complex_condition_tree( ], } - # Create root group: (A AND B) OR (C AND D) + # Create root condition with full tree root_group = DigestCondition.create( db=db, data={ **content_condition, - **group_condition_or, - "sort_order": 1, "condition_tree": condition_tree, }, ) - # Create first nested group: (A AND B) - for backward compatibility - nested_group1 = DigestCondition.create( - db=db, - data={ - **content_condition, - **group_condition_and, - "parent_id": root_group.id, - "sort_order": 1, - }, - ) - - # Create second nested group: (C AND D) - for backward compatibility - nested_group2 = DigestCondition.create( - db=db, - data={ - **content_condition, - **group_condition_and, - "parent_id": root_group.id, - "sort_order": 2, - }, - ) - - # Create leaf conditions for first group - for backward compatibility - leaf_a = DigestCondition.create( - db=db, - data={ - **content_condition, - "parent_id": nested_group1.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "task.assignee", - "operator": Operator.eq, - "value": "user123", - "sort_order": 1, - }, - ) - - leaf_b = DigestCondition.create( - db=db, - data={ - **content_condition, - "parent_id": nested_group1.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "task.due_date", - "operator": Operator.lte, - "value": "2024-01-01", - "sort_order": 2, - }, - ) - - # Create leaf conditions for second group - for backward compatibility - leaf_c = DigestCondition.create( - db=db, - data={ - **content_condition, - "parent_id": nested_group2.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "task.category", - "operator": Operator.eq, - "value": "urgent", - "sort_order": 1, - }, - ) - - leaf_d = DigestCondition.create( - db=db, - data={ - **content_condition, - "parent_id": nested_group2.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "task.created_at", - "operator": Operator.gte, - "value": "2024-01-01T00:00:00Z", - "sort_order": 2, - }, - ) - yield { "root": root_group, - "nested_groups": [nested_group1, nested_group2], - "leaves": [leaf_a, leaf_b, leaf_c, leaf_d], + "condition_tree": condition_tree, } root_group.delete(db) diff --git a/tests/api/models/digest/test_conditional_dependencies.py b/tests/api/models/digest/test_conditional_dependencies.py index 6a0c7f9bb2..27b2a65f72 100644 --- a/tests/api/models/digest/test_conditional_dependencies.py +++ b/tests/api/models/digest/test_conditional_dependencies.py @@ -3,12 +3,10 @@ from typing import Any import pytest -from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from fides.api.models.conditional_dependency.conditional_dependency_base import ( ConditionalDependencyError, - ConditionalDependencyType, ) from fides.api.models.digest import DigestConfig, DigestType from fides.api.models.digest.conditional_dependencies import ( @@ -16,31 +14,12 @@ DigestConditionType, ) from fides.api.task.conditional_dependencies.schemas import ( - Condition, ConditionGroup, ConditionLeaf, GroupOperator, Operator, ) - -def assert_group_condition( - condition: DigestCondition, - logical_operator: GroupOperator, - expected_children: list[DigestCondition], -): - """Assert properties of a group condition.""" - assert condition.condition_type == ConditionalDependencyType.group - assert condition.logical_operator == logical_operator - assert condition.field_address is None - assert condition.operator is None - assert condition.value is None - # Compare by id since objects may be different instances - actual_child_ids = {c.id for c in condition.children} - expected_child_ids = {c.id for c in expected_children} - assert actual_child_ids == expected_child_ids - - # ============================================================================ # DigestConditionType Tests # ============================================================================ @@ -86,27 +65,21 @@ def test_create_leaf_condition_types( digest_condition_type: DigestConditionType, sample_eq_condition_leaf: ConditionLeaf, ): - """Test creating a receiver leaf condition.""" + """Test creating a leaf condition for different types.""" + condition_tree = sample_eq_condition_leaf.model_dump() condition = DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, "digest_condition_type": digest_condition_type, - **sample_eq_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, + "condition_tree": condition_tree, }, ) assert condition.id is not None assert condition.digest_config_id == digest_config.id assert condition.digest_condition_type == digest_condition_type - assert condition.condition_type == ConditionalDependencyType.leaf - assert condition.field_address == sample_eq_condition_leaf.field_address - assert condition.operator == sample_eq_condition_leaf.operator - assert condition.value == sample_eq_condition_leaf.value - assert condition.sort_order == 1 - assert condition.parent_id is None + assert condition.condition_tree == condition_tree # Test relationship assert condition.digest_config == digest_config @@ -120,29 +93,33 @@ def test_create_leaf_condition_types( DigestConditionType.PRIORITY, ], ) - @pytest.mark.parametrize("group_condition", [GroupOperator.or_, GroupOperator.and_]) + @pytest.mark.parametrize( + "logical_operator", [GroupOperator.or_, GroupOperator.and_] + ) def test_create_group_condition_types( self, db: Session, digest_config: DigestConfig, digest_condition_type: DigestConditionType, - group_condition: GroupOperator, + logical_operator: GroupOperator, + sample_eq_condition_leaf: ConditionLeaf, ): - """Test creating a content group condition.""" + """Test creating a group condition for different types.""" + condition_tree = { + "logical_operator": logical_operator, + "conditions": [sample_eq_condition_leaf.model_dump()], + } condition = DigestCondition.create( db=db, data={ - "condition_type": ConditionalDependencyType.group, - "logical_operator": group_condition, "digest_config_id": digest_config.id, "digest_condition_type": digest_condition_type, - "sort_order": 1, + "condition_tree": condition_tree, }, ) assert condition.digest_condition_type == digest_condition_type - assert condition.condition_type == ConditionalDependencyType.group - assert_group_condition(condition, group_condition, []) + assert condition.condition_tree["logical_operator"] == logical_operator condition.delete(db) def test_required_fields_validation(self, db: Session, digest_config: DigestConfig): @@ -153,7 +130,11 @@ def test_required_fields_validation(self, db: Session, digest_config: DigestConf db=db, data={ "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, + "condition_tree": { + "field_address": "test", + "operator": "eq", + "value": 1, + }, }, ) @@ -163,17 +144,11 @@ def test_required_fields_validation(self, db: Session, digest_config: DigestConf db=db, data={ "digest_config_id": digest_config.id, - "condition_type": ConditionalDependencyType.leaf, - }, - ) - - # Missing condition_type - with pytest.raises(ConditionalDependencyError): - DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, + "condition_tree": { + "field_address": "test", + "operator": "eq", + "value": 1, + }, }, ) @@ -189,9 +164,7 @@ def test_cascade_delete_with_digest_config( db=db, data={ **receiver_condition, - **sample_eq_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, + "condition_tree": sample_eq_condition_leaf.model_dump(), }, ) @@ -204,6 +177,36 @@ def test_cascade_delete_with_digest_config( ) assert deleted_condition is None + def test_unique_constraint_per_config_and_type( + self, + db: Session, + digest_config: DigestConfig, + sample_eq_condition_leaf: ConditionLeaf, + ): + """Test that only one condition per (digest_config_id, digest_condition_type) is allowed.""" + condition_tree = sample_eq_condition_leaf.model_dump() + + # Create first condition + DigestCondition.create( + db=db, + data={ + "digest_config_id": digest_config.id, + "digest_condition_type": DigestConditionType.RECEIVER, + "condition_tree": condition_tree, + }, + ) + + # Try to create second condition with same config and type + with pytest.raises(Exception): # Should raise unique constraint violation + DigestCondition.create( + db=db, + data={ + "digest_config_id": digest_config.id, + "digest_condition_type": DigestConditionType.RECEIVER, + "condition_tree": condition_tree, + }, + ) + # ============================================================================ # Condition Tree Tests @@ -213,152 +216,65 @@ def test_cascade_delete_with_digest_config( class TestDigestConditionTrees: """Test building and managing condition trees.""" - def test_create_group_condition_tree_with_children( + def test_create_nested_condition_tree( self, db: Session, content_condition: dict[str, Any], - group_condition_and: dict[str, Any], content_condition_leaf: ConditionLeaf, priority_condition_leaf: ConditionLeaf, ): - """Test creating a group condition tree with child conditions.""" - # Create root group condition - root_group = DigestCondition.create( - db=db, data={**content_condition, **group_condition_and, "sort_order": 1} - ) + """Test creating a nested group condition tree.""" + condition_tree = { + "logical_operator": GroupOperator.and_, + "conditions": [ + content_condition_leaf.model_dump(), + priority_condition_leaf.model_dump(), + ], + } - # Create child leaf conditions - child1 = DigestCondition.create( + root_group = DigestCondition.create( db=db, data={ **content_condition, - **content_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "parent_id": root_group.id, - "sort_order": 1, + "condition_tree": condition_tree, }, ) - child2 = DigestCondition.create( - db=db, - data={ - **content_condition, - **priority_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "parent_id": root_group.id, - "sort_order": 2, - }, + # Verify tree structure + assert root_group.condition_tree["logical_operator"] == GroupOperator.and_ + assert len(root_group.condition_tree["conditions"]) == 2 + assert ( + root_group.condition_tree["conditions"][0]["field_address"] == "task.status" + ) + assert ( + root_group.condition_tree["conditions"][1]["field_address"] + == "task.priority" ) - # Refresh to get children - db.refresh(root_group) - - # Test conversion to condition group - condition_group = root_group.to_condition_group() - assert isinstance(condition_group, ConditionGroup) - assert_group_condition(root_group, GroupOperator.and_, [child1, child2]) - - # Check children are properly ordered - assert isinstance(condition_group.conditions[0], ConditionLeaf) - assert condition_group.conditions[0].field_address == "task.status" - assert isinstance(condition_group.conditions[1], ConditionLeaf) - assert condition_group.conditions[1].field_address == "task.priority" - - # Test parent-child relationships - assert child1.parent == root_group - assert child2.parent == root_group - assert len(root_group.children) == 2 root_group.delete(db) - def test_nested_group_condition_tree( - self, - complex_condition_tree: dict[str, Any], - ): - """Test creating nested group condition trees.""" + def test_complex_nested_tree(self, complex_condition_tree: dict[str, Any]): + """Test creating complex nested group condition trees.""" root_group = complex_condition_tree["root"] - nested_groups = complex_condition_tree["nested_groups"] - leaves = complex_condition_tree["leaves"] - - # Test conversion to nested condition group - assert isinstance(root_group.to_condition_group(), ConditionGroup) - assert_group_condition(root_group, GroupOperator.or_, nested_groups) - assert len(root_group.children) == 2 - - # Test first nested group (A AND B) - first_nested = root_group.children[0] - assert isinstance(first_nested.to_condition_group(), ConditionGroup) - assert_group_condition(first_nested, GroupOperator.and_, leaves[0:2]) - assert len(first_nested.children) == 2 - assert first_nested.children[0].field_address == "task.assignee" - assert first_nested.children[1].field_address == "task.due_date" - - # Test second nested group (C AND D) - second_nested = root_group.children[1] - assert isinstance(second_nested.to_condition_group(), ConditionGroup) - assert_group_condition(second_nested, GroupOperator.and_, leaves[2:4]) - assert len(second_nested.children) == 2 - assert second_nested.children[0].field_address == leaves[2].field_address - assert second_nested.children[1].field_address == leaves[3].field_address - - def test_cascade_delete_with_parent_condition( - self, - db: Session, - receiver_condition: dict[str, Any], - sample_exists_condition_leaf: ConditionLeaf, - ): - """Test that child conditions are deleted when parent is deleted.""" - # Create parent group - parent_group = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "condition_type": ConditionalDependencyType.group, - "logical_operator": GroupOperator.and_, - "sort_order": 1, - }, - ) - - # Create child conditions - child1 = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **sample_exists_condition_leaf.model_dump(), - "parent_id": parent_group.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) + tree = complex_condition_tree["condition_tree"] - child2 = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "parent_id": parent_group.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "user.active", - "operator": Operator.eq, - "value": True, - "sort_order": 2, - }, - ) + # Verify root structure + assert root_group.condition_tree["logical_operator"] == GroupOperator.or_ + assert len(root_group.condition_tree["conditions"]) == 2 - child1_id = child1.id - child2_id = child2.id - - # Delete parent - parent_group.delete(db) - - # Verify children are also deleted - deleted_child1 = ( - db.query(DigestCondition).filter(DigestCondition.id == child1_id).first() - ) - deleted_child2 = ( - db.query(DigestCondition).filter(DigestCondition.id == child2_id).first() - ) + # Verify first nested group + first_group = root_group.condition_tree["conditions"][0] + assert first_group["logical_operator"] == GroupOperator.and_ + assert len(first_group["conditions"]) == 2 + assert first_group["conditions"][0]["field_address"] == "task.assignee" + assert first_group["conditions"][1]["field_address"] == "task.due_date" - assert deleted_child1 is None - assert deleted_child2 is None + # Verify second nested group + second_group = root_group.condition_tree["conditions"][1] + assert second_group["logical_operator"] == GroupOperator.and_ + assert len(second_group["conditions"]) == 2 + assert second_group["conditions"][0]["field_address"] == "task.category" + assert second_group["conditions"][1]["field_address"] == "task.created_at" # ============================================================================ @@ -372,8 +288,6 @@ class TestDigestConditionQueries: @pytest.mark.usefixtures("sample_conditions") def test_get_root_condition_by_type(self, db: Session, digest_config: DigestConfig): """Test getting root condition by digest condition type.""" - # sample_conditions fixture creates conditions, so we can test retrieval - # Test getting receiver condition receiver_condition = DigestCondition.get_root_condition( db, @@ -383,7 +297,7 @@ def test_get_root_condition_by_type(self, db: Session, digest_config: DigestConf assert receiver_condition is not None assert isinstance(receiver_condition, ConditionLeaf) assert receiver_condition.field_address == "user.email" - assert receiver_condition.value == None + assert receiver_condition.value is None # Test getting content condition content_condition = DigestCondition.get_root_condition( @@ -426,7 +340,6 @@ def test_get_root_condition_nonexistent(self, db: Session): @pytest.mark.usefixtures("sample_conditions") def test_get_all_root_conditions(self, db: Session, digest_config: DigestConfig): """Test getting all root conditions for a digest config.""" - # sample_conditions fixture creates conditions, so we can test retrieval all_conditions = DigestCondition.get_all_root_conditions(db, digest_config.id) assert len(all_conditions) == 3 @@ -488,59 +401,43 @@ def test_filter_conditions_by_type(self, db: Session, digest_config: DigestConfi content_conditions[0].digest_condition_type == DigestConditionType.CONTENT ) - def test_filter_root_conditions_only( + def test_get_root_condition_returns_group( self, db: Session, digest_config: DigestConfig, - receiver_condition: dict[str, Any], - group_condition_and: dict[str, Any], - sample_exists_condition_leaf: ConditionLeaf, + sample_eq_condition_leaf: ConditionLeaf, ): - """Test filtering for root conditions only (parent_id is None).""" - # Create root condition - root_condition = DigestCondition.create( - db=db, data={**receiver_condition, **group_condition_and, "sort_order": 1} - ) + """Test that get_root_condition returns ConditionGroup for group trees.""" + condition_tree = { + "logical_operator": GroupOperator.and_, + "conditions": [ + sample_eq_condition_leaf.model_dump(), + { + "field_address": "user.active", + "operator": Operator.eq, + "value": True, + }, + ], + } - # Create child condition - child_condition = DigestCondition.create( + DigestCondition.create( db=db, data={ - **receiver_condition, - **sample_exists_condition_leaf.model_dump(), - "parent_id": root_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, + "digest_config_id": digest_config.id, + "digest_condition_type": DigestConditionType.RECEIVER, + "condition_tree": condition_tree, }, ) - # Query for root conditions only - root_conditions = ( - db.query(DigestCondition) - .filter( - DigestCondition.digest_config_id == digest_config.id, - DigestCondition.parent_id.is_(None), - ) - .all() - ) - - assert len(root_conditions) == 1 - assert root_conditions[0].id == root_condition.id - assert root_conditions[0].parent_id is None - - # Query for child conditions - child_conditions = ( - db.query(DigestCondition) - .filter( - DigestCondition.digest_config_id == digest_config.id, - DigestCondition.parent_id.isnot(None), - ) - .all() + result = DigestCondition.get_root_condition( + db, + digest_config_id=digest_config.id, + digest_condition_type=DigestConditionType.RECEIVER, ) - assert len(child_conditions) == 1 - assert child_conditions[0].id == child_condition.id - assert child_conditions[0].parent_id == root_condition.id + assert isinstance(result, ConditionGroup) + assert result.logical_operator == GroupOperator.and_ + assert len(result.conditions) == 2 # ============================================================================ @@ -558,6 +455,7 @@ def test_digest_config_relationship_loading( receiver_condition: dict[str, Any], content_condition: dict[str, Any], sample_exists_condition_leaf: ConditionLeaf, + content_condition_leaf: ConditionLeaf, ): """Test that digest config properly loads condition relationships.""" # Create multiple conditions @@ -565,9 +463,7 @@ def test_digest_config_relationship_loading( db=db, data={ **receiver_condition, - **sample_exists_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, + "condition_tree": sample_exists_condition_leaf.model_dump(), }, ) @@ -575,11 +471,7 @@ def test_digest_config_relationship_loading( db=db, data={ **content_condition, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "task.status", - "operator": Operator.eq, - "value": "pending", - "sort_order": 1, + "condition_tree": content_condition_leaf.model_dump(), }, ) @@ -606,54 +498,6 @@ def test_digest_config_relationship_loading( class TestDigestConditionValidation: """Test validation and error cases for digest conditions.""" - def test_invalid_condition_conversion( - self, - db: Session, - receiver_condition: dict[str, Any], - group_condition_and: dict[str, Any], - sample_eq_condition_leaf: ConditionLeaf, - ): - """Test error handling for invalid condition conversions.""" - # Create group condition - group_condition = DigestCondition.create( - db=db, data={**receiver_condition, **group_condition_and, "sort_order": 0} - ) - - # Test converting group to leaf fails - with pytest.raises(ValueError, match="Cannot convert group condition to leaf"): - group_condition.to_condition_leaf() - - # Create leaf condition as a child of the group condition - leaf_condition = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **sample_eq_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "parent_id": group_condition.id, - "sort_order": 1, - }, - ) - - # Test converting leaf to group fails - with pytest.raises(ValueError, match="Cannot convert leaf condition to group"): - leaf_condition.to_condition_group() - - def test_group_condition_with_empty_children( - self, - db: Session, - receiver_condition: dict[str, Any], - group_condition_or: dict[str, Any], - ): - """Test that group condition with no children raises validation error.""" - group_condition = DigestCondition.create( - db=db, data={**receiver_condition, **group_condition_or, "sort_order": 1} - ) - - # Test that converting empty group to condition group fails - with pytest.raises(ValueError, match="conditions list cannot be empty"): - group_condition.to_condition_group() - def test_invalid_digest_config_reference( self, db: Session, sample_exists_condition_leaf: ConditionLeaf ): @@ -662,57 +506,8 @@ def test_invalid_digest_config_reference( DigestCondition.create( db=db, data={ - **sample_exists_condition_leaf.model_dump(), + "digest_config_id": "nonexistent_config_id", "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, + "condition_tree": sample_exists_condition_leaf.model_dump(), }, ) - - def test_invalid_parent_reference( - self, - db: Session, - receiver_condition: dict[str, Any], - sample_exists_condition_leaf: ConditionLeaf, - ): - """Test creating condition with invalid parent reference.""" - with pytest.raises( - ValueError, - match="Parent condition with id 'nonexistent_parent_id' does not exist", - ): - DigestCondition.create( - db=db, - data={ - **receiver_condition, - **sample_exists_condition_leaf.model_dump(), - "parent_id": "nonexistent_parent_id", - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - def test_mixed_condition_types_in_tree( - self, - db: Session, - receiver_condition: dict[str, Any], - group_condition_and: dict[str, Any], - sample_eq_condition_leaf: ConditionLeaf, - ): - """Test that conditions in the same tree must have same digest_condition_type.""" - # Create parent with RECEIVER type - parent_condition = DigestCondition.create( - db=db, data={**receiver_condition, **group_condition_and, "sort_order": 1} - ) - child_condition1 = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **sample_eq_condition_leaf.model_dump(), - "parent_id": parent_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - assert child_condition1.digest_condition_type == DigestConditionType.RECEIVER - assert child_condition1.parent_id == parent_condition.id diff --git a/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py b/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py index 01773f25f5..4313e022a0 100644 --- a/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py +++ b/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py @@ -1,9 +1,6 @@ import pytest from sqlalchemy.orm import Session -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.manual_task import ManualTask, ManualTaskConditionalDependency from fides.api.task.conditional_dependencies.schemas import ( ConditionGroup, @@ -40,73 +37,44 @@ def sample_condition_group() -> ConditionGroup: class TestManualTaskConditionalDependencyCRUD: """Test basic CRUD operations for ManualTaskConditionalDependency""" - def test_manual_task_conditional_dependency_creation( + def test_manual_task_conditional_dependency_creation_with_leaf_tree( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): - """Test creating a basic conditional dependency""" + """Test creating a conditional dependency with a leaf condition tree""" + condition_tree = sample_condition_leaf.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, + "condition_tree": condition_tree, }, ) assert dependency.id is not None assert dependency.manual_task_id == manual_task.id - assert dependency.condition_type == ConditionalDependencyType.leaf - assert dependency.field_address == sample_condition_leaf.field_address - assert dependency.operator == sample_condition_leaf.operator - assert dependency.value == sample_condition_leaf.value - assert dependency.sort_order == 1 - assert dependency.parent_id is None + assert dependency.condition_tree == condition_tree assert dependency.created_at is not None assert dependency.updated_at is not None - def test_manual_task_conditional_dependency_with_group_condition( - self, - db: Session, - manual_task: ManualTask, - sample_condition_group: ConditionGroup, - ): - """Test creating a conditional dependency with a group condition""" - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, - }, - ) - - assert dependency.condition_type == ConditionalDependencyType.group - assert dependency.logical_operator == "and" - - def test_manual_task_conditional_dependency_serialization( + def test_manual_task_conditional_dependency_with_group_tree( self, db: Session, manual_task: ManualTask, sample_condition_group: ConditionGroup, ): - """Test that condition data can be serialized and deserialized correctly""" + """Test creating a conditional dependency with a group condition tree""" + condition_tree = sample_condition_group.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, + "condition_tree": condition_tree, }, ) - # Verify the logical operator is stored correctly - assert dependency.logical_operator == "and" - assert dependency.condition_type == ConditionalDependencyType.group + assert dependency.condition_tree == condition_tree + assert dependency.condition_tree["logical_operator"] == GroupOperator.and_ + assert len(dependency.condition_tree["conditions"]) == 2 class TestManualTaskConditionalDependencyRelationships: @@ -116,15 +84,12 @@ def test_manual_task_conditional_dependency_relationship( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): """Test the relationship between ManualTask and ManualTaskConditionalDependency""" + condition_tree = sample_condition_leaf.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, + "condition_tree": condition_tree, }, ) @@ -140,6 +105,7 @@ def test_manual_task_conditional_dependency_foreign_key_constraint( self, db: Session, sample_condition_leaf: ConditionLeaf ): """Test that foreign key constraints are enforced""" + condition_tree = sample_condition_leaf.model_dump() # Try to create a dependency with non-existent manual_task_id with pytest.raises( Exception @@ -148,155 +114,34 @@ def test_manual_task_conditional_dependency_foreign_key_constraint( db=db, data={ "manual_task_id": "non_existent_id", - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, + "condition_tree": condition_tree, }, ) - def test_manual_task_conditional_dependency_parent_foreign_key_constraint( + def test_manual_task_unique_constraint( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): - """Test that parent foreign key constraints are enforced""" - # Create a dependency - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, - }, - ) + """Test that only one conditional dependency is allowed per manual task""" + condition_tree = sample_condition_leaf.model_dump() - # Try to create a child dependency with non-existent parent_id - with pytest.raises( - Exception - ): # Should raise an exception for invalid parent foreign key - ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 2, - "parent_id": "non_existent_parent_id", - }, - ) - - -class TestManualTaskConditionalDependencyHierarchy: - """Test hierarchical relationships and complex structures""" - - def test_manual_task_conditional_dependency_hierarchy( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test hierarchical relationships between conditional dependencies""" - # Create parent dependency - parent_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, - }, - ) - - # Create child dependency - child_dependency = ManualTaskConditionalDependency.create( + # Create first dependency + ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 2, - "parent_id": parent_dependency.id, - }, - ) - - # Test parent-child relationship - assert child_dependency.parent_id == parent_dependency.id - assert child_dependency.parent.id == parent_dependency.id - - # Test children relationship - db.refresh(parent_dependency) - assert len(parent_dependency.children) == 1 - assert parent_dependency.children[0].id == child_dependency.id - - def test_manual_task_conditional_dependency_complex_hierarchy( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test complex hierarchical structure with multiple levels""" - # Create root dependency - root_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, + "condition_tree": condition_tree, }, ) - # Create level 1 children - level1_children = [] - for i in range(2): - child = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": i + 2, - "parent_id": root_dependency.id, - }, - ) - level1_children.append(child) - - # Create level 2 children (grandchildren) - level2_children = [] - for i, parent in enumerate(level1_children): - grandchild = ManualTaskConditionalDependency.create( + # Try to create second dependency for the same task + with pytest.raises(Exception): # Should raise unique constraint violation + ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": i + 4, - "parent_id": parent.id, + "condition_tree": condition_tree, }, ) - level2_children.append(grandchild) - - # Verify hierarchy structure - db.refresh(root_dependency) - assert len(root_dependency.children) == 2 - - for child in level1_children: - db.refresh(child) - assert child.parent_id == root_dependency.id - assert len(child.children) == 1 - - for grandchild in level2_children: - db.refresh(grandchild) - assert grandchild.parent_id in [child.id for child in level1_children] class TestManualTaskConditionalDependencyCascadeDeletes: @@ -306,15 +151,12 @@ def test_manual_task_conditional_dependency_cascade_delete( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): """Test that conditional dependencies are deleted when the manual task is deleted""" + condition_tree = sample_condition_leaf.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, + "condition_tree": condition_tree, }, ) @@ -338,236 +180,6 @@ def test_manual_task_conditional_dependency_cascade_delete( is None ) - def test_manual_task_conditional_dependency_hierarchy_cascade_delete( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test that child dependencies are deleted when parent is deleted""" - # Create parent dependency - parent_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, - }, - ) - - # Create child dependency - child_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 2, - "parent_id": parent_dependency.id, - }, - ) - - # Verify both dependencies exist - assert ( - db.query(ManualTaskConditionalDependency) - .filter_by(id=parent_dependency.id) - .first() - is not None - ) - assert ( - db.query(ManualTaskConditionalDependency) - .filter_by(id=child_dependency.id) - .first() - is not None - ) - - # Delete the parent dependency - db.delete(parent_dependency) - db.commit() - - # Verify both dependencies are deleted - assert ( - db.query(ManualTaskConditionalDependency) - .filter_by(id=parent_dependency.id) - .first() - is None - ) - assert ( - db.query(ManualTaskConditionalDependency) - .filter_by(id=child_dependency.id) - .first() - is None - ) - - -class TestManualTaskConditionalDependencyValidation: - """Test validation and ordering behavior""" - - def test_manual_task_conditional_dependency_sort_order( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test that sort_order is respected when creating multiple dependencies""" - # Create multiple dependencies with different sort orders - dependencies = [] - for i in range(3): - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": i + 1, - }, - ) - dependencies.append(dependency) - - # Refresh the manual task to get the dependencies - db.refresh(manual_task) - - # Verify dependencies are ordered correctly - sorted_dependencies = sorted( - manual_task.conditional_dependencies, key=lambda x: x.sort_order - ) - assert len(sorted_dependencies) == 3 - assert sorted_dependencies[0].sort_order == 1 - assert sorted_dependencies[1].sort_order == 2 - assert sorted_dependencies[2].sort_order == 3 - - def test_manual_task_conditional_dependency_validation( - self, db: Session, manual_task: ManualTask - ): - """Test validation of conditional dependency data""" - # Test with invalid condition type - with pytest.raises( - Exception - ): # Should raise an exception for invalid condition type - ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": "invalid_type", - "field_address": "test_field", - "operator": "eq", - "value": "test_value", - "sort_order": 1, - }, - ) - - -class TestManualTaskConditionalDependencyConversion: - """Test conversion methods for condition types""" - - def test_to_condition_leaf( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test converting a leaf dependency to a ConditionLeaf object""" - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, - }, - ) - - # Convert to ConditionLeaf - condition_leaf = dependency.to_condition_leaf() - - # Verify the conversion - assert isinstance(condition_leaf, ConditionLeaf) - assert condition_leaf.field_address == sample_condition_leaf.field_address - assert condition_leaf.operator == sample_condition_leaf.operator - assert condition_leaf.value == sample_condition_leaf.value - - def test_to_condition_leaf_raises_error_for_group( - self, db: Session, manual_task: ManualTask - ): - """Test that to_condition_leaf raises an error for group conditions""" - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, - }, - ) - - # Should raise ValueError for group condition - with pytest.raises(ValueError, match="Cannot convert group condition to leaf"): - dependency.to_condition_leaf() - - def test_to_condition_group( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test converting a group dependency to a ConditionGroup object""" - # Create parent group dependency - parent_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, - }, - ) - - # Create child leaf dependency - child_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 2, - "parent_id": parent_dependency.id, - }, - ) - - # Convert to ConditionGroup - condition_group = parent_dependency.to_condition_group() - - # Verify the conversion - assert isinstance(condition_group, ConditionGroup) - assert condition_group.logical_operator == "and" - assert len(condition_group.conditions) == 1 - assert isinstance(condition_group.conditions[0], ConditionLeaf) - assert ( - condition_group.conditions[0].field_address - == sample_condition_leaf.field_address - ) - assert condition_group.conditions[0].operator == sample_condition_leaf.operator - assert condition_group.conditions[0].value == sample_condition_leaf.value - - def test_to_condition_group_raises_error_for_leaf( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test that to_condition_group raises an error for leaf conditions""" - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, - }, - ) - - # Should raise ValueError for leaf condition - with pytest.raises(ValueError, match="Cannot convert leaf condition to group"): - dependency.to_condition_group() - class TestManualTaskConditionalDependencyClassMethods: """Test class methods for ManualTaskConditionalDependency""" @@ -576,21 +188,11 @@ def test_get_root_condition_leaf( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): """Test getting root condition when it's a leaf condition""" - # Build condition_tree for JSONB storage - condition_tree = { - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - } - dependency = ManualTaskConditionalDependency.create( + condition_tree = sample_condition_leaf.model_dump() + ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 1, "condition_tree": condition_tree, }, ) @@ -607,46 +209,21 @@ def test_get_root_condition_leaf( assert root_condition.value == sample_condition_leaf.value def test_get_root_condition_group( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf + self, + db: Session, + manual_task: ManualTask, + sample_condition_group: ConditionGroup, ): """Test getting root condition when it's a group condition""" - # Build condition_tree for JSONB storage - condition_tree = { - "logical_operator": "and", - "conditions": [ - { - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - } - ], - } - # Create parent group dependency - parent_dependency = ManualTaskConditionalDependency.create( + condition_tree = sample_condition_group.model_dump() + ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, "condition_tree": condition_tree, }, ) - # Create child leaf dependency (for backward compatibility) - child_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": sample_condition_leaf.field_address, - "operator": sample_condition_leaf.operator, - "value": sample_condition_leaf.value, - "sort_order": 2, - "parent_id": parent_dependency.id, - }, - ) - # Get root condition root_condition = ManualTaskConditionalDependency.get_root_condition( db, manual_task_id=manual_task.id @@ -654,15 +231,10 @@ def test_get_root_condition_group( # Verify the result assert isinstance(root_condition, ConditionGroup) - assert root_condition.logical_operator == "and" - assert len(root_condition.conditions) == 1 + assert root_condition.logical_operator == GroupOperator.and_ + assert len(root_condition.conditions) == 2 assert isinstance(root_condition.conditions[0], ConditionLeaf) - assert ( - root_condition.conditions[0].field_address - == sample_condition_leaf.field_address - ) - assert root_condition.conditions[0].operator == sample_condition_leaf.operator - assert root_condition.conditions[0].value == sample_condition_leaf.value + assert root_condition.conditions[0].field_address == "user.age" def test_get_root_condition_none(self, db: Session, manual_task: ManualTask): """Test getting root condition when none exists""" @@ -674,62 +246,33 @@ def test_get_root_condition_none(self, db: Session, manual_task: ManualTask): # Should return None assert root_condition is None - def test_get_root_condition_complex_hierarchy( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf + def test_get_root_condition_complex_nested( + self, db: Session, manual_task: ManualTask ): - """Test getting root condition with complex hierarchy""" - # Build condition_tree for JSONB storage + """Test getting root condition with complex nested hierarchy""" + # Build nested condition tree condition_tree = { - "logical_operator": "and", + "logical_operator": GroupOperator.and_, "conditions": [ { - "logical_operator": "or", + "logical_operator": GroupOperator.or_, "conditions": [ {"field_address": "user.field_0", "operator": "eq", "value": 0}, {"field_address": "user.field_1", "operator": "eq", "value": 1}, ], - } + }, + {"field_address": "user.active", "operator": "eq", "value": True}, ], } - # Create root group dependency - root_dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, - "condition_tree": condition_tree, - }, - ) - # Create level 1 child group (for backward compatibility) - level1_child = ManualTaskConditionalDependency.create( + ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "or", - "sort_order": 2, - "parent_id": root_dependency.id, + "condition_tree": condition_tree, }, ) - # Create level 2 leaf children (for backward compatibility) - for i in range(2): - ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"user.field_{i}", - "operator": "eq", - "value": i, - "sort_order": i + 3, - "parent_id": level1_child.id, - }, - ) - # Get root condition root_condition = ManualTaskConditionalDependency.get_root_condition( db, manual_task_id=manual_task.id @@ -737,12 +280,20 @@ def test_get_root_condition_complex_hierarchy( # Verify the result assert isinstance(root_condition, ConditionGroup) - assert root_condition.logical_operator == "and" - assert len(root_condition.conditions) == 1 - assert isinstance(root_condition.conditions[0], ConditionGroup) - assert root_condition.conditions[0].logical_operator == "or" - assert len(root_condition.conditions[0].conditions) == 2 - assert all( - isinstance(cond, ConditionLeaf) - for cond in root_condition.conditions[0].conditions - ) + assert root_condition.logical_operator == GroupOperator.and_ + assert len(root_condition.conditions) == 2 + + # First condition is a nested group + nested_group = root_condition.conditions[0] + assert isinstance(nested_group, ConditionGroup) + assert nested_group.logical_operator == GroupOperator.or_ + assert len(nested_group.conditions) == 2 + + # Second condition is a leaf + assert isinstance(root_condition.conditions[1], ConditionLeaf) + assert root_condition.conditions[1].field_address == "user.active" + + def test_get_root_condition_missing_manual_task_id(self, db: Session): + """Test that get_root_condition raises error when manual_task_id is missing""" + with pytest.raises(ValueError, match="manual_task_id is required"): + ManualTaskConditionalDependency.get_root_condition(db) diff --git a/tests/api/models/test_conditional_dependency_base.py b/tests/api/models/test_conditional_dependency_base.py index 35abe439c3..96d4284d18 100644 --- a/tests/api/models/test_conditional_dependency_base.py +++ b/tests/api/models/test_conditional_dependency_base.py @@ -1,189 +1,13 @@ """Tests for the ConditionalDependencyBase abstract model.""" -from typing import Any from unittest.mock import create_autospec -from uuid import uuid4 import pytest from sqlalchemy.orm import Session from fides.api.models.conditional_dependency.conditional_dependency_base import ( ConditionalDependencyBase, - ConditionalDependencyType, ) -from fides.api.task.conditional_dependencies.schemas import ( - ConditionGroup, - ConditionLeaf, - GroupOperator, - Operator, -) - - -def create_mock_conditional_dependency_leaf(**kwargs): - """Create a mock conditional dependency.""" - return MockConditionalDependency( - condition_type=ConditionalDependencyType.leaf, - **kwargs, - ) - - -class MockConditionalDependency: - """Concrete implementation for testing base class methods without SQLAlchemy.""" - - def __init__(self, **kwargs): - # Set required attributes for testing - for key, value in kwargs.items(): - setattr(self, key, value) - - if not hasattr(self, "id"): - self.id = str(uuid4()) - - # Set defaults if not provided - if not hasattr(self, "children"): - self.children = [] - - if not hasattr(self, "parent_id"): - self.parent_id = None - - if not hasattr(self, "parent"): - self.parent = None - - if not hasattr(self, "sort_order"): - self.sort_order = 0 - - def to_correct_condition_type(self): - """Use the base class method via composition.""" - # Create a temporary object with base class methods - temp = ConditionalDependencyBase() - temp.condition_type = self.condition_type - temp.field_address = getattr(self, "field_address", None) - temp.operator = getattr(self, "operator", None) - temp.value = getattr(self, "value", None) - temp.logical_operator = getattr(self, "logical_operator", None) - temp.children = self.children - temp.parent_id = getattr(self, "parent_id", None) - temp.parent = getattr(self, "parent", None) - return temp.to_correct_condition_type() - - def to_condition_leaf(self): - """Use the base class method via composition.""" - # Create a temporary object with base class methods - temp = ConditionalDependencyBase() - temp.condition_type = self.condition_type - temp.field_address = getattr(self, "field_address", None) - temp.operator = getattr(self, "operator", None) - temp.value = getattr(self, "value", None) - temp.parent_id = getattr(self, "parent_id", None) - temp.parent = getattr(self, "parent", None) - return temp.to_condition_leaf() - - def to_condition_group(self): - """Use the base class method via composition.""" - # Create a temporary object with base class methods - temp = ConditionalDependencyBase() - temp.condition_type = self.condition_type - temp.logical_operator = getattr(self, "logical_operator", None) - temp.children = self.children - temp.parent_id = getattr(self, "parent_id", None) - temp.parent = getattr(self, "parent", None) - return temp.to_condition_group() - - @classmethod - def get_root_condition(cls, db: Session, *, parent_id: str, **kwargs: Any): - """Mock implementation.""" - temp = ConditionalDependencyBase() - temp.condition_type = cls.condition_type - temp.parent_id = parent_id - temp.parent = getattr(cls, "parent", None) - return temp.get_root_condition(db, parent_id=parent_id) - - -@pytest.fixture -def leaf_dependency_name(): - """Create a leaf dependency factory that returns fresh instances.""" - - def _create(): - return MockConditionalDependency( - condition_type=ConditionalDependencyType.leaf, - field_address="user.name", - operator=Operator.eq, - value="test_user", - ) - - return _create() - - -@pytest.fixture -def leaf_dependency_active(): - """Create a leaf dependency factory that returns fresh instances.""" - condition = {"field_address": "user.active", "operator": Operator.eq, "value": True} - return create_mock_conditional_dependency_leaf(**condition) - - -@pytest.fixture -def leaf_dependency_role(): - """Create a leaf dependency factory that returns fresh instances.""" - condition = { - "field_address": "user.role", - "operator": Operator.eq, - "value": "admin", - } - return create_mock_conditional_dependency_leaf(**condition) - - -@pytest.fixture -def leaf_dependency_verified(): - """Create a leaf dependency factory that returns fresh instances.""" - condition = { - "field_address": "user.verified", - "operator": Operator.eq, - "value": True, - } - return create_mock_conditional_dependency_leaf(**condition) - - -@pytest.fixture -def leaf_dependency_premium(): - """Create a leaf dependency factory that returns fresh instances.""" - condition = { - "field_address": "user.premium", - "operator": Operator.eq, - "value": True, - } - return create_mock_conditional_dependency_leaf(**condition) - - -@pytest.fixture -def leaf_dependency_age(): - """Create a leaf dependency factory that returns fresh instances.""" - condition = {"field_address": "user.age", "operator": Operator.gte, "value": 18} - return create_mock_conditional_dependency_leaf(**condition) - - -@pytest.fixture -def group_dependency(): - """Create a group dependency factory that returns fresh instances.""" - - def _create(): - return MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.and_, - ) - - return _create() - - -class TestConditionalDependencyType: - """Test the shared ConditionalDependencyType enum.""" - - def test_enum_values(self): - """Test that the enum has the expected values.""" - assert ConditionalDependencyType.leaf.value == "leaf" - assert ConditionalDependencyType.group.value == "group" - with pytest.raises( - ValueError, match="'invalid' is not a valid ConditionalDependencyType" - ): - ConditionalDependencyType("invalid") class TestConditionalDependencyBase: @@ -203,393 +27,6 @@ def test_get_root_condition_not_implemented(self): ): ConditionalDependencyBase.get_root_condition(db, test_id="test_id") - def test_abstract_class_attributes(self): - """Test that the abstract class has the required attributes.""" - # Test the abstract class attributes are present - abstract_class_attributes = ["__module__", "__doc__", "__abstract__"] - assert all( - attr in ConditionalDependencyBase.__dict__ - for attr in abstract_class_attributes - ) - - # Test the attributes that are common to all conditional dependency models - common_attributes = [ - "condition_type", - "field_address", - "operator", - "value", - "logical_operator", - "sort_order", - ] - assert all( - attr in ConditionalDependencyBase.__dict__ for attr in common_attributes - ) - - # Test the functions that are common to all conditional dependency models - common_functions = [ - "to_correct_condition_type", - "to_condition_leaf", - "to_condition_group", - "get_root_condition", - ] - assert all( - attr in ConditionalDependencyBase.__dict__ for attr in common_functions - ) - - -class TestConditionalDependencyBaseMethods: - """Test the shared methods in ConditionalDependencyBase.""" - - def test_to_correct_condition_type_leaf( - self, leaf_dependency_name: MockConditionalDependency - ): - """Test successful conversion to the correct condition type.""" - dependency = leaf_dependency_name - - result = dependency.to_correct_condition_type() - - assert isinstance(result, ConditionLeaf) - assert result.field_address == "user.name" - assert result.operator == Operator.eq - assert result.value == "test_user" - - def test_to_correct_condition_type_group( - self, - group_dependency: MockConditionalDependency, - leaf_dependency_name: MockConditionalDependency, - ): - """Test successful conversion to the correct condition type.""" - leaf_dependency_name.parent_id = group_dependency.id - group_dependency.children = [leaf_dependency_name] - group_dependency.conditions = [leaf_dependency_name] - - result = group_dependency.to_correct_condition_type() - - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.and_ - assert len(result.conditions) == 1 - assert isinstance(result.conditions[0], ConditionLeaf) - assert result.conditions[0].field_address == "user.name" - - def test_to_condition_leaf_success( - self, leaf_dependency_name: MockConditionalDependency - ): - """Test successful conversion to ConditionLeaf.""" - result = leaf_dependency_name.to_condition_leaf() - - assert isinstance(result, ConditionLeaf) - assert result.field_address == "user.name" - assert result.operator == Operator.eq - assert result.value == "test_user" - - def test_to_condition_leaf_failure_on_group( - self, - group_dependency: MockConditionalDependency, - leaf_dependency_name: MockConditionalDependency, - ): - """Test that to_condition_leaf fails when called on group condition.""" - leaf_dependency_name.parent_id = group_dependency.id - - with pytest.raises(ValueError, match="Cannot convert group condition to leaf"): - group_dependency.to_condition_leaf() - - def test_to_condition_group_success_single_child( - self, - leaf_dependency_active: MockConditionalDependency, - group_dependency: MockConditionalDependency, - ): - """Test successful conversion to ConditionGroup with single child.""" - # Create child leaf condition - child = leaf_dependency_active - child.parent_id = group_dependency.id - group_dependency.children = [child] - group_dependency.conditions = [child] - - result = group_dependency.to_condition_group() - - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.and_ - assert len(result.conditions) == 1 - assert isinstance(result.conditions[0], ConditionLeaf) - assert result.conditions[0].field_address == "user.active" - - def test_to_condition_group_success_with_leaf_children( - self, - leaf_dependency_age: MockConditionalDependency, - leaf_dependency_active: MockConditionalDependency, - group_dependency: MockConditionalDependency, - ): - """Test successful conversion to ConditionGroup with leaf children.""" - # Create child leaf conditions - child1 = leaf_dependency_age - child1.sort_order = 1 - child1.parent_id = group_dependency.id - child2 = leaf_dependency_active - child2.sort_order = 2 - child2.parent_id = group_dependency.id - group_dependency.children = [child1, child2] - group_dependency.conditions = [child1, child2] - - result = group_dependency.to_condition_group() - - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.and_ - assert len(result.conditions) == 2 - - def test_to_condition_group_success_with_nested_groups( - self, - leaf_dependency_verified: MockConditionalDependency, - leaf_dependency_role: MockConditionalDependency, - ): - """Test successful conversion to ConditionGroup with nested group children.""" - # Create parent group - dependency = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.and_, - ) - - # Create nested child group - nested_leaf = leaf_dependency_verified - nested_leaf.sort_order = 1 - nested_leaf.parent_id = dependency.id - nested_group = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.or_, - children=[nested_leaf], - ) - nested_group.sort_order = 1 - nested_group.parent_id = dependency.id - nested_group.children = [nested_leaf] - - # Create direct child leaf - direct_leaf = leaf_dependency_role - direct_leaf.sort_order = 2 - direct_leaf.parent_id = dependency.id - - dependency.children = [nested_group, direct_leaf] - dependency.conditions = [nested_group, direct_leaf] - - result = dependency.to_condition_group() - - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.and_ - assert len(result.conditions) == 2 - - # First child should be the nested group (sort_order=1) - assert isinstance(result.conditions[0], ConditionGroup) - assert result.conditions[0].logical_operator == GroupOperator.or_ - assert len(result.conditions[0].conditions) == 1 - assert isinstance(result.conditions[0].conditions[0], ConditionLeaf) - assert result.conditions[0].conditions[0].field_address == "user.verified" - - # Second child should be the direct leaf (sort_order=2) - assert isinstance(result.conditions[1], ConditionLeaf) - assert result.conditions[1].field_address == "user.role" - - def test_to_condition_group_failure_on_leaf( - self, leaf_dependency_name: MockConditionalDependency - ): - """Test that to_condition_group fails when called on leaf condition.""" - with pytest.raises(ValueError, match="Cannot convert leaf condition to group"): - leaf_dependency_name.to_condition_group() - - def test_to_condition_group_sorts_children_by_sort_order(self): - """Test that children are properly sorted by sort_order.""" - # Create children with explicit sort orders - children = [] - expected_order = [3, 1, 4, 2] # Intentionally out of order - - for i, sort_order in enumerate(expected_order): - child = MockConditionalDependency( - condition_type=ConditionalDependencyType.leaf, - field_address=f"field_{i}", - operator=Operator.exists, - value=None, - sort_order=sort_order, - ) - children.append(child) - - dependency = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.or_, - children=children, - ) - - result = dependency.to_condition_group() - - # Check that conditions are sorted by sort_order (1, 2, 3, 4) - assert len(result.conditions) == 4 - assert result.conditions[0].field_address == "field_1" # sort_order=1 - assert result.conditions[1].field_address == "field_3" # sort_order=2 - assert result.conditions[2].field_address == "field_0" # sort_order=3 - assert result.conditions[3].field_address == "field_2" # sort_order=4 - - -class TestConditionalDependencyBaseEdgeCases: - """Test edge cases and error conditions.""" - - def test_to_condition_leaf_with_none_values(self): - """Test to_condition_leaf with None values.""" - dependency = MockConditionalDependency( - condition_type=ConditionalDependencyType.leaf, - field_address="user.optional_field", - operator=Operator.not_exists, - value=None, - ) - - result = dependency.to_condition_leaf() - - assert isinstance(result, ConditionLeaf) - assert result.field_address == "user.optional_field" - assert result.operator == Operator.not_exists - assert result.value is None - - def test_to_condition_group_validation_error_with_empty_children(self): - """Test that to_condition_group raises validation error with empty children list.""" - dependency = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.or_, - children=[], - ) - - with pytest.raises(ValueError, match="conditions list cannot be empty"): - dependency.to_condition_group() - - def test_complex_nested_structure( - self, - leaf_dependency_name: MockConditionalDependency, - leaf_dependency_age: MockConditionalDependency, - leaf_dependency_role: MockConditionalDependency, - leaf_dependency_verified: MockConditionalDependency, - leaf_dependency_premium: MockConditionalDependency, - ): - """Test complex nested group structure with multiple levels.""" - # Build a complex structure: (A AND B) OR (C AND (D OR E)) - - # Leaf conditions - leaf_a = leaf_dependency_name - leaf_b = leaf_dependency_age - leaf_c = leaf_dependency_role - leaf_d = leaf_dependency_verified - leaf_e = leaf_dependency_premium - - # Inner groups - group_ab = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.and_, - children=[leaf_a, leaf_b], - sort_order=1, - ) - leaf_a.parent_id = group_ab.id - leaf_a.sort_order = 1 # First in group_ab - leaf_b.parent_id = group_ab.id - leaf_b.sort_order = 2 # Second in group_ab - - group_de = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.or_, - children=[leaf_d, leaf_e], - sort_order=2, - ) - leaf_d.parent_id = group_de.id - leaf_d.sort_order = 1 # First in group_de - leaf_e.parent_id = group_de.id - leaf_e.sort_order = 2 # Second in group_de - - group_c_de = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.and_, - children=[leaf_c, group_de], - sort_order=2, - ) - leaf_c.parent_id = group_c_de.id - leaf_c.sort_order = 1 - group_de.parent_id = group_c_de.id - group_de.sort_order = 2 - - # Root group - root = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator=GroupOperator.or_, - children=[group_ab, group_c_de], - ) - group_ab.parent_id = root.id - group_c_de.parent_id = root.id - - result = root.to_condition_group() - - # Verify structure: (A AND B) OR (C AND (D OR E)) - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.or_ - assert len(result.conditions) == 2 - - # First group: (A AND B) - first_group = result.conditions[0] - assert isinstance(first_group, ConditionGroup) - assert first_group.logical_operator == GroupOperator.and_ - assert len(first_group.conditions) == 2 - assert first_group.conditions[0].field_address == "user.name" - assert first_group.conditions[1].field_address == "user.age" - - # Second group: (C AND (D OR E)) - second_group = result.conditions[1] - assert isinstance(second_group, ConditionGroup) - assert second_group.logical_operator == GroupOperator.and_ - assert len(second_group.conditions) == 2 - assert second_group.conditions[0].field_address == "user.role" - - # Nested OR group: (D OR E) - nested_or = second_group.conditions[1] - assert isinstance(nested_or, ConditionGroup) - assert nested_or.logical_operator == GroupOperator.or_ - assert len(nested_or.conditions) == 2 - assert nested_or.conditions[0].field_address == "user.verified" - assert nested_or.conditions[1].field_address == "user.premium" - - -class TestConditionalDependencyBaseEnhancedErrorMessages: - """Test the enhanced error messages in conversion methods.""" - - def test_to_condition_leaf_enhanced_error_message(self): - """Test enhanced error message when converting group to leaf.""" - dependency = MockConditionalDependency( - condition_type=ConditionalDependencyType.group, - logical_operator="and", - ) - - with pytest.raises(ValueError) as exc_info: - dependency.to_condition_leaf() - - error_message = str(exc_info.value) - assert "Cannot convert group condition to leaf" in error_message - assert "Only conditions with condition_type='leaf'" in error_message - assert "should be converted using to_condition_group()" in error_message - - def test_to_condition_group_enhanced_error_message( - self, leaf_dependency_name: MockConditionalDependency - ): - """Test enhanced error message when converting leaf to group.""" - - with pytest.raises(ValueError) as exc_info: - leaf_dependency_name.to_condition_group() - - error_message = str(exc_info.value) - assert "Cannot convert leaf condition to group" in error_message - assert "Only conditions with condition_type='group'" in error_message - assert "should be converted using to_condition_leaf()" in error_message - - def test_to_condition_group_children_relationship_error(self): - """Test error message when children relationship is missing.""" - # Create a ConditionalDependencyBase instance directly (no children relationship) - temp = ConditionalDependencyBase() - temp.condition_type = ConditionalDependencyType.group - temp.logical_operator = "and" - - with pytest.raises(AttributeError) as exc_info: - temp.to_condition_group() - - error_message = str(exc_info.value) - assert "The 'children' relationship is not defined" in error_message - assert ( - "Concrete subclasses must define a 'children' relationship" in error_message - ) + def test_abstract_class_has_condition_tree_column(self): + """Test that the abstract class has the condition_tree column.""" + assert "condition_tree" in ConditionalDependencyBase.__dict__ diff --git a/tests/api/task/manual/conftest.py b/tests/api/task/manual/conftest.py index 7736111758..aa95c76e51 100644 --- a/tests/api/task/manual/conftest.py +++ b/tests/api/task/manual/conftest.py @@ -18,9 +18,6 @@ AttachmentReference, AttachmentReferenceType, ) -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.connectionconfig import ( AccessLevel, ConnectionConfig, @@ -49,6 +46,7 @@ from fides.api.schemas.privacy_request import PrivacyRequestStatus from fides.api.task.manual.manual_task_graph_task import ManualTaskGraphTask from fides.api.task.manual.manual_task_utils import ( + extract_field_addresses, get_manual_task_for_connection_config, ) from fides.api.task.task_resources import TaskResources @@ -394,21 +392,25 @@ def _build_request_task( incoming_edges = [] if manual_task: - # Get conditional dependency field addresses - field_addresses = [] + # Get conditional dependency field addresses from condition_tree + all_field_addresses: set[str] = set() for dependency in manual_task.conditional_dependencies: - if ( - dependency.condition_type == ConditionalDependencyType.leaf - and dependency.field_address - and not dependency.field_address.startswith("privacy_request.") - ): - field_addresses.append(dependency.field_address) + tree = dependency.condition_tree + if isinstance(tree, dict) or tree is None: + addresses = extract_field_addresses(tree) + # Filter out privacy_request fields + addresses = { + addr + for addr in addresses + if not addr.startswith("privacy_request.") + } + all_field_addresses.update(addresses) # For testing, we'll create input keys and edges based on the field addresses # In a real scenario, these would be determined by the graph traversal - if field_addresses: + if all_field_addresses: # Use the existing field address parsing utilities to create mappings - for field_address in field_addresses: + for field_address in all_field_addresses: source_field_address = field_address target_field_address = ( f"{connection_config.key}:manual_data:{field_address}" @@ -983,21 +985,13 @@ def create_condition_gt_18_tree() -> dict: } -def create_condition_gt_18( - db: Session, manual_task: ManualTask, parent_id: int = None, sort_order: int = 1 -): - condition_tree = create_condition_gt_18_tree() if parent_id is None else None +def create_condition_gt_18(db: Session, manual_task: ManualTask): + """Create a conditional dependency for age >= 18.""" return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": parent_id, - "field_address": "postgres_example:customer:profile.age", - "operator": "gte", - "value": 18, - "sort_order": sort_order, - "condition_tree": condition_tree, + "condition_tree": create_condition_gt_18_tree(), }, ) @@ -1011,21 +1005,13 @@ def create_condition_age_lt_65_tree() -> dict: } -def create_condition_age_lt_65( - db: Session, manual_task: ManualTask, parent_id: int = None, sort_order: int = 2 -): - condition_tree = create_condition_age_lt_65_tree() if parent_id is None else None +def create_condition_age_lt_65(db: Session, manual_task: ManualTask): + """Create a conditional dependency for age < 65.""" return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": parent_id, - "field_address": "postgres_example:customer:profile.age", - "operator": "lt", - "value": 65, - "sort_order": sort_order, - "condition_tree": condition_tree, + "condition_tree": create_condition_age_lt_65_tree(), }, ) @@ -1039,21 +1025,13 @@ def create_condition_eq_active_tree() -> dict: } -def create_condition_eq_active( - db: Session, manual_task: ManualTask, parent_id: int = None, sort_order: int = 1 -): - condition_tree = create_condition_eq_active_tree() if parent_id is None else None +def create_condition_eq_active(db: Session, manual_task: ManualTask): + """Create a conditional dependency for status == active.""" return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": parent_id, - "field_address": "postgres_example:payment_card:subscription.status", - "operator": "eq", - "value": "active", - "sort_order": sort_order, - "condition_tree": condition_tree, + "condition_tree": create_condition_eq_active_tree(), }, ) @@ -1067,21 +1045,13 @@ def create_condition_eq_admin_tree() -> dict: } -def create_condition_eq_admin( - db: Session, manual_task: ManualTask, parent_id: int = None, sort_order: int = 1 -): - condition_tree = create_condition_eq_admin_tree() if parent_id is None else None +def create_condition_eq_admin(db: Session, manual_task: ManualTask): + """Create a conditional dependency for role == admin.""" return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example:customer:role", - "operator": "eq", - "value": "admin", - "sort_order": sort_order, - "parent_id": parent_id, - "condition_tree": condition_tree, + "condition_tree": create_condition_eq_admin_tree(), }, ) @@ -1089,7 +1059,7 @@ def create_condition_eq_admin( @pytest.fixture() def condition_gt_18(db: Session, manual_task: ManualTask): """Create a conditional dependency with field_address 'user.age' and operator 'gte' and value 18""" - condition = create_condition_gt_18(db, manual_task, None) + condition = create_condition_gt_18(db, manual_task) yield condition condition.delete(db) @@ -1097,7 +1067,7 @@ def condition_gt_18(db: Session, manual_task: ManualTask): @pytest.fixture() def condition_age_lt_65(db: Session, manual_task: ManualTask): """Create a conditional dependency with field_address 'user.age' and operator 'lt' and value 65""" - condition = create_condition_age_lt_65(db, manual_task, None) + condition = create_condition_age_lt_65(db, manual_task) yield condition condition.delete(db) @@ -1105,7 +1075,7 @@ def condition_age_lt_65(db: Session, manual_task: ManualTask): @pytest.fixture() def condition_eq_active(db: Session, manual_task: ManualTask): """Create a conditional dependency with field_address 'billing.subscription.status' and operator 'eq' and value 'active'""" - condition = create_condition_eq_active(db, manual_task, None) + condition = create_condition_eq_active(db, manual_task) yield condition condition.delete(db) @@ -1113,7 +1083,7 @@ def condition_eq_active(db: Session, manual_task: ManualTask): @pytest.fixture() def condition_eq_admin(db: Session, manual_task: ManualTask): """Create a conditional dependency with field_address 'user.role' and operator 'eq' and value 'admin'""" - condition = create_condition_eq_admin(db, manual_task, None) + condition = create_condition_eq_admin(db, manual_task) yield condition condition.delete(db) @@ -1133,15 +1103,9 @@ def group_condition(db: Session, manual_task: ManualTask): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, "condition_tree": condition_tree, }, ) - # Also create child rows for backward compatibility during transition - create_condition_gt_18(db, manual_task, root_condition.id, 2) - create_condition_eq_active(db, manual_task, root_condition.id, 3) yield root_condition root_condition.delete(db) @@ -1167,25 +1131,8 @@ def nested_group_condition(db: Session, manual_task: ManualTask): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 1, "condition_tree": condition_tree, }, ) - # Also create child rows for backward compatibility during transition - nested_group = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.group, - "parent_id": root_condition.id, - "logical_operator": "or", - "sort_order": 2, - }, - ) - create_condition_gt_18(db, manual_task, nested_group.id, 3) - create_condition_eq_active(db, manual_task, nested_group.id, 4) - create_condition_eq_admin(db, manual_task, nested_group.id, 5) yield root_condition root_condition.delete(db) From 98acd32085406a53ad23919e83fa03f7aad58343 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 14:37:41 +0000 Subject: [PATCH 02/12] update to remove orphan cols and update db yml --- .fides/db_dataset.yml | 28 ------------------- ...emove_un_used_columns_from_conditional_.py | 28 ++++++++++++------- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/.fides/db_dataset.yml b/.fides/db_dataset.yml index eaafbbee6e..bbba0c77c2 100644 --- a/.fides/db_dataset.yml +++ b/.fides/db_dataset.yml @@ -1066,22 +1066,8 @@ dataset: data_categories: [system.operations] - name: condition_tree data_categories: [system.operations] - - name: parent_id - data_categories: [system.operations] - name: digest_condition_type data_categories: [system.operations] - - name: condition_type - data_categories: [system.operations] - - name: field_address - data_categories: [system.operations] - - name: operator - data_categories: [system.operations] - - name: value - data_categories: [system.operations] - - name: logical_operator - data_categories: [system.operations] - - name: sort_order - data_categories: [system.operations] - name: created_at data_categories: [system.operations] - name: updated_at @@ -1281,20 +1267,6 @@ dataset: data_categories: [system.operations] - name: condition_tree data_categories: [system.operations] - - name: parent_id - data_categories: [system.operations] - - name: condition_type - data_categories: [system.operations] - - name: field_address - data_categories: [system.operations] - - name: operator - data_categories: [system.operations] - - name: value - data_categories: [system.operations] - - name: logical_operator - data_categories: [system.operations] - - name: sort_order - data_categories: [system.operations] - name: created_at data_categories: [system.operations] - name: updated_at diff --git a/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py b/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py index 5ea9ba4489..f03dd9eb89 100644 --- a/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py +++ b/src/fides/api/alembic/migrations/versions/xx_2025_12_30_1421_9cf7bb472a7c_remove_un_used_columns_from_conditional_.py @@ -19,13 +19,18 @@ def upgrade(): # === digest_condition table === + # Delete child rows (non-root) - their data is now in the root's condition_tree JSONB + op.execute("DELETE FROM digest_condition WHERE parent_id IS NOT NULL") + # Drop indexes on columns being removed op.drop_index("ix_digest_condition_condition_type", table_name="digest_condition") op.drop_index("ix_digest_condition_parent_id", table_name="digest_condition") op.drop_index("ix_digest_condition_sort_order", table_name="digest_condition") # Replace partial unique index with a full unique constraint - op.drop_index("ix_digest_condition_unique_root_per_type", table_name="digest_condition") + op.drop_index( + "ix_digest_condition_unique_root_per_type", table_name="digest_condition" + ) op.create_unique_constraint( "uq_digest_condition_config_type", "digest_condition", @@ -47,6 +52,11 @@ def upgrade(): op.drop_column("digest_condition", "field_address") # === manual_task_conditional_dependency table === + # Delete child rows (non-root) - their data is now in the root's condition_tree JSONB + op.execute( + "DELETE FROM manual_task_conditional_dependency WHERE parent_id IS NOT NULL" + ) + # Drop indexes on columns being removed op.drop_index( "ix_manual_task_conditional_dependency_condition_type", @@ -96,6 +106,10 @@ def upgrade(): def downgrade(): + # NOTE: Child rows (parent_id IS NOT NULL) that were deleted during upgrade + # cannot be recovered. Only the schema is restored, not the hierarchical data. + # The condition_tree JSONB column still contains the full tree structure. + # === manual_task_conditional_dependency table === # Re-add columns op.add_column( @@ -104,9 +118,7 @@ def downgrade(): ) op.add_column( "manual_task_conditional_dependency", - sa.Column( - "value", postgresql.JSONB(astext_type=sa.Text()), nullable=True - ), + sa.Column("value", postgresql.JSONB(astext_type=sa.Text()), nullable=True), ) op.add_column( "manual_task_conditional_dependency", @@ -181,9 +193,7 @@ def downgrade(): ) op.add_column( "digest_condition", - sa.Column( - "value", postgresql.JSONB(astext_type=sa.Text()), nullable=True - ), + sa.Column("value", postgresql.JSONB(astext_type=sa.Text()), nullable=True), ) op.add_column( "digest_condition", @@ -232,9 +242,7 @@ def downgrade(): op.create_index( "ix_digest_condition_sort_order", "digest_condition", ["sort_order"] ) - op.create_index( - "ix_digest_condition_parent_id", "digest_condition", ["parent_id"] - ) + op.create_index("ix_digest_condition_parent_id", "digest_condition", ["parent_id"]) op.create_index( "ix_digest_condition_condition_type", "digest_condition", From 62e3fe54be3fffe9c6d8b7420bc9a46d44da9592 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 15:03:58 +0000 Subject: [PATCH 03/12] add commit --- .../xx_2025_12_16_1630_85ce2c1c9579_add_jsonb_tree_column.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fides/api/alembic/migrations/versions/xx_2025_12_16_1630_85ce2c1c9579_add_jsonb_tree_column.py b/src/fides/api/alembic/migrations/versions/xx_2025_12_16_1630_85ce2c1c9579_add_jsonb_tree_column.py index 531b5c4490..1ee6d7424a 100644 --- a/src/fides/api/alembic/migrations/versions/xx_2025_12_16_1630_85ce2c1c9579_add_jsonb_tree_column.py +++ b/src/fides/api/alembic/migrations/versions/xx_2025_12_16_1630_85ce2c1c9579_add_jsonb_tree_column.py @@ -117,6 +117,7 @@ def upgrade(): db = Session(op.get_bind()) migrate_conditions(db, "manual_task_conditional_dependency") migrate_conditions(db, "digest_condition") + db.commit() def downgrade(): From 075ff2f1d84483b2e5014d2465e27bcf3bc19083 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 16:03:42 +0000 Subject: [PATCH 04/12] more test updates --- ...nual_task_with_conditional_dependencies.py | 149 +++---- .../digest/test_condition_validation.py | 417 ------------------ .../digest/test_digest_config_conditions.py | 88 +--- ...test_manual_task_conditional_evaluation.py | 112 +---- .../manual/test_manual_task_graph_task.py | 8 - .../api/task/manual/test_manual_task_utils.py | 8 - .../test_manual_task_privacy_request.py | 58 +-- 7 files changed, 65 insertions(+), 775 deletions(-) delete mode 100644 tests/api/models/digest/test_condition_validation.py diff --git a/qa/scenarios/manual_task_with_conditional_dependencies.py b/qa/scenarios/manual_task_with_conditional_dependencies.py index 5edd51463c..9292697bce 100644 --- a/qa/scenarios/manual_task_with_conditional_dependencies.py +++ b/qa/scenarios/manual_task_with_conditional_dependencies.py @@ -26,9 +26,6 @@ from fides.api.db.ctl_session import sync_engine from fides.api.db.base import * from fides.api.db.database import seed_db -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.connectionconfig import ( AccessLevel, ConnectionConfig, @@ -103,68 +100,52 @@ } ] -DEPENDENCIES_DATA = [ - # Root group condition (AND) - { - "parent_id": None, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "and", - "sort_order": 0 - }, - # Customer name exists condition (leaf) - references postgres dataset field - { - "parent_id": None, # Will be updated after root group is created - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"{POSTGRES_DATASET_KEY}:customer:name", - "operator": "exists", - "value": None, # exists operator doesn't need a value - "sort_order": 1 - }, - # Customer email contains specific value condition (leaf) - references postgres dataset field - { - "parent_id": None, # Will be updated after root group is created - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"{POSTGRES_DATASET_KEY}:customer:email", - "operator": "starts_with", - "value": "customer-1", - "sort_order": 2 - }, - # Nested group condition (OR) - { - "parent_id": None, # Will be updated after root group is created - "condition_type": ConditionalDependencyType.group, - "logical_operator": "or", - "sort_order": 3 - }, - # Customer ID condition (leaf) - references postgres dataset field - { - "parent_id": None, # Will be updated after nested group is created - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"{POSTGRES_DATASET_KEY}:customer:id", - "operator": "gt", - "value": 0, - "sort_order": 4 - }, - # Customer address city starts with Example condition (leaf) - references postgres dataset field - { - "parent_id": None, # Will be updated after nested group is created - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"{POSTGRES_DATASET_KEY}:address:city", - "operator": "starts_with", - "value": "Example", - "sort_order": 5 - } -] +# Full condition tree stored as JSONB: +# (customer.name exists AND customer.email starts_with "customer-1") AND +# (customer.id > 0 OR address.city starts_with "Example") +CONDITION_TREE = { + "logical_operator": "and", + "conditions": [ + # Customer name exists condition + { + "field_address": f"{POSTGRES_DATASET_KEY}:customer:name", + "operator": "exists", + "value": None, + }, + # Customer email starts with specific value condition + { + "field_address": f"{POSTGRES_DATASET_KEY}:customer:email", + "operator": "starts_with", + "value": "customer-1", + }, + # Nested OR group + { + "logical_operator": "or", + "conditions": [ + # Customer ID condition + { + "field_address": f"{POSTGRES_DATASET_KEY}:customer:id", + "operator": "gt", + "value": 0, + }, + # Customer address city starts with Example condition + { + "field_address": f"{POSTGRES_DATASET_KEY}:address:city", + "operator": "starts_with", + "value": "Example", + }, + ], + }, + ], +} -def create_dependencies_data(manual_task_id: str) -> list[dict]: - return [ - { - "manual_task_id": manual_task_id, - **dependency_data, - } - for dependency_data in DEPENDENCIES_DATA - ] +def create_dependency_data(manual_task_id: str) -> dict: + """Create a single dependency with the full condition tree.""" + return { + "manual_task_id": manual_task_id, + "condition_tree": CONDITION_TREE, + } def create_fields_data(manual_task_id: str, manual_task_config_id: str) -> list[dict]: @@ -623,46 +604,20 @@ def _create_manual_task_config_fields(self) -> bool: return False def _create_conditional_dependencies(self) -> bool: - """Create ManualTaskConditionalDependencies based on ManualTask fields.""" + """Create ManualTaskConditionalDependency with full condition tree.""" try: db = self._get_db_session() - dependencies_data = create_dependencies_data(self.manual_task.id) - - # Create dependencies in order and establish parent-child relationships - root_group = None - nested_group = None - - for i, dep_data in enumerate(dependencies_data): - dep = ManualTaskConditionalDependency.create(db=db, data=dep_data) - self.conditional_dependencies.append(dep) - - # Set up parent-child relationships - if i == 0: # Root group - root_group = dep - self.info(f"Created root group dependency: {dep.id}") - elif i == 3: # Nested group - nested_group = dep - # Update parent to root group - dep.parent_id = root_group.id - db.commit() - self.info(f"Created nested group dependency: {dep.id}") - elif i in [1, 2]: # Leaf conditions under root group - # Update parent to root group - dep.parent_id = root_group.id - db.commit() - self.info(f"Created leaf dependency under root: {dep.id}") - elif i in [4, 5]: # Leaf conditions under nested group - # Update parent to nested group - dep.parent_id = nested_group.id - db.commit() - self.info(f"Created leaf dependency under nested group: {dep.id}") - - self.success(f"Created {len(self.conditional_dependencies)} ManualTaskConditionalDependencies") + dependency_data = create_dependency_data(self.manual_task.id) + dep = ManualTaskConditionalDependency.create(db=db, data=dependency_data) + self.conditional_dependencies.append(dep) + self.info(f"Created conditional dependency with full condition tree: {dep.id}") + + self.success("Created ManualTaskConditionalDependency with condition tree") return True except Exception as e: - self.error(f"Error creating ManualTaskConditionalDependencies: {e}") + self.error(f"Error creating ManualTaskConditionalDependency: {e}") return False def _create_policy(self) -> bool: diff --git a/tests/api/models/digest/test_condition_validation.py b/tests/api/models/digest/test_condition_validation.py deleted file mode 100644 index 09afd4ef7b..0000000000 --- a/tests/api/models/digest/test_condition_validation.py +++ /dev/null @@ -1,417 +0,0 @@ -"""Tests for DigestCondition validation logic.""" - -from typing import Any - -import pytest -from sqlalchemy.orm import Session - -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) -from fides.api.models.digest import DigestConfig -from fides.api.models.digest.conditional_dependencies import ( - DigestCondition, - DigestConditionType, -) -from fides.api.task.conditional_dependencies.schemas import ConditionLeaf, Operator - - -class TestDigestConditionValidation: - """Test DigestCondition validation for mixed condition types.""" - - def test_create_root_condition_no_validation(self, sample_conditions): - """Test that root conditions don't require validation.""" - # sample_conditions fixture creates root conditions of different types - receiver_condition, content_condition, priority_condition = sample_conditions - - # Verify they have different types (proving root conditions can be any type) - assert receiver_condition.digest_condition_type == DigestConditionType.RECEIVER - assert content_condition.digest_condition_type == DigestConditionType.CONTENT - assert priority_condition.digest_condition_type == DigestConditionType.PRIORITY - - # Verify they are all root conditions (no parent) - assert receiver_condition.parent_id is None - assert content_condition.parent_id is None - assert priority_condition.parent_id is None - - def test_create_child_condition_same_type_succeeds( - self, - db: Session, - digest_config: DigestConfig, - receiver_group: DigestCondition, - receiver_condition_leaf: ConditionLeaf, - ): - """Test that child conditions with same type as parent succeed.""" - parent = receiver_group - - child = DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - **receiver_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - assert child.digest_condition_type == DigestConditionType.RECEIVER - assert child.parent_id == parent.id - - def test_create_child_condition_different_type_fails( - self, - db: Session, - receiver_group: DigestCondition, - content_condition: dict[str, Any], - content_condition_leaf: ConditionLeaf, - ): - """Test that child conditions with different type from parent fail.""" - # Use receiver_group as parent - parent = receiver_group - - # Try to create child with different type - should fail - with pytest.raises( - ValueError, match="Cannot create condition with type 'content'" - ): - DigestCondition.create( - db=db, - data={ - **content_condition, - **content_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - def test_create_child_condition_nonexistent_parent_fails( - self, - db: Session, - receiver_condition: dict[str, Any], - receiver_condition_leaf: ConditionLeaf, - ): - """Test that creating child with nonexistent parent fails.""" - with pytest.raises( - ValueError, match="Parent condition with id 'nonexistent' does not exist" - ): - DigestCondition.create( - db=db, - data={ - **receiver_condition, - **receiver_condition_leaf.model_dump(), - "parent_id": "nonexistent", - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - def test_create_nested_conditions_same_type_succeeds(self, complex_condition_tree): - """Test that deeply nested conditions with same type succeed.""" - # complex_condition_tree fixture creates a nested structure with consistent types - root = complex_condition_tree["root"] - nested_groups = complex_condition_tree["nested_groups"] - leaves = complex_condition_tree["leaves"] - - # Verify all conditions have the same type (CONTENT) - assert root.digest_condition_type == DigestConditionType.CONTENT - for group in nested_groups: - assert group.digest_condition_type == DigestConditionType.CONTENT - for leaf in leaves: - assert leaf.digest_condition_type == DigestConditionType.CONTENT - - # Verify the tree structure is intact - assert root.parent_id is None - for group in nested_groups: - assert group.parent_id == root.id - # Each leaf should have one of the nested groups as parent - for leaf in leaves: - assert leaf.parent_id in [group.id for group in nested_groups] - - def test_update_condition_type_to_different_type_fails( - self, db: Session, complex_condition_tree - ): - """Test that updating a child condition to different type fails.""" - # Use a leaf from the complex tree (all are CONTENT type) - leaf = complex_condition_tree["leaves"][0] - - # Verify it's a child condition with CONTENT type - assert leaf.digest_condition_type == DigestConditionType.CONTENT - assert leaf.parent_id is not None - - # Try to update child to different type - should fail - with pytest.raises( - ValueError, match="Cannot create condition with type 'priority'" - ): - leaf.update( - db=db, data={"digest_condition_type": DigestConditionType.PRIORITY} - ) - - def test_update_condition_same_type_succeeds( - self, db: Session, complex_condition_tree - ): - """Test that updating condition with same type succeeds.""" - # Use a leaf from the complex tree - leaf = complex_condition_tree["leaves"][0] - original_type = leaf.digest_condition_type - - # Update other fields - should succeed - updated_leaf = leaf.update( - db=db, - data={"field_address": "task.updated_field", "value": "updated_value"}, - ) - assert updated_leaf.field_address == "task.updated_field" - assert updated_leaf.value == "updated_value" - assert updated_leaf.digest_condition_type == original_type # Type unchanged - - def test_update_root_condition_type_succeeds( - self, db: Session, receiver_digest_condition_leaf: DigestCondition - ): - """Test that updating root condition type succeeds (no parent to validate against).""" - # Use receiver_digest_condition_leaf as root condition (it has no parent) - root = receiver_digest_condition_leaf - assert root.parent_id is None # Verify it's a root condition - - # Update root condition type - should succeed (no parent to validate against) - updated_root = root.update( - db=db, - data={ - "digest_condition_type": DigestConditionType.CONTENT, - "field_address": "task.status", - "value": "pending", - }, - ) - assert updated_root.digest_condition_type == DigestConditionType.CONTENT - assert updated_root.field_address == "task.status" - - def test_validation_error_messages_are_descriptive( - self, - db: Session, - receiver_group: DigestCondition, - content_condition: dict[str, Any], - receiver_condition_leaf: ConditionLeaf, - ): - """Test that validation error messages are clear and helpful.""" - parent = receiver_group - - # Try to create child with different type - with pytest.raises(ValueError) as exc_info: - DigestCondition.create( - db=db, - data={ - **content_condition, - **receiver_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - error_message = str(exc_info.value) - assert "Cannot create condition with type 'content'" in error_message - assert "under parent with type 'receiver'" in error_message - assert "must have the same digest_condition_type" in error_message - - @pytest.mark.parametrize( - "condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) - def test_all_condition_types_can_be_validated( - self, - db: Session, - digest_config: DigestConfig, - condition_type: DigestConditionType, - group_condition_and: dict[str, Any], - ): - """Test validation works for all digest condition types.""" - - parent_condition = DigestCondition.create( - db=db, - data={ - **group_condition_and, - "digest_condition_type": condition_type, - "digest_config_id": digest_config.id, - }, - ) - - # Child with same type should succeed - child = DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": condition_type, - "parent_id": parent_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "test.field", - "operator": Operator.eq, - "value": "test_value", - "sort_order": 1, - }, - ) - assert child.digest_condition_type == condition_type - - # Child with different type should fail - different_types = [t for t in DigestConditionType if t != condition_type] - if different_types: # Only test if there are other types - with pytest.raises(ValueError): - DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": different_types[0], - "parent_id": parent_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "test.field2", - "operator": Operator.eq, - "value": "test_value2", - "sort_order": 2, - }, - ) - - # Clean up child for next iteration - parent_condition.delete(db) - - @pytest.mark.usefixtures("digest_config") - def test_deep_tree_validation_prevents_type_mixing( - self, - db: Session, - receiver_condition: dict[str, Any], - content_condition: dict[str, Any], - group_condition_and: dict[str, Any], - ): - """Test that validation works across multiple levels of nesting.""" - # Create root RECEIVER condition - root = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "sort_order": 0, - }, - ) - - # Create intermediate RECEIVER condition (child of root) - intermediate = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "parent_id": root.id, - "sort_order": 1, - }, - ) - - # Create another intermediate RECEIVER condition (grandchild of root) - deep_intermediate = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "parent_id": intermediate.id, - "sort_order": 1, - }, - ) - - # Now try to create a CONTENT condition as great-grandchild - # This should fail because the parent is RECEIVER type - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - DigestCondition.create( - db=db, - data={ - **content_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": deep_intermediate.id, - "field_address": "content.title", - "operator": Operator.eq, - "value": "test", - "sort_order": 1, - }, - ) - - # But creating a RECEIVER condition should work - valid_deep_child = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": deep_intermediate.id, - "field_address": "receiver.email", - "operator": Operator.eq, - "value": "test@example.com", - "sort_order": 1, - }, - ) - assert valid_deep_child.digest_condition_type == DigestConditionType.RECEIVER - - # Clean up - valid_deep_child.delete(db) - deep_intermediate.delete(db) - intermediate.delete(db) - root.delete(db) - - @pytest.mark.usefixtures("digest_config") - def test_validation_prevents_crud_that_breaks_tree_consistency( - self, - db: Session, - receiver_condition: dict[str, Any], - group_condition_and: dict[str, Any], - ): - """Test that updating a condition to different type fails if it breaks tree consistency.""" - # Create RECEIVER tree - root = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "sort_order": 0, - }, - ) - - # Create receiver leaf condition - child = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": root.id, - "field_address": "receiver.email", - "operator": Operator.eq, - "value": "test@example.com", - "sort_order": 1, - }, - ) - - # Try to update child to CONTENT type - should fail - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - child.update( - db=db, data={"digest_condition_type": DigestConditionType.CONTENT} - ) - - # Verify child is still RECEIVER type - db.refresh(child) - assert child.digest_condition_type == DigestConditionType.RECEIVER - - # Try to save - should fail validation - child.digest_condition_type = DigestConditionType.CONTENT - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - child.save(db) - - # Verify child's type was not saved to database - db.refresh(child) - assert child.digest_condition_type == DigestConditionType.RECEIVER - - # Clean up - child.delete(db) - root.delete(db) diff --git a/tests/api/models/digest/test_digest_config_conditions.py b/tests/api/models/digest/test_digest_config_conditions.py index 3e1cec1462..f8a42399e7 100644 --- a/tests/api/models/digest/test_digest_config_conditions.py +++ b/tests/api/models/digest/test_digest_config_conditions.py @@ -1,13 +1,8 @@ """Tests for DigestConfig condition-related methods.""" -from typing import Any - import pytest from sqlalchemy.orm import Session -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.digest import DigestConfig, DigestType from fides.api.models.digest.conditional_dependencies import ( DigestCondition, @@ -57,9 +52,6 @@ def test_get_type_specific_condition_leaf( data={ "digest_config_id": digest_config.id, "digest_condition_type": digest_condition_type, - **sample_exists_condition_leaf.model_dump(), - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, "condition_tree": sample_exists_condition_leaf.model_dump(), }, ) @@ -88,7 +80,6 @@ def test_get_type_specific_condition_group( db: Session, digest_config: DigestConfig, digest_condition_type: DigestConditionType, - group_condition_and: dict[str, Any], sample_exists_condition_leaf: ConditionLeaf, sample_eq_condition_leaf: ConditionLeaf, ): @@ -106,39 +97,12 @@ def test_get_type_specific_condition_group( root_group = DigestCondition.create( db=db, data={ - **group_condition_and, "digest_config_id": digest_config.id, "digest_condition_type": digest_condition_type, - "sort_order": 0, "condition_tree": condition_tree, }, ) - # Create child conditions (for backward compatibility/relationships) - DigestCondition.create( - db=db, - data={ - **sample_exists_condition_leaf.model_dump(), - "digest_config_id": digest_config.id, - "digest_condition_type": digest_condition_type, - "parent_id": root_group.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - DigestCondition.create( - db=db, - data={ - **sample_eq_condition_leaf.model_dump(), - "digest_config_id": digest_config.id, - "digest_condition_type": digest_condition_type, - "parent_id": root_group.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 2, - }, - ) - # Test getting receiver conditions as group db.refresh(digest_config) get_by_type = GET_BY_TYPE[digest_condition_type] @@ -297,11 +261,6 @@ def test_multiple_digest_configs_isolation(self, db: Session): data={ "digest_config_id": config1.id, "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "user.team", - "operator": Operator.eq, - "value": "alpha", - "sort_order": 1, "condition_tree": condition1_tree, }, ) @@ -317,11 +276,6 @@ def test_multiple_digest_configs_isolation(self, db: Session): data={ "digest_config_id": config2.id, "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "user.team", - "operator": Operator.eq, - "value": "beta", - "sort_order": 1, "condition_tree": condition2_tree, }, ) @@ -364,11 +318,6 @@ def test_condition_updates_reflected_immediately( data={ "digest_config_id": digest_config.id, "digest_condition_type": DigestConditionType.RECEIVER, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "user.status", - "operator": Operator.eq, - "value": "active", - "sort_order": 1, "condition_tree": condition_tree, }, ) @@ -379,13 +328,13 @@ def test_condition_updates_reflected_immediately( assert isinstance(receiver_conditions, ConditionLeaf) assert receiver_conditions.value == "active" - # Update the condition (including condition_tree) + # Update the condition_tree updated_tree = { "field_address": "user.status", "operator": "eq", "value": "verified", } - condition.update(db, data={"value": "verified", "condition_tree": updated_tree}) + condition.update(db, data={"condition_tree": updated_tree}) # Should reflect the update updated_receiver_conditions = digest_config.get_receiver_condition(db) @@ -423,43 +372,10 @@ def test_performance_with_large_condition_trees( data={ "digest_config_id": digest_config.id, "digest_condition_type": DigestConditionType.CONTENT, - "condition_type": ConditionalDependencyType.group, - "logical_operator": GroupOperator.or_, - "sort_order": 1, "condition_tree": condition_tree, }, ) - # Create multiple branches (for backward compatibility/relationships) - for branch_idx in range(3): - branch_group = DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.CONTENT, - "parent_id": root_group.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": GroupOperator.and_, - "sort_order": branch_idx + 1, - }, - ) - - # Create multiple leaves per branch - for leaf_idx in range(5): - DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.CONTENT, - "parent_id": branch_group.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": f"task.field_{branch_idx}_{leaf_idx}", - "operator": Operator.eq, - "value": f"value_{branch_idx}_{leaf_idx}", - "sort_order": leaf_idx + 1, - }, - ) - # Test that retrieval still works efficiently content_conditions = digest_config.get_content_condition(db) assert isinstance(content_conditions, ConditionGroup) diff --git a/tests/api/task/manual/test_manual_task_conditional_evaluation.py b/tests/api/task/manual/test_manual_task_conditional_evaluation.py index e5ea0c1580..825d7ce76c 100644 --- a/tests/api/task/manual/test_manual_task_conditional_evaluation.py +++ b/tests/api/task/manual/test_manual_task_conditional_evaluation.py @@ -39,11 +39,6 @@ def email_exists_conditional_dependency(db, manual_task): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "exists", - "value": None, - "sort_order": 1, "condition_tree": _email_exists_tree(), }, ) @@ -55,11 +50,6 @@ def city_eq_new_york_conditional_dependency(db, manual_task): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "postgres_example_test_dataset:address:city", - "operator": "eq", - "value": "New York", - "sort_order": 2, "condition_tree": _city_eq_new_york_tree(), }, ) @@ -74,39 +64,16 @@ def _input_group_tree(): @pytest.fixture -def input_group_conditional_dependency( - db, - manual_task, - email_exists_conditional_dependency, - city_eq_new_york_conditional_dependency, -): - dependency = ManualTaskConditionalDependency.create( +def input_group_conditional_dependency(db, manual_task): + """Create a single dependency with the full input group condition tree.""" + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "group", - "logical_operator": "and", - "sort_order": 1, "condition_tree": _input_group_tree(), }, ) - email_exists_conditional_dependency.parent = dependency - email_exists_conditional_dependency.sort_order = 3 - email_exists_conditional_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - city_eq_new_york_conditional_dependency.parent = dependency - city_eq_new_york_conditional_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - dependency.children = [ - email_exists_conditional_dependency, - city_eq_new_york_conditional_dependency, - ] - db.commit() - return dependency - def _privacy_request_location_tree(): """Return condition tree dict for privacy_request.location == New York.""" @@ -132,11 +99,6 @@ def privacy_request_location_dependency(db, manual_task): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "privacy_request.location", - "operator": "eq", - "value": "New York", - "sort_order": 1, "condition_tree": _privacy_request_location_tree(), }, ) @@ -148,11 +110,6 @@ def privacy_request_access_rule_dependency(db, manual_task): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "privacy_request.policy.has_access_rule", - "operator": "eq", - "value": True, - "sort_order": 1, "condition_tree": _privacy_request_access_rule_tree(), }, ) @@ -169,10 +126,6 @@ def privacy_request_policy_id_dependency(db, manual_task, policy): db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "privacy_request.policy.id", - "operator": "eq", - "value": policy.id, "condition_tree": condition_tree, }, ) @@ -190,36 +143,15 @@ def _privacy_request_group_tree(): @pytest.fixture -def privacy_request_group_conditional_dependency( - db, - manual_task, - privacy_request_location_dependency, - privacy_request_access_rule_dependency, -): - dependency = ManualTaskConditionalDependency.create( +def privacy_request_group_conditional_dependency(db, manual_task): + """Create a single dependency with the full privacy request group condition tree.""" + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "group", - "logical_operator": "and", - "sort_order": 1, "condition_tree": _privacy_request_group_tree(), }, ) - privacy_request_location_dependency.parent = dependency - privacy_request_location_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - privacy_request_access_rule_dependency.parent = dependency - privacy_request_access_rule_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - dependency.children = [ - privacy_request_location_dependency, - privacy_request_access_rule_dependency, - ] - db.commit() - return dependency def _full_group_tree(): @@ -231,36 +163,15 @@ def _full_group_tree(): @pytest.fixture -def group_conditional_dependency( - db, - manual_task, - input_group_conditional_dependency, - privacy_request_group_conditional_dependency, -): - dependency = ManualTaskConditionalDependency.create( +def group_conditional_dependency(db, manual_task): + """Create a single dependency with the full condition tree.""" + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "group", - "logical_operator": "and", - "sort_order": 1, "condition_tree": _full_group_tree(), }, ) - input_group_conditional_dependency.parent = dependency - input_group_conditional_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - privacy_request_group_conditional_dependency.parent = dependency - privacy_request_group_conditional_dependency.condition_tree = ( - None # Clear since parent has full tree - ) - dependency.children = [ - input_group_conditional_dependency, - privacy_request_group_conditional_dependency, - ] - db.commit() - return dependency class TestManualTaskConditionalDependencies: @@ -479,11 +390,6 @@ def test_extract_conditional_dependency_data_from_inputs_nested_field( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": "leaf", - "field_address": "postgres_example_test_dataset:customer:profile:preferences:theme", - "operator": "eq", - "value": "dark", - "sort_order": 1, "condition_tree": condition_tree, }, ) diff --git a/tests/api/task/manual/test_manual_task_graph_task.py b/tests/api/task/manual/test_manual_task_graph_task.py index 6259fc285e..3342a71e68 100644 --- a/tests/api/task/manual/test_manual_task_graph_task.py +++ b/tests/api/task/manual/test_manual_task_graph_task.py @@ -4,9 +4,6 @@ from pytest import param from fides.api.common_exceptions import AwaitingAsyncTask -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.manual_task import ( ManualTaskConfigurationType, ManualTaskFieldType, @@ -764,11 +761,6 @@ def test_manual_task_with_privacy_request_only_conditions( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": privacy_request_field_address, - "operator": "eq", - "value": privacy_request_value, - "sort_order": 1, "condition_tree": condition_tree, }, ) diff --git a/tests/api/task/manual/test_manual_task_utils.py b/tests/api/task/manual/test_manual_task_utils.py index 8b7be18262..883727a94b 100644 --- a/tests/api/task/manual/test_manual_task_utils.py +++ b/tests/api/task/manual/test_manual_task_utils.py @@ -6,9 +6,6 @@ from fides.api.common_exceptions import FidesopsException from fides.api.graph.config import CollectionAddress, FieldAddress, ScalarField -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.connectionconfig import ( AccessLevel, ConnectionConfig, @@ -408,11 +405,6 @@ def test_multiple_manual_tasks_get_separate_collections( db=db, data={ "manual_task_id": second_manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example:payment_card:billing.subscription.status", - "operator": "eq", - "value": "active", - "sort_order": 1, "condition_tree": condition_tree, }, ) diff --git a/tests/ops/service/privacy_request/test_manual_task_privacy_request.py b/tests/ops/service/privacy_request/test_manual_task_privacy_request.py index 6237fd2b6f..0e511c367f 100644 --- a/tests/ops/service/privacy_request/test_manual_task_privacy_request.py +++ b/tests/ops/service/privacy_request/test_manual_task_privacy_request.py @@ -7,9 +7,6 @@ from fides.api.graph.config import CollectionAddress from fides.api.graph.graph import DatasetGraph -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.connectionconfig import ( AccessLevel, ConnectionConfig, @@ -154,11 +151,6 @@ def conditional_name_exists( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "list_contains", - "value": "customer-1@example.com", # Use an email that actually exists in test data - "sort_order": 1, "condition_tree": condition_tree, }, ) @@ -178,11 +170,6 @@ def conditional_email_exists( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "exists", - "value": None, # exists operator doesn't need a value - "sort_order": 1, "condition_tree": condition_tree, }, ) @@ -481,43 +468,12 @@ def test_manual_task_with_conditional_dependencies( ], } - # Root group condition (OR) - root_dependency_or = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task_or.id, - "condition_type": ConditionalDependencyType.group, - "logical_operator": "or", - "sort_order": 1, - "condition_tree": condition_tree_or, - }, - ) - - # First condition: customer email exists (this will be True for any customer) + # Create conditional dependency with the full condition tree ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task_or.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "exists", - "value": None, # exists operator doesn't need a value - "sort_order": 2, - "parent_id": root_dependency_or.id, - }, - ) - - # Second condition: customer name contains 'customer' (this will be True for customer-1, customer-2) - ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task_or.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:name", - "operator": "list_contains", - "value": "customer", - "sort_order": 3, - "parent_id": root_dependency_or.id, + "condition_tree": condition_tree_or, }, ) @@ -1089,11 +1045,6 @@ def test_manual_task_conditional_dependencies_skip_execution( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "eq", - "value": "nonexistent@example.com", # Email that doesn't exist in test data - "sort_order": 1, "condition_tree": condition_tree, }, ) @@ -1196,11 +1147,6 @@ def test_manual_task_conditional_dependencies_execute_when_met( db=db, data={ "manual_task_id": manual_task.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "postgres_example_test_dataset:customer:email", - "operator": "exists", - "value": None, # exists operator doesn't need a value - "sort_order": 1, "condition_tree": condition_tree, }, ) From 41fd4a6f7000ae4de0dd87815afbe9bf8f89d1b2 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 16:33:05 +0000 Subject: [PATCH 05/12] more test updates --- tests/api/task/manual/conftest.py | 21 +++++++++ ...test_manual_task_conditional_evaluation.py | 46 +++++++++++++++---- .../manual/test_manual_task_integration.py | 10 ++-- .../api/task/manual/test_manual_task_utils.py | 4 +- 4 files changed, 64 insertions(+), 17 deletions(-) diff --git a/tests/api/task/manual/conftest.py b/tests/api/task/manual/conftest.py index aa95c76e51..54f5ff655a 100644 --- a/tests/api/task/manual/conftest.py +++ b/tests/api/task/manual/conftest.py @@ -1136,3 +1136,24 @@ def nested_group_condition(db: Session, manual_task: ManualTask): ) yield root_condition root_condition.delete(db) + + +@pytest.fixture() +def condition_age_range(db: Session, manual_task: ManualTask): + """Create a conditional dependency with age >= 18 AND age < 65 (same field, multiple conditions).""" + condition_tree = { + "logical_operator": "and", + "conditions": [ + create_condition_gt_18_tree(), + create_condition_age_lt_65_tree(), + ], + } + condition = ManualTaskConditionalDependency.create( + db=db, + data={ + "manual_task_id": manual_task.id, + "condition_tree": condition_tree, + }, + ) + yield condition + condition.delete(db) diff --git a/tests/api/task/manual/test_manual_task_conditional_evaluation.py b/tests/api/task/manual/test_manual_task_conditional_evaluation.py index 825d7ce76c..b6512abf6e 100644 --- a/tests/api/task/manual/test_manual_task_conditional_evaluation.py +++ b/tests/api/task/manual/test_manual_task_conditional_evaluation.py @@ -75,6 +75,42 @@ def input_group_conditional_dependency(db, manual_task): ) +@pytest.fixture +def email_and_city_conditional_dependency(db, manual_task): + """Create a single dependency with both email and city conditions.""" + return ManualTaskConditionalDependency.create( + db=db, + data={ + "manual_task_id": manual_task.id, + "condition_tree": _input_group_tree(), + }, + ) + + +def _email_and_privacy_request_tree(): + """Return condition tree with email exists AND privacy request conditions.""" + return { + "logical_operator": "and", + "conditions": [ + _email_exists_tree(), + _privacy_request_location_tree(), + _privacy_request_access_rule_tree(), + ], + } + + +@pytest.fixture +def email_and_privacy_request_conditional_dependency(db, manual_task): + """Create a single dependency with email and privacy request conditions.""" + return ManualTaskConditionalDependency.create( + db=db, + data={ + "manual_task_id": manual_task.id, + "condition_tree": _email_and_privacy_request_tree(), + }, + ) + + def _privacy_request_location_tree(): """Return condition tree dict for privacy_request.location == New York.""" return { @@ -273,11 +309,7 @@ def setup_method(self): self.CollectionAddress = CollectionAddress - @pytest.mark.usefixtures( - "email_exists_conditional_dependency", - "privacy_request_location_dependency", - "privacy_request_access_rule_dependency", - ) + @pytest.mark.usefixtures("email_and_privacy_request_conditional_dependency") def test_extract_conditional_dependency_data_from_inputs_and_privacy_request( self, manual_task_graph_task, db, manual_task, privacy_request ): @@ -444,9 +476,7 @@ def test_extract_conditional_dependency_data_from_inputs_missing_field( } assert result == expected - @pytest.mark.usefixtures( - "email_exists_conditional_dependency", "city_eq_new_york_conditional_dependency" - ) + @pytest.mark.usefixtures("email_and_city_conditional_dependency") def test_extract_conditional_dependency_data_from_inputs_multiple_collections( self, manual_task_graph_task, db, manual_task, privacy_request ): diff --git a/tests/api/task/manual/test_manual_task_integration.py b/tests/api/task/manual/test_manual_task_integration.py index 846ec02712..38a8a3512f 100644 --- a/tests/api/task/manual/test_manual_task_integration.py +++ b/tests/api/task/manual/test_manual_task_integration.py @@ -615,7 +615,7 @@ def test_disabled_connection_configs_filtered_from_artificial_graphs( class TestManualTaskTraversalExecution: """Test that manual task traversal executes correctly with conditional dependencies and upstream data""" - @pytest.mark.usefixtures("condition_gt_18", "condition_eq_active") + @pytest.mark.usefixtures("group_condition") def test_manual_task_traversal_with_conditional_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph ): @@ -738,9 +738,7 @@ def test_manual_task_traversal_with_nested_group_conditional_dependencies( class TestManualTaskUpstreamDataFlow: """Test that manual tasks receive and process upstream data correctly""" - @pytest.mark.usefixtures( - "condition_gt_18", "condition_eq_active", "privacy_request" - ) + @pytest.mark.usefixtures("group_condition", "privacy_request") def test_manual_task_receives_upstream_data_from_conditional_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph ): @@ -816,7 +814,7 @@ class TestManualTaskExecutionOrder: """Test that manual tasks execute in the correct order relative to their dependencies""" @pytest.mark.usefixtures( - "condition_gt_18", "condition_eq_active", "privacy_request" + "group_condition", "privacy_request" ) def test_manual_task_executes_after_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph @@ -881,7 +879,7 @@ class TestManualTaskTraversalIntegration: """Integration tests for manual task traversal with real graph execution""" @pytest.mark.usefixtures( - "condition_gt_18", "condition_eq_active", "privacy_request" + "group_condition", "privacy_request" ) def test_manual_task_traversal_integration_with_conditional_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph diff --git a/tests/api/task/manual/test_manual_task_utils.py b/tests/api/task/manual/test_manual_task_utils.py index 883727a94b..670a8c2fc4 100644 --- a/tests/api/task/manual/test_manual_task_utils.py +++ b/tests/api/task/manual/test_manual_task_utils.py @@ -337,9 +337,7 @@ def test_conditional_dependency_nested_group_fields_extraction( "postgres_example:payment_card:subscription.status" in field_names ) # from "payment_card.subscription.status" - @pytest.mark.usefixtures( - "connection_with_manual_access_task", "condition_gt_18", "condition_age_lt_65" - ) + @pytest.mark.usefixtures("connection_with_manual_access_task", "condition_age_range") def test_no_duplicate_fields_from_conditional_dependencies( self, db: Session, manual_task: ManualTask ): From 28ef656e3d4ab6ad4d77829a4b547db35ff8a975 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 30 Dec 2025 16:39:22 +0000 Subject: [PATCH 06/12] . --- tests/api/task/manual/test_manual_task_integration.py | 8 ++------ tests/api/task/manual/test_manual_task_utils.py | 4 +++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/api/task/manual/test_manual_task_integration.py b/tests/api/task/manual/test_manual_task_integration.py index 38a8a3512f..afabb2fc5e 100644 --- a/tests/api/task/manual/test_manual_task_integration.py +++ b/tests/api/task/manual/test_manual_task_integration.py @@ -813,9 +813,7 @@ def test_manual_task_upstream_data_with_group_conditions( class TestManualTaskExecutionOrder: """Test that manual tasks execute in the correct order relative to their dependencies""" - @pytest.mark.usefixtures( - "group_condition", "privacy_request" - ) + @pytest.mark.usefixtures("group_condition", "privacy_request") def test_manual_task_executes_after_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph ): @@ -878,9 +876,7 @@ def test_manual_task_execution_order_with_multiple_dependencies( class TestManualTaskTraversalIntegration: """Integration tests for manual task traversal with real graph execution""" - @pytest.mark.usefixtures( - "group_condition", "privacy_request" - ) + @pytest.mark.usefixtures("group_condition", "privacy_request") def test_manual_task_traversal_integration_with_conditional_dependencies( self, db, connection_with_manual_access_task, mock_dataset_graph ): diff --git a/tests/api/task/manual/test_manual_task_utils.py b/tests/api/task/manual/test_manual_task_utils.py index 670a8c2fc4..7e8181f8c1 100644 --- a/tests/api/task/manual/test_manual_task_utils.py +++ b/tests/api/task/manual/test_manual_task_utils.py @@ -337,7 +337,9 @@ def test_conditional_dependency_nested_group_fields_extraction( "postgres_example:payment_card:subscription.status" in field_names ) # from "payment_card.subscription.status" - @pytest.mark.usefixtures("connection_with_manual_access_task", "condition_age_range") + @pytest.mark.usefixtures( + "connection_with_manual_access_task", "condition_age_range" + ) def test_no_duplicate_fields_from_conditional_dependencies( self, db: Session, manual_task: ManualTask ): From 19ba7ec6f9ec46a55a3a23d78ba1499f61ddda4b Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 5 Jan 2026 21:13:26 +0000 Subject: [PATCH 07/12] clean up --- tests/api/models/digest/conftest.py | 20 +--- .../digest/test_conditional_dependencies.py | 103 +++++++----------- .../digest/test_digest_config_conditions.py | 42 ++----- 3 files changed, 48 insertions(+), 117 deletions(-) diff --git a/tests/api/models/digest/conftest.py b/tests/api/models/digest/conftest.py index 919bf63cca..2736812f07 100644 --- a/tests/api/models/digest/conftest.py +++ b/tests/api/models/digest/conftest.py @@ -56,7 +56,7 @@ def priority_condition(digest_config: DigestConfig) -> dict[str, Any]: @pytest.fixture def digest_config(db: Session) -> DigestConfig: """Create a test digest configuration.""" - config = DigestConfig.create( + return DigestConfig.create( db=db, data={ "digest_type": DigestType.MANUAL_TASKS, @@ -65,8 +65,6 @@ def digest_config(db: Session) -> DigestConfig: "enabled": True, }, ) - yield config - config.delete(db) # ============================================================================ @@ -167,7 +165,7 @@ def receiver_digest_condition_leaf( receiver_condition_leaf: ConditionLeaf, ) -> DigestCondition: """Create a receiver condition in the database.""" - condition = DigestCondition.create( + return DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, @@ -175,8 +173,6 @@ def receiver_digest_condition_leaf( "condition_tree": receiver_condition_leaf.model_dump(), }, ) - yield condition - condition.delete(db) @pytest.fixture @@ -186,7 +182,7 @@ def priority_digest_condition_leaf( priority_condition_leaf: ConditionLeaf, ) -> DigestCondition: """Create a priority condition in the database.""" - condition = DigestCondition.create( + return DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, @@ -194,8 +190,6 @@ def priority_digest_condition_leaf( "condition_tree": priority_condition_leaf.model_dump(), }, ) - yield condition - condition.delete(db) @pytest.fixture @@ -242,16 +236,10 @@ def complex_condition_tree( } # Create root condition with full tree - root_group = DigestCondition.create( + return DigestCondition.create( db=db, data={ **content_condition, "condition_tree": condition_tree, }, ) - - yield { - "root": root_group, - "condition_tree": condition_tree, - } - root_group.delete(db) diff --git a/tests/api/models/digest/test_conditional_dependencies.py b/tests/api/models/digest/test_conditional_dependencies.py index 27b2a65f72..5613ecd31a 100644 --- a/tests/api/models/digest/test_conditional_dependencies.py +++ b/tests/api/models/digest/test_conditional_dependencies.py @@ -50,14 +50,13 @@ def test_invalid_condition_type(self): class TestDigestConditionCRUD: """Test basic CRUD operations for DigestCondition.""" - @pytest.mark.parametrize( - "digest_condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) + DIGEST_CONDITION_TYPES = [ + DigestConditionType.RECEIVER, + DigestConditionType.CONTENT, + DigestConditionType.PRIORITY, + ] + + @pytest.mark.parametrize("digest_condition_type", DIGEST_CONDITION_TYPES) def test_create_leaf_condition_types( self, db: Session, @@ -66,33 +65,24 @@ def test_create_leaf_condition_types( sample_eq_condition_leaf: ConditionLeaf, ): """Test creating a leaf condition for different types.""" - condition_tree = sample_eq_condition_leaf.model_dump() condition = DigestCondition.create( db=db, data={ "digest_config_id": digest_config.id, "digest_condition_type": digest_condition_type, - "condition_tree": condition_tree, + "condition_tree": sample_eq_condition_leaf.model_dump(), }, ) assert condition.id is not None assert condition.digest_config_id == digest_config.id assert condition.digest_condition_type == digest_condition_type - assert condition.condition_tree == condition_tree + assert condition.condition_tree == sample_eq_condition_leaf.model_dump() # Test relationship assert condition.digest_config == digest_config - condition.delete(db) - @pytest.mark.parametrize( - "digest_condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) + @pytest.mark.parametrize("digest_condition_type", DIGEST_CONDITION_TYPES) @pytest.mark.parametrize( "logical_operator", [GroupOperator.or_, GroupOperator.and_] ) @@ -120,36 +110,29 @@ def test_create_group_condition_types( assert condition.digest_condition_type == digest_condition_type assert condition.condition_tree["logical_operator"] == logical_operator - condition.delete(db) def test_required_fields_validation(self, db: Session, digest_config: DigestConfig): """Test that required fields are validated.""" + data = { + "condition_tree": { + "field_address": "test", + "operator": "eq", + "value": 1, + }, + } # Missing digest_config_id with pytest.raises(ConditionalDependencyError): DigestCondition.create( db=db, - data={ - "digest_condition_type": DigestConditionType.RECEIVER, - "condition_tree": { - "field_address": "test", - "operator": "eq", - "value": 1, - }, - }, + data=data.update( + {"digest_condition_type": DigestConditionType.RECEIVER} + ), ) - # Missing digest_condition_type with pytest.raises(ConditionalDependencyError): DigestCondition.create( db=db, - data={ - "digest_config_id": digest_config.id, - "condition_tree": { - "field_address": "test", - "operator": "eq", - "value": 1, - }, - }, + data=data.update({"digest_config_id": digest_config.id}), ) def test_cascade_delete_with_digest_config( @@ -184,27 +167,23 @@ def test_unique_constraint_per_config_and_type( sample_eq_condition_leaf: ConditionLeaf, ): """Test that only one condition per (digest_config_id, digest_condition_type) is allowed.""" - condition_tree = sample_eq_condition_leaf.model_dump() + data = { + "digest_config_id": digest_config.id, + "digest_condition_type": DigestConditionType.RECEIVER, + "condition_tree": sample_eq_condition_leaf.model_dump(), + } # Create first condition DigestCondition.create( db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - "condition_tree": condition_tree, - }, + data=data, ) # Try to create second condition with same config and type with pytest.raises(Exception): # Should raise unique constraint violation DigestCondition.create( db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - "condition_tree": condition_tree, - }, + data=data, ) @@ -232,7 +211,7 @@ def test_create_nested_condition_tree( ], } - root_group = DigestCondition.create( + group = DigestCondition.create( db=db, data={ **content_condition, @@ -241,36 +220,28 @@ def test_create_nested_condition_tree( ) # Verify tree structure - assert root_group.condition_tree["logical_operator"] == GroupOperator.and_ - assert len(root_group.condition_tree["conditions"]) == 2 - assert ( - root_group.condition_tree["conditions"][0]["field_address"] == "task.status" - ) - assert ( - root_group.condition_tree["conditions"][1]["field_address"] - == "task.priority" - ) - - root_group.delete(db) + assert group.condition_tree["logical_operator"] == GroupOperator.and_ + assert len(group.condition_tree["conditions"]) == 2 + assert group.condition_tree["conditions"][0]["field_address"] == "task.status" + assert group.condition_tree["conditions"][1]["field_address"] == "task.priority" def test_complex_nested_tree(self, complex_condition_tree: dict[str, Any]): """Test creating complex nested group condition trees.""" - root_group = complex_condition_tree["root"] tree = complex_condition_tree["condition_tree"] # Verify root structure - assert root_group.condition_tree["logical_operator"] == GroupOperator.or_ - assert len(root_group.condition_tree["conditions"]) == 2 + assert tree["logical_operator"] == GroupOperator.or_ + assert len(tree["conditions"]) == 2 # Verify first nested group - first_group = root_group.condition_tree["conditions"][0] + first_group = tree["conditions"][0] assert first_group["logical_operator"] == GroupOperator.and_ assert len(first_group["conditions"]) == 2 assert first_group["conditions"][0]["field_address"] == "task.assignee" assert first_group["conditions"][1]["field_address"] == "task.due_date" # Verify second nested group - second_group = root_group.condition_tree["conditions"][1] + second_group = tree["conditions"][1] assert second_group["logical_operator"] == GroupOperator.and_ assert len(second_group["conditions"]) == 2 assert second_group["conditions"][0]["field_address"] == "task.category" diff --git a/tests/api/models/digest/test_digest_config_conditions.py b/tests/api/models/digest/test_digest_config_conditions.py index f8a42399e7..bacb751437 100644 --- a/tests/api/models/digest/test_digest_config_conditions.py +++ b/tests/api/models/digest/test_digest_config_conditions.py @@ -29,14 +29,7 @@ class TestDigestConfigConditionMethods: """Test DigestConfig methods for retrieving conditions.""" - @pytest.mark.parametrize( - "digest_condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) + @pytest.mark.parametrize("digest_condition_type", list(GET_BY_TYPE.keys())) def test_get_type_specific_condition_leaf( self, db: Session, @@ -67,14 +60,7 @@ def test_get_type_specific_condition_leaf( assert conditions.value == None condition.delete(db) - @pytest.mark.parametrize( - "digest_condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) + @pytest.mark.parametrize("digest_condition_type", list(GET_BY_TYPE.keys())) def test_get_type_specific_condition_group( self, db: Session, @@ -125,14 +111,7 @@ def test_get_type_specific_condition_group( ) root_group.delete(db) - @pytest.mark.parametrize( - "digest_condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) + @pytest.mark.parametrize("digest_condition_type", list(GET_BY_TYPE.keys())) def test_get_type_specific_condition_none( self, db: Session, @@ -152,12 +131,11 @@ def test_get_all_conditions_complete( # Test getting all conditions all_conditions = digest_config.get_all_conditions(db) - assert len(all_conditions) == 3 - assert DigestConditionType.RECEIVER in all_conditions - assert DigestConditionType.CONTENT in all_conditions - assert DigestConditionType.PRIORITY in all_conditions + expected_conditions = sorted(list(GET_BY_TYPE.keys())) + assert len(all_conditions) == len(expected_conditions) + assert sorted(all_conditions) == expected_conditions - # Test receiver condition + # Test conditions receiver = all_conditions[DigestConditionType.RECEIVER] assert isinstance(receiver, ConditionLeaf) assert receiver.field_address == "user.email" @@ -296,10 +274,6 @@ def test_multiple_digest_configs_isolation(self, db: Session): assert config1_all[DigestConditionType.RECEIVER].value == "alpha" assert config2_all[DigestConditionType.RECEIVER].value == "beta" - # Clean up - config1.delete(db) - config2.delete(db) - def test_condition_updates_reflected_immediately( self, db: Session, digest_config: DigestConfig ): @@ -389,5 +363,3 @@ def test_performance_with_large_condition_trees( assert isinstance(leaf, ConditionLeaf) assert leaf.field_address == f"task.field_{i}_{j}" assert leaf.value == f"value_{i}_{j}" - - root_group.delete(db) From e1f34c6603a3ac96b56c53e70a20b6987dbbb70f Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Mon, 5 Jan 2026 21:48:43 +0000 Subject: [PATCH 08/12] . --- tests/api/models/manual_task/conftest.py | 69 +----- ...st_manual_task_conditional_dependencies.py | 69 ++---- .../manual_task/test_manual_task_instance.py | 74 +++--- tests/api/task/manual/conftest.py | 220 +++--------------- 4 files changed, 100 insertions(+), 332 deletions(-) diff --git a/tests/api/models/manual_task/conftest.py b/tests/api/models/manual_task/conftest.py index 6f6e4ae67e..8ca02fd63f 100644 --- a/tests/api/models/manual_task/conftest.py +++ b/tests/api/models/manual_task/conftest.py @@ -22,7 +22,7 @@ def manual_task( db: Session, connection_config: ConnectionConfig ) -> Generator[ManualTask, None, None]: """Create a test manual task.""" - task = ManualTask.create( + return ManualTask.create( db=db, data={ "task_type": ManualTaskType.privacy_request, @@ -30,11 +30,6 @@ def manual_task( "parent_entity_type": ManualTaskParentEntityType.connection_config, }, ) - yield task - try: - task.delete(db) - except Exception as e: - pass @pytest.fixture @@ -51,10 +46,10 @@ def manual_task_config(db: Session, manual_task: ManualTask) -> ManualTaskConfig @pytest.fixture -def manual_task_config_field_1( +def manual_task_config_field( db: Session, manual_task_config: ManualTaskConfig ) -> Generator[ManualTaskConfigField, None, None]: - field = ManualTaskConfigField.create( + return ManualTaskConfigField.create( db, data={ "task_id": manual_task_config.task_id, @@ -66,45 +61,6 @@ def manual_task_config_field_1( }, }, ) - yield field - - # Clean up submissions first if they exist and field still exists - try: - if db.query(ManualTaskConfigField).filter_by(id=field.id).first(): - from fides.api.models.manual_task import ManualTaskSubmission - - for submission in field.submissions: - if db.query(ManualTaskSubmission).filter_by(id=submission.id).first(): - submission.delete(db) - db.commit() - - # Now delete the field if it still exists - if db.query(ManualTaskConfigField).filter_by(id=field.id).first(): - db.delete(field) - db.commit() - except Exception: - # Field may have been cascade deleted, which is fine - pass - - -@pytest.fixture -def manual_task_config_field_2( - db: Session, manual_task_config: ManualTaskConfig -) -> Generator[ManualTaskConfigField, None, None]: - field = ManualTaskConfigField.create( - db, - data={ - "task_id": manual_task_config.task_id, - "config_id": manual_task_config.id, - "field_type": "text", - "field_key": "field_2", - "field_metadata": { - "required": False, - }, - }, - ) - yield field - field.delete(db) @pytest.fixture @@ -115,7 +71,7 @@ def manual_task_instance( privacy_request: PrivacyRequest, ) -> Generator[ManualTaskInstance, None, None]: """Create a test manual task instance.""" - instance = ManualTaskInstance.create( + return ManualTaskInstance.create( db=db, data={ "task_id": manual_task.id, @@ -124,15 +80,6 @@ def manual_task_instance( "entity_type": "privacy_request", }, ) - yield instance - # Only try to delete if instance still exists (may have been cascade deleted) - try: - if db.query(ManualTaskInstance).filter_by(id=instance.id).first(): - instance.delete(db) - db.commit() - except Exception: - # Instance may have been cascade deleted, which is fine - pass @pytest.fixture @@ -141,20 +88,18 @@ def manual_task_submission( manual_task: ManualTask, manual_task_config: ManualTaskConfig, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ) -> Generator[ManualTaskSubmission, None, None]: """Create a test manual task submission.""" - submission = ManualTaskSubmission.create( + return ManualTaskSubmission.create( db=db, data={ "task_id": manual_task.id, "config_id": manual_task_config.id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, }, ) - yield submission - submission.delete(db) diff --git a/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py b/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py index 4313e022a0..c26d6444d7 100644 --- a/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py +++ b/tests/api/models/manual_task/test_manual_task_conditional_dependencies.py @@ -21,12 +21,12 @@ def sample_condition_leaf() -> ConditionLeaf: @pytest.fixture -def sample_condition_group() -> ConditionGroup: +def sample_condition_group(sample_condition_leaf: ConditionLeaf) -> ConditionGroup: """Create a sample group condition""" return ConditionGroup( logical_operator=GroupOperator.and_, conditions=[ - ConditionLeaf(field_address="user.age", operator=Operator.gte, value=18), + sample_condition_leaf, ConditionLeaf( field_address="user.active", operator=Operator.eq, value=True ), @@ -41,20 +41,25 @@ def test_manual_task_conditional_dependency_creation_with_leaf_tree( self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf ): """Test creating a conditional dependency with a leaf condition tree""" - condition_tree = sample_condition_leaf.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": condition_tree, + "condition_tree": sample_condition_leaf.model_dump(), }, ) assert dependency.id is not None - assert dependency.manual_task_id == manual_task.id - assert dependency.condition_tree == condition_tree - assert dependency.created_at is not None - assert dependency.updated_at is not None + + # Test the relationship from dependency to task + assert dependency.task.id == manual_task.id + + # Test the relationship from task to dependencies + db.refresh(manual_task) + assert len(manual_task.conditional_dependencies) == 1 + assert manual_task.conditional_dependencies[0].id == dependency.id + + assert dependency.condition_tree == sample_condition_leaf.model_dump() def test_manual_task_conditional_dependency_with_group_tree( self, @@ -63,16 +68,15 @@ def test_manual_task_conditional_dependency_with_group_tree( sample_condition_group: ConditionGroup, ): """Test creating a conditional dependency with a group condition tree""" - condition_tree = sample_condition_group.model_dump() dependency = ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": condition_tree, + "condition_tree": sample_condition_group.model_dump(), }, ) - assert dependency.condition_tree == condition_tree + assert dependency.condition_tree == sample_condition_group.model_dump() assert dependency.condition_tree["logical_operator"] == GroupOperator.and_ assert len(dependency.condition_tree["conditions"]) == 2 @@ -80,27 +84,6 @@ def test_manual_task_conditional_dependency_with_group_tree( class TestManualTaskConditionalDependencyRelationships: """Test relationships and foreign key constraints""" - def test_manual_task_conditional_dependency_relationship( - self, db: Session, manual_task: ManualTask, sample_condition_leaf: ConditionLeaf - ): - """Test the relationship between ManualTask and ManualTaskConditionalDependency""" - condition_tree = sample_condition_leaf.model_dump() - dependency = ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_tree": condition_tree, - }, - ) - - # Test the relationship from dependency to task - assert dependency.task.id == manual_task.id - - # Test the relationship from task to dependencies - db.refresh(manual_task) - assert len(manual_task.conditional_dependencies) == 1 - assert manual_task.conditional_dependencies[0].id == dependency.id - def test_manual_task_conditional_dependency_foreign_key_constraint( self, db: Session, sample_condition_leaf: ConditionLeaf ): @@ -204,9 +187,7 @@ def test_get_root_condition_leaf( # Verify the result assert isinstance(root_condition, ConditionLeaf) - assert root_condition.field_address == sample_condition_leaf.field_address - assert root_condition.operator == sample_condition_leaf.operator - assert root_condition.value == sample_condition_leaf.value + assert root_condition.model_dump() == sample_condition_leaf.model_dump() def test_get_root_condition_group( self, @@ -231,10 +212,7 @@ def test_get_root_condition_group( # Verify the result assert isinstance(root_condition, ConditionGroup) - assert root_condition.logical_operator == GroupOperator.and_ - assert len(root_condition.conditions) == 2 - assert isinstance(root_condition.conditions[0], ConditionLeaf) - assert root_condition.conditions[0].field_address == "user.age" + assert root_condition.model_dump() == sample_condition_group.model_dump() def test_get_root_condition_none(self, db: Session, manual_task: ManualTask): """Test getting root condition when none exists""" @@ -280,18 +258,7 @@ def test_get_root_condition_complex_nested( # Verify the result assert isinstance(root_condition, ConditionGroup) - assert root_condition.logical_operator == GroupOperator.and_ - assert len(root_condition.conditions) == 2 - - # First condition is a nested group - nested_group = root_condition.conditions[0] - assert isinstance(nested_group, ConditionGroup) - assert nested_group.logical_operator == GroupOperator.or_ - assert len(nested_group.conditions) == 2 - - # Second condition is a leaf - assert isinstance(root_condition.conditions[1], ConditionLeaf) - assert root_condition.conditions[1].field_address == "user.active" + assert root_condition.model_dump() == condition_tree def test_get_root_condition_missing_manual_task_id(self, db: Session): """Test that get_root_condition raises error when manual_task_id is missing""" diff --git a/tests/api/models/manual_task/test_manual_task_instance.py b/tests/api/models/manual_task/test_manual_task_instance.py index eb07c6124d..5374517e34 100644 --- a/tests/api/models/manual_task/test_manual_task_instance.py +++ b/tests/api/models/manual_task/test_manual_task_instance.py @@ -66,38 +66,38 @@ def test_required_fields( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, ): """Test getting required fields.""" # Update the field to be required - manual_task_config_field_1.field_metadata = {"required": True} + manual_task_config_field.field_metadata = {"required": True} db.commit() required_fields = manual_task_instance.required_fields assert len(required_fields) == 1 - assert required_fields[0].id == manual_task_config_field_1.id + assert required_fields[0].id == manual_task_config_field.id assert required_fields[0].field_metadata["required"] is True def test_incomplete_fields( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, ): """Test getting incomplete fields.""" # Update the field to be required - manual_task_config_field_1.field_metadata = {"required": True} + manual_task_config_field.field_metadata = {"required": True} db.commit() incomplete_fields = manual_task_instance.incomplete_fields assert len(incomplete_fields) == 1 - assert incomplete_fields[0].id == manual_task_config_field_1.id + assert incomplete_fields[0].id == manual_task_config_field.id def test_completed_fields( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test getting completed fields.""" @@ -110,7 +110,7 @@ def test_completed_fields( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -120,14 +120,14 @@ def test_completed_fields( # Now field1 should be completed completed_fields = manual_task_instance.completed_fields assert len(completed_fields) == 1 - assert completed_fields[0].id == manual_task_config_field_1.id + assert completed_fields[0].id == manual_task_config_field.id @pytest.mark.usefixtures("mock_s3_client", "s3_client") def test_attachments( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, attachment_data: dict[str, Any], user: FidesUser, ): @@ -138,7 +138,7 @@ def test_attachments( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -191,7 +191,7 @@ def test_submissions_relationship_auto_sync( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that submissions relationship automatically syncs without manual expire/refresh.""" @@ -204,7 +204,7 @@ def test_submissions_relationship_auto_sync( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test1"}, @@ -221,7 +221,7 @@ def test_submissions_relationship_auto_sync( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test2"}, @@ -246,7 +246,7 @@ def test_attachments_relationship_auto_sync( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, attachment_data: dict[str, Any], user: FidesUser, mock_s3_client, @@ -258,7 +258,7 @@ def test_attachments_relationship_auto_sync( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -295,7 +295,7 @@ def test_bidirectional_relationship_consistency( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that bidirectional relationships stay consistent automatically.""" @@ -305,7 +305,7 @@ def test_bidirectional_relationship_consistency( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -382,7 +382,7 @@ def test_create_manual_task_submission( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test creating a manual task submission.""" @@ -391,7 +391,7 @@ def test_create_manual_task_submission( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -400,7 +400,7 @@ def test_create_manual_task_submission( assert submission.task_id == manual_task_instance.task_id assert submission.config_id == manual_task_instance.config_id - assert submission.field_id == manual_task_config_field_1.id + assert submission.field_id == manual_task_config_field.id assert submission.instance_id == manual_task_instance.id assert submission.submitted_by == user.id assert submission.data == {"value": "test"} @@ -478,7 +478,7 @@ def test_submission_data_validation( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test submission data validation.""" @@ -488,7 +488,7 @@ def test_submission_data_validation( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -503,7 +503,7 @@ def test_submission_data_validation( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {}, @@ -514,7 +514,7 @@ def test_submission_cascade_delete( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that submissions are deleted when instance is deleted.""" @@ -524,7 +524,7 @@ def test_submission_cascade_delete( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -545,7 +545,7 @@ def test_submission_timestamps( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test submission timestamp handling.""" @@ -555,7 +555,7 @@ def test_submission_timestamps( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -677,7 +677,7 @@ def test_cascade_delete_with_attachments( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, attachment_data: dict[str, Any], user: FidesUser, mock_s3_client, @@ -689,7 +689,7 @@ def test_cascade_delete_with_attachments( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -815,7 +815,7 @@ def test_relationship_consistency_across_transactions( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that relationships remain consistent across separate transactions.""" @@ -829,7 +829,7 @@ def test_relationship_consistency_across_transactions( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, @@ -855,7 +855,7 @@ def test_cascade_deletion_behavior( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that cascade deletion properly removes related records.""" @@ -865,7 +865,7 @@ def test_cascade_deletion_behavior( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test1"}, @@ -877,7 +877,7 @@ def test_cascade_deletion_behavior( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test2"}, @@ -912,7 +912,7 @@ def test_relationship_loading_behavior( self, db: Session, manual_task_instance: ManualTaskInstance, - manual_task_config_field_1: ManualTaskConfigField, + manual_task_config_field: ManualTaskConfigField, user: FidesUser, ): """Test that relationships load fresh data when accessed after changes.""" @@ -922,7 +922,7 @@ def test_relationship_loading_behavior( data={ "task_id": manual_task_instance.task_id, "config_id": manual_task_instance.config_id, - "field_id": manual_task_config_field_1.id, + "field_id": manual_task_config_field.id, "instance_id": manual_task_instance.id, "submitted_by": user.id, "data": {"value": "test"}, diff --git a/tests/api/task/manual/conftest.py b/tests/api/task/manual/conftest.py index 54f5ff655a..0a47542981 100644 --- a/tests/api/task/manual/conftest.py +++ b/tests/api/task/manual/conftest.py @@ -264,60 +264,6 @@ def connection_with_manual_access_task( yield connection_config, manual_task, manual_task_access_config, field -@pytest.fixture() -def manual_setup( - db: Session, - connection_config, - manual_task, - manual_task_access_config, - manual_task_erasure_config, -): - """Create a connection config, manual task and two configs (access & erasure).""" - # Create access config field - access_field_data = { - "field_key": "user_email", - "field_type": ManualTaskFieldType.text, - "field_metadata": { - "label": "Access Confirmation", - "required": True, - "data_categories": ["user.contact.email"], - }, - } - _create_manual_task_config_field( - db, manual_task, manual_task_access_config, access_field_data - ) - - # Create erasure config field - erasure_field_data = { - "field_key": "confirm_erasure", - "field_type": ManualTaskFieldType.text, - "field_metadata": { - "label": "Erasure Confirmation", - "required": True, - "data_categories": ["user.contact.email"], - }, - } - _create_manual_task_config_field( - db, manual_task, manual_task_erasure_config, erasure_field_data - ) - - yield { - "connection_config": connection_config, - "manual_task": manual_task, - "access_config": manual_task_access_config, - "erasure_config": manual_task_erasure_config, - } - - # Cleanup - try: - for config in [manual_task_access_config, manual_task_erasure_config]: - config.delete(db) - manual_task.delete(db) - connection_config.delete(db) - except Exception as e: - print(f"Error deleting manual task: {e}") - - # ============================================================================= # Privacy Request Fixtures # ============================================================================= @@ -326,7 +272,7 @@ def manual_setup( @pytest.fixture() def mixed_privacy_request(db: Session, mixed_policy): """Minimal PrivacyRequest for testing with mixed policy.""" - pr = PrivacyRequest.create( + return PrivacyRequest.create( db=db, data={ "requested_at": datetime.utcnow(), @@ -334,14 +280,12 @@ def mixed_privacy_request(db: Session, mixed_policy): "status": PrivacyRequestStatus.pending, }, ) - yield pr - pr.delete(db) @pytest.fixture() def access_privacy_request(db: Session, access_policy): """Privacy request with access-only policy.""" - pr = PrivacyRequest.create( + return PrivacyRequest.create( db=db, data={ "requested_at": datetime.utcnow(), @@ -349,14 +293,12 @@ def access_privacy_request(db: Session, access_policy): "status": PrivacyRequestStatus.pending, }, ) - yield pr - pr.delete(db) @pytest.fixture() def erasure_privacy_request(db: Session, erasure_policy): """Privacy request with erasure-only policy.""" - pr = PrivacyRequest.create( + return PrivacyRequest.create( db=db, data={ "requested_at": datetime.utcnow(), @@ -364,8 +306,6 @@ def erasure_privacy_request(db: Session, erasure_policy): "status": PrivacyRequestStatus.pending, }, ) - yield pr - pr.delete(db) # ============================================================================= @@ -817,28 +757,6 @@ def complete_manual_task_setup( } -@pytest.fixture() -def complete_manual_task_setup_with_attachment( - db: Session, - connection_with_manual_access_task, - manual_task_instance, - manual_task_submission_attachment, - attachment_for_access_package, -): - """Create a complete manual task setup with instance, submission, and attachment.""" - connection_config, manual_task, config, field = connection_with_manual_access_task - - return { - "connection_config": connection_config, - "manual_task": manual_task, - "config": config, - "field": field, - "instance": manual_task_instance, - "submission": manual_task_submission_attachment, - "attachment": attachment_for_access_package, - } - - # ============================================================================= # Manual Task Conditional Dependency Fixtures # ============================================================================= @@ -985,17 +903,6 @@ def create_condition_gt_18_tree() -> dict: } -def create_condition_gt_18(db: Session, manual_task: ManualTask): - """Create a conditional dependency for age >= 18.""" - return ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_tree": create_condition_gt_18_tree(), - }, - ) - - def create_condition_age_lt_65_tree() -> dict: """Return the condition tree dict for age < 65.""" return { @@ -1025,17 +932,6 @@ def create_condition_eq_active_tree() -> dict: } -def create_condition_eq_active(db: Session, manual_task: ManualTask): - """Create a conditional dependency for status == active.""" - return ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_tree": create_condition_eq_active_tree(), - }, - ) - - def create_condition_eq_admin_tree() -> dict: """Return the condition tree dict for role == admin.""" return { @@ -1045,115 +941,75 @@ def create_condition_eq_admin_tree() -> dict: } -def create_condition_eq_admin(db: Session, manual_task: ManualTask): - """Create a conditional dependency for role == admin.""" +@pytest.fixture() +def condition_gt_18(db: Session, manual_task: ManualTask): + """Create a conditional dependency with field_address 'user.age' and operator 'gte' and value 18""" return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": create_condition_eq_admin_tree(), + "condition_tree": create_condition_gt_18_tree(), }, ) -@pytest.fixture() -def condition_gt_18(db: Session, manual_task: ManualTask): - """Create a conditional dependency with field_address 'user.age' and operator 'gte' and value 18""" - condition = create_condition_gt_18(db, manual_task) - yield condition - condition.delete(db) - - -@pytest.fixture() -def condition_age_lt_65(db: Session, manual_task: ManualTask): - """Create a conditional dependency with field_address 'user.age' and operator 'lt' and value 65""" - condition = create_condition_age_lt_65(db, manual_task) - yield condition - condition.delete(db) - - -@pytest.fixture() -def condition_eq_active(db: Session, manual_task: ManualTask): - """Create a conditional dependency with field_address 'billing.subscription.status' and operator 'eq' and value 'active'""" - condition = create_condition_eq_active(db, manual_task) - yield condition - condition.delete(db) - - -@pytest.fixture() -def condition_eq_admin(db: Session, manual_task: ManualTask): - """Create a conditional dependency with field_address 'user.role' and operator 'eq' and value 'admin'""" - condition = create_condition_eq_admin(db, manual_task) - yield condition - condition.delete(db) - - @pytest.fixture() def group_condition(db: Session, manual_task: ManualTask): """Create a group conditional dependency with logical_operator 'and'""" # Build the full condition tree for JSONB storage - condition_tree = { - "logical_operator": "and", - "conditions": [ - create_condition_gt_18_tree(), - create_condition_eq_active_tree(), - ], - } - root_condition = ManualTaskConditionalDependency.create( + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": condition_tree, + "condition_tree": { + "logical_operator": "and", + "conditions": [ + create_condition_gt_18_tree(), + create_condition_eq_active_tree(), + ], + }, }, ) - yield root_condition - root_condition.delete(db) @pytest.fixture() def nested_group_condition(db: Session, manual_task: ManualTask): - """Create a nested group conditional dependency with logical_operator 'or'""" + """Create a nested group conditional dependency: (age >= 18 OR status == active) AND role == admin""" # Build the full condition tree for JSONB storage - condition_tree = { - "logical_operator": "and", - "conditions": [ - { - "logical_operator": "or", - "conditions": [ - create_condition_gt_18_tree(), - create_condition_eq_active_tree(), - create_condition_eq_admin_tree(), - ], - } - ], - } - root_condition = ManualTaskConditionalDependency.create( + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": condition_tree, + "condition_tree": { + "logical_operator": "and", + "conditions": [ + { + "logical_operator": "or", + "conditions": [ + create_condition_gt_18_tree(), + create_condition_eq_active_tree(), + ], + }, + create_condition_eq_admin_tree(), + ], + }, }, ) - yield root_condition - root_condition.delete(db) @pytest.fixture() def condition_age_range(db: Session, manual_task: ManualTask): """Create a conditional dependency with age >= 18 AND age < 65 (same field, multiple conditions).""" - condition_tree = { - "logical_operator": "and", - "conditions": [ - create_condition_gt_18_tree(), - create_condition_age_lt_65_tree(), - ], - } - condition = ManualTaskConditionalDependency.create( + return ManualTaskConditionalDependency.create( db=db, data={ "manual_task_id": manual_task.id, - "condition_tree": condition_tree, + "condition_tree": { + "logical_operator": "and", + "conditions": [ + create_condition_gt_18_tree(), + create_condition_age_lt_65_tree(), + ], + }, }, ) - yield condition - condition.delete(db) From 122e77197c91de1f823160ab2650011d0c032719 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 6 Jan 2026 18:00:25 +0000 Subject: [PATCH 09/12] test clean up --- .../digest/test_condition_validation.py | 426 ++---------------- 1 file changed, 42 insertions(+), 384 deletions(-) diff --git a/tests/api/models/digest/test_condition_validation.py b/tests/api/models/digest/test_condition_validation.py index af063c2e3c..bb390bb3a8 100644 --- a/tests/api/models/digest/test_condition_validation.py +++ b/tests/api/models/digest/test_condition_validation.py @@ -1,417 +1,75 @@ -"""Tests for DigestCondition validation logic.""" +"""Tests for DigestCondition tree validation scenarios.""" from typing import Any import pytest from sqlalchemy.orm import Session -from fides.api.models.conditional_dependency.conditional_dependency_base import ( - ConditionalDependencyType, -) from fides.api.models.digest import DigestConfig from fides.api.models.digest.conditional_dependencies import ( DigestCondition, DigestConditionType, ) -from fides.api.task.conditional_dependencies.schemas import ConditionLeaf, Operator - - -class TestDigestConditionValidation: - """Test DigestCondition validation for mixed condition types.""" - - def test_create_condition_tree_no_validation(self, sample_conditions): - """Test that root conditions don't require validation.""" - # sample_conditions fixture creates root conditions of different types - receiver_condition, content_condition, priority_condition = sample_conditions - - # Verify they have different types (proving root conditions can be any type) - assert receiver_condition.digest_condition_type == DigestConditionType.RECEIVER - assert content_condition.digest_condition_type == DigestConditionType.CONTENT - assert priority_condition.digest_condition_type == DigestConditionType.PRIORITY - - # Verify they are all root conditions (no parent) - assert receiver_condition.parent_id is None - assert content_condition.parent_id is None - assert priority_condition.parent_id is None - - def test_create_child_condition_same_type_succeeds( - self, - db: Session, - digest_config: DigestConfig, - receiver_group: DigestCondition, - receiver_condition_leaf: ConditionLeaf, - ): - """Test that child conditions with same type as parent succeed.""" - parent = receiver_group +from fides.api.task.conditional_dependencies.schemas import ConditionLeaf, GroupOperator - child = DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - **receiver_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - assert child.digest_condition_type == DigestConditionType.RECEIVER - assert child.parent_id == parent.id - def test_create_child_condition_different_type_fails( - self, - db: Session, - receiver_group: DigestCondition, - content_condition: dict[str, Any], - content_condition_leaf: ConditionLeaf, - ): - """Test that child conditions with different type from parent fail.""" - # Use receiver_group as parent - parent = receiver_group +class TestConditionTreeValidation: + """Test condition tree validation scenarios.""" - # Try to create child with different type - should fail - with pytest.raises( - ValueError, match="Cannot create condition with type 'content'" - ): - DigestCondition.create( - db=db, - data={ - **content_condition, - **content_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - def test_create_child_condition_nonexistent_parent_fails( + def test_condition_tree_can_be_none( self, db: Session, receiver_condition: dict[str, Any], - receiver_condition_leaf: ConditionLeaf, - ): - """Test that creating child with nonexistent parent fails.""" - with pytest.raises( - ValueError, match="Parent condition with id 'nonexistent' does not exist" - ): - DigestCondition.create( - db=db, - data={ - **receiver_condition, - **receiver_condition_leaf.model_dump(), - "parent_id": "nonexistent", - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) - - def test_create_nested_conditions_same_type_succeeds(self, complex_condition_tree): - """Test that deeply nested conditions with same type succeed.""" - # complex_condition_tree fixture creates a nested structure with consistent types - root = complex_condition_tree["root"] - nested_groups = complex_condition_tree["nested_groups"] - leaves = complex_condition_tree["leaves"] - - # Verify all conditions have the same type (CONTENT) - assert root.digest_condition_type == DigestConditionType.CONTENT - for group in nested_groups: - assert group.digest_condition_type == DigestConditionType.CONTENT - for leaf in leaves: - assert leaf.digest_condition_type == DigestConditionType.CONTENT - - # Verify the tree structure is intact - assert root.parent_id is None - for group in nested_groups: - assert group.parent_id == root.id - # Each leaf should have one of the nested groups as parent - for leaf in leaves: - assert leaf.parent_id in [group.id for group in nested_groups] - - def test_update_condition_type_to_different_type_fails( - self, db: Session, complex_condition_tree ): - """Test that updating a child condition to different type fails.""" - # Use a leaf from the complex tree (all are CONTENT type) - leaf = complex_condition_tree["leaves"][0] - - # Verify it's a child condition with CONTENT type - assert leaf.digest_condition_type == DigestConditionType.CONTENT - assert leaf.parent_id is not None - - # Try to update child to different type - should fail - with pytest.raises( - ValueError, match="Cannot create condition with type 'priority'" - ): - leaf.update( - db=db, data={"digest_condition_type": DigestConditionType.PRIORITY} - ) - - def test_update_condition_same_type_succeeds( - self, db: Session, complex_condition_tree - ): - """Test that updating condition with same type succeeds.""" - # Use a leaf from the complex tree - leaf = complex_condition_tree["leaves"][0] - original_type = leaf.digest_condition_type - - # Update other fields - should succeed - updated_leaf = leaf.update( + """Test that a condition can be created with no condition_tree.""" + condition = DigestCondition.create( db=db, - data={"field_address": "task.updated_field", "value": "updated_value"}, + data={**receiver_condition, "condition_tree": None}, ) - assert updated_leaf.field_address == "task.updated_field" - assert updated_leaf.value == "updated_value" - assert updated_leaf.digest_condition_type == original_type # Type unchanged - def test_update_condition_tree_type_succeeds( - self, db: Session, receiver_digest_condition_leaf: DigestCondition - ): - """Test that updating root condition type succeeds (no parent to validate against).""" - # Use receiver_digest_condition_leaf as root condition (it has no parent) - root = receiver_digest_condition_leaf - assert root.parent_id is None # Verify it's a root condition + assert condition.condition_tree is None - # Update root condition type - should succeed (no parent to validate against) - updated_root = root.update( - db=db, - data={ - "digest_condition_type": DigestConditionType.CONTENT, - "field_address": "task.status", - "value": "pending", - }, - ) - assert updated_root.digest_condition_type == DigestConditionType.CONTENT - assert updated_root.field_address == "task.status" - - def test_validation_error_messages_are_descriptive( + def test_update_condition_tree_to_none( self, db: Session, - receiver_group: DigestCondition, - content_condition: dict[str, Any], - receiver_condition_leaf: ConditionLeaf, + receiver_digest_condition_leaf: DigestCondition, ): - """Test that validation error messages are clear and helpful.""" - parent = receiver_group - - # Try to create child with different type - with pytest.raises(ValueError) as exc_info: - DigestCondition.create( - db=db, - data={ - **content_condition, - **receiver_condition_leaf.model_dump(), - "parent_id": parent.id, - "condition_type": ConditionalDependencyType.leaf, - "sort_order": 1, - }, - ) + """Test updating a condition tree to None.""" + assert receiver_digest_condition_leaf.condition_tree is not None - error_message = str(exc_info.value) - assert "Cannot create condition with type 'content'" in error_message - assert "under parent with type 'receiver'" in error_message - assert "must have the same digest_condition_type" in error_message + updated = receiver_digest_condition_leaf.update(db, data={"condition_tree": None}) + assert updated.condition_tree is None - @pytest.mark.parametrize( - "condition_type", - [ - DigestConditionType.RECEIVER, - DigestConditionType.CONTENT, - DigestConditionType.PRIORITY, - ], - ) - def test_all_condition_types_can_be_validated( + def test_update_from_leaf_to_group( self, db: Session, - digest_config: DigestConfig, - condition_type: DigestConditionType, - group_condition_and: dict[str, Any], - ): - """Test validation works for all digest condition types.""" - - parent_condition = DigestCondition.create( - db=db, - data={ - **group_condition_and, - "digest_condition_type": condition_type, - "digest_config_id": digest_config.id, - }, - ) - - # Child with same type should succeed - child = DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": condition_type, - "parent_id": parent_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "test.field", - "operator": Operator.eq, - "value": "test_value", - "sort_order": 1, - }, - ) - assert child.digest_condition_type == condition_type - - # Child with different type should fail - different_types = [t for t in DigestConditionType if t != condition_type] - if different_types: # Only test if there are other types - with pytest.raises(ValueError): - DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": different_types[0], - "parent_id": parent_condition.id, - "condition_type": ConditionalDependencyType.leaf, - "field_address": "test.field2", - "operator": Operator.eq, - "value": "test_value2", - "sort_order": 2, - }, - ) - - # Clean up child for next iteration - parent_condition.delete(db) - - @pytest.mark.usefixtures("digest_config") - def test_deep_tree_validation_prevents_type_mixing( - self, - db: Session, - receiver_condition: dict[str, Any], - content_condition: dict[str, Any], - group_condition_and: dict[str, Any], + receiver_digest_condition_leaf: DigestCondition, + content_condition_leaf: ConditionLeaf, + priority_condition_leaf: ConditionLeaf, ): - """Test that validation works across multiple levels of nesting.""" - # Create root RECEIVER condition - root = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "sort_order": 0, - }, - ) - - # Create intermediate RECEIVER condition (child of root) - intermediate = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "parent_id": root.id, - "sort_order": 1, - }, - ) - - # Create another intermediate RECEIVER condition (grandchild of root) - deep_intermediate = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "parent_id": intermediate.id, - "sort_order": 1, - }, - ) - - # Now try to create a CONTENT condition as great-grandchild - # This should fail because the parent is RECEIVER type - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - DigestCondition.create( - db=db, - data={ - **content_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": deep_intermediate.id, - "field_address": "content.title", - "operator": Operator.eq, - "value": "test", - "sort_order": 1, - }, - ) - - # But creating a RECEIVER condition should work - valid_deep_child = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": deep_intermediate.id, - "field_address": "receiver.email", - "operator": Operator.eq, - "value": "test@example.com", - "sort_order": 1, - }, - ) - assert valid_deep_child.digest_condition_type == DigestConditionType.RECEIVER - - # Clean up - valid_deep_child.delete(db) - deep_intermediate.delete(db) - intermediate.delete(db) - root.delete(db) - - @pytest.mark.usefixtures("digest_config") - def test_validation_prevents_crud_that_breaks_tree_consistency( + """Test updating from a leaf to a group condition tree.""" + group_tree = { + "logical_operator": GroupOperator.and_, + "conditions": [ + content_condition_leaf.model_dump(), + priority_condition_leaf.model_dump(), + ], + } + updated = receiver_digest_condition_leaf.update(db, data={"condition_tree": group_tree}) + + assert updated.condition_tree["logical_operator"] == GroupOperator.and_ + assert len(updated.condition_tree["conditions"]) == 2 + + def test_mixed_leaf_and_group_in_tree( self, db: Session, - receiver_condition: dict[str, Any], - group_condition_and: dict[str, Any], + complex_condition_tree: DigestCondition, ): - """Test that updating a condition to different type fails if it breaks tree consistency.""" - # Create RECEIVER tree - root = DigestCondition.create( - db=db, - data={ - **receiver_condition, - **group_condition_and, - "sort_order": 0, - }, - ) - - # Create receiver leaf condition - child = DigestCondition.create( - db=db, - data={ - **receiver_condition, - "condition_type": ConditionalDependencyType.leaf, - "parent_id": root.id, - "field_address": "receiver.email", - "operator": Operator.eq, - "value": "test@example.com", - "sort_order": 1, - }, - ) - - # Try to update child to CONTENT type - should fail - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - child.update( - db=db, data={"digest_condition_type": DigestConditionType.CONTENT} - ) - - # Verify child is still RECEIVER type - db.refresh(child) - assert child.digest_condition_type == DigestConditionType.RECEIVER - - # Try to save - should fail validation - child.digest_condition_type = DigestConditionType.CONTENT - with pytest.raises( - ValueError, - match="Cannot create condition with type 'content' under parent with type 'receiver'", - ): - child.save(db) - - # Verify child's type was not saved to database - db.refresh(child) - assert child.digest_condition_type == DigestConditionType.RECEIVER - - # Clean up - child.delete(db) - root.delete(db) + """Test a tree with both leaf and nested group conditions.""" + tree = complex_condition_tree.condition_tree + + assert tree["logical_operator"] == GroupOperator.or_ + assert len(tree["conditions"]) == 2 + # Both are nested groups in complex_condition_tree fixture + assert "logical_operator" in tree["conditions"][0] + assert "logical_operator" in tree["conditions"][1] From 6a7095ffba04117f4a3fbc6f1c25a4640c576a53 Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 6 Jan 2026 18:00:39 +0000 Subject: [PATCH 10/12] test clean up --- tests/api/models/digest/test_condition_validation.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/api/models/digest/test_condition_validation.py b/tests/api/models/digest/test_condition_validation.py index bb390bb3a8..35b837ca15 100644 --- a/tests/api/models/digest/test_condition_validation.py +++ b/tests/api/models/digest/test_condition_validation.py @@ -37,7 +37,9 @@ def test_update_condition_tree_to_none( """Test updating a condition tree to None.""" assert receiver_digest_condition_leaf.condition_tree is not None - updated = receiver_digest_condition_leaf.update(db, data={"condition_tree": None}) + updated = receiver_digest_condition_leaf.update( + db, data={"condition_tree": None} + ) assert updated.condition_tree is None def test_update_from_leaf_to_group( @@ -55,7 +57,9 @@ def test_update_from_leaf_to_group( priority_condition_leaf.model_dump(), ], } - updated = receiver_digest_condition_leaf.update(db, data={"condition_tree": group_tree}) + updated = receiver_digest_condition_leaf.update( + db, data={"condition_tree": group_tree} + ) assert updated.condition_tree["logical_operator"] == GroupOperator.and_ assert len(updated.condition_tree["conditions"]) == 2 From ce2b24e3f7f11d19bf639b9fb1eed76eb0b19aac Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 6 Jan 2026 18:26:43 +0000 Subject: [PATCH 11/12] update tests --- .../digest/test_conditional_dependencies.py | 244 +++--------------- 1 file changed, 29 insertions(+), 215 deletions(-) diff --git a/tests/api/models/digest/test_conditional_dependencies.py b/tests/api/models/digest/test_conditional_dependencies.py index 49127f4537..8cb5dd95de 100644 --- a/tests/api/models/digest/test_conditional_dependencies.py +++ b/tests/api/models/digest/test_conditional_dependencies.py @@ -13,12 +13,7 @@ DigestCondition, DigestConditionType, ) -from fides.api.task.conditional_dependencies.schemas import ( - ConditionGroup, - ConditionLeaf, - GroupOperator, - Operator, -) +from fides.api.task.conditional_dependencies.schemas import ConditionLeaf, GroupOperator # ============================================================================ # DigestConditionType Tests @@ -78,8 +73,6 @@ def test_create_leaf_condition_types( assert condition.digest_config_id == digest_config.id assert condition.digest_condition_type == digest_condition_type assert condition.condition_tree == sample_eq_condition_leaf.model_dump() - - # Test relationship assert condition.digest_config == digest_config @pytest.mark.parametrize("digest_condition_type", DIGEST_CONDITION_TYPES) @@ -154,7 +147,6 @@ def test_cascade_delete_with_digest_config( condition_id = condition.id digest_config.delete(db) - # Verify condition is also deleted deleted_condition = ( db.query(DigestCondition).filter(DigestCondition.id == condition_id).first() ) @@ -173,79 +165,10 @@ def test_unique_constraint_per_config_and_type( "condition_tree": sample_eq_condition_leaf.model_dump(), } - # Create first condition - DigestCondition.create( - db=db, - data=data, - ) - - # Try to create second condition with same config and type - with pytest.raises(Exception): # Should raise unique constraint violation - DigestCondition.create( - db=db, - data=data, - ) - - -# ============================================================================ -# Condition Tree Tests -# ============================================================================ - - -class TestDigestConditionTrees: - """Test building and managing condition trees.""" - - def test_create_nested_condition_tree( - self, - db: Session, - content_condition: dict[str, Any], - content_condition_leaf: ConditionLeaf, - priority_condition_leaf: ConditionLeaf, - ): - """Test creating a nested group condition tree.""" - condition_tree = { - "logical_operator": GroupOperator.and_, - "conditions": [ - content_condition_leaf.model_dump(), - priority_condition_leaf.model_dump(), - ], - } - - group = DigestCondition.create( - db=db, - data={ - **content_condition, - "condition_tree": condition_tree, - }, - ) - - # Verify tree structure - assert group.condition_tree["logical_operator"] == GroupOperator.and_ - assert len(group.condition_tree["conditions"]) == 2 - assert group.condition_tree["conditions"][0]["field_address"] == "task.status" - assert group.condition_tree["conditions"][1]["field_address"] == "task.priority" - - def test_complex_nested_tree(self, complex_condition_tree: dict[str, Any]): - """Test creating complex nested group condition trees.""" - tree = complex_condition_tree["condition_tree"] - - # Verify root structure - assert tree["logical_operator"] == GroupOperator.or_ - assert len(tree["conditions"]) == 2 + DigestCondition.create(db=db, data=data) - # Verify first nested group - first_group = tree["conditions"][0] - assert first_group["logical_operator"] == GroupOperator.and_ - assert len(first_group["conditions"]) == 2 - assert first_group["conditions"][0]["field_address"] == "task.assignee" - assert first_group["conditions"][1]["field_address"] == "task.due_date" - - # Verify second nested group - second_group = tree["conditions"][1] - assert second_group["logical_operator"] == GroupOperator.and_ - assert len(second_group["conditions"]) == 2 - assert second_group["conditions"][0]["field_address"] == "task.category" - assert second_group["conditions"][1]["field_address"] == "task.created_at" + with pytest.raises(Exception): + DigestCondition.create(db=db, data=data) # ============================================================================ @@ -257,44 +180,35 @@ class TestDigestConditionQueries: """Test querying digest conditions.""" @pytest.mark.usefixtures("sample_conditions") - def test_get_condition_tree_by_type(self, db: Session, digest_config: DigestConfig): - """Test getting root condition by digest condition type.""" - # Test getting receiver condition - receiver_condition = DigestCondition.get_condition_tree( - db, - digest_config_id=digest_config.id, - digest_condition_type=DigestConditionType.RECEIVER, - ) - assert receiver_condition is not None - assert isinstance(receiver_condition, ConditionLeaf) - assert receiver_condition.field_address == "user.email" - assert receiver_condition.value is None - - # Test getting content condition - content_condition = DigestCondition.get_condition_tree( - db, - digest_config_id=digest_config.id, - digest_condition_type=DigestConditionType.CONTENT, - ) - assert content_condition is not None - assert isinstance(content_condition, ConditionLeaf) - assert content_condition.field_address == "task.status" - assert content_condition.value == "pending" - - # Test getting priority condition - priority_condition = DigestCondition.get_condition_tree( + @pytest.mark.parametrize( + "condition_type,expected_field,expected_value", + [ + (DigestConditionType.RECEIVER, "user.email", None), + (DigestConditionType.CONTENT, "task.status", "pending"), + (DigestConditionType.PRIORITY, "task.priority", "high"), + ], + ) + def test_get_condition_tree_by_type( + self, + db: Session, + digest_config: DigestConfig, + condition_type: DigestConditionType, + expected_field: str, + expected_value: str, + ): + """Test getting condition tree by type.""" + result = DigestCondition.get_condition_tree( db, digest_config_id=digest_config.id, - digest_condition_type=DigestConditionType.PRIORITY, + digest_condition_type=condition_type, ) - assert priority_condition is not None - assert isinstance(priority_condition, ConditionLeaf) - assert priority_condition.field_address == "task.priority" - assert priority_condition.value == "high" + assert result is not None + assert isinstance(result, ConditionLeaf) + assert result.field_address == expected_field + assert result.value == expected_value def test_get_condition_tree_nonexistent(self, db: Session): - """Test getting root condition for nonexistent type returns None.""" - # Test with empty digest config (no conditions) + """Test getting condition tree for nonexistent config returns None.""" empty_config = DigestConfig.create( db=db, data={"digest_type": DigestType.PRIVACY_REQUESTS, "name": "Empty Config"}, @@ -308,32 +222,6 @@ def test_get_condition_tree_nonexistent(self, db: Session): assert result is None empty_config.delete(db) - @pytest.mark.usefixtures("sample_conditions") - def test_get_all_condition_trees(self, db: Session, digest_config: DigestConfig): - """Test getting all root conditions for a digest config.""" - # sample_conditions fixture creates conditions, so we can test retrieval - all_conditions = DigestCondition.get_all_condition_trees(db, digest_config.id) - - assert len(all_conditions) == 3 - assert DigestConditionType.RECEIVER in all_conditions - assert DigestConditionType.CONTENT in all_conditions - assert DigestConditionType.PRIORITY in all_conditions - - # Test receiver condition - receiver = all_conditions[DigestConditionType.RECEIVER] - assert isinstance(receiver, ConditionLeaf) - assert receiver.field_address == "user.email" - - # Test content condition - content = all_conditions[DigestConditionType.CONTENT] - assert isinstance(content, ConditionLeaf) - assert content.field_address == "task.status" - - # Test priority condition - priority = all_conditions[DigestConditionType.PRIORITY] - assert isinstance(priority, ConditionLeaf) - assert priority.field_address == "task.priority" - def test_get_condition_tree_missing_args(self, db: Session): """Test that get_condition_tree raises error with missing arguments.""" with pytest.raises( @@ -342,75 +230,6 @@ def test_get_condition_tree_missing_args(self, db: Session): ): DigestCondition.get_condition_tree(db, digest_config_id="only_one_arg") - @pytest.mark.usefixtures("sample_conditions") - def test_filter_conditions_by_type(self, db: Session, digest_config: DigestConfig): - """Test filtering conditions by digest condition type.""" - receiver_conditions = ( - db.query(DigestCondition) - .filter( - DigestCondition.digest_config_id == digest_config.id, - DigestCondition.digest_condition_type == DigestConditionType.RECEIVER, - ) - .all() - ) - - assert len(receiver_conditions) == 1 - assert ( - receiver_conditions[0].digest_condition_type == DigestConditionType.RECEIVER - ) - - content_conditions = ( - db.query(DigestCondition) - .filter( - DigestCondition.digest_config_id == digest_config.id, - DigestCondition.digest_condition_type == DigestConditionType.CONTENT, - ) - .all() - ) - - assert len(content_conditions) == 1 - assert ( - content_conditions[0].digest_condition_type == DigestConditionType.CONTENT - ) - - def test_filter_condition_trees_by_parent_id( - self, - db: Session, - digest_config: DigestConfig, - sample_eq_condition_leaf: ConditionLeaf, - ): - """Test that get_root_condition returns ConditionGroup for group trees.""" - condition_tree = { - "logical_operator": GroupOperator.and_, - "conditions": [ - sample_eq_condition_leaf.model_dump(), - { - "field_address": "user.active", - "operator": Operator.eq, - "value": True, - }, - ], - } - - DigestCondition.create( - db=db, - data={ - "digest_config_id": digest_config.id, - "digest_condition_type": DigestConditionType.RECEIVER, - "condition_tree": condition_tree, - }, - ) - - result = DigestCondition.get_root_condition( - db, - digest_config_id=digest_config.id, - digest_condition_type=DigestConditionType.RECEIVER, - ) - - assert isinstance(result, ConditionGroup) - assert result.logical_operator == GroupOperator.and_ - assert len(result.conditions) == 2 - # ============================================================================ # DigestConfig Integration Tests @@ -430,7 +249,6 @@ def test_digest_config_relationship_loading( content_condition_leaf: ConditionLeaf, ): """Test that digest config properly loads condition relationships.""" - # Create multiple conditions condition1 = DigestCondition.create( db=db, data={ @@ -447,19 +265,15 @@ def test_digest_config_relationship_loading( }, ) - # Refresh to load relationships db.refresh(digest_config) - # Test that conditions are accessible through relationship assert len(digest_config.conditions) == 2 - condition_ids = [condition.id for condition in digest_config.conditions] + condition_ids = [c.id for c in digest_config.conditions] assert condition1.id in condition_ids assert condition2.id in condition_ids - # Test reverse relationship for condition in digest_config.conditions: assert condition.digest_config == digest_config - assert condition.digest_config_id == digest_config.id # ============================================================================ From 2b2c07f537a5081c9d0daad8bcc6a9af4636289a Mon Sep 17 00:00:00 2001 From: Jade Wibbels Date: Tue, 6 Jan 2026 18:33:51 +0000 Subject: [PATCH 12/12] updated tests --- ...test_manual_task_conditional_evaluation.py | 145 ++---------------- 1 file changed, 16 insertions(+), 129 deletions(-) diff --git a/tests/api/task/manual/test_manual_task_conditional_evaluation.py b/tests/api/task/manual/test_manual_task_conditional_evaluation.py index 9c8050bf85..1125b461cd 100644 --- a/tests/api/task/manual/test_manual_task_conditional_evaluation.py +++ b/tests/api/task/manual/test_manual_task_conditional_evaluation.py @@ -3,6 +3,7 @@ import pytest from pytest import param +from fides.api.graph.config import CollectionAddress from fides.api.models.manual_task.conditional_dependency import ( ManualTaskConditionalDependency, ) @@ -63,18 +64,6 @@ def _input_group_tree(): } -@pytest.fixture -def input_group_conditional_dependency(db, manual_task): - """Create a single dependency with the full input group condition tree.""" - return ManualTaskConditionalDependency.create( - db=db, - data={ - "manual_task_id": manual_task.id, - "condition_tree": _input_group_tree(), - }, - ) - - @pytest.fixture def email_and_city_conditional_dependency(db, manual_task): """Create a single dependency with both email and city conditions.""" @@ -303,35 +292,24 @@ def test_evaluate_group_conditional_dependencies_with_root_condition( class TestManualTaskDataExtraction: """Test the _extract_conditional_dependency_data_from_inputs method in ManualTaskGraphTask""" - def setup_method(self): - """Import CollectionAddress for all test methods""" - from fides.api.graph.config import CollectionAddress - - self.CollectionAddress = CollectionAddress - @pytest.mark.usefixtures("email_and_privacy_request_conditional_dependency") def test_extract_conditional_dependency_data_from_inputs_and_privacy_request( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test extracting a simple field like 'postgres_example_test_dataset:customer:email'""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create test input data inputs = [ [ {"id": 1, "email": "customer-1@example.com", "name": "Customer 1"}, {"id": 2, "email": "customer-2@example.com", "name": "Customer 2"}, ] ] - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -339,12 +317,9 @@ def test_extract_conditional_dependency_data_from_inputs_and_privacy_request( privacy_request=privacy_request, ) - # Should extract the email field data expected = { "postgres_example_test_dataset": { - "customer": { - "email": "customer-1@example.com" # First non-None value found - } + "customer": {"email": "customer-1@example.com"} }, "privacy_request": { "location": None, @@ -358,24 +333,19 @@ def test_extract_conditional_dependency_data_from_inputs_simple_field( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test extracting a simple field like 'postgres_example_test_dataset:customer:email'""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create test input data inputs = [ [ {"id": 1, "email": "customer-1@example.com", "name": "Customer 1"}, {"id": 2, "email": "customer-2@example.com", "name": "Customer 2"}, ] ] - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -383,12 +353,9 @@ def test_extract_conditional_dependency_data_from_inputs_simple_field( privacy_request=privacy_request, ) - # Should extract the email field data expected = { "postgres_example_test_dataset": { - "customer": { - "email": "customer-1@example.com" # First non-None value found - } + "customer": {"email": "customer-1@example.com"} } } assert result == expected @@ -397,22 +364,17 @@ def test_extract_conditional_dependency_data_from_inputs_nested_field( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test extracting nested fields like 'dataset:collection:subcollection:field'""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create test input data with nested structure inputs = [ [{"id": 1, "profile": {"age": 25, "preferences": {"theme": "dark"}}}] ] - # Create a conditional dependency that references a deeply nested field condition_tree = { "field_address": "postgres_example_test_dataset:customer:profile:preferences:theme", "operator": "eq", @@ -426,7 +388,6 @@ def test_extract_conditional_dependency_data_from_inputs_nested_field( }, ) - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -434,7 +395,6 @@ def test_extract_conditional_dependency_data_from_inputs_nested_field( privacy_request=privacy_request, ) - # Should extract the nested field data expected = { "postgres_example_test_dataset": { "customer": {"profile": {"preferences": {"theme": "dark"}}} @@ -447,20 +407,15 @@ def test_extract_conditional_dependency_data_from_inputs_missing_field( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test behavior when the field doesn't exist in input data""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create test input data without the expected field inputs = [[{"id": 1, "name": "Customer 1"}]] # No email field - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -468,12 +423,7 @@ def test_extract_conditional_dependency_data_from_inputs_missing_field( privacy_request=privacy_request, ) - # Should include the field with None value - expected = { - "postgres_example_test_dataset": { - "customer": {"email": None} # Field not found, so None - } - } + expected = {"postgres_example_test_dataset": {"customer": {"email": None}}} assert result == expected @pytest.mark.usefixtures("email_and_city_conditional_dependency") @@ -481,26 +431,19 @@ def test_extract_conditional_dependency_data_from_inputs_multiple_collections( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test extracting from multiple input collections""" - # Mock the execution node to have multiple input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ), - self.CollectionAddress.from_string( - "postgres_example_test_dataset:address" - ), + CollectionAddress.from_string("postgres_example_test_dataset:customer"), + CollectionAddress.from_string("postgres_example_test_dataset:address"), ] - # Create test input data for multiple collections inputs = [ - [{"id": 1, "email": "customer-1@example.com"}], # customer collection - [{"id": 1, "city": "New York"}], # address collection + [{"id": 1, "email": "customer-1@example.com"}], + [{"id": 1, "city": "New York"}], ] - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -508,7 +451,6 @@ def test_extract_conditional_dependency_data_from_inputs_multiple_collections( privacy_request=privacy_request, ) - # Should extract data from both collections expected = { "postgres_example_test_dataset": { "customer": {"email": "customer-1@example.com"}, @@ -517,59 +459,20 @@ def test_extract_conditional_dependency_data_from_inputs_multiple_collections( } assert result == expected - @pytest.mark.usefixtures("email_exists_conditional_dependency") - def test_extract_conditional_dependency_data_from_inputs_field_address_parsing( - self, manual_task_graph_task, db, manual_task, privacy_request - ): - """Test that FieldAddress.from_string() works correctly""" - # Mock the execution node to have specific input keys - with patch.object( - manual_task_graph_task, "execution_node", autospec=True - ) as mock_node: - mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) - ] - - # Create test input data - inputs = [[{"id": 1, "email": "customer-1@example.com"}]] - - # Extract the data - result = extract_conditional_dependency_data_from_inputs( - *inputs, - manual_task=manual_task, - input_keys=mock_node.input_keys, - privacy_request=privacy_request, - ) - - # Should correctly parse the field address and extract the data - expected = { - "postgres_example_test_dataset": { - "customer": {"email": "customer-1@example.com"} - } - } - assert result == expected - @pytest.mark.usefixtures("email_exists_conditional_dependency") def test_extract_conditional_dependency_data_from_inputs_empty_inputs( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test behavior with empty input data""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create empty input data - inputs = [[]] # Empty list for the collection + inputs = [[]] - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -577,34 +480,22 @@ def test_extract_conditional_dependency_data_from_inputs_empty_inputs( privacy_request=privacy_request, ) - # Should handle empty inputs gracefully - expected = { - "postgres_example_test_dataset": { - "customer": {"email": None} # No data to extract - } - } + expected = {"postgres_example_test_dataset": {"customer": {"email": None}}} assert result == expected def test_extract_conditional_dependency_data_from_inputs_no_conditional_dependencies( self, manual_task_graph_task, db, manual_task, privacy_request ): """Test behavior when there are no conditional dependencies""" - # Mock the execution node to have specific input keys with patch.object( manual_task_graph_task, "execution_node", autospec=True ) as mock_node: mock_node.input_keys = [ - self.CollectionAddress.from_string( - "postgres_example_test_dataset:customer" - ) + CollectionAddress.from_string("postgres_example_test_dataset:customer") ] - # Create test input data inputs = [[{"id": 1, "email": "customer-1@example.com"}]] - # No conditional dependencies created - - # Extract the data result = extract_conditional_dependency_data_from_inputs( *inputs, manual_task=manual_task, @@ -612,7 +503,6 @@ def test_extract_conditional_dependency_data_from_inputs_no_conditional_dependen privacy_request=privacy_request, ) - # Should return empty dict when no conditional dependencies exist assert result == {} @pytest.mark.parametrize( @@ -801,14 +691,12 @@ def test_extract_location_convenience_fields( privacy_request=privacy_request, ) - # Verify the extracted location convenience fields match expected for field_path, expected_value in conditional_data[ "privacy_request" ].items(): if field_path in result.get("privacy_request", {}): actual_value = result["privacy_request"][field_path] if isinstance(expected_value, list): - # For list fields, check if expected items are in actual if expected_result: assert all( item in actual_value for item in expected_value @@ -818,7 +706,6 @@ def test_extract_location_convenience_fields( item in actual_value for item in expected_value ), f"Expected {expected_value} not to be in {actual_value}" else: - # For scalar fields, check equality if expected_result: assert ( actual_value == expected_value