diff --git a/.env.example b/.env.example index f742b1ca74..b8e98a0eee 100644 --- a/.env.example +++ b/.env.example @@ -81,6 +81,7 @@ DATABASE_NAME=baserow # BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES= # BASEROW_AUTOMATION_WORKFLOW_HISTORY_MIN_RETENTION_DAYS= # BASEROW_AUTOMATION_WORKFLOW_HISTORY_CLEANUP_INTERVAL_MINUTES= +# BASEROW_AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN= # BASEROW_EXTRA_ALLOWED_HOSTS= # ADDITIONAL_APPS= # ADDITIONAL_MODULES= diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a76ef4261b..fb01f962dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,6 +211,7 @@ jobs: backend: ${{ steps.filter.outputs.backend }} frontend: ${{ steps.filter.outputs.frontend }} dockerfiles: ${{ steps.filter.outputs.dockerfiles }} + e2e: ${{ steps.filter.outputs.e2e }} mjml: ${{ steps.filter.outputs.mjml }} zapier: ${{ steps.filter.outputs.zapier }} helm: ${{ steps.filter.outputs.helm }} @@ -236,6 +237,9 @@ jobs: dockerfiles: - '**/Dockerfile' - '.github/workflows/ci.yml' + e2e: + - 'e2e-tests/**' + - '.github/workflows/ci.yml' mjml: - '**/*.eta' - '.github/workflows/ci.yml' @@ -673,7 +677,7 @@ jobs: - detect-changes - build-backend - build-frontend - if: needs.detect-changes.outputs.backend == 'true' || needs.detect-changes.outputs.frontend == 'true' || needs.detect-changes.outputs.dockerfiles == 'true' || github.ref_name == 'develop' || github.ref_name == 'master' + if: needs.detect-changes.outputs.backend == 'true' || needs.detect-changes.outputs.frontend == 'true' || needs.detect-changes.outputs.dockerfiles == 'true' || needs.detect-changes.outputs.e2e == 'true' || github.ref_name == 'develop' || github.ref_name == 'master' permissions: contents: read packages: read @@ -866,6 +870,10 @@ jobs: PUBLIC_BACKEND_URL: http://localhost:8000 PUBLIC_WEB_FRONTEND_URL: http://localhost:3000 PRIVATE_BACKEND_URL: http://backend:8000 + # The CI web-frontend runs with the default (empty) cookie prefix, so + # tests that read/set cookies (e.g. the user_source_token) must use the + # same empty prefix rather than playwright.config's baserow_e2e_ default. + BASEROW_FRONTEND_COOKIE_PREFIX: "" CI: 1 run: | cd e2e-tests diff --git a/backend/src/baserow/config/settings/base.py b/backend/src/baserow/config/settings/base.py index b34884e157..b5de2e9316 100644 --- a/backend/src/baserow/config/settings/base.py +++ b/backend/src/baserow/config/settings/base.py @@ -975,6 +975,11 @@ def __setitem__(self, key, value): AUTOMATION_WORKFLOW_HISTORY_CLEANUP_INTERVAL_MINUTES = int( os.getenv("BASEROW_AUTOMATION_WORKFLOW_HISTORY_CLEANUP_INTERVAL_MINUTES", 60) ) +# The maximum number of node dispatches allowed in a single workflow run. +# This protects against infinite dispatches due to a misconfigured node. +AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN = int( + os.getenv("BASEROW_AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN", 1000) +) TRASH_PAGE_SIZE_LIMIT = 200 # How many trash entries can be requested at once. diff --git a/backend/src/baserow/contrib/automation/api/history/serializers.py b/backend/src/baserow/contrib/automation/api/history/serializers.py index 2c8b374e42..ca9a74f042 100644 --- a/backend/src/baserow/contrib/automation/api/history/serializers.py +++ b/backend/src/baserow/contrib/automation/api/history/serializers.py @@ -15,6 +15,9 @@ class AutomationNodeHistorySerializer(serializers.ModelSerializer): iteration = serializers.SerializerMethodField() iteration_path = serializers.SerializerMethodField() edge_label = serializers.SerializerMethodField() + destination_node_id = serializers.SerializerMethodField() + destination_node_type = serializers.SerializerMethodField() + destination_label = serializers.SerializerMethodField() class Meta: model = AutomationNodeHistory @@ -32,6 +35,9 @@ class Meta: "iteration", "iteration_path", "edge_label", + "destination_node_id", + "destination_node_type", + "destination_label", ) @extend_schema_field(OpenApiTypes.STR) @@ -71,6 +77,18 @@ def get_iteration_path(self, obj): def get_edge_label(self, obj): return self.context.get("edge_labels", {}).get(obj.id, "") + @extend_schema_field(OpenApiTypes.INT) + def get_destination_node_id(self, obj): + return self.context.get("destinations", {}).get(obj.id, {}).get("id") + + @extend_schema_field(OpenApiTypes.STR) + def get_destination_node_type(self, obj): + return self.context.get("destinations", {}).get(obj.id, {}).get("type", "") + + @extend_schema_field(OpenApiTypes.STR) + def get_destination_label(self, obj): + return self.context.get("destinations", {}).get(obj.id, {}).get("label", "") + class AutomationNodeResultSerializer(serializers.ModelSerializer): class Meta: diff --git a/backend/src/baserow/contrib/automation/api/history/views.py b/backend/src/baserow/contrib/automation/api/history/views.py index 9dff0d5726..af44c7eb0d 100644 --- a/backend/src/baserow/contrib/automation/api/history/views.py +++ b/backend/src/baserow/contrib/automation/api/history/views.py @@ -57,10 +57,14 @@ def get(self, request, workflow_history_id: int): service = AutomationHistoryService() node_histories = service.get_node_histories(request.user, workflow_history_id) edge_labels = service.get_edge_labels(request.user, node_histories) + destinations = service.get_destination_labels(request.user, node_histories) serializer = AutomationNodeHistorySerializer( node_histories, many=True, - context={"edge_labels": edge_labels}, + context={ + "edge_labels": edge_labels, + "destinations": destinations, + }, ) return Response(serializer.data) diff --git a/backend/src/baserow/contrib/automation/apps.py b/backend/src/baserow/contrib/automation/apps.py index 36eedace5a..3f892dc45b 100644 --- a/backend/src/baserow/contrib/automation/apps.py +++ b/backend/src/baserow/contrib/automation/apps.py @@ -20,6 +20,7 @@ def ready(self): from baserow.contrib.automation.nodes.node_types import ( AIAgentActionNodeType, CoreCSVFileReaderNodeType, + CoreGotoActionNodeType, CoreHttpRequestNodeType, CoreHTTPTriggerNodeType, CoreIteratorNodeType, @@ -186,6 +187,7 @@ def ready(self): automation_node_type_registry.register(CoreSMTPEmailNodeType()) automation_node_type_registry.register(CoreRouterActionNodeType()) automation_node_type_registry.register(CoreStartWorkflowNodeType()) + automation_node_type_registry.register(CoreGotoActionNodeType()) automation_node_type_registry.register(LocalBaserowRowsCreatedNodeTriggerType()) automation_node_type_registry.register(LocalBaserowRowsUpdatedNodeTriggerType()) automation_node_type_registry.register(LocalBaserowRowsDeletedNodeTriggerType()) diff --git a/backend/src/baserow/contrib/automation/history/constants.py b/backend/src/baserow/contrib/automation/history/constants.py index 54a8fabc99..7057c3e3cd 100644 --- a/backend/src/baserow/contrib/automation/history/constants.py +++ b/backend/src/baserow/contrib/automation/history/constants.py @@ -6,3 +6,6 @@ class HistoryStatusChoices(models.TextChoices): ERROR = "error" DISABLED = "disabled" STARTED = "started" + # The node ran without doing anything, e.g. a "Go to node" whose + # condition resolved to false, so no jump was followed. + SKIPPED = "skipped" diff --git a/backend/src/baserow/contrib/automation/history/handler.py b/backend/src/baserow/contrib/automation/history/handler.py index 2eae84af4e..b38d6a140b 100644 --- a/backend/src/baserow/contrib/automation/history/handler.py +++ b/backend/src/baserow/contrib/automation/history/handler.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Dict, Iterable, List, Optional, Union +from typing import Any, Dict, Iterable, List, Optional, Union from django.db.models import Prefetch, QuerySet @@ -126,15 +126,25 @@ def create_node_result( def get_node_result(self, history, node, iteration_path): """ Returns the result for the given history/node/iteration_path. + + A node can be dispatched several times within a single run when a jump + (e.g. the "Go to node" node) loops execution back to it. Each pass writes a + new result with the same iteration_path, so we return the most recent one + rather than assuming a single result exists. """ - try: - node_result = AutomationNodeResult.objects.only("result").get( + node_result = ( + AutomationNodeResult.objects.only("result") + .filter( node_history__workflow_history_id=history.id, node_history__node_id=node.id, iteration_path=iteration_path, ) - except AutomationNodeResult.DoesNotExist: + .order_by("-node_history__started_on", "-node_history_id") + .first() + ) + + if node_result is None: raise AutomationWorkflowHistoryNodeResultDoesNotExist() return node_result.result @@ -271,3 +281,46 @@ def get_edge_labels( for nr in results if (label := nr.result.get("edge", {}).get("label")) } + + def get_destination_labels( + self, node_histories: List[AutomationNodeHistory] + ) -> Dict[int, Dict[str, Any]]: + """ + For each history entry whose node redirected execution to another node + (e.g. the "Go to node" node), return a mapping that describes the node + it jumped to, e.g.: + { + "id": , + "type": , + "label": , + }` + + The destination node belongs to the published workflow copy that + actually ran, so its id cannot be resolved against the editor workflow. + We therefore also return its type, allowing the frontend to display a + human-readable name from the node type registry when the node has no + custom label. + """ + + # A node can be dispatched many times within a single run when a jump + # loops execution back to it, so we resolve each distinct node only + # once. Resolving a destination costs several queries, and the result + # only depends on the node, not on the individual dispatch. + resolved: Dict[int, Optional[Dict[str, Any]]] = {} + labels = {} + for nh in node_histories: + node = nh.node + if node.id not in resolved: + destination = node.get_type().get_history_destination_node(node) + resolved[node.id] = ( + { + "id": destination.id, + "type": destination.get_type().type, + "label": destination.label, + } + if destination is not None + else None + ) + if (destination_label := resolved[node.id]) is not None: + labels[nh.id] = destination_label + return labels diff --git a/backend/src/baserow/contrib/automation/history/service.py b/backend/src/baserow/contrib/automation/history/service.py index 5f8b754efd..969aaf20e8 100644 --- a/backend/src/baserow/contrib/automation/history/service.py +++ b/backend/src/baserow/contrib/automation/history/service.py @@ -1,4 +1,4 @@ -from typing import Dict, List +from typing import Any, Dict, List from django.contrib.auth.models import AbstractUser from django.db.models import QuerySet @@ -66,3 +66,15 @@ def get_edge_labels( workflow = node_histories[0].workflow_history.original_workflow self._check_workflow_permissions(user, workflow) return self.handler.get_edge_labels(node_histories) + + def get_destination_labels( + self, + user: AbstractUser, + node_histories: List[AutomationNodeHistory], + ) -> Dict[int, Dict[str, Any]]: + if not node_histories: + return {} + + workflow = node_histories[0].workflow_history.original_workflow + self._check_workflow_permissions(user, workflow) + return self.handler.get_destination_labels(node_histories) diff --git a/backend/src/baserow/contrib/automation/migrations/0035_coregotoactionnode.py b/backend/src/baserow/contrib/automation/migrations/0035_coregotoactionnode.py new file mode 100644 index 0000000000..b425dc273f --- /dev/null +++ b/backend/src/baserow/contrib/automation/migrations/0035_coregotoactionnode.py @@ -0,0 +1,34 @@ +# Generated by Django 5.2.14 on 2026-06-24 09:14 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('automation', '0034_corestartworkflowactionnode'), + ] + + operations = [ + migrations.CreateModel( + name='CoreGotoActionNode', + fields=[ + ('automationnode_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='automation.automationnode')), + ], + options={ + 'abstract': False, + }, + bases=('automation.automationnode',), + ), + migrations.AlterField( + model_name='automationnodehistory', + name='status', + field=models.CharField(choices=[('success', 'Success'), ('error', 'Error'), ('disabled', 'Disabled'), ('started', 'Started'), ('skipped', 'Skipped')], max_length=8), + ), + migrations.AlterField( + model_name='automationworkflowhistory', + name='status', + field=models.CharField(choices=[('success', 'Success'), ('error', 'Error'), ('disabled', 'Disabled'), ('started', 'Started'), ('skipped', 'Skipped')], max_length=8), + ), + ] diff --git a/backend/src/baserow/contrib/automation/nodes/actions.py b/backend/src/baserow/contrib/automation/nodes/actions.py index 5920b21e4d..cd7a63c6f9 100644 --- a/backend/src/baserow/contrib/automation/nodes/actions.py +++ b/backend/src/baserow/contrib/automation/nodes/actions.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any from django.contrib.auth.models import AbstractUser @@ -10,6 +10,7 @@ ) from baserow.contrib.automation.nodes.models import AutomationActionNode, AutomationNode from baserow.contrib.automation.nodes.node_types import AutomationNodeType +from baserow.contrib.automation.nodes.registries import automation_node_type_registry from baserow.contrib.automation.nodes.service import AutomationNodeService from baserow.contrib.automation.nodes.trash_types import AutomationNodeTrashableItemType from baserow.contrib.automation.workflows.models import AutomationWorkflow @@ -392,6 +393,9 @@ class Params: destination_reference_node_id: int destination_position: GraphPointPositionType destination_output: str + # Reversible modifications node types made to reconcile the workflow + # after the move, keyed by node type. Reverted on undo. + move_extra_data: dict = field(default_factory=dict) @classmethod def do( @@ -425,6 +429,7 @@ def do( reference_node_id, position, output, + move.move_extra_data, ), scope=cls.scope(workflow.id), workspace=workflow.automation.workspace, @@ -449,6 +454,12 @@ def undo( params.origin_position, params.origin_output, ) + # The node is back at its original level, so revert the reconciliations + # the move made (e.g. restore Go to links it cleared). + for node_type_str, modifications in params.move_extra_data.items(): + automation_node_type_registry.get(node_type_str).revert_move( + user, modifications + ) @classmethod def redo( diff --git a/backend/src/baserow/contrib/automation/nodes/exceptions.py b/backend/src/baserow/contrib/automation/nodes/exceptions.py index da7ac2df3c..087ff45c7e 100644 --- a/backend/src/baserow/contrib/automation/nodes/exceptions.py +++ b/backend/src/baserow/contrib/automation/nodes/exceptions.py @@ -75,3 +75,10 @@ class AutomationNodeMissingOutput(AutomationNodeError): """ Raised when the target output is missing in the reference node. """ + + +class AutomationNodeMaxDispatchesExceeded(AutomationNodeError): + """ + Raised when a workflow run exceeds the maximum number of node dispatches, + a safety backstop against infinite loops. + """ diff --git a/backend/src/baserow/contrib/automation/nodes/handler.py b/backend/src/baserow/contrib/automation/nodes/handler.py index 6805e4080a..14de46443a 100644 --- a/backend/src/baserow/contrib/automation/nodes/handler.py +++ b/backend/src/baserow/contrib/automation/nodes/handler.py @@ -1,6 +1,8 @@ from collections import defaultdict from typing import Any, Dict, Iterable, List, Optional, Type, Union +from django.conf import settings +from django.core.cache import cache from django.core.files.storage import Storage from django.db.models import QuerySet from django.utils import timezone @@ -26,6 +28,7 @@ from baserow.contrib.automation.models import AutomationWorkflow from baserow.contrib.automation.nodes.exceptions import ( AutomationNodeDoesNotExist, + AutomationNodeMaxDispatchesExceeded, ) from baserow.contrib.automation.nodes.models import AutomationNode from baserow.contrib.automation.nodes.node_types import ( @@ -173,6 +176,25 @@ def get_node( except AutomationNode.DoesNotExist: raise AutomationNodeDoesNotExist(node_id) + def get_node_by_service_id(self, service_id: int) -> AutomationNode: + """ + Return the AutomationNode that owns the given service. The node<->service + relation is one-to-one, so a service maps to exactly one node. + + :param service_id: The ID of the node's service. + :raises AutomationNodeDoesNotExist: If no node owns the service. + :return: The specific model instance of the AutomationNode. + """ + + try: + return ( + AutomationNode.objects.select_related("workflow__automation__workspace") + .get(service_id=service_id) + .specific + ) + except AutomationNode.DoesNotExist: + raise AutomationNodeDoesNotExist(service_id) + def create_node( self, node_type: AutomationNodeType, @@ -232,6 +254,10 @@ def duplicate_node(self, source_node: AutomationNode) -> AutomationNode: id_mapping = defaultdict(lambda: MirrorDict()) id_mapping["automation_workflow_nodes"] = MirrorDict() + # A single-node duplicate leaves referenced nodes (and thus their + # services) in place, so mirror service ids too. This lets a self + # reference like a "Go to node" destination carry over unchanged. + id_mapping["services"] = MirrorDict() import_export_config = ImportExportConfig( include_permission_data=True, @@ -342,13 +368,15 @@ def import_nodes( if progress: progress.increment(state=IMPORT_SERIALIZED_IMPORTING) - # We migrate service formulas here to make sure all nodes are imported before - # we migrate them + # We migrate service formulas and remap cross-service references here to + # make sure all nodes are imported before we migrate them for imported_node in imported_nodes: service = imported_node.service.specific - updated_models = service.get_type().import_formulas( + service_type = service.get_type() + updated_models = service_type.import_formulas( service, id_mapping, import_formula, **kwargs ) + updated_models |= service_type.after_import(service, id_mapping, **kwargs) [u.save() for u in updated_models] @@ -398,6 +426,23 @@ def _handle_workflow_error( iteration_path=iteration_path, ) + def _node_dispatch_count_cache_key(self, history_id: int) -> str: + return f"automation_node_dispatch_count_{history_id}" + + def _check_node_dispatch_limit(self, history_id: int) -> bool: + """ + Increments and checks the per-run node dispatch counter. The counter + is keyed per workflow run and self-expires after the workflow timeout. + + This is a safety backstop against infinite loops, e.g. a misconfigured + "Go to node". + """ + + key = self._node_dispatch_count_cache_key(history_id) + cache.add(key, 0, settings.AUTOMATION_WORKFLOW_TIMEOUT_HOURS * 60 * 60) + dispatch_count = cache.incr(key) + return dispatch_count > settings.AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN + def _handle_simulation_notify( self, simulate_until_node: AutomationNode | None, node: AutomationNode ) -> bool: @@ -418,12 +463,26 @@ def _handle_simulation_notify( return False def _before_node_dispatch( - self, node: AutomationNode, workflow_history: AutomationWorkflowHistory + self, + node: AutomationNode, + workflow_history: AutomationWorkflowHistory, ) -> None: """ - Sends a signal before a node is dispatched. + Runs pre-dispatch checks and emits the started signal. + + :raises AutomationNodeMaxDispatchesExceeded: If the workflow run has + exceeded the maximum number of node dispatches allowed. """ + # Stop the workflow run if it exceeds the max dispatches allowed. + # Safety backstop against infinite loops, e.g. a misconfigured + # "Go to node". + if self._check_node_dispatch_limit(workflow_history.id): + limit = settings.AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN + raise AutomationNodeMaxDispatchesExceeded( + f"Workflow exceeded the maximum of {limit} node dispatches." + ) + automation_node_dispatch_started.send( sender=self, node=node, @@ -522,6 +581,22 @@ def dispatch_node( try: self._before_node_dispatch(node, workflow_history) dispatch_result = node_type.dispatch(node, dispatch_context) + if ( + dispatch_result.destination_service_id is not None + and not simulate_until_node + ): + # The dispatch requested a jump the runner is about to follow + # (jumps are never followed while simulating). Let the node type + # re-validate the destination against the live graph first. + node_type.validate_jump_destination( + node, dispatch_result.destination_service_id + ) + except AutomationNodeMaxDispatchesExceeded as e: + error = str(e) + logger.warning(error) + self._handle_workflow_error(node_history, iteration_path, error) + self._handle_simulation_notify(simulate_until_node, node) + return None except ServiceImproperlyConfiguredDispatchException as e: error = f"The node is misconfigured and cannot be dispatched. {str(e)}" self._handle_workflow_error(node_history, iteration_path, error) @@ -557,7 +632,7 @@ def dispatch_node( # can accurately rely on the completed_on field. now = timezone.now() node_history.completed_on = now - node_history.status = HistoryStatusChoices.SUCCESS + node_history.status = node_type.get_history_status(dispatch_result) node_history.save() # The post-dispatch hook should also be able to safely access the @@ -602,16 +677,31 @@ def dispatch_node( canvas = chain(*groups_to_chain) to_chain.append(canvas) - # Handle non-iterator nodes, including iterator children. - next_nodes = node.get_next_points(dispatch_result.output_uid) - if next_nodes: + if ( + dispatch_result.destination_service_id is not None + and not simulate_until_node + ): + # A node (e.g. "Go to node") requested a jump to a specific node, + # identified by its service, rather than the natural next node. + # While simulating, the jump is never followed: execution walks the + # natural path towards the simulated node instead of looping on it. + next_node_ids = [ + self.get_node_by_service_id(dispatch_result.destination_service_id).id + ] + else: + # Handle non-iterator nodes, including iterator children. + next_node_ids = [ + n.id for n in node.get_next_points(dispatch_result.output_uid) + ] + + if next_node_ids: to_chain.append( group( [ dispatch_node_celery_task.si( - n.id, history_id, current_iterations + next_id, history_id, current_iterations ) - for n in next_nodes + for next_id in next_node_ids ] ), ) diff --git a/backend/src/baserow/contrib/automation/nodes/models.py b/backend/src/baserow/contrib/automation/nodes/models.py index 6b291fbd39..a3c264911a 100644 --- a/backend/src/baserow/contrib/automation/nodes/models.py +++ b/backend/src/baserow/contrib/automation/nodes/models.py @@ -185,6 +185,9 @@ class CoreSMTPEmailActionNode(AutomationActionNode): ... class CoreRouterActionNode(AutomationActionNode): ... +class CoreGotoActionNode(AutomationActionNode): ... + + class CoreIteratorActionNode(AutomationActionNode): ... diff --git a/backend/src/baserow/contrib/automation/nodes/node_types.py b/backend/src/baserow/contrib/automation/nodes/node_types.py index 78ab531537..cb3521346d 100644 --- a/backend/src/baserow/contrib/automation/nodes/node_types.py +++ b/backend/src/baserow/contrib/automation/nodes/node_types.py @@ -6,7 +6,9 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from baserow.contrib.automation.history.constants import HistoryStatusChoices from baserow.contrib.automation.nodes.exceptions import ( + AutomationNodeDoesNotExist, AutomationNodeFirstNodeMustBeTrigger, AutomationNodeMisconfiguredService, AutomationNodeNotDeletable, @@ -20,6 +22,7 @@ AutomationNode, AutomationTriggerNode, CoreCSVFileReaderActionNode, + CoreGotoActionNode, CoreHTTPRequestActionNode, CoreHTTPTriggerNode, CoreIteratorActionNode, @@ -43,11 +46,14 @@ SlackWriteMessageActionNode, ) from baserow.contrib.automation.nodes.registries import AutomationNodeType +from baserow.contrib.automation.nodes.signals import automation_node_updated from baserow.contrib.automation.workflows.constants import WorkflowState from baserow.contrib.automation.workflows.models import AutomationWorkflow from baserow.contrib.integrations.ai.service_types import AIAgentServiceType +from baserow.contrib.integrations.core.models import CoreGotoService from baserow.contrib.integrations.core.service_types import ( CoreCSVFileReaderServiceType, + CoreGotoServiceType, CoreHTTPRequestServiceType, CoreHTTPTriggerServiceType, CoreIteratorServiceType, @@ -75,8 +81,12 @@ ) from baserow.core.graph.types import GraphPointPositionType from baserow.core.registry import Instance +from baserow.core.services.exceptions import ( + ServiceImproperlyConfiguredDispatchException, +) from baserow.core.services.models import Service from baserow.core.services.registries import service_type_registry +from baserow.core.services.types import DispatchResult class AutomationNodeActionNodeType(AutomationNodeType): @@ -354,6 +364,264 @@ def prepare_values( return super().prepare_values(values, user, instance) +class CoreGotoActionNodeType(AutomationNodeActionNodeType): + type = "goto" + model_class = CoreGotoActionNode + service_type = CoreGotoServiceType.type + + def get_history_status(self, dispatch_result: DispatchResult) -> str: + """ + A "Go to" node only jumps when its condition resolves to true, so a + dispatch without a destination means the jump was not followed. The + history is marked as skipped in that case, so it isn't presented as + if execution had been redirected. + """ + + if dispatch_result.destination_service_id is None: + return HistoryStatusChoices.SKIPPED + return HistoryStatusChoices.SUCCESS + + def get_history_destination_node( + self, node: AutomationNode + ) -> Optional[AutomationNode]: + """ + Resolves the node this "Go to" node jumps to via its service's + configured destination. Returns None when no destination has been + configured (or the destination service is not backed by a node). + """ + + service = node.service.specific + destination_service = service.destination_service + + if destination_service is None: + return None + + return getattr(destination_service, "automation_workflow_node", None) + + @staticmethod + def validate_goto_destination( + source_node: AutomationNode, + destination_node: Optional[AutomationNode], + ) -> Optional[str]: + """ + Validates that destination_node is an eligible "Go to node" destination + for source_node. + + A destination is eligible when it belongs to the same workflow, is at + the same level (i.e. has the same parent/container nodes), is not a + trigger node and runs before the Go to node on its own path (a backward + jump). Forward jumps are not allowed for now: they would leave the + skipped nodes unexecuted, so a later node that reads a skipped node's + output via the previous-node data provider would fail at dispatch time. + A node may not target itself. + """ + + if destination_node is None: + return None + + if destination_node.id == source_node.id: + return "The destination node cannot be the Go to node itself." + + if destination_node.workflow_id != source_node.workflow_id: + return "The destination node must belong to the same workflow." + + if destination_node.get_type().is_workflow_trigger: + return "The destination node cannot be a trigger node." + + source_level = sorted(node.id for node in source_node.get_parent_points()) + destination_level = sorted( + node.id for node in destination_node.get_parent_points() + ) + if source_level != destination_level: + return "The destination node must be at the same level as the Go to node." + + # The destination must run before the Go to node on its own path (a + # backward jump). `get_previous_points` returns the whole root-to-node + # path, so this both rejects forward jumps and a same-level node on a + # different branch, whose own predecessors would not have run when the + # jump lands on it. + source_previous_ids = {node.id for node in source_node.get_previous_points()} + if destination_node.id not in source_previous_ids: + return "The destination node must run before the Go to node." + + return None + + def validate_jump_destination( + self, + automation_node: AutomationNode, + destination_service_id: int, + ) -> None: + """ + Re-validates the configured jump against the live graph before the + runner follows it. The service only resolves the intent to jump (and to + which destination); a link that became invalid after the service was + configured (e.g. the destination was moved to another level) raises a + clean misconfigured error instead of jumping. + + The runner calls this only when the jump is about to be followed, so a + jump that is never followed (e.g. while simulating) is never validated. + """ + + from baserow.contrib.automation.nodes.handler import AutomationNodeHandler + + try: + destination_node = AutomationNodeHandler().get_node_by_service_id( + destination_service_id + ) + except AutomationNodeDoesNotExist: + # The destination was deleted after the jump was configured. The + # link is only nulled when the destination is permanently deleted, + # so a trashed destination still resolves to a service here. + raise ServiceImproperlyConfiguredDispatchException( + "The destination node no longer exists." + ) + + if error := self.validate_goto_destination(automation_node, destination_node): + raise ServiceImproperlyConfiguredDispatchException(error) + + def prepare_values( + self, + values: Dict[str, Any], + user: AbstractUser, + instance: AutomationNode = None, + ) -> Dict[str, Any]: + """ + Validates the configured destination node before the service is updated. + + The destination must be a same-level, non-trigger node of the same + workflow, and cannot be the Go to node itself. We can only check this + when updating an existing node, as the source node must already exist + in the graph to determine its level. + + A destination that no longer exists is dropped instead of rejected: the + link is only nulled when the destination is permanently deleted, so an + update can carry a destination whose node has since been trashed (e.g. + when undoing an update that predates the deletion). + """ + + from baserow.contrib.automation.nodes.handler import AutomationNodeHandler + + service_values = values.get("service", {}) + if instance is not None and service_values.get("destination_service_id"): + try: + destination_node = AutomationNodeHandler().get_node_by_service_id( + service_values["destination_service_id"] + ) + except AutomationNodeDoesNotExist: + values = { + **values, + "service": {**service_values, "destination_service_id": None}, + } + else: + if error := self.validate_goto_destination(instance, destination_node): + raise AutomationNodeMisconfiguredService(error) + + return super().prepare_values(values, user, instance) + + def after_move( + self, user: AbstractUser, workflow: AutomationWorkflow + ) -> list[tuple[int, int]] | None: + # A move can change a node's level or take it off the source node's + # path, either of which may invalidate a "Go to node" link that targets + # - or originates from - the moved node. Clear any now-invalid links and + # report them so the move can be undone. + return self.clear_invalidated_links(user, workflow) or None + + def revert_move( + self, user: AbstractUser, modifications: list[tuple[int, int]] + ) -> None: + self.restore_links(user, modifications) + + @classmethod + def clear_invalidated_links( + cls, + user: AbstractUser, + workflow: AutomationWorkflow, + ) -> list[tuple[int, int]]: + """ + Nulls every "Go to node" destination in the workflow that is no longer a + valid jump from its source node - i.e. it left the source's level or its + path. Intended to be called after a move, which can change either. We + simply re-validate every goto link in the workflow: there are few of + them, and this avoids depending on the exact descendant semantics of the + graph to work out which links the move could have touched. A link + survives when it is still a valid backward jump (e.g. source and + destination moved together inside a container). + + An `automation_node_updated` signal is sent for each cleared Go to node + so connected clients drop the stale link. + + :return: A list of (goto_node_id, previous_destination_node_id) tuples + describing the links that were cleared, so the caller can restore + them when a move is undone. + """ + + from baserow.contrib.automation.nodes.handler import AutomationNodeHandler + + handler = AutomationNodeHandler() + goto_services = CoreGotoService.objects.filter( + automation_workflow_node__workflow=workflow, + destination_service__isnull=False, + ).select_related( + "automation_workflow_node", + "destination_service__automation_workflow_node", + ) + + cleared_goto_links: list[tuple[int, int]] = [] + for service in goto_services: + source_node = service.automation_workflow_node + destination_node = service.destination_service.automation_workflow_node + if cls.validate_goto_destination(source_node, destination_node) is None: + continue + + service.destination_service = None + service.save(update_fields=["destination_service"]) + cleared_goto_links.append((source_node.id, destination_node.id)) + + automation_node_updated.send( + cls, user=user, node=handler.get_node(source_node.id) + ) + + return cleared_goto_links + + @classmethod + def restore_links( + cls, + user: AbstractUser, + links: list[tuple[int, int]], + ) -> None: + """ + Re-applies "Go to node" destinations that a move cleared, used when that + move is undone. Each link is re-validated against the (restored) graph + and skipped if it would still be invalid, so we never persist a + cross-level link. + + :param links: (goto_node_id, destination_node_id) tuples to restore. + """ + + from baserow.contrib.automation.nodes.handler import AutomationNodeHandler + + handler = AutomationNodeHandler() + for goto_node_id, destination_node_id in links: + try: + goto_node = handler.get_node(goto_node_id) + destination_node = handler.get_node(destination_node_id) + except AutomationNodeDoesNotExist: + # An endpoint was deleted after the move was made; nothing to + # restore. (The link stays cleared, which is correct.) + continue + if cls.validate_goto_destination(goto_node, destination_node) is not None: + continue + + service = goto_node.service.specific + service.destination_service_id = destination_node.service_id + service.save(update_fields=["destination_service"]) + + automation_node_updated.send( + cls, user=user, node=handler.get_node(goto_node_id) + ) + + class AutomationNodeTriggerType(AutomationNodeType): is_workflow_trigger = True diff --git a/backend/src/baserow/contrib/automation/nodes/registries.py b/backend/src/baserow/contrib/automation/nodes/registries.py index 4f34f8eebb..e4982cf0a1 100644 --- a/backend/src/baserow/contrib/automation/nodes/registries.py +++ b/backend/src/baserow/contrib/automation/nodes/registries.py @@ -8,6 +8,7 @@ from baserow.contrib.automation.automation_dispatch_context import ( AutomationDispatchContext, ) +from baserow.contrib.automation.history.constants import HistoryStatusChoices from baserow.contrib.automation.nodes.exceptions import ( AutomationNodeMisconfiguredService, AutomationNodeNotReplaceable, @@ -132,11 +133,72 @@ def after_create(self, node: AutomationNode) -> None: :param node: The node instance that was just created. """ + def after_move( + self, user: AbstractUser, workflow: AutomationWorkflow + ) -> Any | None: + """ + A hook called after any node is moved within `workflow`. A node type + can override this to reconcile state that the move may have invalidated + (for example cross-level references between nodes), returning an opaque, + JSON-serializable payload describing the modifications it made so + `revert_move` can undo them. Returns None (the default) + when the type made no changes. + + :param user: The user that performed the move. + :param workflow: The workflow the moved node belongs to. + :return: A JSON-serializable payload describing the modifications made, + or None if nothing changed. + """ + + return None + + def revert_move(self, user: AbstractUser, modifications: Any) -> None: + """ + Reverses the modifications previously returned by `after_move`, + used when a node move is undone. + + :param user: The user undoing the move. + :param modifications: The payload returned by `after_move`. + """ + def get_service_type(self) -> Optional[ServiceTypeSubClass]: return ( service_type_registry.get(self.service_type) if self.service_type else None ) + def get_history_destination_node( + self, node: AutomationNode + ) -> Optional[AutomationNode]: + """ + Returns the node that execution jumped to from the given node during a + run, or None when this node type does not redirect execution. + + Most nodes simply hand off to their natural next node and so have no + explicit destination. Node types that jump elsewhere in the graph (e.g. + the "Go to" node) override this so run histories can show where + execution went. + + :param node: The node instance to resolve the destination for. + :return: The destination node, or None. + """ + + return None + + def get_history_status(self, dispatch_result: DispatchResult) -> str: + """ + Returns the status the node history should be marked with after a + successful dispatch. Most node types always did something, so they + report a success. Node types whose dispatch can legitimately be a + no-op (e.g. a "Go to" node whose condition resolved to false) override + this to report a skip instead, so run histories don't suggest that + something happened. + + :param dispatch_result: The result of the node's dispatch. + :return: The history status to store on the node history. + """ + + return HistoryStatusChoices.SUCCESS + def is_replaceable_with(self, other_node_type: "AutomationNodeType") -> bool: """ Determines if this node type can be replaced with another node type. @@ -357,6 +419,30 @@ def dispatch( automation_node.service.specific, dispatch_context ) + def validate_jump_destination( + self, + automation_node: AutomationNode, + destination_service_id: int, + ) -> None: + """ + Hook for node types whose dispatch can request a jump to another node + (by returning a destination_service_id on the DispatchResult). + + The runner calls this only when the jump is about to be followed, i.e. + never while simulating, where jumps are suppressed so a backward jump + doesn't loop the path leading to the simulated node. The node type can + then re-validate the destination against the live graph and raise + ServiceImproperlyConfiguredDispatchException if the link is no longer a + valid jump. + + The default is a no-op, as most node types never request a jump. + + :param automation_node: The node that requested the jump. + :param destination_service_id: The service the jump targets. + :raises ServiceImproperlyConfiguredDispatchException: If the jump is no + longer valid against the current graph. + """ + class AutomationNodeTypeRegistry( Registry, diff --git a/backend/src/baserow/contrib/automation/nodes/service.py b/backend/src/baserow/contrib/automation/nodes/service.py index ef21537e68..e48dff0908 100644 --- a/backend/src/baserow/contrib/automation/nodes/service.py +++ b/backend/src/baserow/contrib/automation/nodes/service.py @@ -1,4 +1,4 @@ -from typing import Iterable, Optional +from typing import Any, Iterable, Optional from django.contrib.auth.models import AbstractUser @@ -509,6 +509,19 @@ def move_node( workflow.get_graph().move(node_to_move, reference_node, position, output) + # A move can change a node's level, which may invalidate cross-node + # references (e.g. a "Go to node" link that now points across levels). + # Let each node type reconcile the workflow and record any reversible + # changes it made, keyed by node type, so the move can be undone. + move_extra_data: dict[str, Any] = {} + for node_type in automation_node_type_registry.get_all(): + modifications = node_type.after_move(user, workflow) + if modifications is not None: + move_extra_data[node_type.type] = modifications + + cache_key = WORKFLOW_DIRTY_CACHE_KEY.format(workflow.id) + global_cache.update(cache_key, lambda _: True) + automation_workflow_updated.send(self, workflow=workflow, user=user) return AutomationNodeMove( @@ -516,4 +529,5 @@ def move_node( previous_reference_node=previous_reference_node, previous_position=previous_position, previous_output=previous_output, + move_extra_data=move_extra_data, ) diff --git a/backend/src/baserow/contrib/automation/nodes/types.py b/backend/src/baserow/contrib/automation/nodes/types.py index 0748817a4b..5fedc65f4a 100644 --- a/backend/src/baserow/contrib/automation/nodes/types.py +++ b/backend/src/baserow/contrib/automation/nodes/types.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any, NewType, TypedDict from baserow.contrib.automation.nodes.models import AutomationActionNode, AutomationNode @@ -28,6 +28,10 @@ class AutomationNodeMove: previous_reference_node: AutomationActionNode | None previous_position: GraphPointPositionType previous_output: str + # Reversible modifications node types made to reconcile the workflow after + # the move (e.g. clearing now-invalid "Go to node" links), keyed by node + # type. Captured so the move action can revert them on undo. + move_extra_data: dict[str, Any] = field(default_factory=dict) class AutomationNodeDict(TypedDict): diff --git a/backend/src/baserow/contrib/integrations/apps.py b/backend/src/baserow/contrib/integrations/apps.py index 0401ebd222..5c431a65b7 100644 --- a/backend/src/baserow/contrib/integrations/apps.py +++ b/backend/src/baserow/contrib/integrations/apps.py @@ -57,6 +57,7 @@ def ready(self): from baserow.contrib.integrations.core.service_types import ( CoreCSVFileReaderServiceType, + CoreGotoServiceType, CoreHTTPRequestServiceType, CoreHTTPTriggerServiceType, CoreIteratorServiceType, @@ -70,6 +71,7 @@ def ready(self): service_type_registry.register(CoreHTTPRequestServiceType()) service_type_registry.register(CoreSMTPEmailServiceType()) service_type_registry.register(CoreRouterServiceType()) + service_type_registry.register(CoreGotoServiceType()) service_type_registry.register(CoreHTTPTriggerServiceType()) service_type_registry.register(CoreManualTriggerServiceType()) service_type_registry.register(CoreIteratorServiceType()) diff --git a/backend/src/baserow/contrib/integrations/core/models.py b/backend/src/baserow/contrib/integrations/core/models.py index 4d706d9696..3376b19ea4 100644 --- a/backend/src/baserow/contrib/integrations/core/models.py +++ b/backend/src/baserow/contrib/integrations/core/models.py @@ -264,6 +264,27 @@ class Meta: ordering = ("order",) +class CoreGotoService(Service): + """ + A service that, when its condition formula evaluates to true, redirects + execution to the configured destination instead of the natural next step. + + This makes while loops, retries, and conditional jumps possible. + """ + + condition = FormulaField( + help_text="The formula that must evaluate to true for the jump to the " + "destination service to be followed.", + ) + destination_service = models.ForeignKey( + Service, + null=True, + on_delete=models.SET_NULL, + related_name="+", + help_text="The service to jump to when the condition evaluates to true.", + ) + + class CorePeriodicService(Service): last_periodic_run = models.DateTimeField( null=True, diff --git a/backend/src/baserow/contrib/integrations/core/service_types.py b/backend/src/baserow/contrib/integrations/core/service_types.py index 49568dd746..f159750a19 100644 --- a/backend/src/baserow/contrib/integrations/core/service_types.py +++ b/backend/src/baserow/contrib/integrations/core/service_types.py @@ -43,6 +43,7 @@ from baserow.contrib.integrations.core.integration_types import SMTPIntegrationType from baserow.contrib.integrations.core.models import ( CoreCSVFileReaderService, + CoreGotoService, CoreHTTPRequestService, CoreHTTPTriggerService, CoreIteratorService, @@ -1262,6 +1263,197 @@ def get_edges(self, service: Service) -> Dict[str, Dict[str, str]]: } +class CoreGotoServiceType(CoreServiceType): + type = "goto" + model_class = CoreGotoService + allowed_fields = ["condition", "destination_service_id"] + dispatch_types = [DispatchTypes.ACTION] + serializer_field_names = ["condition", "destination_service_id"] + simple_formula_fields = ["condition"] + + class SerializedDict(ServiceDict): + condition: str + destination_service_id: int + + @property + def serializer_field_overrides(self): + from baserow.core.formula.serializers import FormulaSerializerField + + return { + "condition": FormulaSerializerField( + help_text=CoreGotoService._meta.get_field("condition").help_text, + required=False, + default="", + ), + "destination_service_id": serializers.IntegerField( + required=False, + allow_null=True, + help_text=CoreGotoService._meta.get_field( + "destination_service" + ).help_text, + ), + } + + def create_instance_from_serialized( + self, + serialized_values, + id_mapping, + files_zip=None, + storage=None, + cache=None, + **kwargs, + ): + """ + The destination is a reference to another service which may not have + been imported yet, as the import order is not guaranteed to follow the + graph order. We therefore null it on this first pass and remap it + during the second pass in after_import(), once all the workflow's + services have been imported. + """ + + original_destination_id = serialized_values.pop("destination_service_id", None) + + service = super().create_instance_from_serialized( + serialized_values, + id_mapping, + files_zip=files_zip, + storage=storage, + cache=cache, + **kwargs, + ) + + if original_destination_id is not None: + id_mapping.setdefault("goto_destination_services", {})[service.id] = ( + original_destination_id + ) + + return service + + def after_import(self, instance, id_mapping, **kwargs): + """ + Performs the second-pass remap of the destination service reference. + + By this point every service of the workflow has been imported, so + id_mapping["services"] can resolve every jump destination. + + When the destination service was part of this import - e.g. a full + workflow duplicate - it is remapped to the newly imported service. For a + partial duplicate, which copies just this Go to service and leaves the + destination service in place, the destination is carried over unchanged + so the duplicate jumps to the same place as the original. The link is + only reset when the destination service is genuinely absent from this + import. + """ + + updated_models = super().after_import(instance, id_mapping, **kwargs) + + pending_destinations = id_mapping.get("goto_destination_services", {}) + if instance.id in pending_destinations: + original_destination_id = pending_destinations[instance.id] + service_mapping = id_mapping.get("services", {}) + # A partial duplicate seeds the service mapping with a MirrorDict, + # whose `get` echoes back any unmapped id, so the destination is + # carried over unchanged. A regular dict returns None instead. + instance.destination_service_id = service_mapping.get( + original_destination_id + ) + updated_models.add(instance) + + return updated_models + + def get_schema_name(self, service: CoreGotoService) -> str: + return f"CoreGoto{service.id}Schema" + + def generate_schema( + self, + service: CoreGotoService, + allowed_fields: Optional[List[str]] = None, + ) -> Dict[str, Any]: + properties = {} + if allowed_fields is None or "condition" in allowed_fields: + properties["condition"] = { + "type": "boolean", + "title": _("Condition"), + "description": _( + "Whether the condition evaluated to true and the jump was followed." + ), + } + + return { + "title": self.get_schema_name(service), + "type": "object", + "properties": properties, + } + + def formulas_to_resolve(self, service: CoreGotoService) -> list[FormulaToResolve]: + return [ + FormulaToResolve( + "condition", + service.condition, + lambda x: ensure_boolean(x, False), + 'property "condition"', + ) + ] + + def _condition_is_set(self, service: CoreGotoService) -> bool: + """ + Whether a condition formula has actually been configured. An unset + condition means the jump is unconditional, so it must be distinguished + from a configured condition that resolves to false (which suppresses + the jump). The resolved boolean alone can't tell these two apart, as + both arrive as `False`. + """ + + formula = (service.condition or {}).get("formula") or "" + # a formula of just white spaces should be considered unset + return bool(formula.strip()) + + def dispatch_data( + self, + service: CoreGotoService, + resolved_values: Dict[str, Any], + dispatch_context: DispatchContext, + ) -> Dict[str, Any]: + """ + Decides whether to follow the jump to the configured destination. The + jump is followed when no condition has been configured (an unconditional + jump) or when the condition resolves to true. It is only skipped when a + condition is set and explicitly resolves to false. + + This only resolves the intent to jump; how the jump is validated against + the graph, and whether it should be suppressed (e.g. while simulating), + is decided by the consumer that owns the graph. The returned + destination_service_id is a plain reference to the configured destination + service, which the consumer resolves within its own graph. + """ + + # An empty condition means "always jump"; only a configured condition + # that resolves to false can prevent it. + should_jump = resolved_values["condition"] or not self._condition_is_set( + service + ) + destination_service_id = None + if should_jump: + if service.destination_service_id is None: + raise ServiceImproperlyConfiguredDispatchException( + "No destination has been configured for this service." + ) + destination_service_id = service.destination_service_id + + return { + "destination_service_id": destination_service_id, + "data": {"condition": bool(should_jump)}, + } + + def dispatch_transform( + self, + data: Any, + ) -> DispatchResult: + return DispatchResult( + destination_service_id=data["destination_service_id"], data=data["data"] + ) + + class CorePeriodicServiceType(TriggerServiceTypeMixin, CoreServiceType): type = "periodic" model_class = CorePeriodicService diff --git a/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py b/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py index 14207571bd..812c57d023 100644 --- a/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py +++ b/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py @@ -2,9 +2,14 @@ from baserow.contrib.integrations.local_baserow.models import ( LocalBaserowTableServiceFilter, + LocalBaserowTableServiceFilterGroup, LocalBaserowTableServiceSort, ) from baserow.core.formula.serializers import FormulaSerializerField +from baserow.core.services.models import ( + SERVICE_FILTER_TYPE_AND, + SERVICE_FILTER_TYPES, +) class LocalBaserowTableServiceSortSerializer(serializers.ModelSerializer): @@ -53,6 +58,33 @@ def to_internal_value(self, data): return data +class LocalBaserowTableServiceFilterGroupSerializer(serializers.ModelSerializer): + id = serializers.CharField( + help_text="A unique identifier for the filter group. On read this is the " + "group's id; on write it may be a client-generated id used to link filters " + "and nested groups to this group within the request payload." + ) + parent_group = serializers.CharField( + source="parent_group_id", + required=False, + allow_null=True, + default=None, + help_text="The id of the parent filter group, or null for a group directly " + "under the service.", + ) + filter_type = serializers.ChoiceField( + choices=SERVICE_FILTER_TYPES, + required=False, + default=SERVICE_FILTER_TYPE_AND, + help_text="Indicates whether all the filters in the group should match (AND) " + "or any of them (OR). Defaults to AND when omitted from the payload.", + ) + + class Meta: + model = LocalBaserowTableServiceFilterGroup + fields = ("id", "filter_type", "parent_group") + + class LocalBaserowTableServiceFilterSerializer(serializers.ModelSerializer): value = FormulaSerializerField( help_text="A formula for the filter's value.", @@ -66,6 +98,14 @@ class LocalBaserowTableServiceFilterSerializer(serializers.ModelSerializer): help_text="A filter is considered trashed if " "the field it's associated with is trashed.", ) + group = serializers.CharField( + source="group_id", + required=False, + allow_null=True, + default=None, + help_text="The id of the filter group this filter belongs to, or null if it " + "applies directly to the service.", + ) order = serializers.IntegerField(read_only=True) class Meta: @@ -78,6 +118,7 @@ class Meta: "value", "trashed", "value_is_formula", + "group", ) @@ -98,10 +139,16 @@ def to_representation(self, instance): many=True, context=self.context, ).data + representation["filter_groups"] = LocalBaserowTableServiceFilterGroupSerializer( + instance.service_filter_groups.all(), + many=True, + context=self.context, + ).data return representation def to_internal_value(self, data): filters = data.pop("filters", None) + filter_groups = data.pop("filter_groups", None) data = super().to_internal_value(data) if filters is not None: data["service_filters"] = [ @@ -110,6 +157,13 @@ def to_internal_value(self, data): ).to_internal_value(sf) for sf in filters ] + if filter_groups is not None: + data["service_filter_groups"] = [ + LocalBaserowTableServiceFilterGroupSerializer( + context=self.context + ).to_internal_value(fg) + for fg in filter_groups + ] return data diff --git a/backend/src/baserow/contrib/integrations/local_baserow/mixins.py b/backend/src/baserow/contrib/integrations/local_baserow/mixins.py index c0b97d9cf6..339bb809a9 100644 --- a/backend/src/baserow/contrib/integrations/local_baserow/mixins.py +++ b/backend/src/baserow/contrib/integrations/local_baserow/mixins.py @@ -3,12 +3,11 @@ from django.db.models import OrderBy, Prefetch, QuerySet from baserow.contrib.database.api.utils import extract_field_ids_from_list -from baserow.contrib.database.fields.field_filters import FilterBuilder +from baserow.contrib.database.fields.field_filters import AdvancedFilterBuilder from baserow.contrib.database.fields.models import Field from baserow.contrib.database.search.handler import SearchHandler from baserow.contrib.database.views.filters import AdHocFilters from baserow.contrib.database.views.handler import ViewHandler -from baserow.contrib.database.views.registries import view_filter_type_registry from baserow.contrib.integrations.local_baserow.api.serializers import ( LocalBaserowTableServiceFilterSerializerMixin, LocalBaserowTableServiceSortSerializerMixin, @@ -17,9 +16,13 @@ LocalBaserowGetRow, LocalBaserowListRows, LocalBaserowTableServiceFilter, + LocalBaserowTableServiceFilterGroup, LocalBaserowTableServiceSort, LocalBaserowViewService, ) +from baserow.contrib.integrations.local_baserow.service_filter_groups import ( + LocalBaserowServiceGroupedFiltersAdapter, +) from baserow.core.formula import BaserowFormulaObject, resolve_formula from baserow.core.formula.registries import formula_runtime_function_registry from baserow.core.formula.serializers import FormulaSerializerField @@ -64,6 +67,7 @@ class LocalBaserowTableServiceFilterableMixin: class SerializedDict(ServiceDict): filter_type: str filters: List[Dict] + filter_groups: List[Dict] def enhance_queryset(self, queryset): return ( @@ -76,6 +80,10 @@ def enhance_queryset(self, queryset): "field" ).all(), ), + Prefetch( + "service_filter_groups", + queryset=LocalBaserowTableServiceFilterGroup.objects.all(), + ), ) ) @@ -93,10 +101,28 @@ def serialize_filters(self, service: ServiceSubClass): "type": f.type, "value": f.value, "value_is_formula": f.value_is_formula, + "group": f.group_id, } for f in service.service_filters_with_untrashed_fields ] + def serialize_filter_groups(self, service: ServiceSubClass): + """ + Responsible for serializing the service `filter_groups`. + + :param service: the service instance. + :return: A list of serialized filter group dictionaries. + """ + + return [ + { + "id": g.id, + "filter_type": g.filter_type, + "parent_group": g.parent_group_id, + } + for g in service.service_filter_groups.all() + ] + def serialize_property( self, service: ServiceSubClass, @@ -106,7 +132,7 @@ def serialize_property( cache=None, ): """ - Responsible for serializing the `filters` properties. + Responsible for serializing the `filters` and `filter_groups` properties. :param service: The LocalBaserowListRows service. :param prop_name: The property name we're serializing. @@ -116,6 +142,9 @@ def serialize_property( if prop_name == "filters": return self.serialize_filters(service) + if prop_name == "filter_groups": + return self.serialize_filter_groups(service) + return super().serialize_property( service, prop_name, files_zip=files_zip, storage=storage, cache=cache ) @@ -178,6 +207,7 @@ def create_instance_from_serialized( """ filters = serialized_values.pop("filters", []) + filter_groups = serialized_values.pop("filter_groups", []) service = super().create_instance_from_serialized( serialized_values, @@ -188,11 +218,32 @@ def create_instance_from_serialized( **kwargs, ) - # Create filters + # Create the filter groups first, so that filters can reference them. Groups + # are serialized with parents before children (see the model's `Meta.ordering`), + # so the parent group has always been created (and mapped) by the time a child + # references it. + group_id_mapping = id_mapping.setdefault( + "integration_service_filter_groups", {} + ) + for filter_group in filter_groups: + parent_group_id = filter_group["parent_group"] + new_group = LocalBaserowTableServiceFilterGroup.objects.create( + service=service, + filter_type=filter_group["filter_type"], + parent_group_id=group_id_mapping.get(parent_group_id) + if parent_group_id is not None + else None, + ) + group_id_mapping[filter_group["id"]] = new_group.id + + # Create filters, mapping each filter's group to the newly-created group. LocalBaserowTableServiceFilter.objects.bulk_create( [ LocalBaserowTableServiceFilter( - **service_filter, + **{k: v for k, v in service_filter.items() if k != "group"}, + group_id=group_id_mapping.get(service_filter.get("group")) + if service_filter.get("group") is not None + else None, order=index, service=service, ) @@ -261,40 +312,16 @@ def get_dispatch_filters( "One or more filtered properties no longer exist.", ) - service_filter_builder = FilterBuilder(filter_type=service.filter_type) - for service_filter in service.service_filters_with_untrashed_fields: - field_object = model._field_objects[service_filter.field_id] - field_name = field_object["name"] - model_field = model._meta.get_field(field_name) - view_filter_type = view_filter_type_registry.get(service_filter.type) - - # We need this test for compatibility purposes with old values - if ( - service_filter.value_is_formula - or service_filter.value["mode"] == BASEROW_FORMULA_MODE_RAW - ): - try: - resolved_value = ensure_string( - resolve_formula( - service_filter.value, - formula_runtime_function_registry, - dispatch_context, - ) - ) - except Exception as exc: - raise ServiceImproperlyConfiguredDispatchException( - f"The {field_name} service filter formula can't be " - "resolved: {exc}" - ) from exc - else: - resolved_value = service_filter.value["formula"] - - service_filter_builder.filter( - view_filter_type.get_filter( - field_name, resolved_value, model_field, field_object["field"] - ) - ) - + # Build the service filters, nesting them into their filter groups (if any) + # using the same machinery the database views use. Filters without a group + # are combined under the service's top-level `filter_type`, preserving the + # behaviour of services which don't use groups. + adapter = LocalBaserowServiceGroupedFiltersAdapter( + service, model, dispatch_context + ) + service_filter_builder = AdvancedFilterBuilder( + adapter + ).construct_filter_builder() return service_filter_builder.apply_to_queryset(queryset) def formula_generator( @@ -362,22 +389,157 @@ def get_table_queryset( queryset = adhoc_filters.apply_to_queryset(model, queryset) return queryset + def sync_service_filter_groups( + self, + service: Union[LocalBaserowGetRow, LocalBaserowListRows], + service_filter_groups: List[Dict], + ) -> Dict[str, LocalBaserowTableServiceFilterGroup]: + """ + Reconciles the service's filter groups against the given payload **in place**, + preserving the primary keys of groups that already exist. Groups present in the + payload are updated (or created when new), and groups no longer present are + deleted. + + Preserving group primary keys is essential: filters reference their group by id, + and because the data source is saved as a whole payload with only the changed + keys sent, a partial update (e.g. only the filters, or only the groups) must not + invalidate the links between the filters and groups that were not resent. + + Every incoming group id is treated as an opaque correlation key: it is either the + id of an existing group (returned on read) or a client-generated id for a new + group. Both are used only to link filters and nested groups together. + + :param service: The service the groups belong to. + :param service_filter_groups: The list of validated filter group dictionaries. + :return: A mapping of client group id to the persisted group instance. + """ + + existing_by_id = { + str(group.id): group for group in service.service_filter_groups.all() + } + incoming_ids = {str(group["id"]) for group in service_filter_groups} + + # Delete groups which are no longer present in the payload. This cascades to + # their filters, which is correct: removing a group removes its filters. + for group_id, group in list(existing_by_id.items()): + if group_id not in incoming_ids: + group.delete() + del existing_by_id[group_id] + + client_id_to_group: Dict[str, LocalBaserowTableServiceFilterGroup] = {} + pending = list(service_filter_groups) + + def upsert(group, parent): + group_id = str(group["id"]) + existing = existing_by_id.get(group_id) + if existing is not None: + existing.filter_type = group["filter_type"] + existing.parent_group = parent + existing.save() + return existing + return LocalBaserowTableServiceFilterGroup.objects.create( + service=service, + filter_type=group["filter_type"], + parent_group=parent, + ) + + # Resolve parent references iteratively so that a parent group is always + # persisted before its children, regardless of the order in the payload. + while pending: + still_pending = [] + for group in pending: + parent_client_id = group.get("parent_group_id") + parent_resolved = ( + parent_client_id is None + or str(parent_client_id) in client_id_to_group + ) + if not parent_resolved: + still_pending.append(group) + continue + parent = ( + client_id_to_group[str(parent_client_id)] + if parent_client_id is not None + else None + ) + client_id_to_group[str(group["id"])] = upsert(group, parent) + + if len(still_pending) == len(pending): + # No progress: the remaining groups reference missing parents. Persist + # them as top-level groups so we never loop forever or lose data. + for group in still_pending: + client_id_to_group[str(group["id"])] = upsert(group, None) + break + + pending = still_pending + + return client_id_to_group + def update_service_filters( self, service: Union[LocalBaserowGetRow, LocalBaserowListRows], service_filters: Optional[List[ServiceFilterDictSubClass]] = None, + service_filter_groups: Optional[List[Dict]] = None, ): + """ + Persists the given filters and/or filter groups for the service. + + Because the data source is saved as a whole payload but only the changed keys + are sent, `service_filters` and/or `service_filter_groups` may be `None`, + meaning "this part was not part of this update, leave it as-is". An empty list + means "clear this part". This decoupling is what keeps a filter-only edit from + wiping the groups (and vice versa). + """ + with atomic_if_not_already(): + # Reconcile groups first (preserving their ids) so that filters can be + # linked to them. When the groups are not part of this update, index the + # existing groups by id so filters can still reference them. + if service_filter_groups is not None: + client_id_to_group = self.sync_service_filter_groups( + service, service_filter_groups + ) + else: + client_id_to_group = { + str(group.id): group + for group in service.service_filter_groups.all() + } + + if service_filters is None: + return + service.service_filters.all().delete() + + def build_filter(index, service_filter): + service_filter = {**service_filter} + group_client_id = service_filter.pop("group_id", None) + group = ( + client_id_to_group.get(str(group_client_id)) + if group_client_id is not None + else None + ) + return LocalBaserowTableServiceFilter( + **service_filter, service=service, order=index, group=group + ) + LocalBaserowTableServiceFilter.objects.bulk_create( [ - LocalBaserowTableServiceFilter( - **service_filter, service=service, order=index - ) + build_filter(index, service_filter) for index, service_filter in enumerate(service_filters) ] ) + def _invalidate_refinement_prefetch_cache(self, service): + """ + The service may have been fetched with its filters/groups prefetched (see + `enhance_queryset`). After mutating them, drop the stale prefetch cache so that + a response serialized from the same instance reflects the changes. + """ + + prefetch_cache = getattr(service, "_prefetched_objects_cache", None) + if prefetch_cache is not None: + prefetch_cache.pop("service_filters", None) + prefetch_cache.pop("service_filter_groups", None) + def after_update( self, instance: ServiceSubClass, @@ -385,28 +547,40 @@ def after_update( changes: Dict[str, Tuple], ) -> None: """ - Responsible for updating service filters which have been - PATCHED to the data source / service endpoint. At the moment we - destroy all current filters, and create the ones present - in `service_filters`. + Responsible for updating the service filters and filter groups which have been + PATCHED to the data source / service endpoint. Because only the changed keys are + sent, filters and filter groups are updated independently: a part that is absent + from the payload is left untouched (see `update_service_filters`). :param instance: The service we want to manage filters for. - :param values: A dictionary which may contain filters. + :param values: A dictionary which may contain `service_filters` and/or + `service_filter_groups`. :param changes: A dictionary containing all changes which were made to the service prior to `after_update` being called. """ super().after_update(instance, values, changes) - # Following a Table change, from one Table to another, we drop all filters. - # This is due to the fact that they point at specific table fields. + # Following a Table change, from one Table to another, we drop all filters and + # filter groups. This is due to the fact that they point at specific table + # fields. from_table, to_table = changes.get("table", (None, None)) if from_table and to_table: instance.service_filters.all().delete() + instance.service_filter_groups.all().delete() + self._invalidate_refinement_prefetch_cache(instance) else: - if "service_filters" in values: - self.update_service_filters(instance, values["service_filters"]) + if "service_filters" in values or "service_filter_groups" in values: + # Pass `None` (not `[]`) for a part that wasn't sent, so it is left + # untouched rather than cleared. This is what keeps a filter-only edit + # from wiping the groups, and a group-only edit from wiping the filters. + self.update_service_filters( + instance, + values.get("service_filters"), + values.get("service_filter_groups"), + ) + self._invalidate_refinement_prefetch_cache(instance) class LocalBaserowTableServiceSortableMixin: diff --git a/backend/src/baserow/contrib/integrations/local_baserow/models.py b/backend/src/baserow/contrib/integrations/local_baserow/models.py index 09fbfefb69..e22e1ac454 100644 --- a/backend/src/baserow/contrib/integrations/local_baserow/models.py +++ b/backend/src/baserow/contrib/integrations/local_baserow/models.py @@ -17,6 +17,7 @@ SearchableServiceMixin, Service, ServiceFilter, + ServiceFilterGroup, ServiceSort, ) @@ -214,6 +215,23 @@ class LocalBaserowTableServiceRefinementManager(models.Manager): use_in_migrations = True +class LocalBaserowTableServiceFilterGroup(ServiceFilterGroup): + """ + A service filter group applicable to a `LocalBaserowTableService` integration + service. Mirrors `baserow.contrib.database.views.models.ViewFilterGroup`. + """ + + objects = LocalBaserowTableServiceRefinementManager() + + class Meta: + # Ordering by `id` guarantees parent groups are returned before their + # children, which the `AdvancedFilterBuilder` tree construction relies on. + ordering = ("id",) + + def __repr__(self): + return f"" + + class LocalBaserowTableServiceFilter(ServiceFilter): """ A service filter applicable to a `LocalBaserowTableService` integration service. @@ -221,6 +239,16 @@ class LocalBaserowTableServiceFilter(ServiceFilter): objects = LocalBaserowTableServiceRefinementManager() + group = models.ForeignKey( + LocalBaserowTableServiceFilterGroup, + related_name="filters", + help_text="The filter group to which the filter applies. If null, the filter " + "applies directly to the service.", + null=True, + default=None, + db_default=None, + on_delete=models.CASCADE, + ) field = models.ForeignKey( "database.Field", help_text="The database Field, in the LocalBaserowTableService, " diff --git a/backend/src/baserow/contrib/integrations/local_baserow/service_filter_groups.py b/backend/src/baserow/contrib/integrations/local_baserow/service_filter_groups.py new file mode 100644 index 0000000000..61828bf613 --- /dev/null +++ b/backend/src/baserow/contrib/integrations/local_baserow/service_filter_groups.py @@ -0,0 +1,80 @@ +from typing import TYPE_CHECKING, Type, Union + +from django.db.models import Q + +from baserow.contrib.database.fields.field_filters import ( + AnnotatedQ, + GroupedFiltersAdapter, +) +from baserow.contrib.database.views.registries import view_filter_type_registry +from baserow.core.formula import resolve_formula +from baserow.core.formula.registries import formula_runtime_function_registry +from baserow.core.formula.types import BASEROW_FORMULA_MODE_RAW +from baserow.core.formula.validator import ensure_string +from baserow.core.services.dispatch_context import DispatchContext +from baserow.core.services.exceptions import ( + ServiceImproperlyConfiguredDispatchException, +) + +if TYPE_CHECKING: + from baserow.contrib.database.table.models import GeneratedTableModel + + +class LocalBaserowServiceGroupedFiltersAdapter(GroupedFiltersAdapter): + """ + A `GroupedFiltersAdapter` implementation for `LocalBaserow` services. It exposes + the service's stored filters and filter groups to the `AdvancedFilterBuilder` so + they can be applied to a queryset with correct nesting and per-group AND/OR + operators. + + Unlike the views adapter, a service filter's `value` may be a runtime formula which + must be resolved against the `dispatch_context` before it can be turned into a `Q`. + """ + + def __init__( + self, + service, + model: Type["GeneratedTableModel"], + dispatch_context: DispatchContext, + **kwargs, + ): + super().__init__(service, model, **kwargs) + self.dispatch_context = dispatch_context + + @property + def filters(self): + return self.instance.service_filters_with_untrashed_fields + + @property + def groups(self): + return self.instance.service_filter_groups.all() + + def get_q_from_filter(self, service_filter) -> Union[Q, AnnotatedQ]: + field_object = self.model._field_objects[service_filter.field_id] + field_name = field_object["name"] + model_field = self.model._meta.get_field(field_name) + view_filter_type = view_filter_type_registry.get(service_filter.type) + + # We need this test for compatibility purposes with old values. + if ( + service_filter.value_is_formula + or service_filter.value["mode"] == BASEROW_FORMULA_MODE_RAW + ): + try: + resolved_value = ensure_string( + resolve_formula( + service_filter.value, + formula_runtime_function_registry, + self.dispatch_context, + ) + ) + except Exception as exc: + raise ServiceImproperlyConfiguredDispatchException( + f"The {field_name} service filter formula can't be resolved: {exc}" + ) from exc + else: + resolved_value = service_filter.value["formula"] + + return view_filter_type.get_filter( + field_name, resolved_value, model_field, field_object["field"] + ) diff --git a/backend/src/baserow/contrib/integrations/migrations/0032_localbaserowtableservicefiltergroup_and_more.py b/backend/src/baserow/contrib/integrations/migrations/0032_localbaserowtableservicefiltergroup_and_more.py new file mode 100644 index 0000000000..6b57ba6999 --- /dev/null +++ b/backend/src/baserow/contrib/integrations/migrations/0032_localbaserowtableservicefiltergroup_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 5.2.15 on 2026-07-10 10:45 + +import baserow.contrib.integrations.local_baserow.models +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0114_alter_workspaceinvitation_message'), + ('integrations', '0031_corestartworkflowservice'), + ] + + operations = [ + migrations.CreateModel( + name='LocalBaserowTableServiceFilterGroup', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('filter_type', models.CharField(choices=[('AND', 'And'), ('OR', 'Or')], default='AND', help_text='Indicates whether all the rows should apply to all filters (AND) or to any filter (OR) in the group to be shown.', max_length=3)), + ('parent_group', models.ForeignKey(default=None, help_text='The parent filter group, allowing groups to be nested. If null, the group is directly under the service.', null=True, on_delete=django.db.models.deletion.CASCADE, to='integrations.localbaserowtableservicefiltergroup')), + ('service', models.ForeignKey(help_text='The service which this filter group belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='service_filter_groups', to='core.service')), + ], + options={ + 'ordering': ('id',), + }, + managers=[ + ('objects', baserow.contrib.integrations.local_baserow.models.LocalBaserowTableServiceRefinementManager()), + ], + ), + migrations.AddField( + model_name='localbaserowtableservicefilter', + name='group', + field=models.ForeignKey(db_default=None, default=None, help_text='The filter group to which the filter applies. If null, the filter applies directly to the service.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='filters', to='integrations.localbaserowtableservicefiltergroup'), + ), + ] diff --git a/backend/src/baserow/contrib/integrations/migrations/0033_coregotonodeservice.py b/backend/src/baserow/contrib/integrations/migrations/0033_coregotonodeservice.py new file mode 100644 index 0000000000..7461025513 --- /dev/null +++ b/backend/src/baserow/contrib/integrations/migrations/0033_coregotonodeservice.py @@ -0,0 +1,29 @@ +# Generated by Django 5.2.14 on 2026-06-24 09:14 + +import baserow.core.formula.field +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('automation', '0035_coregotoactionnode'), + ('core', '0114_alter_workspaceinvitation_message'), + ('integrations', '0032_localbaserowtableservicefiltergroup_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='CoreGotoService', + fields=[ + ('service_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='core.service')), + ('condition', baserow.core.formula.field.FormulaField(blank=True, default='', help_text='The formula that must evaluate to true for the jump to the destination service to be followed.', null=True)), + ('destination_service', models.ForeignKey(help_text='The service to jump to when the condition evaluates to true.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to='core.service')), + ], + options={ + 'abstract': False, + }, + bases=('core.service',), + ), + ] diff --git a/backend/src/baserow/core/registry.py b/backend/src/baserow/core/registry.py index e1c22cbaac..a3ba9e7a86 100644 --- a/backend/src/baserow/core/registry.py +++ b/backend/src/baserow/core/registry.py @@ -730,6 +730,25 @@ def import_serialized( return created_instance + def after_import( + self, + instance: T, + id_mapping: Dict[str, Any], + **kwargs: Dict[str, Any], + ) -> Set[T]: + """ + Second-pass hook to remap references to other instances that may not + have existed yet during the first import pass (e.g. a forward + reference). It is meant to run once every instance of the import has + been created, so both backward and forward references can be resolved. + + This method does not save any updates made to the instance. Instead, + it returns a set of all updated model instances, and the caller should + call `.save()` on them to persist the changes. + """ + + return set() + class Registry(Generic[InstanceSubClass]): name: str diff --git a/backend/src/baserow/core/services/models.py b/backend/src/baserow/core/services/models.py index ad36cc3f50..363419602f 100644 --- a/backend/src/baserow/core/services/models.py +++ b/backend/src/baserow/core/services/models.py @@ -10,6 +10,17 @@ WithRegistry, ) +# Mirrors `baserow.contrib.database.fields.field_filters.FILTER_TYPE_{AND,OR}` and +# `views.models.FILTER_TYPES`. Redefined here as plain literals to avoid importing the +# `contrib.database` filter modules into `core`, which would cause a circular import +# (`core.models` is loaded before `contrib.database`). +SERVICE_FILTER_TYPE_AND = "AND" +SERVICE_FILTER_TYPE_OR = "OR" +SERVICE_FILTER_TYPES = ( + (SERVICE_FILTER_TYPE_AND, "And"), + (SERVICE_FILTER_TYPE_OR, "Or"), +) + def get_default_service_service(): return ContentType.objects.get_for_model(Integration) @@ -95,6 +106,45 @@ def get_parent(self): return self.service +class ServiceFilterGroup(HierarchicalModelMixin): + """ + An abstract Model which service subclass's filter group model can inherit from. + + A filter group allows service filters to be nested and combined with their own + `AND`/`OR` operator, independently from the service's top-level operator. Groups + can themselves be nested via `parent_group` (a `parent_group` of `None` denotes a + top-level group directly under the service). + """ + + filter_type = models.CharField( + max_length=3, + choices=SERVICE_FILTER_TYPES, + default=SERVICE_FILTER_TYPE_AND, + help_text="Indicates whether all the rows should apply to all filters (AND) " + "or to any filter (OR) in the group to be shown.", + ) + parent_group = models.ForeignKey( + "self", + on_delete=models.CASCADE, + null=True, + default=None, + help_text="The parent filter group, allowing groups to be nested. If null, " + "the group is directly under the service.", + ) + service = models.ForeignKey( + Service, + related_name="service_filter_groups", + help_text="The service which this filter group belongs to.", + on_delete=models.CASCADE, + ) + + class Meta: + abstract = True + + def get_parent(self): + return self.service + + class ServiceSort(HierarchicalModelMixin): """ An abstract Model which service subclass's sort model can inherit from. diff --git a/backend/src/baserow/core/services/types.py b/backend/src/baserow/core/services/types.py index 942343996c..e7f8e81763 100644 --- a/backend/src/baserow/core/services/types.py +++ b/backend/src/baserow/core/services/types.py @@ -32,6 +32,9 @@ class DispatchResult: data: dict = field(default_factory=dict) status: int = 200 output_uid: str = "" + # When set, the runner redirects execution to this service instead of + # resolving the natural next step. Used to implement conditional jumps. + destination_service_id: Optional[int] = None @dataclass diff --git a/backend/src/baserow/test_utils/fixtures/automation_node.py b/backend/src/baserow/test_utils/fixtures/automation_node.py index 68d2e302ae..68d0370aca 100644 --- a/backend/src/baserow/test_utils/fixtures/automation_node.py +++ b/backend/src/baserow/test_utils/fixtures/automation_node.py @@ -11,6 +11,7 @@ LocalBaserowUpdateRowsActionNode, ) from baserow.contrib.automation.nodes.node_types import ( + CoreGotoActionNodeType, CoreHTTPTriggerNodeType, CoreIteratorNodeType, CoreManualTriggerNodeType, @@ -200,6 +201,13 @@ def create_core_router_action_node_with_edges(self, user=None, **kwargs): fallback_output_node=fallback_output_node, ) + def create_core_goto_node(self, user=None, **kwargs): + return self.create_automation_node( + user=user, + type=CoreGotoActionNodeType.type, + **kwargs, + ) + def create_periodic_trigger_node(self, user=None, **kwargs): return self.create_automation_node( user=user, diff --git a/backend/src/baserow/test_utils/fixtures/service.py b/backend/src/baserow/test_utils/fixtures/service.py index f7176c2b88..2dbf610bee 100644 --- a/backend/src/baserow/test_utils/fixtures/service.py +++ b/backend/src/baserow/test_utils/fixtures/service.py @@ -3,6 +3,7 @@ from baserow.contrib.integrations.ai.models import AIAgentService from baserow.contrib.integrations.core.models import ( CoreCSVFileReaderService, + CoreGotoService, CoreHTTPRequestService, CoreHTTPTriggerService, CoreIteratorService, @@ -23,6 +24,7 @@ LocalBaserowRowsDeleted, LocalBaserowRowsUpdated, LocalBaserowTableServiceFilter, + LocalBaserowTableServiceFilterGroup, LocalBaserowTableServiceSort, LocalBaserowUpdateRows, LocalBaserowUpsertRow, @@ -105,6 +107,11 @@ def create_local_baserow_table_service_filter( kwargs["order"] = 0 return LocalBaserowTableServiceFilter.objects.create(**kwargs) + def create_local_baserow_table_service_filter_group( + self, **kwargs + ) -> LocalBaserowTableServiceFilterGroup: + return LocalBaserowTableServiceFilterGroup.objects.create(**kwargs) + def create_local_baserow_table_service_sort( self, **kwargs ) -> LocalBaserowTableServiceSort: @@ -167,6 +174,9 @@ def create_core_router_service_edge(self, service: CoreRouterService, **kwargs): return edge + def create_core_goto_service(self, **kwargs) -> CoreGotoService: + return self.create_service(CoreGotoService, **kwargs) + def create_core_http_trigger_service(self, **kwargs) -> CoreSMTPEmailService: if "uid" not in kwargs: kwargs["uid"] = uuid4() diff --git a/backend/tests/baserow/contrib/automation/api/history/test_history_views.py b/backend/tests/baserow/contrib/automation/api/history/test_history_views.py index cac6ab3e52..2a44e30a52 100644 --- a/backend/tests/baserow/contrib/automation/api/history/test_history_views.py +++ b/backend/tests/baserow/contrib/automation/api/history/test_history_views.py @@ -71,6 +71,40 @@ def test_get_node_histories_surfaces_router_edge_label(api_client, data_fixture) assert rows[router_history.id]["edge_label"] == "Foo label" +@pytest.mark.django_db +def test_get_node_histories_surfaces_goto_destination(api_client, data_fixture): + user, token = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination node" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + goto_node.service.specific.destination_service = destination.service + goto_node.service.specific.save() + + workflow_history = data_fixture.create_automation_workflow_history( + workflow=workflow, + ) + goto_history = data_fixture.create_automation_node_history( + workflow_history=workflow_history, node=goto_node + ) + + url = reverse( + API_URL_NODE_HISTORIES, kwargs={"workflow_history_id": workflow_history.id} + ) + response = api_client.get(url, **get_api_kwargs(token)) + + assert response.status_code == HTTP_200_OK + rows = {row["id"]: row for row in response.json()} + row = rows[goto_history.id] + assert row["destination_node_id"] == destination.id + assert row["destination_node_type"] == destination.get_type().type + assert row["destination_label"] == "destination node" + + @pytest.mark.django_db def test_get_node_histories_permission_error(api_client, data_fixture): user = data_fixture.create_user() diff --git a/backend/tests/baserow/contrib/automation/history/test_history_handler.py b/backend/tests/baserow/contrib/automation/history/test_history_handler.py index 56d37f2a7e..f366dd5af7 100644 --- a/backend/tests/baserow/contrib/automation/history/test_history_handler.py +++ b/backend/tests/baserow/contrib/automation/history/test_history_handler.py @@ -1,3 +1,5 @@ +from django.db import connection +from django.test.utils import CaptureQueriesContext from django.utils import timezone import pytest @@ -12,6 +14,7 @@ AutomationNodeHistory, AutomationWorkflowHistory, ) +from baserow.contrib.automation.history.service import AutomationHistoryService from baserow.contrib.automation.workflows.constants import WorkflowState @@ -322,6 +325,178 @@ def test_get_node_history_result(data_fixture): assert result.result == {"foo": "bar"} +@pytest.mark.django_db +def test_get_node_result_returns_latest_pass_when_node_loops(data_fixture): + """ + When a node is dispatched multiple times in a single run (e.g. a "Go to node" + jump loops back to it), several results share the same iteration_path. Reading + the previous result should return the most recent pass rather than raising + MultipleObjectsReturned. + """ + + user, _ = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + workflow_history = data_fixture.create_automation_workflow_history( + user=user, + workflow=workflow, + ) + + earlier = timezone.now() + later = earlier + timezone.timedelta(seconds=10) + + first_history = data_fixture.create_automation_node_history( + user=user, + workflow_history=workflow_history, + node=trigger, + started_on=earlier, + ) + data_fixture.create_automation_node_result( + node_history=first_history, + iteration_path="", + result={"pass": 1}, + ) + + latest_history = data_fixture.create_automation_node_history( + user=user, + workflow_history=workflow_history, + node=trigger, + started_on=later, + ) + data_fixture.create_automation_node_result( + node_history=latest_history, + iteration_path="", + result={"pass": 2}, + ) + + result = AutomationHistoryHandler().get_node_result(workflow_history, trigger, "") + + assert result == {"pass": 2} + + +@pytest.mark.django_db +def test_get_destination_labels(data_fixture): + user, _ = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination node" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + goto_node.service.specific.destination_service = destination.service + goto_node.service.specific.save() + + workflow_history = data_fixture.create_automation_workflow_history( + user=user, workflow=workflow + ) + goto_history = data_fixture.create_automation_node_history( + user=user, workflow_history=workflow_history, node=goto_node + ) + # A non-goto entry should be ignored. + trigger_history = data_fixture.create_automation_node_history( + user=user, workflow_history=workflow_history, node=trigger + ) + + labels = AutomationHistoryHandler().get_destination_labels( + [goto_history, trigger_history] + ) + + assert labels == { + goto_history.id: { + "id": destination.id, + "type": destination.get_type().type, + "label": "destination node", + } + } + + +@pytest.mark.django_db +def test_get_destination_labels_without_custom_label(data_fixture): + """ + When the destination node has no custom label, the returned label is empty but + the node type is still provided so the frontend can show a generic name. + """ + + user, _ = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + goto_node.service.specific.destination_service = destination.service + goto_node.service.specific.save() + + workflow_history = data_fixture.create_automation_workflow_history( + user=user, workflow=workflow + ) + goto_history = data_fixture.create_automation_node_history( + user=user, workflow_history=workflow_history, node=goto_node + ) + + labels = AutomationHistoryHandler().get_destination_labels([goto_history]) + + assert destination.label == "" + assert labels == { + goto_history.id: { + "id": destination.id, + "type": destination.get_type().type, + "label": "", + } + } + + +@pytest.mark.django_db +def test_get_destination_labels_query_count_does_not_grow_with_loops(data_fixture): + """ + A "Go to node" that loops writes one history entry per pass. Resolving a + destination costs several queries, so they must be resolved once per node + rather than once per pass, otherwise a long loop causes an N+1. + """ + + user, _ = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination node" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + goto_node.service.specific.destination_service = destination.service + goto_node.service.specific.save() + + service = AutomationHistoryService() + + def count_queries_for(passes): + workflow_history = data_fixture.create_automation_workflow_history( + user=user, workflow=workflow + ) + for _ in range(passes): + data_fixture.create_automation_node_history( + user=user, workflow_history=workflow_history, node=goto_node + ) + data_fixture.create_automation_node_history( + user=user, workflow_history=workflow_history, node=destination + ) + # Fetched the same way the API view does, so the node histories carry + # non-specific nodes. + node_histories = service.get_node_histories(user, workflow_history.id) + with CaptureQueriesContext(connection) as captured: + labels = AutomationHistoryHandler().get_destination_labels(node_histories) + assert len(labels) == passes + return len(captured.captured_queries) + + # Warm the content type cache so it doesn't skew the first measurement. + count_queries_for(1) + + assert count_queries_for(1) == count_queries_for(10) + + @pytest.mark.django_db def test_get_node_history_result_does_not_exist(data_fixture): user, _ = data_fixture.create_user_and_token() diff --git a/backend/tests/baserow/contrib/automation/nodes/test_goto_node.py b/backend/tests/baserow/contrib/automation/nodes/test_goto_node.py new file mode 100644 index 0000000000..0d040c9254 --- /dev/null +++ b/backend/tests/baserow/contrib/automation/nodes/test_goto_node.py @@ -0,0 +1,443 @@ +import uuid + +import pytest + +from baserow.contrib.automation.action_scopes import WorkflowActionScopeType +from baserow.contrib.automation.history.constants import HistoryStatusChoices +from baserow.contrib.automation.history.models import AutomationNodeHistory +from baserow.contrib.automation.nodes.actions import ( + DeleteAutomationNodeActionType, + UpdateAutomationNodeActionType, +) +from baserow.contrib.automation.nodes.exceptions import ( + AutomationNodeMisconfiguredService, +) +from baserow.contrib.automation.nodes.handler import AutomationNodeHandler +from baserow.contrib.automation.nodes.service import AutomationNodeService +from baserow.core.action.handler import ActionHandler +from baserow.core.services.exceptions import ( + ServiceImproperlyConfiguredDispatchException, +) + + +def _build_goto_workflow(data_fixture, condition="'false'", with_destination=True): + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + + service = goto_node.service.specific + service.condition = condition + if with_destination: + service.destination_service = destination.service + service.save() + + return { + "user": user, + "workflow": workflow, + "trigger": trigger, + "destination": destination, + "goto_node": goto_node, + } + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_trigger_destination(data_fixture): + data = _build_goto_workflow(data_fixture) + goto_node = data["goto_node"] + trigger = data["trigger"] + + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": trigger.service_id}}, + data["user"], + instance=goto_node, + ) + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_cross_level_destination(data_fixture): + iterator_data = data_fixture.iterator_graph_fixture() + workflow = iterator_data["workflow"] + trigger = iterator_data["trigger_node"] + iterator_child = iterator_data["iterator_child_1_node"] + + user = data_fixture.create_user() + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=trigger, position="south", output="" + ) + + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": iterator_child.service_id}}, + user, + instance=goto_node, + ) + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_cross_workflow_destination(data_fixture): + data = _build_goto_workflow(data_fixture) + goto_node = data["goto_node"] + + # A destination node belonging to a different workflow is not eligible. + other_workflow = data_fixture.create_automation_workflow(user=data["user"]) + other_destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=other_workflow, + reference_node=other_workflow.get_trigger(), + label="other workflow destination", + ) + + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": other_destination.service_id}}, + data["user"], + instance=goto_node, + ) + + +@pytest.mark.django_db +def test_goto_node_prepare_values_drops_deleted_destination(data_fixture): + # A destination is only nulled when it is permanently deleted, so an update + # can still carry the service of a trashed node (e.g. when undoing an update + # made before the deletion). The unresolvable link is dropped instead of + # failing the update. + data = _build_goto_workflow(data_fixture) + goto_node = data["goto_node"] + destination = data["destination"] + + AutomationNodeService().delete_node(data["user"], destination.id) + + values = goto_node.get_type().prepare_values( + {"service": {"destination_service_id": destination.service_id}}, + data["user"], + instance=goto_node, + ) + assert values["service"].destination_service_id is None + + +@pytest.mark.django_db +@pytest.mark.undo_redo +def test_undo_goto_node_update_after_destination_deleted(data_fixture): + # Deleting the destination, pointing the Go to node at another node, and then + # undoing that update must not fail: the undo restores a destination that no + # longer exists, so the link is cleared instead. + session_id = str(uuid.uuid4()) + user = data_fixture.create_user(session_id=session_id) + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination" + ) + other_destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=destination, label="other destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=other_destination + ) + service = goto_node.service.specific + service.destination_service = destination.service + service.save() + + DeleteAutomationNodeActionType.do(user, destination.id) + UpdateAutomationNodeActionType.do( + user, + goto_node.id, + {"service": {"destination_service_id": other_destination.service_id}}, + ) + + [undone_action] = ActionHandler.undo( + user, [WorkflowActionScopeType.value(workflow.id)], session_id + ) + + assert undone_action.error is None + service.refresh_from_db() + assert service.destination_service_id is None + + +@pytest.mark.django_db +def test_dispatch_node_reports_deleted_destination_as_misconfigured(data_fixture): + # A jump to a node that was deleted after it was configured is reported as a + # workflow error rather than crashing the run with a lookup error. + data = _build_goto_workflow(data_fixture, condition="'true'") + workflow = data["workflow"] + goto_node = data["goto_node"] + + AutomationNodeService().delete_node(data["user"], data["destination"].id) + + history = data_fixture.create_automation_workflow_history( + original_workflow=workflow, + workflow=workflow, + event_payload={"results": [], "has_next_page": False}, + ) + + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + + assert result is None + history.refresh_from_db() + assert history.status == HistoryStatusChoices.ERROR + assert "misconfigured" in history.message + + +@pytest.mark.django_db +def test_goto_node_prepare_values_allows_same_level_destination(data_fixture): + data = _build_goto_workflow(data_fixture) + goto_node = data["goto_node"] + destination = data["destination"] + + values = goto_node.get_type().prepare_values( + {"service": {"destination_service_id": destination.service_id}}, + data["user"], + instance=goto_node, + ) + assert values["service"].destination_service_id == destination.service_id + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_forward_jump(data_fixture): + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=trigger, position="south", output="" + ) + # A node placed after the Go to node is a forward jump: reaching it would + # skip the nodes in between, leaving their outputs unset for later nodes. + # Only backward jumps are allowed for now. + forward_node = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=goto_node, label="forward", position="south" + ) + + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": forward_node.service_id}}, + user, + instance=goto_node, + ) + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_cross_branch_destination(data_fixture): + # A node in one router branch cannot jump to a node in a sibling branch: + # that node does not run before the Go to node. The two branches share the + # same level, so the same-level rule alone wouldn't catch this - the + # backward-path rule does. + user = data_fixture.create_user() + router_data = data_fixture.create_core_router_action_node_with_edges(user=user) + workflow = router_data.router.workflow + branch_a_node = router_data.edge1_output + branch_b_node = router_data.edge2_output + + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=branch_a_node, position="south", output="" + ) + + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": branch_b_node.service_id}}, + user, + instance=goto_node, + ) + + # ...but jumping back to an earlier node in its own branch is allowed. + values = goto_node.get_type().prepare_values( + {"service": {"destination_service_id": branch_a_node.service_id}}, + user, + instance=goto_node, + ) + assert values["service"].destination_service_id == branch_a_node.service_id + + +@pytest.mark.django_db +def test_goto_node_prepare_values_rejects_self_target(data_fixture): + data = _build_goto_workflow(data_fixture) + goto_node = data["goto_node"] + + # Self-targeting is rejected: an empty loop body re-evaluates the same condition + # against unchanged state, so it can only be a no-op or a guaranteed infinite loop. + with pytest.raises(AutomationNodeMisconfiguredService): + goto_node.get_type().prepare_values( + {"service": {"destination_service_id": goto_node.service_id}}, + data["user"], + instance=goto_node, + ) + + +@pytest.mark.django_db +def test_dispatch_node_jumps_to_destination_when_condition_true(data_fixture): + data = _build_goto_workflow(data_fixture, condition="'true'") + workflow = data["workflow"] + goto_node = data["goto_node"] + destination = data["destination"] + + history = data_fixture.create_automation_workflow_history( + workflow=workflow.get_original(), + event_payload={"results": [], "has_next_page": False}, + ) + + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + + # The runner should redirect to the destination node rather than the natural + # next node (there is none after the goto node). + assert result is not None + leaf = result.tasks[0] + if hasattr(leaf, "tasks"): + leaf = leaf.tasks[0] + assert leaf.args[0] == destination.id + + # The jump was followed, so the run is a regular success. + node_history = AutomationNodeHistory.objects.get(node=goto_node) + assert node_history.status == HistoryStatusChoices.SUCCESS + + +@pytest.mark.django_db +def test_dispatch_node_falls_through_when_condition_false(data_fixture): + data = _build_goto_workflow(data_fixture, condition="'false'") + workflow = data["workflow"] + goto_node = data["goto_node"] + + history = data_fixture.create_automation_workflow_history( + workflow=workflow.get_original(), + event_payload={"results": [], "has_next_page": False}, + ) + + # The goto node is the last node in the graph, so falling through ends the branch. + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + assert result is None + + # No jump was followed, so the history must not suggest one happened. + node_history = AutomationNodeHistory.objects.get(node=goto_node) + assert node_history.status == HistoryStatusChoices.SKIPPED + + +def _cross_level_goto_workflow(data_fixture): + """ + Builds a workflow whose goto (at the root level) targets a node living + inside an iterator (a different level), i.e. a jump that is no longer valid. + """ + + data = data_fixture.iterator_graph_fixture() + workflow = data["workflow"] + iterator_child = data["iterator_child_1_node"] + trigger = data["trigger_node"] + + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=trigger, position="south", output="" + ) + service = goto_node.service.specific + service.condition = "'true'" + service.destination_service = iterator_child.service + service.save() + + return {"workflow": workflow, "goto_node": goto_node, "destination": iterator_child} + + +@pytest.mark.django_db +def test_validate_jump_destination_allows_valid_destination(data_fixture): + # The hook is a no-op for a link that is still a valid jump. + data = _build_goto_workflow(data_fixture, condition="'true'") + goto_node = data["goto_node"] + destination = data["destination"] + + # Does not raise. + goto_node.get_type().validate_jump_destination(goto_node, destination.service_id) + + +@pytest.mark.django_db +def test_validate_jump_destination_rejects_cross_level_destination(data_fixture): + # When the destination has been moved into a container (becomes cross-level) + # after being configured, the hook raises a clean misconfigured error rather + # than letting the runner follow the jump into a 500/KeyError. + data = _cross_level_goto_workflow(data_fixture) + goto_node = data["goto_node"] + destination = data["destination"] + + with pytest.raises(ServiceImproperlyConfiguredDispatchException): + goto_node.get_type().validate_jump_destination( + goto_node, destination.service_id + ) + + +@pytest.mark.django_db +def test_dispatch_node_does_not_follow_jump_when_simulating(data_fixture): + # The runner owns the simulation gate: while simulating, a requested jump is + # never followed. The goto lies on the path to the simulated node, so it is + # dispatched, but execution must continue to its natural next node rather than + # looping back to the jump destination. + data = _build_goto_workflow(data_fixture, condition="'true'") + workflow = data["workflow"] + goto_node = data["goto_node"] + + final_node = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=goto_node, label="final" + ) + + history = data_fixture.create_automation_workflow_history( + original_workflow=workflow, + workflow=workflow, + event_payload={"results": [], "has_next_page": False}, + simulate_until_node=final_node, + ) + + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + + # The natural next node runs, not the (backward) jump destination. + assert result is not None + leaf = result.tasks[0] + if hasattr(leaf, "tasks"): + leaf = leaf.tasks[0] + assert leaf.args[0] == final_node.id + + +@pytest.mark.django_db +def test_dispatch_node_validates_jump_on_real_run(data_fixture): + # On a real run the jump is followed, so the runner validates it against the + # live graph: an invalid (cross-level) destination is reported as a workflow + # error instead of crashing. + data = _cross_level_goto_workflow(data_fixture) + workflow = data["workflow"] + goto_node = data["goto_node"] + + history = data_fixture.create_automation_workflow_history( + original_workflow=workflow, + workflow=workflow, + event_payload={"results": [], "has_next_page": False}, + ) + + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + + assert result is None + history.refresh_from_db() + assert history.status == HistoryStatusChoices.ERROR + assert "misconfigured" in history.message + + +@pytest.mark.django_db +def test_dispatch_node_skips_jump_validation_when_simulating(data_fixture): + # The jump is never followed while simulating, so the runner must not validate + # it: an invalid (cross-level) destination that errors on a real run is left + # untouched, and the simulation completes without a misconfigured error. + data = _cross_level_goto_workflow(data_fixture) + workflow = data["workflow"] + goto_node = data["goto_node"] + + history = data_fixture.create_automation_workflow_history( + original_workflow=workflow, + workflow=workflow, + event_payload={"results": [], "has_next_page": False}, + simulate_until_node=goto_node, + ) + + result = AutomationNodeHandler().dispatch_node(goto_node.id, history.id) + + assert result is None + history.refresh_from_db() + assert history.status != HistoryStatusChoices.ERROR diff --git a/backend/tests/baserow/contrib/automation/nodes/test_goto_node_move.py b/backend/tests/baserow/contrib/automation/nodes/test_goto_node_move.py new file mode 100644 index 0000000000..c64b79da1d --- /dev/null +++ b/backend/tests/baserow/contrib/automation/nodes/test_goto_node_move.py @@ -0,0 +1,206 @@ +import uuid + +import pytest + +from baserow.contrib.automation.action_scopes import WorkflowActionScopeType +from baserow.contrib.automation.nodes.actions import MoveAutomationNodeActionType +from baserow.contrib.automation.nodes.node_types import CoreGotoActionNodeType +from baserow.contrib.automation.nodes.service import AutomationNodeService +from baserow.contrib.integrations.core.models import CoreGotoService +from baserow.core.action.handler import ActionHandler + + +def _root_goto_workflow(data_fixture, user=None): + """ + Builds a workflow whose root level is: + trigger -> destination -> iterator -> goto (destination_service=destination.service) + """ + + user = user or data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user) + trigger = workflow.get_trigger() + destination = data_fixture.create_automation_node( + workflow=workflow, label="destination" + ) + iterator = data_fixture.create_core_iterator_action_node( + workflow=workflow, label="iterator" + ) + goto = data_fixture.create_core_goto_node(workflow=workflow, label="goto") + + service = goto.service.specific + service.condition = "'true'" + service.destination_service = destination.service + service.save() + + # Sanity check: same root level, so the link is valid to begin with. + assert CoreGotoActionNodeType.validate_goto_destination(goto, destination) is None + + return { + "user": user, + "workflow": workflow, + "trigger": trigger, + "destination": destination, + "iterator": iterator, + "goto": goto, + "service": service, + } + + +@pytest.mark.django_db +def test_move_destination_into_container_clears_goto_link(data_fixture): + data = _root_goto_workflow(data_fixture) + + # Moving the destination inside the iterator breaks the same-level rule. + AutomationNodeService().move_node( + data["user"], data["destination"].id, data["iterator"].id, "child", "" + ) + + data["service"].refresh_from_db() + assert data["service"].destination_service_id is None + + +@pytest.mark.django_db +def test_move_goto_into_container_clears_goto_link(data_fixture): + data = _root_goto_workflow(data_fixture) + + # Moving the goto node itself inside the iterator breaks the rule too. + AutomationNodeService().move_node( + data["user"], data["goto"].id, data["iterator"].id, "child", "" + ) + + data["service"].refresh_from_db() + assert data["service"].destination_service_id is None + + +@pytest.mark.django_db +def test_move_goto_before_destination_clears_goto_link(data_fixture): + data = _root_goto_workflow(data_fixture) + + # Moving the goto node before its destination turns the backward jump into a + # forward jump (trigger -> goto -> destination -> iterator). Forward jumps + # are not allowed, so the link is cleared. + AutomationNodeService().move_node( + data["user"], data["goto"].id, data["trigger"].id, "south", "" + ) + + data["service"].refresh_from_db() + assert data["service"].destination_service_id is None + + +@pytest.mark.django_db +def test_move_unrelated_node_keeps_goto_link(data_fixture): + data = _root_goto_workflow(data_fixture) + other = data_fixture.create_automation_node( + workflow=data["workflow"], label="other" + ) + + # A reorder that doesn't change either endpoint's level leaves the link be. + AutomationNodeService().move_node( + data["user"], other.id, data["trigger"].id, "south", "" + ) + + data["service"].refresh_from_db() + assert data["service"].destination_service_id == data["destination"].service_id + + +@pytest.mark.django_db +def test_move_container_holding_both_endpoints_keeps_goto_link(data_fixture): + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user) + workflow.get_trigger() + iterator = data_fixture.create_core_iterator_action_node( + workflow=workflow, label="iterator" + ) + after = data_fixture.create_automation_node(workflow=workflow, label="after") + + # Both endpoints live inside the iterator, at the same level as each other. + destination = data_fixture.create_automation_node( + workflow=workflow, + label="destination", + reference_node=iterator, + position="child", + ) + goto = data_fixture.create_core_goto_node( + workflow=workflow, label="goto", reference_node=destination, position="south" + ) + service = goto.service.specific + service.destination_service = destination.service + service.save() + assert CoreGotoActionNodeType.validate_goto_destination(goto, destination) is None + + # Moving the whole container carries both endpoints together, so their + # relative level is unchanged and the link must survive. + AutomationNodeService().move_node(user, iterator.id, after.id, "south", "") + + service.refresh_from_db() + assert service.destination_service_id == destination.service_id + + +@pytest.mark.django_db +@pytest.mark.undo_redo +def test_undo_redo_move_restores_and_reclears_goto_link(data_fixture): + session_id = str(uuid.uuid4()) + user = data_fixture.create_user(session_id=session_id) + data = _root_goto_workflow(data_fixture, user=user) + workflow = data["workflow"] + scope = WorkflowActionScopeType.value(workflow.id) + + MoveAutomationNodeActionType.do( + user, data["destination"].id, data["iterator"].id, "child", "" + ) + data["service"].refresh_from_db() + assert data["service"].destination_service_id is None + + # Undo returns the destination to the root, so the link is restored. + ActionHandler.undo(user, [scope], session_id) + data["service"].refresh_from_db() + assert data["service"].destination_service_id == data["destination"].service_id + + # Redo breaks it again. + ActionHandler.redo(user, [scope], session_id) + data["service"].refresh_from_db() + assert data["service"].destination_service_id is None + + +@pytest.mark.django_db +def test_duplicate_goto_node_copies_its_destination(data_fixture): + data = _root_goto_workflow(data_fixture) + + duplicated = AutomationNodeService().duplicate_node(data["user"], data["goto"].id) + + # A single-node duplicate leaves the destination node in place, so the copy + # keeps jumping to the same destination as the original. + duplicated_service = duplicated.service.specific + assert duplicated_service.destination_service_id == data["destination"].service_id + # The original link is untouched. + data["service"].refresh_from_db() + assert data["service"].destination_service_id == data["destination"].service_id + + +@pytest.mark.django_db +def test_duplicate_destination_node_is_not_referenced_by_any_goto(data_fixture): + data = _root_goto_workflow(data_fixture) + + duplicated_destination = AutomationNodeService().duplicate_node( + data["user"], data["destination"].id + ) + + # The copy is a standalone node: no goto points at it, and the original + # link is untouched. + assert not CoreGotoService.objects.filter( + destination_service=duplicated_destination.service + ).exists() + data["service"].refresh_from_db() + assert data["service"].destination_service_id == data["destination"].service_id + + +@pytest.mark.django_db +def test_deleting_destination_preserves_link_for_restore(data_fixture): + data = _root_goto_workflow(data_fixture) + + # Deleting trashes the node (soft delete), which must NOT fire SET_NULL, so + # the link survives and an undo/restore brings the jump back. + AutomationNodeService().delete_node(data["user"], data["destination"].id) + + data["service"].refresh_from_db() + assert data["service"].destination_service_id == data["destination"].service_id diff --git a/backend/tests/baserow/contrib/automation/nodes/test_node_dispatch_async.py b/backend/tests/baserow/contrib/automation/nodes/test_node_dispatch_async.py index eacc855d4a..678aa10664 100644 --- a/backend/tests/baserow/contrib/automation/nodes/test_node_dispatch_async.py +++ b/backend/tests/baserow/contrib/automation/nodes/test_node_dispatch_async.py @@ -1,5 +1,7 @@ from unittest.mock import ANY, patch +from django.test.utils import override_settings + import pytest from celery.canvas import Signature @@ -175,6 +177,22 @@ def create_workflow_history(data_fixture, workflow, trigger_table_fields): ) +def create_dispatch_limit_workflow(data_fixture, simulate=False): + """ + Build a workflow + history for exercising the per-run node dispatch cap. + """ + + data = create_workflow(data_fixture) + node = data["trigger_node"] + history = data["workflow_history"] + + if simulate: + history.simulate_until_node = node + history.save() + + return node, history + + @pytest.mark.django_db def test_dispatch_node_service_error(data_fixture): user = data_fixture.create_user() @@ -521,6 +539,7 @@ def test_dispatch_node_dispatches_iterator_children(data_fixture): @pytest.mark.django_db +@override_settings(AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN=100) def test_dispatch_node_fully_dispatches_nested_iterator_workflow(data_fixture): data = data_fixture.nested_iterator_graph_fixture() trigger_node = data["trigger_node"] @@ -646,6 +665,7 @@ def test_dispatch_node_dispatches_trigger_simulation( }, "status": 200, "output_uid": "", + "destination_service_id": None, } mock_automation_node_updated.send.assert_called_once_with( @@ -722,6 +742,7 @@ def test_dispatch_node_dispatches_action_simulation( "order": AnyStr(), }, "output_uid": "", + "destination_service_id": None, "status": 200, } @@ -943,6 +964,7 @@ def test_dispatch_node_dispatches_iterator_simulation( "order": AnyStr(), }, "output_uid": "", + "destination_service_id": None, "status": 200, } @@ -1444,6 +1466,7 @@ def test_dispatch_node_dispatches_router_edge_simulation( }, "output_uid": AnyStr(), "status": 200, + "destination_service_id": None, } mock_automation_node_updated.send.assert_called_once_with( @@ -1740,32 +1763,60 @@ def test_dispatch_node_does_not_send_completed_signal_on_error( @pytest.mark.django_db -@patch(f"{NODE_HANDLER_PATH}.automation_node_dispatch_started") -@patch(f"{NODE_HANDLER_PATH}.automation_node_dispatch_completed") -@patch(f"{TRIGGER_NODE_TYPE_PATH}.dispatch") -def test_dispatch_node_returns_early_if_started_signal_has_error( - mock_dispatch, - mock_dispatch_completed, - mock_dispatch_started, - data_fixture, -): - # Simulate an exception raised when the started signal is sent. - mock_dispatch_started.send.side_effect = Exception("Foo error") +@override_settings(AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN=1) +def test_dispatch_node_enforces_per_run_dispatch_limit(data_fixture): + node, history = create_dispatch_limit_workflow(data_fixture) - data = create_workflow(data_fixture) - trigger_node = data["trigger_node"] - workflow_history = data["workflow_history"] + # First dispatch is within the limit. + AutomationNodeHandler().dispatch_node(node.id, history.id) + clear_local() + history.refresh_from_db() + assert history.status != HistoryStatusChoices.ERROR - result = AutomationNodeHandler().dispatch_node( - trigger_node.id, - history_id=workflow_history.id, - ) + # Second dispatch exceeds the limit and errors the run at the workflow level. + result = AutomationNodeHandler().dispatch_node(node.id, history.id) + assert result is None + history.refresh_from_db() + assert history.status == HistoryStatusChoices.ERROR + assert "exceeded the maximum of 1 node dispatches" in history.message + + +@pytest.mark.django_db +@override_settings(AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN=1) +@patch(f"{NODE_HANDLER_PATH}.automation_node_updated") +def test_dispatch_node_enforces_dispatch_limit_when_simulating( + mock_automation_node_updated, data_fixture +): + # The cap is a backstop against infinite loops and applies to simulations + # too. A legitimate simulation dispatches each node on the path to the + # simulated node once, so it never gets near the cap. + node, history = create_dispatch_limit_workflow(data_fixture, simulate=True) + + # First dispatch is within the limit. + AutomationNodeHandler().dispatch_node(node.id, history.id) + clear_local() + history.refresh_from_db() + assert history.status != HistoryStatusChoices.ERROR + # Second dispatch exceeds the limit and errors the run at the workflow level. + result = AutomationNodeHandler().dispatch_node(node.id, history.id) assert result is None - node_history = AutomationNodeHistory.objects.get(node=trigger_node) - assert node_history.status == HistoryStatusChoices.ERROR + history.refresh_from_db() + assert history.status == HistoryStatusChoices.ERROR + assert "exceeded the maximum of 1 node dispatches" in history.message - # Both node_dispatch() and the completed signal shouldn't be called - # because the started signal raised an error. - mock_dispatch.assert_not_called() - mock_dispatch_completed.send.assert_not_called() + # The frontend is notified about the simulated node on the error path too, + # in addition to the notification sent by the first, successful dispatch. + assert mock_automation_node_updated.send.call_count == 2 + mock_automation_node_updated.send.assert_called_with(ANY, user=None, node=node) + + +@pytest.mark.django_db +def test_check_node_dispatch_limit_counts_per_run(data_fixture): + _, history = create_dispatch_limit_workflow(data_fixture) + handler = AutomationNodeHandler() + + with override_settings(AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN=2): + assert handler._check_node_dispatch_limit(history.id) is False + assert handler._check_node_dispatch_limit(history.id) is False + assert handler._check_node_dispatch_limit(history.id) is True diff --git a/backend/tests/baserow/contrib/automation/nodes/test_node_service.py b/backend/tests/baserow/contrib/automation/nodes/test_node_service.py index e84601ee66..8917e44967 100644 --- a/backend/tests/baserow/contrib/automation/nodes/test_node_service.py +++ b/backend/tests/baserow/contrib/automation/nodes/test_node_service.py @@ -834,6 +834,39 @@ def test_update_node_updates_workflow_dirty_cache(data_fixture): assert global_cache.get(cache_key, default=False) is True +@pytest.mark.django_db +def test_move_node_updates_workflow_dirty_cache(data_fixture): + """ + When a node is moved, the workflow's dirty cache flag should be set so that the + next test run creates a fresh clone instead of reusing a stale one. Otherwise a + test run would dispatch the pre-move graph (e.g. a "Go to node" whose destination + has since changed), surfacing errors that no longer reflect the workflow. + """ + + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user) + trigger = workflow.get_trigger() + node_a = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, position="south" + ) + node_b = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=node_a, position="south" + ) + + cache_key = WORKFLOW_DIRTY_CACHE_KEY.format(workflow.id) + assert global_cache.get(cache_key, default=False) is False + + AutomationNodeService().move_node( + user, + node_b.id, + reference_node_id=trigger.id, + position="south", + output="", + ) + + assert global_cache.get(cache_key, default=False) is True + + @pytest.mark.django_db def test_update_node_rejects_integration_from_another_application(data_fixture): user = data_fixture.create_user() diff --git a/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py b/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py index ff8b441d70..797f7c7d1a 100644 --- a/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py +++ b/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py @@ -436,6 +436,7 @@ def test_update_data_source_with_filters(api_client, data_fixture): ), "trashed": False, "value_is_formula": False, + "group": None, }, { "id": service_filters[1].id, @@ -449,6 +450,7 @@ def test_update_data_source_with_filters(api_client, data_fixture): mode=BASEROW_FORMULA_MODE_SIMPLE, ), "value_is_formula": True, + "group": None, }, ] @@ -502,10 +504,233 @@ def test_update_data_source_with_filters(api_client, data_fixture): ), "trashed": False, "value_is_formula": False, + "group": None, } ] +@pytest.mark.django_db +def test_update_data_source_with_filter_groups(api_client, data_fixture): + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + table = data_fixture.create_database_table(user=user) + text_field = data_fixture.create_text_field(table=table) + data_source1 = data_fixture.create_builder_local_baserow_list_rows_data_source( + page=page, table=table + ) + + url = reverse( + "api:builder:data_source:item", kwargs={"data_source_id": data_source1.id} + ) + + # The frontend sends the whole filters + filter_groups payload, linking filters to + # groups (and nested groups to their parents) via client-generated correlation ids. + response = api_client.patch( + url, + { + "filter_groups": [ + {"id": "group-a", "filter_type": "OR", "parent_group": None}, + {"id": "group-b", "filter_type": "AND", "parent_group": "group-a"}, + ], + "filters": [ + { + "field": text_field.id, + "type": "equal", + "value": BaserowFormulaObject( + formula="foo", + version=BASEROW_FORMULA_VERSION_INITIAL, + mode=BASEROW_FORMULA_MODE_RAW, + ), + "value_is_formula": False, + "group": "group-a", + }, + { + "field": text_field.id, + "type": "equal", + "value": BaserowFormulaObject( + formula="bar", + version=BASEROW_FORMULA_VERSION_INITIAL, + mode=BASEROW_FORMULA_MODE_RAW, + ), + "value_is_formula": False, + "group": "group-b", + }, + ], + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_200_OK + + groups = list(data_source1.service.service_filter_groups.order_by("id")) + assert len(groups) == 2 + group_a, group_b = groups + assert group_a.filter_type == "OR" + assert group_a.parent_group_id is None + assert group_b.filter_type == "AND" + assert group_b.parent_group_id == group_a.id + + filters = list(data_source1.service.service_filters.order_by("order")) + assert [f.group_id for f in filters] == [group_a.id, group_b.id] + + # The response exposes the persisted groups, and each filter references its group. + response_json = response.json() + assert {g["id"] for g in response_json["filter_groups"]} == { + str(group_a.id), + str(group_b.id), + } + assert [f["group"] for f in response_json["filters"]] == [ + str(group_a.id), + str(group_b.id), + ] + + +@pytest.mark.django_db +def test_update_data_source_filters_only_preserves_groups(api_client, data_fixture): + """ + Regression: editing a filter sends only `filters` in the PATCH (not + `filter_groups`). The existing groups must be preserved and the filters re-linked + to them, rather than the groups being wiped and the filters flattened. + """ + + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + table = data_fixture.create_database_table(user=user) + text_field = data_fixture.create_text_field(table=table) + data_source = data_fixture.create_builder_local_baserow_list_rows_data_source( + page=page, table=table + ) + url = reverse( + "api:builder:data_source:item", kwargs={"data_source_id": data_source.id} + ) + + # Create a group with a filter inside it. + api_client.patch( + url, + { + "filter_groups": [{"id": "g1", "filter_type": "AND", "parent_group": None}], + "filters": [ + { + "field": text_field.id, + "type": "equal", + "value": BaserowFormulaObject( + formula="foo", + version=BASEROW_FORMULA_VERSION_INITIAL, + mode=BASEROW_FORMULA_MODE_RAW, + ), + "value_is_formula": False, + "group": "g1", + } + ], + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + group = data_source.service.service_filter_groups.get() + + # Simulate editing the filter value: only `filters` is sent, referencing the + # existing group by its persisted id. `filter_groups` is intentionally absent. + response = api_client.patch( + url, + { + "filters": [ + { + "field": text_field.id, + "type": "equal", + "value": BaserowFormulaObject( + formula="bar", + version=BASEROW_FORMULA_VERSION_INITIAL, + mode=BASEROW_FORMULA_MODE_RAW, + ), + "value_is_formula": False, + "group": str(group.id), + } + ], + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_200_OK + + # The group survives with the same id and the filter is still linked to it. + groups = list(data_source.service.service_filter_groups.all()) + assert [g.id for g in groups] == [group.id] + filters = list(data_source.service.service_filters.all()) + assert len(filters) == 1 + assert filters[0].group_id == group.id + # The response reflects the preserved group (not a stale/empty list). + assert [g["id"] for g in response.json()["filter_groups"]] == [str(group.id)] + assert [f["group"] for f in response.json()["filters"]] == [str(group.id)] + + +@pytest.mark.django_db +def test_update_data_source_filter_groups_only_preserves_filters( + api_client, data_fixture +): + """ + Regression: toggling a group's AND/OR sends only `filter_groups` in the PATCH (not + `filters`). The group must be updated in place (keeping its id) and the filters must + be preserved, rather than the group being recreated (new id) which would cascade + delete the filters. + """ + + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + table = data_fixture.create_database_table(user=user) + text_field = data_fixture.create_text_field(table=table) + data_source = data_fixture.create_builder_local_baserow_list_rows_data_source( + page=page, table=table + ) + url = reverse( + "api:builder:data_source:item", kwargs={"data_source_id": data_source.id} + ) + + api_client.patch( + url, + { + "filter_groups": [{"id": "g1", "filter_type": "AND", "parent_group": None}], + "filters": [ + { + "field": text_field.id, + "type": "equal", + "value": BaserowFormulaObject( + formula="foo", + version=BASEROW_FORMULA_VERSION_INITIAL, + mode=BASEROW_FORMULA_MODE_RAW, + ), + "value_is_formula": False, + "group": "g1", + } + ], + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + group = data_source.service.service_filter_groups.get() + service_filter = data_source.service.service_filters.get() + + # Simulate toggling the group's operator: only `filter_groups` is sent. + response = api_client.patch( + url, + { + "filter_groups": [ + {"id": str(group.id), "filter_type": "OR", "parent_group": None} + ], + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_200_OK + + # The group is updated in place (same id, new operator). + groups = list(data_source.service.service_filter_groups.all()) + assert [(g.id, g.filter_type) for g in groups] == [(group.id, "OR")] + # The filter is untouched (same id) and still linked to the group. + filters = list(data_source.service.service_filters.all()) + assert [f.id for f in filters] == [service_filter.id] + assert filters[0].group_id == group.id + + @pytest.mark.django_db def test_update_data_source_change_type(api_client, data_fixture): user, token = data_fixture.create_user_and_token() diff --git a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py index cf73445dcb..35080ea588 100644 --- a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py +++ b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py @@ -461,7 +461,7 @@ def test_builder_dispatch_context_public_allowed_properties_is_cached( } # Initially calling the property should cause a bunch of DB queries. - with django_assert_num_queries(14): + with django_assert_num_queries(15): result = dispatch_context.public_allowed_properties assert result == expected_results diff --git a/backend/tests/baserow/contrib/builder/test_builder_application_type.py b/backend/tests/baserow/contrib/builder/test_builder_application_type.py index 3bcc0034ec..b03da785fb 100644 --- a/backend/tests/baserow/contrib/builder/test_builder_application_type.py +++ b/backend/tests/baserow/contrib/builder/test_builder_application_type.py @@ -222,6 +222,7 @@ def test_builder_application_export(data_fixture): "integration_id": integration.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "row_id": datasource2.service.row_id, "view_id": None, "table_id": None, @@ -239,6 +240,7 @@ def test_builder_application_export(data_fixture): "integration_id": integration.id, "default_result_count": 20, "filters": [], + "filter_groups": [], "sortings": [], "view_id": None, "table_id": None, @@ -384,6 +386,7 @@ def test_builder_application_export(data_fixture): "integration_id": integration.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "row_id": shared_datasource.service.row_id, "view_id": None, "table_id": None, @@ -437,6 +440,7 @@ def test_builder_application_export(data_fixture): "integration_id": integration.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "row_id": datasource1.service.row_id, "view_id": None, "table_id": None, diff --git a/backend/tests/baserow/contrib/builder/test_builder_handler.py b/backend/tests/baserow/contrib/builder/test_builder_handler.py index 864a01d37e..beb19151f4 100644 --- a/backend/tests/baserow/contrib/builder/test_builder_handler.py +++ b/backend/tests/baserow/contrib/builder/test_builder_handler.py @@ -145,7 +145,7 @@ def test_public_allowed_properties_is_cached(data_fixture, django_assert_num_que } # Initially calling the property should cause a bunch of DB queries. - with django_assert_num_queries(14): + with django_assert_num_queries(15): result = handler.get_builder_public_properties(user_source_user, builder) assert result == expected_results diff --git a/backend/tests/baserow/contrib/dashboard/api/data_sources/test_dashboard_data_source_views.py b/backend/tests/baserow/contrib/dashboard/api/data_sources/test_dashboard_data_source_views.py index 0d83370477..a6166a02b9 100644 --- a/backend/tests/baserow/contrib/dashboard/api/data_sources/test_dashboard_data_source_views.py +++ b/backend/tests/baserow/contrib/dashboard/api/data_sources/test_dashboard_data_source_views.py @@ -45,6 +45,7 @@ def test_get_dashboard_data_sources(api_client, data_fixture): "dashboard_id": dashboard.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "id": data_source1.id, "integration_id": AnyInt(), "name": "Name 1", @@ -63,6 +64,7 @@ def test_get_dashboard_data_sources(api_client, data_fixture): "default_result_count": 20, "filter_type": "AND", "filters": [], + "filter_groups": [], "sortings": [], "id": data_source2.id, "integration_id": AnyInt(), diff --git a/backend/tests/baserow/contrib/dashboard/test_dashboard_application_types.py b/backend/tests/baserow/contrib/dashboard/test_dashboard_application_types.py index 197be08a48..f1817eb71e 100644 --- a/backend/tests/baserow/contrib/dashboard/test_dashboard_application_types.py +++ b/backend/tests/baserow/contrib/dashboard/test_dashboard_application_types.py @@ -115,6 +115,7 @@ def test_dashboard_export_serialized_with_widgets(data_fixture): "field_id": None, "filter_type": "AND", "filters": [], + "filter_groups": [], "id": dashboard_widget.data_source.service.id, "integration_id": integration.id, "search_query": {"formula": "", "mode": "simple", "version": "0.1"}, @@ -133,6 +134,7 @@ def test_dashboard_export_serialized_with_widgets(data_fixture): "field_id": number_field.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "id": dashboard_widget_2.data_source.service.id, "integration_id": integration.id, "search_query": {"formula": "", "mode": "simple", "version": "0.1"}, @@ -232,6 +234,7 @@ def test_dashboard_import_serialized_with_widgets(data_fixture): "field_id": None, "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 1, "integration_id": 1, "search_query": "", @@ -249,6 +252,7 @@ def test_dashboard_import_serialized_with_widgets(data_fixture): "field_id": 1, "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 2, "integration_id": 1, "search_query": "", diff --git a/backend/tests/baserow/contrib/integrations/core/test_core_goto_node_service_type.py b/backend/tests/baserow/contrib/integrations/core/test_core_goto_node_service_type.py new file mode 100644 index 0000000000..cda63fbdda --- /dev/null +++ b/backend/tests/baserow/contrib/integrations/core/test_core_goto_node_service_type.py @@ -0,0 +1,252 @@ +from collections import defaultdict + +import pytest + +from baserow.contrib.automation.nodes.handler import AutomationNodeHandler +from baserow.core.services.exceptions import ( + ServiceImproperlyConfiguredDispatchException, +) +from baserow.core.services.handler import ServiceHandler +from baserow.core.services.registries import service_type_registry +from baserow.core.utils import MirrorDict +from baserow.test_utils.pytest_conftest import FakeDispatchContext + + +@pytest.mark.django_db +def test_create_core_goto_node_service(data_fixture): + user = data_fixture.create_user() + service_type = service_type_registry.get("goto") + values = service_type.prepare_values({"condition": "'true'"}, user) + service = ServiceHandler().create_service(service_type, **values) + assert service.condition["formula"] == "'true'" + assert service.destination_service_id is None + + +@pytest.mark.django_db +def test_update_core_goto_node_service_sets_destination(data_fixture): + user = data_fixture.create_user() + goto_node = data_fixture.create_core_goto_node(user=user) + workflow = goto_node.workflow + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=goto_node, label="destination" + ) + + service = goto_node.service.specific + service_type = service.get_type() + values = service_type.prepare_values( + {"condition": "'true'", "destination_service_id": destination.service_id}, user + ) + result = ServiceHandler().update_service(service_type, service, **values) + + assert result.service.destination_service_id == destination.service_id + + +@pytest.mark.django_db +@pytest.mark.parametrize("truthful_condition", ["'true'", "1", "'yes'"]) +def test_core_goto_node_dispatch_condition_true(data_fixture, truthful_condition): + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + # The destination must run before the Go to node (a backward jump), so it is + # placed between the trigger and the Go to node. + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, position="south", label="destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination, position="south" + ) + + service = goto_node.service.specific + service.condition = truthful_condition + service.destination_service = destination.service + service.save() + + dispatch_result = service.get_type().dispatch(service, FakeDispatchContext()) + + assert dispatch_result.destination_service_id == destination.service_id + assert dispatch_result.data == {"condition": True} + + +@pytest.mark.django_db +def test_core_goto_node_dispatch_condition_false(data_fixture): + user = data_fixture.create_user() + goto_node = data_fixture.create_core_goto_node(user=user) + workflow = goto_node.workflow + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=goto_node, label="destination" + ) + + service = goto_node.service.specific + service.condition = "'false'" + service.destination_service = destination.service + service.save() + + dispatch_result = service.get_type().dispatch(service, FakeDispatchContext()) + + assert dispatch_result.destination_service_id is None + assert dispatch_result.data == {"condition": False} + + +@pytest.mark.django_db +@pytest.mark.parametrize("empty_condition", ["", None]) +def test_core_goto_node_dispatch_empty_condition_jumps(data_fixture, empty_condition): + """ + An unset condition means the jump is unconditional, so it is followed even + though the condition resolves to a falsy value. This is what distinguishes + it from an explicitly configured condition that resolves to false. + """ + + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + # The destination must run before the Go to node (a backward jump), so it is + # placed between the trigger and the Go to node. + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, position="south", label="destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination, position="south" + ) + + service = goto_node.service.specific + service.condition = empty_condition + service.destination_service = destination.service + service.save() + + dispatch_result = service.get_type().dispatch(service, FakeDispatchContext()) + + assert dispatch_result.destination_service_id == destination.service_id + assert dispatch_result.data == {"condition": True} + + +@pytest.mark.django_db +def test_core_goto_node_dispatch_missing_destination_raises(data_fixture): + user = data_fixture.create_user() + goto_node = data_fixture.create_core_goto_node(user=user) + service = goto_node.service.specific + service.condition = "'true'" + service.save() + + with pytest.raises(ServiceImproperlyConfiguredDispatchException): + service.get_type().dispatch(service, FakeDispatchContext()) + + +@pytest.mark.django_db +def test_core_goto_node_generate_schema(data_fixture): + service = data_fixture.create_core_goto_service() + assert service.get_type().generate_schema(service) == { + "title": f"CoreGoto{service.id}Schema", + "type": "object", + "properties": { + "condition": { + "type": "boolean", + "title": "Condition", + "description": "Whether the condition evaluated to true and the " + "jump was followed.", + } + }, + } + + +@pytest.mark.django_db +def test_core_goto_node_destination_set_null_on_delete(data_fixture): + user = data_fixture.create_user() + goto_node = data_fixture.create_core_goto_node(user=user) + workflow = goto_node.workflow + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=goto_node, label="destination" + ) + + service = goto_node.service.specific + service.destination_service = destination.service + service.save() + + # A hard delete of the destination node cascades to its service, which nulls + # the goto's destination_service FK (on_delete=SET_NULL). + destination.delete() + + service.refresh_from_db() + assert service.destination_service_id is None + + +@pytest.mark.django_db +def test_core_goto_node_import_export_remaps_destination(data_fixture): + """ + Exporting then importing the workflow's nodes should remap the goto node's + `destination_service` reference to the newly-imported destination service id. + """ + + user = data_fixture.create_user() + source_workflow = data_fixture.create_automation_workflow(user=user) + trigger = source_workflow.get_trigger() + + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=source_workflow, reference_node=trigger, label="destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=source_workflow, reference_node=destination + ) + service = goto_node.service.specific + service.condition = "'true'" + service.destination_service = destination.service + service.save() + + handler = AutomationNodeHandler() + serialized_destination = handler.export_node(destination) + serialized_goto = handler.export_node(goto_node) + + target_workflow = data_fixture.create_automation_workflow(user=user) + id_mapping = defaultdict(MirrorDict) + id_mapping["services"] = {} + + imported_destination, imported_goto = handler.import_nodes( + target_workflow, + [serialized_destination, serialized_goto], + id_mapping, + ) + + assert ( + imported_goto.service.specific.destination_service_id + == imported_destination.service_id + ) + assert ( + imported_goto.service.specific.destination_service_id != destination.service_id + ) + + +@pytest.mark.django_db +def test_core_goto_node_single_node_duplicate_keeps_destination(data_fixture): + """ + A single-node duplicate copies only the Go to node and leaves the + destination node in place, so the import (seeded with a MirrorDict service + mapping) should carry the original destination over unchanged. + """ + + user = data_fixture.create_user() + workflow = data_fixture.create_automation_workflow(user=user) + trigger = workflow.get_trigger() + + destination = data_fixture.create_local_baserow_create_row_action_node( + workflow=workflow, reference_node=trigger, label="destination" + ) + goto_node = data_fixture.create_core_goto_node( + workflow=workflow, reference_node=destination + ) + service = goto_node.service.specific + service.condition = "'true'" + service.destination_service = destination.service + service.save() + + handler = AutomationNodeHandler() + serialized_goto = handler.export_node(goto_node) + + # A single-node duplicate seeds the service mapping with a MirrorDict. + id_mapping = defaultdict(MirrorDict) + id_mapping["automation_workflow_nodes"] = MirrorDict() + id_mapping["services"] = MirrorDict() + + imported_goto = handler.import_node(workflow, serialized_goto, id_mapping) + + assert ( + imported_goto.service.specific.destination_service_id == destination.service_id + ) diff --git a/backend/tests/baserow/contrib/integrations/core/test_core_http_trigger_service_type.py b/backend/tests/baserow/contrib/integrations/core/test_core_http_trigger_service_type.py index f77c02f57b..430633bab4 100644 --- a/backend/tests/baserow/contrib/integrations/core/test_core_http_trigger_service_type.py +++ b/backend/tests/baserow/contrib/integrations/core/test_core_http_trigger_service_type.py @@ -34,6 +34,7 @@ def test_generate_schema(data_fixture): }, "status": 200, "output_uid": "", + "destination_service_id": None, } service.save() diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py index bdd985bcab..f61a96125d 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py @@ -107,8 +107,10 @@ def test_export_import_local_baserow_get_row_service(data_fixture): "type": service_filter.type, "value": service_filter.value, "value_is_formula": service_filter.value_is_formula, + "group": service_filter.group_id, } ], + "filter_groups": [], } id_mapping = {} diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_filter_groups.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_filter_groups.py new file mode 100644 index 0000000000..63eddc8d45 --- /dev/null +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_filter_groups.py @@ -0,0 +1,270 @@ +""" +Tests for grouped service filters on Local Baserow services. Filter groups let +service filters be nested and combined with their own AND/OR operator, mirroring the +database grid view. Ungrouped filters must keep behaving exactly as before. +""" + +import pytest + +from baserow.contrib.database.rows.handler import RowHandler +from baserow.contrib.database.table.handler import TableHandler +from baserow.contrib.integrations.local_baserow.service_types import ( + LocalBaserowListRowsUserServiceType, +) +from baserow.core.services.registries import service_type_registry +from baserow.test_utils.pytest_conftest import FakeDispatchContext, fake_import_formula + + +def _build_ingredient_cost_table(data_fixture, user): + builder = data_fixture.create_builder_application(user=user) + integration = data_fixture.create_local_baserow_integration( + application=builder, user=user + ) + database = data_fixture.create_database_application(workspace=builder.workspace) + table = TableHandler().create_table_and_fields( + user=user, + database=database, + name=data_fixture.fake.name(), + fields=[ + ("Ingredient", "text", {}), + ("Cost", "number", {}), + ], + ) + ingredient = table.field_set.get(name="Ingredient") + cost = table.field_set.get(name="Cost") + rows = ( + RowHandler() + .create_rows( + user, + table, + rows_values=[ + {f"field_{ingredient.id}": "Duck", f"field_{cost.id}": 50}, + {f"field_{ingredient.id}": "Goose", f"field_{cost.id}": 150}, + {f"field_{ingredient.id}": "Beef", f"field_{cost.id}": 250}, + ], + ) + .created_rows + ) + return integration, table, ingredient, cost, rows + + +@pytest.mark.django_db +def test_dispatch_with_filter_groups(data_fixture): + """ + (ingredient=Duck AND cost=50) OR (ingredient=Goose AND cost=150) should match + the first two rows, but not Beef. + """ + + user = data_fixture.create_user() + integration, table, ingredient, cost, rows = _build_ingredient_cost_table( + data_fixture, user + ) + [duck, goose, _beef] = rows + + service = data_fixture.create_local_baserow_list_rows_service( + table=table, integration=integration, filter_type="OR" + ) + + group_duck = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="AND" + ) + group_goose = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="AND" + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="Duck", order=0, group=group_duck + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=cost, value="50", order=1, group=group_duck + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="Goose", order=2, group=group_goose + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=cost, value="150", order=3, group=group_goose + ) + + service_type = LocalBaserowListRowsUserServiceType() + dispatch_context = FakeDispatchContext() + dispatch_values = service_type.resolve_service_formulas(service, dispatch_context) + dispatch_data = service_type.dispatch_data( + service, dispatch_values, dispatch_context + ) + assert [r.id for r in dispatch_data["results"]] == [duck.id, goose.id] + + +@pytest.mark.django_db +def test_dispatch_with_nested_filter_groups(data_fixture): + """ + ingredient=Beef OR (cost=50 OR cost=150). A subgroup nested under a group, both OR, + with the root AND-ing a single (always-true here) condition. Matches all 3 rows. + """ + + user = data_fixture.create_user() + integration, table, ingredient, cost, rows = _build_ingredient_cost_table( + data_fixture, user + ) + [duck, goose, beef] = rows + + service = data_fixture.create_local_baserow_list_rows_service( + table=table, integration=integration, filter_type="OR" + ) + + outer = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="OR" + ) + inner = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="OR", parent_group=outer + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="Beef", order=0, group=outer + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=cost, value="50", order=1, group=inner + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=cost, value="150", order=2, group=inner + ) + + service_type = LocalBaserowListRowsUserServiceType() + dispatch_context = FakeDispatchContext() + dispatch_values = service_type.resolve_service_formulas(service, dispatch_context) + dispatch_data = service_type.dispatch_data( + service, dispatch_values, dispatch_context + ) + assert sorted(r.id for r in dispatch_data["results"]) == sorted( + [duck.id, goose.id, beef.id] + ) + + +@pytest.mark.django_db +def test_dispatch_ungrouped_filters_unchanged(data_fixture): + """ + Backward-compat: filters without a group are combined under the service's + top-level filter_type, exactly as before groups existed. + """ + + user = data_fixture.create_user() + integration, table, ingredient, cost, rows = _build_ingredient_cost_table( + data_fixture, user + ) + [duck, goose, _beef] = rows + + service = data_fixture.create_local_baserow_list_rows_service( + table=table, integration=integration, filter_type="OR" + ) + # ingredient=Duck OR ingredient=Goose, no groups. + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="Duck", order=0 + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="Goose", order=1 + ) + + service_type = LocalBaserowListRowsUserServiceType() + dispatch_context = FakeDispatchContext() + dispatch_values = service_type.resolve_service_formulas(service, dispatch_context) + dispatch_data = service_type.dispatch_data( + service, dispatch_values, dispatch_context + ) + assert sorted(r.id for r in dispatch_data["results"]) == sorted([duck.id, goose.id]) + + +@pytest.mark.django_db +def test_export_import_preserves_filter_groups(data_fixture): + user = data_fixture.create_user() + integration, table, ingredient, cost, _rows = _build_ingredient_cost_table( + data_fixture, user + ) + service = data_fixture.create_local_baserow_list_rows_service( + table=table, integration=integration, filter_type="OR" + ) + outer = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="AND" + ) + inner = data_fixture.create_local_baserow_table_service_filter_group( + service=service, filter_type="OR", parent_group=outer + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=ingredient, value="'Duck'", order=0, group=outer + ) + data_fixture.create_local_baserow_table_service_filter( + service=service, field=cost, value="'50'", order=1, group=inner + ) + + service_type = service_type_registry.get("local_baserow_list_rows") + exported = service_type.export_serialized(service) + + assert len(exported["filter_groups"]) == 2 + assert len(exported["filters"]) == 2 + + imported_service = service_type.import_serialized( + integration, exported, {}, import_formula=fake_import_formula + ) + + groups = list(imported_service.service_filter_groups.order_by("id")) + assert len(groups) == 2 + imported_outer, imported_inner = groups + assert imported_outer.filter_type == "AND" + assert imported_outer.parent_group_id is None + assert imported_inner.filter_type == "OR" + # The nested group's parent was remapped to the newly-created outer group. + assert imported_inner.parent_group_id == imported_outer.id + + filters = list(imported_service.service_filters.order_by("order")) + assert filters[0].group_id == imported_outer.id + assert filters[1].group_id == imported_inner.id + + +@pytest.mark.django_db +def test_update_service_filters_with_groups_via_correlation_ids(data_fixture): + """ + The PATCH path deletes and recreates all filters and groups, linking them via + opaque correlation ids (client-generated strings for new items). + """ + + user = data_fixture.create_user() + integration, table, ingredient, cost, _rows = _build_ingredient_cost_table( + data_fixture, user + ) + service = data_fixture.create_local_baserow_list_rows_service( + table=table, integration=integration, filter_type="AND" + ) + service_type = LocalBaserowListRowsUserServiceType() + + service_filter_groups = [ + {"id": "group-a", "filter_type": "OR", "parent_group_id": None}, + {"id": "group-b", "filter_type": "AND", "parent_group_id": "group-a"}, + ] + service_filters = [ + { + "field": ingredient, + "type": "equal", + "value": {"formula": "Duck", "mode": "raw", "version": "0.1"}, + "value_is_formula": False, + "group_id": "group-a", + }, + { + "field": cost, + "type": "equal", + "value": {"formula": "50", "mode": "raw", "version": "0.1"}, + "value_is_formula": False, + "group_id": "group-b", + }, + ] + + service_type.update_service_filters( + service, + service_filters=service_filters, + service_filter_groups=service_filter_groups, + ) + + groups = list(service.service_filter_groups.order_by("id")) + assert len(groups) == 2 + group_a, group_b = groups + assert group_a.parent_group_id is None + assert group_b.parent_group_id == group_a.id + + filters = list(service.service_filters.order_by("order")) + assert filters[0].group_id == group_a.id + assert filters[1].group_id == group_b.id diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py index 6dd8c14be3..3960db6aed 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py @@ -117,8 +117,10 @@ def test_export_import_local_baserow_list_rows_service(data_fixture): "type": service_filter.type, "value": service_filter.value, "value_is_formula": service_filter.value_is_formula, + "group": service_filter.group_id, } ], + "filter_groups": [], "sortings": [ { "field_id": service_sort.field_id, diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/test_service_types.py b/backend/tests/baserow/contrib/integrations/local_baserow/test_service_types.py index 1d74bda3f4..258c12e257 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/test_service_types.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/test_service_types.py @@ -1316,10 +1316,19 @@ def test_local_baserow_view_service_type_prepare_values(data_fixture): table=table_a, view=view_a ) - # Providing a `view_id` that does not exist. + # Providing a `view_id` that does not exist. Compute an ID guaranteed to be + # free (higher than any view created here) instead of hardcoding one, which + # can collide with a real view ID when DB sequences are high (e.g. parallel + # CI runs), sending us down a different validation branch. + nonexistent_view_id = view_b.id + 1 with pytest.raises(DRFValidationError) as exc: - service_type().prepare_values({"view_id": "123"}, user, instance) - assert str(exc.value.detail["detail"]) == "The view with ID 123 does not exist." + service_type().prepare_values( + {"view_id": str(nonexistent_view_id)}, user, instance + ) + assert ( + str(exc.value.detail["detail"]) + == f"The view with ID {nonexistent_view_id} does not exist." + ) # Providing a `table`, no `view_id`, when the instance already # points to a view, will cause the `view` values to be `None`. diff --git a/changelog/entries/unreleased/bug/5559_fixed_a_bug_where_moving_a_node_didnt_update_the_graph.json b/changelog/entries/unreleased/bug/5559_fixed_a_bug_where_moving_a_node_didnt_update_the_graph.json new file mode 100644 index 0000000000..b52d4dd130 --- /dev/null +++ b/changelog/entries/unreleased/bug/5559_fixed_a_bug_where_moving_a_node_didnt_update_the_graph.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Fixed a bug where moving a node didn't update the graph.", + "issue_origin": "github", + "issue_number": 5559, + "domain": "automation", + "bullet_points": [], + "created_at": "2026-06-26" +} diff --git a/changelog/entries/unreleased/feature/5559_adds_a_go_to_node_action_that_jumps_to_another_node_when_a_c.json b/changelog/entries/unreleased/feature/5559_adds_a_go_to_node_action_that_jumps_to_another_node_when_a_c.json new file mode 100644 index 0000000000..134bea4c49 --- /dev/null +++ b/changelog/entries/unreleased/feature/5559_adds_a_go_to_node_action_that_jumps_to_another_node_when_a_c.json @@ -0,0 +1,9 @@ +{ + "type": "feature", + "message": "Adds a Go to node action that jumps to another node when a condition is true", + "issue_origin": "github", + "issue_number": 5559, + "domain": "automation", + "bullet_points": [], + "created_at": "2026-06-24" +} diff --git a/changelog/entries/unreleased/refactor/5558_introduced_filter_groups_to_filterable_local_baserow_service.json b/changelog/entries/unreleased/refactor/5558_introduced_filter_groups_to_filterable_local_baserow_service.json new file mode 100644 index 0000000000..ce15a1a95f --- /dev/null +++ b/changelog/entries/unreleased/refactor/5558_introduced_filter_groups_to_filterable_local_baserow_service.json @@ -0,0 +1,9 @@ +{ + "type": "refactor", + "message": "Introduced filter groups to filterable Local Baserow services.", + "issue_origin": "github", + "issue_number": 5558, + "domain": "integration", + "bullet_points": [], + "created_at": "2026-07-10" +} diff --git a/docker-compose.no-caddy.yml b/docker-compose.no-caddy.yml index 640d6f94cc..0bcf7e383c 100644 --- a/docker-compose.no-caddy.yml +++ b/docker-compose.no-caddy.yml @@ -101,6 +101,7 @@ x-backend-variables: BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES: BASEROW_AUTOMATION_WORKFLOW_HISTORY_MIN_RETENTION_DAYS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_CLEANUP_INTERVAL_MINUTES: + BASEROW_AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN: BASEROW_EXTRA_ALLOWED_HOSTS: ADDITIONAL_APPS: diff --git a/docker-compose.yml b/docker-compose.yml index 393dadc6cf..4338346907 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -117,6 +117,7 @@ x-backend-variables: BASEROW_AUTOMATION_WORKFLOW_HISTORY_MAX_ENTRIES: BASEROW_AUTOMATION_WORKFLOW_HISTORY_MIN_RETENTION_DAYS: BASEROW_AUTOMATION_WORKFLOW_HISTORY_CLEANUP_INTERVAL_MINUTES: + BASEROW_AUTOMATION_MAX_NODE_DISPATCHES_PER_RUN: BASEROW_EXTRA_ALLOWED_HOSTS: ADDITIONAL_APPS: diff --git a/docs/installation/configuration.md b/docs/installation/configuration.md index 2e0acc2c7e..224590ace3 100644 --- a/docs/installation/configuration.md +++ b/docs/installation/configuration.md @@ -266,6 +266,7 @@ Baserow can throttle the number of concurrent requests a single user (or, option | BASEROW\_AUTOMATION\_WORKFLOW\_TIMEOUT\_HOURS | The number of hours after which a running workflow is considered timed out. | 24 | | BASEROW\_AUTOMATION\_WORKFLOW\_HISTORY\_MAX\_DAYS | The number of days automation workflow history entries are retained. | 30 | | BASEROW\_AUTOMATION\_WORKFLOW\_HISTORY\_MAX\_ENTRIES | The maximum number of workflow history entries retained per workflow. | 50 | +| BASEROW\_AUTOMATION\_MAX\_NODE\_DISPATCHES\_PER\_RUN | The maximum number of node dispatches allowed in a single workflow run. This protects against infinite dispatches caused by a misconfigured node. | 1000 | ### Code runner Configuration diff --git a/premium/backend/tests/baserow_premium_tests/api/dashboard/test_grouped_aggregate_rows_data_source_type.py b/premium/backend/tests/baserow_premium_tests/api/dashboard/test_grouped_aggregate_rows_data_source_type.py index 71b4fdfd45..e4c9d6f5c8 100644 --- a/premium/backend/tests/baserow_premium_tests/api/dashboard/test_grouped_aggregate_rows_data_source_type.py +++ b/premium/backend/tests/baserow_premium_tests/api/dashboard/test_grouped_aggregate_rows_data_source_type.py @@ -115,6 +115,7 @@ def test_grouped_aggregate_rows_get_dashboard_data_sources( "dashboard_id": dashboard.id, "filter_type": "AND", "filters": [], + "filter_groups": [], "aggregation_sorts": [ { "sort_on": "GROUP_BY", @@ -140,6 +141,7 @@ def test_grouped_aggregate_rows_get_dashboard_data_sources( "default_result_count": 20, "filter_type": "AND", "filters": [], + "filter_groups": [], "sortings": [], "id": data_source2.id, "integration_id": AnyInt(), diff --git a/premium/backend/tests/baserow_premium_tests/dashboard/test_dashboard_application_types_charts.py b/premium/backend/tests/baserow_premium_tests/dashboard/test_dashboard_application_types_charts.py index 67837fb71b..e7e2be0476 100644 --- a/premium/backend/tests/baserow_premium_tests/dashboard/test_dashboard_application_types_charts.py +++ b/premium/backend/tests/baserow_premium_tests/dashboard/test_dashboard_application_types_charts.py @@ -96,6 +96,7 @@ def test_dashboard_export_serialized_with_chart_widget(premium_data_fixture): "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": service.id, "sample_data": None, "integration_id": service.integration.id, @@ -177,6 +178,7 @@ def test_dashboard_import_serialized_with_widgets(premium_data_fixture): "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 1, "integration_id": 1, "service_aggregation_group_bys": [ @@ -346,6 +348,7 @@ def test_dashboard_export_serialized_with_chart_widget_config(premium_data_fixtu "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": service.id, "sample_data": None, "integration_id": service.integration.id, @@ -427,6 +430,7 @@ def test_dashboard_import_serialized_with_widget_config(premium_data_fixture): "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 1, "integration_id": 1, "service_aggregation_group_bys": [], @@ -578,6 +582,7 @@ def test_dashboard_export_serialized_with_default_chart_type(premium_data_fixtur "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": service.id, "sample_data": None, "integration_id": service.integration.id, @@ -645,6 +650,7 @@ def test_dashboard_import_serialized_with_default_chart_type(premium_data_fixtur "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 1, "integration_id": 1, "service_aggregation_group_bys": [], diff --git a/premium/backend/tests/baserow_premium_tests/dashboard/test_pie_chart_widget_type.py b/premium/backend/tests/baserow_premium_tests/dashboard/test_pie_chart_widget_type.py index 8e5824bd2e..f093d912cf 100644 --- a/premium/backend/tests/baserow_premium_tests/dashboard/test_pie_chart_widget_type.py +++ b/premium/backend/tests/baserow_premium_tests/dashboard/test_pie_chart_widget_type.py @@ -152,6 +152,7 @@ def test_dashboard_export_serialized_with_pie_chart_widget_config(premium_data_f "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": service.id, "sample_data": None, "integration_id": service.integration.id, @@ -233,6 +234,7 @@ def test_dashboard_import_serialized_with_pie_chart_widget_config(premium_data_f "service": { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 1, "integration_id": 1, "service_aggregation_group_bys": [], diff --git a/premium/backend/tests/baserow_premium_tests/integrations/local_baserow/service_types/test_grouped_aggregate_rows_service_type.py b/premium/backend/tests/baserow_premium_tests/integrations/local_baserow/service_types/test_grouped_aggregate_rows_service_type.py index 7ee471b584..0ef4a0119a 100644 --- a/premium/backend/tests/baserow_premium_tests/integrations/local_baserow/service_types/test_grouped_aggregate_rows_service_type.py +++ b/premium/backend/tests/baserow_premium_tests/integrations/local_baserow/service_types/test_grouped_aggregate_rows_service_type.py @@ -3366,6 +3366,7 @@ def test_grouped_aggregate_rows_service_export_serialized( assert result == { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": service.id, "integration_id": service.integration.id, "sample_data": None, @@ -3411,6 +3412,7 @@ def test_grouped_aggregate_rows_service_import_serialized(data_fixture): serialized_service = { "filter_type": "AND", "filters": [], + "filter_groups": [], "id": 999, "integration_id": integration.id, "service_aggregation_group_bys": [ diff --git a/web-frontend/modules/automation/components/workflow/WorkflowEditor.vue b/web-frontend/modules/automation/components/workflow/WorkflowEditor.vue index ac0781c35d..998f9189a0 100644 --- a/web-frontend/modules/automation/components/workflow/WorkflowEditor.vue +++ b/web-frontend/modules/automation/components/workflow/WorkflowEditor.vue @@ -17,20 +17,21 @@