diff --git a/datajunction-server/datajunction_server/api/graphql/queries/nodes.py b/datajunction-server/datajunction_server/api/graphql/queries/nodes.py index 6134a32da..b014db402 100644 --- a/datajunction-server/datajunction_server/api/graphql/queries/nodes.py +++ b/datajunction-server/datajunction_server/api/graphql/queries/nodes.py @@ -11,7 +11,7 @@ from datajunction_server.api.graphql.resolvers.nodes import find_nodes_by from datajunction_server.api.graphql.scalars import Connection from datajunction_server.api.graphql.scalars.node import Node, NodeSortField -from datajunction_server.models.node import NodeCursor, NodeMode, NodeType +from datajunction_server.models.node import NodeCursor, NodeMode, NodeStatus, NodeType DEFAULT_LIMIT = 1000 UPPER_LIMIT = 10000 @@ -51,6 +51,60 @@ async def find_nodes( "Accepts dimension node names or dimension attributes", ), ] = None, + edited_by: Annotated[ + str | None, + strawberry.argument( + description="Filter to nodes edited by this user", + ), + ] = None, + namespace: Annotated[ + str | None, + strawberry.argument( + description="Filter to nodes in this namespace", + ), + ] = None, + mode: Annotated[ + NodeMode | None, + strawberry.argument( + description="Filter to nodes with this mode (published or draft)", + ), + ] = None, + owned_by: Annotated[ + str | None, + strawberry.argument( + description="Filter to nodes owned by this user", + ), + ] = None, + missing_description: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes missing descriptions (for data quality checks)", + ), + ] = False, + missing_owner: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes without any owners (for data quality checks)", + ), + ] = False, + statuses: Annotated[ + list[NodeStatus] | None, + strawberry.argument( + description="Filter to nodes with these statuses (e.g., VALID, INVALID)", + ), + ] = None, + has_materialization: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes that have materializations configured", + ), + ] = False, + orphaned_dimension: Annotated[ + bool, + strawberry.argument( + description="Filter to dimension nodes that are not linked to by any other node", + ), + ] = False, limit: Annotated[ int | None, strawberry.argument(description="Limit nodes"), @@ -76,12 +130,21 @@ async def find_nodes( limit = UPPER_LIMIT return await find_nodes_by( # type: ignore - info, - names, - fragment, - node_types, - tags, + info=info, + names=names, + fragment=fragment, + node_types=node_types, + tags=tags, dimensions=dimensions, + edited_by=edited_by, + namespace=namespace, + mode=mode, + owned_by=owned_by, + missing_description=missing_description, + missing_owner=missing_owner, + statuses=statuses, + has_materialization=has_materialization, + orphaned_dimension=orphaned_dimension, limit=limit, order_by=order_by, ascending=ascending, @@ -113,6 +176,13 @@ async def find_nodes_paginated( description="Filter to nodes tagged with these tags", ), ] = None, + dimensions: Annotated[ + list[str] | None, + strawberry.argument( + description="Filter to nodes that have ALL of these dimensions. " + "Accepts dimension node names or dimension attributes", + ), + ] = None, edited_by: Annotated[ str | None, strawberry.argument( @@ -131,6 +201,42 @@ async def find_nodes_paginated( description="Filter to nodes with this mode (published or draft)", ), ] = None, + owned_by: Annotated[ + str | None, + strawberry.argument( + description="Filter to nodes owned by this user", + ), + ] = None, + missing_description: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes missing descriptions (for data quality checks)", + ), + ] = False, + missing_owner: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes without any owners (for data quality checks)", + ), + ] = False, + statuses: Annotated[ + list[NodeStatus] | None, + strawberry.argument( + description="Filter to nodes with these statuses (e.g., VALID, INVALID)", + ), + ] = None, + has_materialization: Annotated[ + bool, + strawberry.argument( + description="Filter to nodes that have materializations configured", + ), + ] = False, + orphaned_dimension: Annotated[ + bool, + strawberry.argument( + description="Filter to dimension nodes that are not linked to by any other node", + ), + ] = False, after: str | None = None, before: str | None = None, limit: Annotated[ @@ -148,19 +254,26 @@ async def find_nodes_paginated( if not limit or limit < 0: limit = 100 nodes_list = await find_nodes_by( - info, - names, - fragment, - node_types, - tags, - edited_by, - namespace, - limit + 1, - before, - after, - order_by, - ascending, - mode, + info=info, + names=names, + fragment=fragment, + node_types=node_types, + tags=tags, + dimensions=dimensions, + edited_by=edited_by, + namespace=namespace, + limit=limit + 1, + before=before, + after=after, + order_by=order_by, + ascending=ascending, + mode=mode, + owned_by=owned_by, + missing_description=missing_description, + missing_owner=missing_owner, + statuses=statuses, + has_materialization=has_materialization, + orphaned_dimension=orphaned_dimension, ) return Connection.from_list( items=nodes_list, diff --git a/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py b/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py index 8d90fe102..32b22ab6b 100644 --- a/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py +++ b/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py @@ -17,7 +17,7 @@ from datajunction_server.database.node import Column, ColumnAttribute from datajunction_server.database.node import Node as DBNode from datajunction_server.database.node import NodeRevision as DBNodeRevision -from datajunction_server.models.node import NodeMode, NodeType +from datajunction_server.models.node import NodeMode, NodeStatus, NodeType async def find_nodes_by( @@ -34,7 +34,13 @@ async def find_nodes_by( order_by: NodeSortField = NodeSortField.CREATED_AT, ascending: bool = False, mode: Optional[NodeMode] = None, + owned_by: Optional[str] = None, + missing_description: bool = False, + missing_owner: bool = False, dimensions: Optional[List[str]] = None, + statuses: Optional[List[NodeStatus]] = None, + has_materialization: bool = False, + orphaned_dimension: bool = False, ) -> List[DBNode]: """ Finds nodes based on the search parameters. This function also tries to optimize @@ -64,6 +70,12 @@ async def find_nodes_by( ascending=ascending, options=options, mode=mode, + owned_by=owned_by, + missing_description=missing_description, + missing_owner=missing_owner, + statuses=statuses, + has_materialization=has_materialization, + orphaned_dimension=orphaned_dimension, dimensions=dimensions, ) diff --git a/datajunction-server/datajunction_server/api/graphql/schema.graphql b/datajunction-server/datajunction_server/api/graphql/schema.graphql index 90e46f4f7..bed655f4d 100644 --- a/datajunction-server/datajunction_server/api/graphql/schema.graphql +++ b/datajunction-server/datajunction_server/api/graphql/schema.graphql @@ -407,6 +407,33 @@ type Query { """ dimensions: [String!] = null + """Filter to nodes edited by this user""" + editedBy: String = null + + """Filter to nodes in this namespace""" + namespace: String = null + + """Filter to nodes with this mode (published or draft)""" + mode: NodeMode = null + + """Filter to nodes owned by this user""" + ownedBy: String = null + + """Filter to nodes missing descriptions (for data quality checks)""" + missingDescription: Boolean! = false + + """Filter to nodes without any owners (for data quality checks)""" + missingOwner: Boolean! = false + + """Filter to nodes with these statuses (e.g., VALID, INVALID)""" + statuses: [NodeStatus!] = null + + """Filter to nodes that have materializations configured""" + hasMaterialization: Boolean! = false + + """Filter to dimension nodes that are not linked to by any other node""" + orphanedDimension: Boolean! = false + """Limit nodes""" limit: Int = 1000 orderBy: NodeSortField! = CREATED_AT @@ -427,6 +454,11 @@ type Query { """Filter to nodes tagged with these tags""" tags: [String!] = null + """ + Filter to nodes that have ALL of these dimensions. Accepts dimension node names or dimension attributes + """ + dimensions: [String!] = null + """Filter to nodes edited by this user""" editedBy: String = null @@ -435,6 +467,24 @@ type Query { """Filter to nodes with this mode (published or draft)""" mode: NodeMode = null + + """Filter to nodes owned by this user""" + ownedBy: String = null + + """Filter to nodes missing descriptions (for data quality checks)""" + missingDescription: Boolean! = false + + """Filter to nodes without any owners (for data quality checks)""" + missingOwner: Boolean! = false + + """Filter to nodes with these statuses (e.g., VALID, INVALID)""" + statuses: [NodeStatus!] = null + + """Filter to nodes that have materializations configured""" + hasMaterialization: Boolean! = false + + """Filter to dimension nodes that are not linked to by any other node""" + orphanedDimension: Boolean! = false after: String = null before: String = null diff --git a/datajunction-server/datajunction_server/construction/build_v3/loaders.py b/datajunction-server/datajunction_server/construction/build_v3/loaders.py index 1efa3d901..808876d77 100644 --- a/datajunction-server/datajunction_server/construction/build_v3/loaders.py +++ b/datajunction-server/datajunction_server/construction/build_v3/loaders.py @@ -197,22 +197,24 @@ async def load_dimension_links_batch( Returns a dict mapping link_id to DimensionLink object. Note: Most dimension nodes should already be in ctx.nodes from query2. - We load current+query for pre-parsing cache, but skip heavy relationships. + We load current+query for pre-parsing cache, and columns for type lookups. """ if not link_ids: return {} - # Load dimension links with minimal eager loading - # Need current.query for pre-parsing, but skip columns/dimension_links/etc + # Load dimension links with eager loading for columns (needed for type lookups) stmt = ( select(DimensionLink) .where(DimensionLink.id.in_(link_ids)) .options( joinedload(DimensionLink.dimension).options( joinedload(Node.current).options( - # Only load what's needed for table references and parsing + # Load what's needed for table references, parsing, and type lookups joinedload(NodeRevision.catalog), joinedload(NodeRevision.availability), + selectinload(NodeRevision.columns).options( + load_only(Column.name, Column.type), + ), ), ), ) diff --git a/datajunction-server/datajunction_server/database/node.py b/datajunction-server/datajunction_server/database/node.py index 57e2cc6a8..b94f34512 100644 --- a/datajunction-server/datajunction_server/database/node.py +++ b/datajunction-server/datajunction_server/database/node.py @@ -45,6 +45,7 @@ from datajunction_server.database.history import History from datajunction_server.database.materialization import Materialization from datajunction_server.database.metricmetadata import MetricMetadata +from datajunction_server.database.nodeowner import NodeOwner from datajunction_server.database.tag import Tag from datajunction_server.database.user import User from datajunction_server.errors import ( @@ -599,7 +600,13 @@ async def find_by( ascending: bool = False, options: list[ExecutableOption] = None, mode: NodeMode | None = None, + owned_by: str | None = None, + missing_description: bool = False, + missing_owner: bool = False, dimensions: list[str] | None = None, + statuses: list[NodeStatus] | None = None, + has_materialization: bool = False, + orphaned_dimension: bool = False, ) -> List["Node"]: """ Finds a list of nodes by prefix @@ -676,15 +683,85 @@ async def find_by( if edited_by: edited_node_subquery = ( select(History.entity_name) - .where((History.user == edited_by)) + .where(History.user == edited_by) .distinct() .subquery() ) + # Use WHERE IN instead of JOIN + DISTINCT to avoid ORDER BY conflicts + statement = statement.where( + Node.name.in_(select(edited_node_subquery.c.entity_name)), + ) - statement = statement.join( - edited_node_subquery, - onclause=(edited_node_subquery.c.entity_name == Node.name), - ).distinct() + # Filter by owner username + if owned_by: + owned_node_subquery = ( + select(NodeOwner.node_id) + .join(User, NodeOwner.user_id == User.id) + .where(User.username == owned_by) + .distinct() + .subquery() + ) + statement = statement.where(Node.id.in_(select(owned_node_subquery))) + + # Filter nodes missing descriptions (actionable item) + if missing_description: + if not join_revision: # pragma: no branch + statement = statement.join(NodeRevisionAlias, Node.current) + join_revision = True + statement = statement.where( + or_( + NodeRevisionAlias.description.is_(None), + NodeRevisionAlias.description == "", + ), + ) + + # Filter nodes missing owners (actionable item) + if missing_owner: + nodes_with_owners_subquery = select(NodeOwner.node_id).distinct().subquery() + statement = statement.where( + ~Node.id.in_(select(nodes_with_owners_subquery)), + ) + + # Filter by node statuses + if statuses: + if not join_revision: # pragma: no branch + statement = statement.join(NodeRevisionAlias, Node.current) + join_revision = True + # Strawberry enums need to be converted to their lowercase values for DB comparison + status_values = [ + s.value.lower() if hasattr(s, "value") else str(s).lower() + for s in statuses + ] + statement = statement.where(NodeRevisionAlias.status.in_(status_values)) + + # Filter to nodes with materializations configured + if has_materialization: + if not join_revision: # pragma: no branch + statement = statement.join(NodeRevisionAlias, Node.current) + join_revision = True + nodes_with_mat_subquery = ( + select(Materialization.node_revision_id) + .where(Materialization.deactivated_at.is_(None)) + .distinct() + .subquery() + ) + statement = statement.where( + NodeRevisionAlias.id.in_(select(nodes_with_mat_subquery)), + ) + + # Filter to orphaned dimensions (dimension nodes not linked to by any other node) + if orphaned_dimension: + from datajunction_server.database.dimensionlink import DimensionLink + + # Only dimension nodes can be orphaned + statement = statement.where(Node.type == NodeType.DIMENSION) + # Find dimensions that have no DimensionLink pointing to them + linked_dimension_subquery = ( + select(DimensionLink.dimension_id).distinct().subquery() + ) + statement = statement.where( + ~Node.id.in_(select(linked_dimension_subquery)), + ) if after: cursor = NodeCursor.decode(after) diff --git a/datajunction-server/datajunction_server/sql/parsing/ast.py b/datajunction-server/datajunction_server/sql/parsing/ast.py index 73488b1cf..9ad023934 100644 --- a/datajunction-server/datajunction_server/sql/parsing/ast.py +++ b/datajunction-server/datajunction_server/sql/parsing/ast.py @@ -1709,6 +1709,14 @@ def __str__(self) -> str: return f"({ret})" return ret + # Module-level constant for numeric type ordering (avoids repeated instantiation) + _NUMERIC_TYPES_ORDER = { + "double": 0, + "float": 1, + "bigint": 2, + "int": 3, + } + @property def type(self) -> ColumnType: kind = self.op @@ -1721,17 +1729,7 @@ def raise_binop_exception(): f"{self}. Got left {left_type}, right {right_type}.", ) - numeric_types = { - type_: idx - for idx, type_ in enumerate( - [ - str(DoubleType()), - str(FloatType()), - str(BigIntType()), - str(IntegerType()), - ], - ) - } + numeric_types = self._NUMERIC_TYPES_ORDER def resolve_numeric_types_binary_operations( left: ColumnType, @@ -1747,6 +1745,11 @@ def resolve_numeric_types_binary_operations( return left return left + def check_integer_types(left, right): + if str(left) == "int" and str(right) == "int": + return IntegerType() + return raise_binop_exception() + BINOP_TYPE_COMBO_LOOKUP: Dict[ BinaryOpKind, Callable[[ColumnType, ColumnType], ColumnType], @@ -1761,22 +1764,14 @@ def resolve_numeric_types_binary_operations( BinaryOpKind.Lt: lambda left, right: BooleanType(), BinaryOpKind.GtEq: lambda left, right: BooleanType(), BinaryOpKind.LtEq: lambda left, right: BooleanType(), - BinaryOpKind.BitwiseOr: lambda left, right: IntegerType() - if str(left) == str(IntegerType()) and str(right) == str(IntegerType()) - else raise_binop_exception(), - BinaryOpKind.BitwiseAnd: lambda left, right: IntegerType() - if str(left) == str(IntegerType()) and str(right) == str(IntegerType()) - else raise_binop_exception(), - BinaryOpKind.BitwiseXor: lambda left, right: IntegerType() - if str(left) == str(IntegerType()) and str(right) == str(IntegerType()) - else raise_binop_exception(), + BinaryOpKind.BitwiseOr: check_integer_types, + BinaryOpKind.BitwiseAnd: check_integer_types, + BinaryOpKind.BitwiseXor: check_integer_types, BinaryOpKind.Multiply: resolve_numeric_types_binary_operations, BinaryOpKind.Divide: resolve_numeric_types_binary_operations, BinaryOpKind.Plus: resolve_numeric_types_binary_operations, BinaryOpKind.Minus: resolve_numeric_types_binary_operations, - BinaryOpKind.Modulo: lambda left, right: IntegerType() - if str(left) == str(IntegerType()) and str(right) == str(IntegerType()) - else raise_binop_exception(), + BinaryOpKind.Modulo: check_integer_types, } return BINOP_TYPE_COMBO_LOOKUP[kind](left_type, right_type) diff --git a/datajunction-server/datajunction_server/sql/parsing/types.py b/datajunction-server/datajunction_server/sql/parsing/types.py index 27a6b5558..6ef5b7ced 100644 --- a/datajunction-server/datajunction_server/sql/parsing/types.py +++ b/datajunction-server/datajunction_server/sql/parsing/types.py @@ -37,20 +37,23 @@ FIXED_PARSER = re.compile(r"(?i)fixed\((?P\d+)\)") VARCHAR_PARSER = re.compile(r"(?i)varchar(\((?P\d+)\))?") -# Singleton caching temporarily disabled for Pydantic v2 compatibility -# TODO: Implement proper caching that works with Pydantic v2 +# Singleton caching disabled for Pydantic v2 compatibility +# The singleton pattern causes issues with Pydantic v2's BaseModel initialization, +# particularly when running tests in parallel (pytest-xdist). +# Types are simple immutable objects, so creating new instances is acceptable. class Singleton: """ - Singleton for types - each subclass gets its own singleton instance + Singleton pattern - DISABLED for Pydantic v2 compatibility. + + This class is kept for backwards compatibility but no longer enforces singleton behavior. + Each call to a type constructor now creates a new instance. """ def __new__(cls, *args, **kwargs): - # Each subclass gets its own _instance attribute - if not hasattr(cls, "_instance") or not isinstance(cls._instance, cls): - cls._instance = super(Singleton, cls).__new__(cls) - return cls._instance + # Always create a new instance (singleton disabled) + return super(Singleton, cls).__new__(cls) class ColumnType(BaseModel): @@ -125,22 +128,23 @@ def has_common_ancestor(type1, type2) -> bool: other than the highest-level ancestor types like ColumnType itself. This determines whether they're part of the same type group and are compatible with each other when performing type compatibility checks. + + Uses MRO (Method Resolution Order) to find all ancestors of both types + and checks for meaningful common ancestors. """ - base_types = (ColumnType, Singleton, PrimitiveType) - if type1 in base_types or type2 in base_types: - return False - if type1 == type2: - return True - current_has = False - for ancestor in type1.__bases__: - for ancestor2 in type2.__bases__: - current_has = current_has or has_common_ancestor( - ancestor, - ancestor2, - ) - if current_has: - return current_has - return False + base_types = {ColumnType, Singleton, PrimitiveType, object} + # Get meaningful ancestors from MRO, excluding base types + ancestors1 = {cls for cls in type1.__mro__ if cls not in base_types} + ancestors2 = {cls for cls in type2.__mro__ if cls not in base_types} + # Check for common ancestors (excluding the types themselves and BaseModel) + common = ancestors1 & ancestors2 + # Filter out non-type-related classes like BaseModel + meaningful_common = { + cls + for cls in common + if cls.__module__.startswith("datajunction_server") + } + return bool(meaningful_common) return has_common_ancestor(self.__class__, other.__class__) @@ -584,10 +588,9 @@ class LongType(BigIntType): in Java (returns `-9223372036854775808`) """ - def __new__(cls, *args, **kwargs): - self = super().__new__(BigIntType, *args, **kwargs) - super(BigIntType, self).__init__("long", "LongType()") - return self + def __init__(self): + # Call ColumnType.__init__ directly to set "long" instead of "bigint" + ColumnType.__init__(self, "long", "LongType()") class FloatingBase(NumberType, Singleton): diff --git a/datajunction-server/tests/api/graphql/find_nodes_test.py b/datajunction-server/tests/api/graphql/find_nodes_test.py index 3aa1a53f0..5372f7d44 100644 --- a/datajunction-server/tests/api/graphql/find_nodes_test.py +++ b/datajunction-server/tests/api/graphql/find_nodes_test.py @@ -448,11 +448,11 @@ async def test_find_by_names( }, { "name": "completed_repairs", - "type": "long", + "type": "bigint", }, { "name": "total_repairs_dispatched", - "type": "long", + "type": "bigint", }, { "name": "total_amount_in_region", @@ -468,7 +468,7 @@ async def test_find_by_names( }, { "name": "unique_contractors", - "type": "long", + "type": "bigint", }, ], }, @@ -1630,3 +1630,563 @@ async def test_find_nodes_with_mixed_dimension_formats( assert len(node_names) > 0 # All results should have both dimensions available assert "default.repair_orders_fact" in node_names + + +@pytest.mark.asyncio +async def test_find_nodes_filter_by_owner( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by owner (ownedBy). + """ + # Query for nodes owned by the 'dj' user + query = """ + { + findNodes(ownedBy: "dj") { + name + owners { + username + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have 'dj' as an owner + for node in data["data"]["findNodes"]: + owner_usernames = [owner["username"] for owner in node["owners"]] + assert "dj" in owner_usernames + + # Verify we got some results + assert len(data["data"]["findNodes"]) > 0 + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_by_owner( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by owner (ownedBy) using paginated endpoint. + """ + query = """ + { + findNodesPaginated(ownedBy: "dj", limit: 10) { + edges { + node { + name + owners { + username + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have 'dj' as an owner + for edge in data["data"]["findNodesPaginated"]["edges"]: + owner_usernames = [owner["username"] for owner in edge["node"]["owners"]] + assert "dj" in owner_usernames + + +@pytest.mark.asyncio +async def test_find_nodes_filter_by_status_valid( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by status (VALID). + """ + query = """ + { + findNodes(statuses: [VALID]) { + name + current { + status + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have VALID status + for node in data["data"]["findNodes"]: + assert node["current"]["status"] == "VALID" + + # Verify we got some results + assert len(data["data"]["findNodes"]) > 0 + + +@pytest.mark.asyncio +async def test_find_nodes_filter_by_status_invalid( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by status (INVALID). + First create an invalid node, then filter for it. + """ + # Create a node that references a non-existent parent (will be invalid) + response = await module__client_with_roads.post( + "/nodes/transform/", + json={ + "name": "default.invalid_test_node", + "description": "An invalid test node", + "query": "SELECT * FROM default.nonexistent_table", + "mode": "published", + }, + ) + # This should fail or create an invalid node + + query = """ + { + findNodes(statuses: [INVALID]) { + name + current { + status + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have INVALID status + for node in data["data"]["findNodes"]: + assert node["current"]["status"] == "INVALID" + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_by_status( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by status using paginated endpoint. + """ + query = """ + { + findNodesPaginated(statuses: [VALID], limit: 10) { + edges { + node { + name + current { + status + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have VALID status + for edge in data["data"]["findNodesPaginated"]["edges"]: + assert edge["node"]["current"]["status"] == "VALID" + + +@pytest.mark.asyncio +async def test_find_nodes_filter_missing_description( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that are missing descriptions. + """ + # First create a node without a description + response = await module__client_with_roads.post( + "/nodes/transform/", + json={ + "name": "default.no_description_node", + "description": "", # Empty description + "query": "SELECT 1 as id", + "mode": "published", + }, + ) + + query = """ + { + findNodes(missingDescription: true) { + name + current { + description + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have empty or null descriptions + for node in data["data"]["findNodes"]: + desc = node["current"]["description"] + assert desc is None or desc == "" + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_missing_description( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that are missing descriptions using paginated endpoint. + """ + query = """ + { + findNodesPaginated(missingDescription: true, limit: 10) { + edges { + node { + name + current { + description + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have empty or null descriptions + for edge in data["data"]["findNodesPaginated"]["edges"]: + desc = edge["node"]["current"]["description"] + assert desc is None or desc == "" + + +@pytest.mark.asyncio +async def test_find_nodes_filter_missing_owner( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that are missing owners. + """ + query = """ + { + findNodes(missingOwner: true) { + name + owners { + username + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have no owners + for node in data["data"]["findNodes"]: + assert node["owners"] == [] or node["owners"] is None + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_missing_owner( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that are missing owners using paginated endpoint. + """ + query = """ + { + findNodesPaginated(missingOwner: true, limit: 10) { + edges { + node { + name + owners { + username + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have no owners + for edge in data["data"]["findNodesPaginated"]["edges"]: + owners = edge["node"]["owners"] + assert owners == [] or owners is None + + +@pytest.mark.asyncio +async def test_find_nodes_filter_orphaned_dimension( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering for orphaned dimension nodes (dimensions not linked to by any other node). + """ + # First, create an orphaned dimension (a dimension that no other node links to) + response = await module__client_with_roads.post( + "/nodes/dimension/", + json={ + "name": "default.orphaned_dimension_test", + "description": "An orphaned dimension for testing", + "query": "SELECT 1 as orphan_id, 'test' as orphan_name", + "primary_key": ["orphan_id"], + "mode": "published", + }, + ) + + query = """ + { + findNodes(orphanedDimension: true) { + name + type + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should be dimensions + for node in data["data"]["findNodes"]: + assert node["type"] == "DIMENSION" + + # The orphaned dimension we created should be in the results + node_names = {node["name"] for node in data["data"]["findNodes"]} + assert "default.orphaned_dimension_test" in node_names + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_orphaned_dimension( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering for orphaned dimension nodes using paginated endpoint. + """ + query = """ + { + findNodesPaginated(orphanedDimension: true, limit: 10) { + edges { + node { + name + type + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should be dimensions + for edge in data["data"]["findNodesPaginated"]["edges"]: + assert edge["node"]["type"] == "DIMENSION" + + +@pytest.mark.asyncio +async def test_find_nodes_combined_filters( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test combining multiple filters together. + """ + # Combine ownedBy with status filter + query = """ + { + findNodes(ownedBy: "dj", statuses: [VALID], nodeTypes: [METRIC]) { + name + type + owners { + username + } + current { + status + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should match all filters + for node in data["data"]["findNodes"]: + assert node["type"] == "METRIC" + assert node["current"]["status"] == "VALID" + owner_usernames = [owner["username"] for owner in node["owners"]] + assert "dj" in owner_usernames + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_combined_filters( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test combining multiple filters together using paginated endpoint. + """ + query = """ + { + findNodesPaginated(ownedBy: "dj", statuses: [VALID], nodeTypes: [SOURCE], limit: 10) { + edges { + node { + name + type + owners { + username + } + current { + status + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should match all filters + for edge in data["data"]["findNodesPaginated"]["edges"]: + node = edge["node"] + assert node["type"] == "SOURCE" + assert node["current"]["status"] == "VALID" + owner_usernames = [owner["username"] for owner in node["owners"]] + assert "dj" in owner_usernames + + +@pytest.mark.asyncio +async def test_find_nodes_filter_by_nonexistent_owner( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test that filtering by a nonexistent owner returns empty results. + """ + query = """ + { + findNodes(ownedBy: "nonexistent_user_12345") { + name + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + assert data["data"]["findNodes"] == [] + + +@pytest.mark.asyncio +async def test_find_nodes_filter_multiple_statuses( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes by multiple statuses. + """ + query = """ + { + findNodes(statuses: [VALID, INVALID]) { + name + current { + status + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # All returned nodes should have either VALID or INVALID status + for node in data["data"]["findNodes"]: + assert node["current"]["status"] in ["VALID", "INVALID"] + + # Verify we got some results + assert len(data["data"]["findNodes"]) > 0 + + +@pytest.mark.asyncio +async def test_find_nodes_paginated_filter_has_materialization( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that have materializations configured. + """ + # First, set up a partition column on a node so we can create a materialization + await module__client_with_roads.post( + "/nodes/default.repair_orders_fact/columns/repair_order_id/partition", + json={"type_": "categorical"}, + ) + + # Create a materialization on a node + response = await module__client_with_roads.post( + "/nodes/default.repair_orders_fact/materialization", + json={ + "job": "spark_sql", + "strategy": "full", + "schedule": "@daily", + "config": {}, + }, + ) + # Note: materialization creation may fail in test environment without query service, + # but the node should still be marked as having materialization configured + + # Query for nodes with materializations + query = """ + { + findNodesPaginated(hasMaterialization: true, limit: 10) { + edges { + node { + name + type + current { + materializations { + name + } + } + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # If we got results, all returned nodes should have materializations + for edge in data["data"]["findNodesPaginated"]["edges"]: + node = edge["node"] + materializations = node["current"]["materializations"] + assert materializations is not None and len(materializations) > 0 + + +@pytest.mark.asyncio +async def test_find_nodes_filter_has_materialization( + module__client_with_roads: AsyncClient, +) -> None: + """ + Test filtering nodes that have materializations using non-paginated endpoint. + """ + query = """ + { + findNodes(hasMaterialization: true) { + name + type + current { + materializations { + name + } + } + } + } + """ + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + + # If we got results, all returned nodes should have materializations + for node in data["data"]["findNodes"]: + materializations = node["current"]["materializations"] + assert materializations is not None and len(materializations) > 0 diff --git a/datajunction-server/tests/api/measures_test.py b/datajunction-server/tests/api/measures_test.py index 3ffb65ff7..a12640e0e 100644 --- a/datajunction-server/tests/api/measures_test.py +++ b/datajunction-server/tests/api/measures_test.py @@ -81,7 +81,7 @@ async def test_list_all_measures( { "name": "completed_repairs", "node": "default.regional_level_agg", - "type": "long", + "type": "bigint", }, ], "description": "Number of completed repairs", @@ -117,7 +117,7 @@ async def test_create_measure( { "name": "completed_repairs", "node": "default.regional_level_agg", - "type": "long", + "type": "bigint", }, ], "description": "Number of completed repairs", @@ -169,7 +169,7 @@ async def test_edit_measure( { "name": "completed_repairs", "node": "default.regional_level_agg", - "type": "long", + "type": "bigint", }, ], "description": "random description", @@ -220,7 +220,7 @@ async def test_edit_measure( { "name": "completed_repairs", "node": "default.regional_level_agg", - "type": "long", + "type": "bigint", }, ], "description": "random description", diff --git a/datajunction-server/tests/api/namespaces_test.py b/datajunction-server/tests/api/namespaces_test.py index 6be0ee497..ec37e102a 100644 --- a/datajunction-server/tests/api/namespaces_test.py +++ b/datajunction-server/tests/api/namespaces_test.py @@ -763,7 +763,7 @@ async def test_export_namespaces_deployment(client_with_roads: AsyncClient): "description": None, "display_name": "Num Repair Orders", "name": "default.num_repair_orders", - "type": "long", + "type": "bigint", "partition": None, }, { diff --git a/datajunction-server/tests/api/nodes_test.py b/datajunction-server/tests/api/nodes_test.py index 25154b218..659f36d94 100644 --- a/datajunction-server/tests/api/nodes_test.py +++ b/datajunction-server/tests/api/nodes_test.py @@ -4897,7 +4897,7 @@ async def test_lineage_on_complex_transforms(self, client_with_roads: AsyncClien "dimension_column": None, "display_name": "Completed Repairs", "name": "completed_repairs", - "type": "long", + "type": "bigint", "partition": None, }, { @@ -4907,7 +4907,7 @@ async def test_lineage_on_complex_transforms(self, client_with_roads: AsyncClien "dimension_column": None, "display_name": "Total Repairs Dispatched", "name": "total_repairs_dispatched", - "type": "long", + "type": "bigint", "partition": None, }, { @@ -4947,7 +4947,7 @@ async def test_lineage_on_complex_transforms(self, client_with_roads: AsyncClien "dimension_column": None, "display_name": "Unique Contractors", "name": "unique_contractors", - "type": "long", + "type": "bigint", "partition": None, }, ] diff --git a/datajunction-ui/src/app/icons/FilterIcon.jsx b/datajunction-ui/src/app/icons/FilterIcon.jsx deleted file mode 100644 index 3dd7f29d3..000000000 --- a/datajunction-ui/src/app/icons/FilterIcon.jsx +++ /dev/null @@ -1,7 +0,0 @@ -const FilterIcon = props => ( - - - - -); -export default FilterIcon; diff --git a/datajunction-ui/src/app/pages/NamespacePage/CompactSelect.jsx b/datajunction-ui/src/app/pages/NamespacePage/CompactSelect.jsx new file mode 100644 index 000000000..f1e54619e --- /dev/null +++ b/datajunction-ui/src/app/pages/NamespacePage/CompactSelect.jsx @@ -0,0 +1,100 @@ +import Select from 'react-select'; + +// Compact select with label above - saves horizontal space +export default function CompactSelect({ + label, + name, + options, + value, + onChange, + isMulti = false, + isClearable = true, + placeholder = 'Select...', + minWidth = '100px', + flex = 1, + isLoading = false, + testId = null, +}) { + // For single select, find the matching option + // For multi select, filter to matching options + const selectedValue = isMulti + ? value?.length + ? options.filter(o => value.includes(o.value)) + : [] + : value + ? options.find(o => o.value === value) + : null; + + return ( +
+ + onChange(e)} - styles={{ - control: styles => ({ ...styles, backgroundColor: 'white' }), - }} - options={[ - { value: 'source', label: 'Source' }, - { value: 'transform', label: 'Transform' }, - { value: 'dimension', label: 'Dimension' }, - { value: 'metric', label: 'Metric' }, - { value: 'cube', label: 'Cube' }, - ]} - /> - - ); -} diff --git a/datajunction-ui/src/app/pages/NamespacePage/TagSelect.jsx b/datajunction-ui/src/app/pages/NamespacePage/TagSelect.jsx deleted file mode 100644 index 7fa6d94e4..000000000 --- a/datajunction-ui/src/app/pages/NamespacePage/TagSelect.jsx +++ /dev/null @@ -1,44 +0,0 @@ -import { useContext, useEffect, useState } from 'react'; -import DJClientContext from '../../providers/djclient'; -import Control from './FieldControl'; - -import Select from 'react-select'; - -export default function TagSelect({ onChange }) { - const djClient = useContext(DJClientContext).DataJunctionAPI; - - const [retrieved, setRetrieved] = useState(false); - const [tags, setTags] = useState([]); - - useEffect(() => { - const fetchData = async () => { - const tags = await djClient.listTags(); - setTags(tags); - setRetrieved(true); - }; - fetchData().catch(console.error); - }, [djClient]); - - return ( - - onChange(e)} - defaultValue={{ - value: currentUser, - label: currentUser, - }} - options={users?.map(user => { - return { value: user.username, label: user.username }; - })} - /> - ) : ( - '' - )} - - ); -} diff --git a/datajunction-ui/src/app/pages/NamespacePage/__tests__/CompactSelect.test.jsx b/datajunction-ui/src/app/pages/NamespacePage/__tests__/CompactSelect.test.jsx new file mode 100644 index 000000000..3550c0778 --- /dev/null +++ b/datajunction-ui/src/app/pages/NamespacePage/__tests__/CompactSelect.test.jsx @@ -0,0 +1,190 @@ +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import CompactSelect from '../CompactSelect'; + +describe('', () => { + const defaultOptions = [ + { value: 'option1', label: 'Option 1' }, + { value: 'option2', label: 'Option 2' }, + { value: 'option3', label: 'Option 3' }, + ]; + + const defaultProps = { + label: 'Test Label', + name: 'test-select', + options: defaultOptions, + value: '', + onChange: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders without crashing', () => { + render(); + expect(screen.getByText('Test Label')).toBeInTheDocument(); + }); + + it('displays the label correctly', () => { + render(); + expect(screen.getByText('My Custom Label')).toBeInTheDocument(); + }); + + it('shows placeholder when no value is selected', () => { + render(); + expect(screen.getByText('Choose one...')).toBeInTheDocument(); + }); + + it('displays the selected value for single select', () => { + render(); + expect(screen.getByText('Option 1')).toBeInTheDocument(); + }); + + it('calls onChange when an option is selected', async () => { + const handleChange = jest.fn(); + render(); + + // Open the dropdown + const selectInput = screen.getByRole('combobox'); + fireEvent.keyDown(selectInput, { key: 'ArrowDown' }); + + // Click on an option + await waitFor(() => { + expect(screen.getByText('Option 2')).toBeInTheDocument(); + }); + fireEvent.click(screen.getByText('Option 2')); + + expect(handleChange).toHaveBeenCalledWith( + expect.objectContaining({ value: 'option2', label: 'Option 2' }), + expect.anything(), + ); + }); + + it('supports multi-select mode', async () => { + const handleChange = jest.fn(); + render( + , + ); + + // Should show the selected value + expect(screen.getByText('Option 1')).toBeInTheDocument(); + }); + + it('displays multiple selected values in multi-select mode', () => { + render( + , + ); + + expect(screen.getByText('Option 1')).toBeInTheDocument(); + expect(screen.getByText('Option 2')).toBeInTheDocument(); + }); + + it('shows loading state when isLoading is true', () => { + render(); + // react-select shows a loading indicator when isLoading is true + expect(document.querySelector('.css-1dimb5e-singleValue')).toBeNull(); + }); + + it('allows clearing the selection when isClearable is true', async () => { + const handleChange = jest.fn(); + render( + , + ); + + // Find and click the clear button + const clearButton = document.querySelector( + '[class*="indicatorContainer"]:first-of-type', + ); + if (clearButton) { + fireEvent.mouseDown(clearButton); + } + }); + + it('respects minWidth prop', () => { + const { container } = render( + , + ); + const wrapper = container.firstChild; + expect(wrapper).toHaveStyle({ minWidth: '200px' }); + }); + + it('respects flex prop', () => { + const { container } = render(); + const wrapper = container.firstChild; + expect(wrapper).toHaveStyle({ flex: '2' }); + }); + + it('handles empty options array', () => { + render(); + expect(screen.getByText('Test Label')).toBeInTheDocument(); + }); + + it('handles null value gracefully', () => { + render(); + expect(screen.getByText('Select...')).toBeInTheDocument(); + }); + + it('handles undefined value gracefully', () => { + render(); + expect(screen.getByText('Select...')).toBeInTheDocument(); + }); + + it('renders with custom placeholder', () => { + render(); + expect(screen.getByText('Pick something...')).toBeInTheDocument(); + }); + + it('uses default placeholder when none provided', () => { + render(); + expect(screen.getByText('Select...')).toBeInTheDocument(); + }); + + it('applies compact styling with reduced height', () => { + const { container } = render(); + // The control element should have compact styling + const control = container.querySelector('[class*="control"]'); + expect(control).toBeInTheDocument(); + }); + + it('opens dropdown on click', async () => { + render(); + + const selectInput = screen.getByRole('combobox'); + fireEvent.mouseDown(selectInput); + + await waitFor(() => { + // Menu should be visible with options + expect(screen.getByText('Option 1')).toBeInTheDocument(); + expect(screen.getByText('Option 2')).toBeInTheDocument(); + expect(screen.getByText('Option 3')).toBeInTheDocument(); + }); + }); + + it('filters options based on user input', async () => { + render(); + + const selectInput = screen.getByRole('combobox'); + fireEvent.focus(selectInput); + await userEvent.type(selectInput, '2'); + + await waitFor(() => { + expect(screen.getByText('Option 2')).toBeInTheDocument(); + }); + }); +}); diff --git a/datajunction-ui/src/app/pages/NamespacePage/__tests__/index.test.jsx b/datajunction-ui/src/app/pages/NamespacePage/__tests__/index.test.jsx index 35f8b9861..1f0656b0d 100644 --- a/datajunction-ui/src/app/pages/NamespacePage/__tests__/index.test.jsx +++ b/datajunction-ui/src/app/pages/NamespacePage/__tests__/index.test.jsx @@ -1,6 +1,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter, Route, Routes } from 'react-router-dom'; import DJClientContext from '../../../providers/djclient'; +import UserContext from '../../../providers/UserProvider'; import { NamespacePage } from '../index'; import React from 'react'; import userEvent from '@testing-library/user-event'; @@ -15,6 +16,25 @@ const mockDjClient = { listTags: jest.fn(), }; +const mockCurrentUser = { username: 'dj', email: 'dj@test.com' }; + +const renderWithProviders = (ui, { route = '/namespaces/default' } = {}) => { + return render( + + + + + + + + + + , + ); +}; + describe('NamespacePage', () => { const original = window.location; @@ -164,38 +184,68 @@ describe('NamespacePage', () => { // --- Sorting --- + // Track current call count + const initialCallCount = mockDjClient.listNodesForLanding.mock.calls.length; + // sort by 'name' fireEvent.click(screen.getByText('name')); await waitFor(() => { - expect(mockDjClient.listNodesForLanding).toHaveBeenCalledTimes(2); + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(initialCallCount); }); + const afterFirstSort = mockDjClient.listNodesForLanding.mock.calls.length; + // flip direction fireEvent.click(screen.getByText('name')); await waitFor(() => { - expect(mockDjClient.listNodesForLanding).toHaveBeenCalledTimes(3); + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(afterFirstSort); }); + const afterSecondSort = mockDjClient.listNodesForLanding.mock.calls.length; + // sort by 'displayName' fireEvent.click(screen.getByText('display Name')); await waitFor(() => { - expect(mockDjClient.listNodesForLanding).toHaveBeenCalledTimes(4); + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(afterSecondSort); }); // --- Filters --- - // Node type + // Node type - use react-select properly const selectNodeType = screen.getAllByTestId('select-node-type')[0]; - fireEvent.keyDown(selectNodeType.firstChild, { key: 'ArrowDown' }); - fireEvent.click(screen.getByText('Source')); + const typeInput = selectNodeType.querySelector('input'); + if (typeInput) { + fireEvent.focus(typeInput); + fireEvent.keyDown(typeInput, { key: 'ArrowDown' }); + await waitFor(() => { + const sourceOption = screen.queryByText('Source'); + if (sourceOption) { + fireEvent.click(sourceOption); + } + }); + } // Tag filter const selectTag = screen.getAllByTestId('select-tag')[0]; - fireEvent.keyDown(selectTag.firstChild, { key: 'ArrowDown' }); + const tagInput = selectTag.querySelector('input'); + if (tagInput) { + fireEvent.focus(tagInput); + fireEvent.keyDown(tagInput, { key: 'ArrowDown' }); + } // User filter const selectUser = screen.getAllByTestId('select-user')[0]; - fireEvent.keyDown(selectUser.firstChild, { key: 'ArrowDown' }); + const userInput = selectUser.querySelector('input'); + if (userInput) { + fireEvent.focus(userInput); + fireEvent.keyDown(userInput, { key: 'ArrowDown' }); + } // --- Expand/Collapse Namespace --- fireEvent.click(screen.getByText('common')); @@ -328,4 +378,243 @@ describe('NamespacePage', () => { expect(screen.getByText('you failed')).toBeInTheDocument(); }); }); + + describe('Filter Bar', () => { + it('displays quick filter presets', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('Quick:')).toBeInTheDocument(); + }); + + // Check that preset buttons are rendered + expect(screen.getByText('My Nodes')).toBeInTheDocument(); + expect(screen.getByText('Needs Attention')).toBeInTheDocument(); + expect(screen.getByText('Drafts')).toBeInTheDocument(); + }); + + it('applies My Nodes preset when clicked', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('My Nodes')).toBeInTheDocument(); + }); + + const initialCalls = mockDjClient.listNodesForLanding.mock.calls.length; + fireEvent.click(screen.getByText('My Nodes')); + + await waitFor(() => { + // The API should be called again after clicking preset + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(initialCalls); + }); + }); + + it('applies Needs Attention preset when clicked', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('Needs Attention')).toBeInTheDocument(); + }); + + const initialCalls = mockDjClient.listNodesForLanding.mock.calls.length; + fireEvent.click(screen.getByText('Needs Attention')); + + await waitFor(() => { + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(initialCalls); + }); + }); + + it('applies Drafts preset when clicked', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('Drafts')).toBeInTheDocument(); + }); + + const initialCalls = mockDjClient.listNodesForLanding.mock.calls.length; + fireEvent.click(screen.getByText('Drafts')); + + await waitFor(() => { + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(initialCalls); + }); + }); + + it('shows Clear all button when filters are active', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('My Nodes')).toBeInTheDocument(); + }); + + // Apply a preset to activate filters + fireEvent.click(screen.getByText('My Nodes')); + + await waitFor(() => { + expect(screen.getByText('Clear all ×')).toBeInTheDocument(); + }); + }); + + it('clears all filters when Clear all is clicked', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('My Nodes')).toBeInTheDocument(); + }); + + // Apply a preset + fireEvent.click(screen.getByText('My Nodes')); + + await waitFor(() => { + expect(screen.getByText('Clear all ×')).toBeInTheDocument(); + }); + + // Clear all filters + fireEvent.click(screen.getByText('Clear all ×')); + + await waitFor(() => { + // Clear all button should disappear + expect(screen.queryByText('Clear all ×')).not.toBeInTheDocument(); + }); + }); + + it('displays filter dropdowns', async () => { + renderWithProviders(); + + await waitFor(() => { + // Check for filter labels + expect(screen.getByText('Type')).toBeInTheDocument(); + expect(screen.getByText('Tags')).toBeInTheDocument(); + expect(screen.getByText('Edited By')).toBeInTheDocument(); + expect(screen.getByText('Mode')).toBeInTheDocument(); + expect(screen.getByText('Owner')).toBeInTheDocument(); + expect(screen.getByText('Status')).toBeInTheDocument(); + expect(screen.getByText('Quality')).toBeInTheDocument(); + }); + }); + + it('opens Quality dropdown when clicked', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('Quality')).toBeInTheDocument(); + }); + + // Find and click the Quality button + const qualityButton = screen.getByText('Issues'); + fireEvent.click(qualityButton); + + await waitFor(() => { + expect(screen.getByText('Missing Description')).toBeInTheDocument(); + expect(screen.getByText('Orphaned Dimensions')).toBeInTheDocument(); + expect(screen.getByText('Has Materialization')).toBeInTheDocument(); + }); + }); + + it('toggles quality filters in dropdown', async () => { + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('Quality')).toBeInTheDocument(); + }); + + // Open the Quality dropdown + const qualityButton = screen.getByText('Issues'); + fireEvent.click(qualityButton); + + await waitFor(() => { + expect(screen.getByText('Missing Description')).toBeInTheDocument(); + }); + + // Toggle the Missing Description checkbox + const checkbox = screen.getByLabelText('Missing Description'); + const callsBefore = mockDjClient.listNodesForLanding.mock.calls.length; + fireEvent.click(checkbox); + + await waitFor(() => { + expect( + mockDjClient.listNodesForLanding.mock.calls.length, + ).toBeGreaterThan(callsBefore); + }); + }); + + it('displays no nodes message with clear filter link when no results', async () => { + mockDjClient.listNodesForLanding.mockResolvedValue({ + data: { + findNodesPaginated: { + pageInfo: { + hasNextPage: false, + endCursor: null, + hasPrevPage: false, + startCursor: null, + }, + edges: [], + }, + }, + }); + + renderWithProviders(); + + // Apply a filter first + await waitFor(() => { + expect(screen.getByText('My Nodes')).toBeInTheDocument(); + }); + fireEvent.click(screen.getByText('My Nodes')); + + await waitFor(() => { + expect( + screen.getByText('No nodes found with the current filters.'), + ).toBeInTheDocument(); + expect(screen.getByText('Clear filters')).toBeInTheDocument(); + }); + }); + }); + + describe('URL Parameter Sync', () => { + it('reads filters from URL parameters on load', async () => { + renderWithProviders(, { + route: '/namespaces/default?type=metric&ownedBy=dj', + }); + + await waitFor(() => { + expect(mockDjClient.listNodesForLanding).toHaveBeenCalled(); + }); + }); + + it('reads status filter from URL', async () => { + renderWithProviders(, { + route: '/namespaces/default?statuses=INVALID', + }); + + await waitFor(() => { + expect(mockDjClient.listNodesForLanding).toHaveBeenCalled(); + }); + }); + + it('reads mode filter from URL', async () => { + renderWithProviders(, { + route: '/namespaces/default?mode=draft', + }); + + await waitFor(() => { + expect(mockDjClient.listNodesForLanding).toHaveBeenCalled(); + }); + }); + + it('reads quality filters from URL', async () => { + renderWithProviders(, { + route: + '/namespaces/default?missingDescription=true&orphanedDimension=true', + }); + + await waitFor(() => { + expect(mockDjClient.listNodesForLanding).toHaveBeenCalled(); + }); + }); + }); }); diff --git a/datajunction-ui/src/app/pages/NamespacePage/index.jsx b/datajunction-ui/src/app/pages/NamespacePage/index.jsx index f163d4790..e6f525870 100644 --- a/datajunction-ui/src/app/pages/NamespacePage/index.jsx +++ b/datajunction-ui/src/app/pages/NamespacePage/index.jsx @@ -1,19 +1,14 @@ import * as React from 'react'; -import { useParams } from 'react-router-dom'; -import { useContext, useEffect, useState } from 'react'; +import { useParams, useSearchParams } from 'react-router-dom'; +import { useContext, useEffect, useState, useCallback } from 'react'; import NodeStatus from '../NodePage/NodeStatus'; import DJClientContext from '../../providers/djclient'; import { useCurrentUser } from '../../providers/UserProvider'; import Explorer from '../NamespacePage/Explorer'; import AddNodeDropdown from '../../components/AddNodeDropdown'; import NodeListActions from '../../components/NodeListActions'; -import AddNamespacePopover from './AddNamespacePopover'; -import FilterIcon from '../../icons/FilterIcon'; import LoadingIcon from '../../icons/LoadingIcon'; -import UserSelect from './UserSelect'; -import NodeTypeSelect from './NodeTypeSelect'; -import NodeModeSelect from './NodeModeSelect'; -import TagSelect from './TagSelect'; +import CompactSelect from './CompactSelect'; import 'styles/node-list.css'; import 'styles/sorted-table.css'; @@ -27,6 +22,142 @@ export function NamespacePage() { const djClient = useContext(DJClientContext).DataJunctionAPI; const { currentUser } = useCurrentUser(); var { namespace } = useParams(); + const [searchParams, setSearchParams] = useSearchParams(); + + // Data for select options + const [users, setUsers] = useState([]); + const [tags, setTags] = useState([]); + const [usersLoading, setUsersLoading] = useState(true); + const [tagsLoading, setTagsLoading] = useState(true); + + // Load users and tags for dropdowns + useEffect(() => { + const fetchUsers = async () => { + const data = await djClient.users(); + setUsers(data || []); + setUsersLoading(false); + }; + const fetchTags = async () => { + const data = await djClient.listTags(); + setTags(data || []); + setTagsLoading(false); + }; + fetchUsers().catch(console.error); + fetchTags().catch(console.error); + }, [djClient]); + + // Parse all filters from URL + const getFiltersFromUrl = useCallback( + () => ({ + node_type: searchParams.get('type') || '', + tags: searchParams.get('tags') ? searchParams.get('tags').split(',') : [], + edited_by: searchParams.get('editedBy') || '', + mode: searchParams.get('mode') || '', + ownedBy: searchParams.get('ownedBy') || '', + statuses: searchParams.get('statuses') || '', + missingDescription: searchParams.get('missingDescription') === 'true', + hasMaterialization: searchParams.get('hasMaterialization') === 'true', + orphanedDimension: searchParams.get('orphanedDimension') === 'true', + }), + [searchParams], + ); + + const [filters, setFilters] = useState(getFiltersFromUrl); + const [moreFiltersOpen, setMoreFiltersOpen] = useState(false); + + // Sync filters state when URL changes + useEffect(() => { + setFilters(getFiltersFromUrl()); + }, [searchParams, getFiltersFromUrl]); + + // Update URL when filters change + const updateFilters = useCallback( + newFilters => { + const params = new URLSearchParams(); + + if (newFilters.node_type) params.set('type', newFilters.node_type); + if (newFilters.tags?.length) + params.set('tags', newFilters.tags.join(',')); + if (newFilters.edited_by) params.set('editedBy', newFilters.edited_by); + if (newFilters.mode) params.set('mode', newFilters.mode); + if (newFilters.ownedBy) params.set('ownedBy', newFilters.ownedBy); + if (newFilters.statuses) params.set('statuses', newFilters.statuses); + if (newFilters.missingDescription) + params.set('missingDescription', 'true'); + if (newFilters.hasMaterialization) + params.set('hasMaterialization', 'true'); + if (newFilters.orphanedDimension) params.set('orphanedDimension', 'true'); + + setSearchParams(params); + }, + [setSearchParams], + ); + + const clearAllFilters = () => { + setSearchParams(new URLSearchParams()); + }; + + // Check if any filters are active + const hasActiveFilters = + filters.node_type || + filters.tags?.length || + filters.edited_by || + filters.mode || + filters.ownedBy || + filters.statuses || + filters.missingDescription || + filters.hasMaterialization || + filters.orphanedDimension; + + // Quick presets + const presets = [ + { + id: 'my-nodes', + label: 'My Nodes', + filters: { ownedBy: currentUser?.username }, + }, + { + id: 'needs-attention', + label: 'Needs Attention', + filters: { ownedBy: currentUser?.username, statuses: 'INVALID' }, + }, + { + id: 'drafts', + label: 'Drafts', + filters: { ownedBy: currentUser?.username, mode: 'draft' }, + }, + ]; + + const applyPreset = preset => { + const newFilters = { + node_type: '', + tags: [], + edited_by: '', + mode: preset.filters.mode || '', + ownedBy: preset.filters.ownedBy || '', + statuses: preset.filters.statuses || '', + missingDescription: preset.filters.missingDescription || false, + hasMaterialization: preset.filters.hasMaterialization || false, + orphanedDimension: preset.filters.orphanedDimension || false, + }; + updateFilters(newFilters); + }; + + // Check if a preset is active + const isPresetActive = preset => { + const pf = preset.filters; + return ( + (pf.ownedBy || '') === (filters.ownedBy || '') && + (pf.statuses || '') === (filters.statuses || '') && + (pf.mode || '') === (filters.mode || '') && + !filters.node_type && + !filters.tags?.length && + !filters.edited_by && + !filters.missingDescription && + !filters.hasMaterialization && + !filters.orphanedDimension + ); + }; const [state, setState] = useState({ namespace: namespace ? namespace : '', @@ -34,13 +165,6 @@ export function NamespacePage() { }); const [retrieved, setRetrieved] = useState(false); - const [filters, setFilters] = useState({ - tags: [], - node_type: '', - edited_by: '', - mode: '', - }); - const [namespaceHierarchy, setNamespaceHierarchy] = useState([]); const [sortConfig, setSortConfig] = useState({ @@ -113,6 +237,16 @@ export function NamespacePage() { useEffect(() => { const fetchData = async () => { setRetrieved(false); + + // Build extended filters for API + const extendedFilters = { + ownedBy: filters.ownedBy || null, + statuses: filters.statuses ? [filters.statuses] : null, + missingDescription: filters.missingDescription, + hasMaterialization: filters.hasMaterialization, + orphanedDimension: filters.orphanedDimension, + }; + const nodes = await djClient.listNodesForLanding( namespace, filters.node_type ? [filters.node_type.toUpperCase()] : [], @@ -123,6 +257,7 @@ export function NamespacePage() { 50, sortConfig, filters.mode ? filters.mode.toUpperCase() : null, + extendedFilters, ); setState({ @@ -152,7 +287,15 @@ export function NamespacePage() { setRetrieved(true); }; fetchData().catch(console.error); - }, [djClient, filters, before, after, sortConfig.key, sortConfig.direction]); + }, [ + djClient, + filters, + before, + after, + sortConfig.key, + sortConfig.direction, + namespace, + ]); const loadNext = () => { if (nextCursor) { @@ -167,6 +310,31 @@ export function NamespacePage() { } }; + // Select options + const typeOptions = [ + { value: 'source', label: 'Source' }, + { value: 'transform', label: 'Transform' }, + { value: 'dimension', label: 'Dimension' }, + { value: 'metric', label: 'Metric' }, + { value: 'cube', label: 'Cube' }, + ]; + + const modeOptions = [ + { value: 'published', label: 'Published' }, + { value: 'draft', label: 'Draft' }, + ]; + + const statusOptions = [ + { value: 'VALID', label: 'Valid' }, + { value: 'INVALID', label: 'Invalid' }, + ]; + + const userOptions = users.map(u => ({ + value: u.username, + label: u.username, + })); + const tagOptions = tags.map(t => ({ value: t.name, label: t.display_name })); + const nodesList = retrieved ? ( state.nodes.length > 0 ? ( state.nodes.map(node => ( @@ -234,7 +402,7 @@ export function NamespacePage() { )) ) : ( - + - There are no nodes in{' '} - {namespace} with the above - filters! + No nodes found with the current filters. + {hasActiveFilters && ( + { + e.preventDefault(); + clearAllFilters(); + }} + style={{ marginLeft: '0.5rem' }} + > + Clear filters + + )} @@ -260,63 +438,312 @@ export function NamespacePage() { ); + // Count active quality filters (the ones in the "More" dropdown) + const moreFiltersCount = [ + filters.missingDescription, + filters.hasMaterialization, + filters.orphanedDimension, + ].filter(Boolean).length; + return (
-

Explore

-
+
+

Explore

+ +
+ + {/* Unified Filter Bar */} +
+ {/* Top row: Quick presets + Clear all */}
- +
+ Quick: + {presets.map(preset => ( + + ))} + {hasActiveFilters && ( + + )} +
+ + {/* Bottom row: Dropdowns */}
- Filter + + updateFilters({ ...filters, node_type: e?.value || '' }) + } + flex={1} + minWidth="80px" + testId="select-node-type" + /> + + updateFilters({ + ...filters, + tags: e ? e.map(t => t.value) : [], + }) + } + isMulti + isLoading={tagsLoading} + flex={1.5} + minWidth="100px" + testId="select-tag" + /> + + updateFilters({ ...filters, edited_by: e?.value || '' }) + } + isLoading={usersLoading} + flex={1} + minWidth="80px" + testId="select-user" + /> + + updateFilters({ ...filters, mode: e?.value || '' }) + } + flex={1} + minWidth="80px" + /> + + updateFilters({ ...filters, ownedBy: e?.value || '' }) + } + isLoading={usersLoading} + flex={1} + minWidth="80px" + /> + + updateFilters({ ...filters, statuses: e?.value || '' }) + } + flex={1} + minWidth="80px" + /> + + {/* More Filters (Quality) */} +
+
+ + +
+ + {moreFiltersOpen && ( +
+ + + +
+ )} +
- - setFilters({ ...filters, node_type: entry ? entry.value : '' }) - } - /> - - setFilters({ - ...filters, - tags: entry ? entry.map(tag => tag.value) : [], - }) - } - /> - - setFilters({ ...filters, edited_by: entry ? entry.value : '' }) - } - currentUser={currentUser?.username} - /> - - setFilters({ ...filters, mode: entry ? entry.value : '' }) - } - /> -
+