Skip to content

Commit 2d97d9e

Browse files
authored
fix: previous node data provider returns wrong result when inside an iteration (baserow#5009)
* Fix bad iteration result when multiple nodes
1 parent f6a7d30 commit 2d97d9e

14 files changed

Lines changed: 392 additions & 62 deletions

File tree

backend/src/baserow/contrib/automation/automation_dispatch_context.py

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
from baserow.contrib.automation.data_providers.registries import (
44
automation_data_provider_type_registry,
55
)
6-
from baserow.contrib.automation.history.models import AutomationNodeResult
6+
from baserow.contrib.automation.history.handler import AutomationHistoryHandler
7+
from baserow.contrib.automation.history.models import (
8+
AutomationNodeHistory,
9+
)
710
from baserow.contrib.automation.nodes.models import AutomationActionNode
811
from baserow.contrib.automation.workflows.models import AutomationWorkflow
912
from baserow.core.cache import local_cache
@@ -13,12 +16,12 @@
1316

1417

1518
class AutomationDispatchContext(DispatchContext):
16-
own_properties = ["workflow", "event_payload", "history_id"]
19+
own_properties = ["workflow", "event_payload", "history"]
1720

1821
def __init__(
1922
self,
2023
workflow: AutomationWorkflow,
21-
history_id: int,
24+
history: AutomationNodeHistory,
2225
event_payload: Optional[Union[Dict, List[Dict]]] = None,
2326
simulate_until_node: Optional[AutomationActionNode] = None,
2427
current_iterations: Optional[Dict[int, int]] = None,
@@ -29,7 +32,7 @@ def __init__(
2932
node's changes.
3033
3134
:param workflow: The workflow that this dispatch context is associated with.
32-
:param history_id: The AutomationWorkflowHistory ID from which the
35+
:param history: The AutomationWorkflowHistory from which the
3336
workflow's event payload and node results are derived.
3437
:param event_payload: The event data from the trigger node, if any was
3538
provided, as this is optional.
@@ -39,7 +42,7 @@ def __init__(
3942
"""
4043

4144
self.workflow = workflow
42-
self.history_id = history_id
45+
self.history = history
4346
self.simulate_until_node = simulate_until_node
4447
self.current_iterations: Dict[int, int] = {}
4548

@@ -72,36 +75,30 @@ def clone(self, **kwargs):
7275
new_context.current_iterations = {**self.current_iterations}
7376
return new_context
7477

75-
def _get_previous_results_cache_key(self) -> Optional[str]:
76-
return f"wa_previous_nodes_results_{self.history_id}"
77-
78-
def _load_previous_results(self) -> Dict[int, Any]:
78+
def get_iteration_path(self, node):
7979
"""
80-
Returns a dict where keys are the node IDs and values are the results
81-
of the previous_nodes_results.
80+
Compute the current iteration path for the given node.
8281
"""
82+
parent_nodes = node.get_parent_nodes()
8383

84-
results = {}
85-
previous_results = AutomationNodeResult.objects.filter(
86-
node_history__workflow_history_id=self.history_id
87-
).select_related("node_history__node")
88-
for result in previous_results:
89-
results[result.node_history.node_id] = result.result
84+
return ".".join([str(self.current_iterations[p.id]) for p in parent_nodes])
9085

91-
return results
86+
def _get_previous_result_cache_key(self, node) -> Optional[str]:
87+
return f"wa_previous_node_result_{self.history.id}_{node.id}"
9288

9389
@property
9490
def data_provider_registry(self):
9591
return automation_data_provider_type_registry
9692

97-
@property
98-
def previous_nodes_results(self) -> Dict[int, Any]:
99-
if cache_key := self._get_previous_results_cache_key():
100-
return local_cache.get(
101-
cache_key,
102-
lambda: self._load_previous_results(),
103-
)
104-
return {}
93+
def get_previous_node_result(self, node) -> Dict[int, Any]:
94+
# We don't need to cache per iteration path because it won't change in this
95+
# dispatch
96+
return local_cache.get(
97+
self._get_previous_result_cache_key(node),
98+
lambda: AutomationHistoryHandler().get_node_result(
99+
self.history, node, self.get_iteration_path(node)
100+
),
101+
)
105102

106103
def get_timezone_name(self) -> str:
107104
"""
@@ -120,17 +117,21 @@ def sortings(self) -> Optional[str]:
120117
def filters(self) -> Optional[str]:
121118
return None
122119

120+
@property
123121
def is_publicly_sortable(self) -> bool:
124122
return False
125123

124+
@property
126125
def is_publicly_filterable(self) -> bool:
127126
return False
128127

128+
@property
129129
def is_publicly_searchable(self) -> bool:
130130
return False
131131

132+
@property
132133
def public_allowed_properties(self) -> Optional[Dict[str, Dict[int, List[str]]]]:
133-
return {}
134+
return None
134135

135136
def search_query(self) -> Optional[str]:
136137
return None

backend/src/baserow/contrib/automation/data_providers/data_provider_types.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
from baserow.contrib.automation.automation_dispatch_context import (
55
AutomationDispatchContext,
66
)
7+
from baserow.contrib.automation.history.exceptions import (
8+
AutomationWorkflowHistoryNodeResultDoesNotExist,
9+
)
710
from baserow.contrib.automation.nodes.exceptions import AutomationNodeDoesNotExist
811
from baserow.contrib.automation.nodes.handler import AutomationNodeHandler
912
from baserow.core.formula.exceptions import InvalidFormulaContext
@@ -33,10 +36,10 @@ def get_data_chunk(
3336
raise InvalidFormulaContext(message) from exc
3437

3538
try:
36-
previous_node_results = dispatch_context.previous_nodes_results[
37-
int(previous_node.id)
38-
]
39-
except KeyError as exc:
39+
previous_node_result = dispatch_context.get_previous_node_result(
40+
previous_node
41+
)
42+
except AutomationWorkflowHistoryNodeResultDoesNotExist as exc:
4043
message = (
4144
"The previous node id is not present in the dispatch context results"
4245
)
@@ -45,7 +48,7 @@ def get_data_chunk(
4548
service = previous_node.service.specific
4649

4750
if service.get_type().returns_list:
48-
previous_node_results = previous_node_results["results"]
51+
previous_node_result = previous_node_result["results"]
4952
if len(rest) >= 2:
5053
prepared_path = [
5154
rest[0],
@@ -56,7 +59,7 @@ def get_data_chunk(
5659
else:
5760
prepared_path = service.get_type().prepare_value_path(service, rest)
5861

59-
return get_value_at_path(previous_node_results, prepared_path)
62+
return get_value_at_path(previous_node_result, prepared_path)
6063

6164
def import_path(self, path, id_mapping, **kwargs):
6265
"""
@@ -99,9 +102,7 @@ def get_data_chunk(
99102
raise InvalidFormulaContext(message) from exc
100103

101104
try:
102-
parent_node_results = dispatch_context.previous_nodes_results[
103-
parent_node.id
104-
]
105+
parent_node_result = dispatch_context.get_previous_node_result(parent_node)
105106
except KeyError as exc:
106107
message = (
107108
"The parent node id is not present in the dispatch context results"
@@ -116,7 +117,7 @@ def get_data_chunk(
116117
)
117118
raise InvalidFormulaContext(message) from exc
118119

119-
current_item = parent_node_results["results"][current_iteration]
120+
current_item = parent_node_result["results"][current_iteration]
120121
data = {"index": current_iteration, "item": current_item}
121122

122123
return get_value_at_path(data, rest)

backend/src/baserow/contrib/automation/history/exceptions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,7 @@ def __init__(self, history_id=None, *args, **kwargs):
1515
*args,
1616
**kwargs,
1717
)
18+
19+
20+
class AutomationWorkflowHistoryNodeResultDoesNotExist(AutomationWorkflowHistoryError):
21+
"""When the result entry doesn't exist for the given node/history."""

backend/src/baserow/contrib/automation/history/handler.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from baserow.contrib.automation.history.constants import HistoryStatusChoices
77
from baserow.contrib.automation.history.exceptions import (
88
AutomationWorkflowHistoryDoesNotExist,
9+
AutomationWorkflowHistoryNodeResultDoesNotExist,
910
)
1011
from baserow.contrib.automation.history.models import (
1112
AutomationNodeHistory,
@@ -105,13 +106,29 @@ def create_node_result(
105106
self,
106107
node_history: AutomationNodeHistory,
107108
result: Optional[Union[Dict, List[Dict]]] = None,
108-
iteration: int = 0,
109+
iteration_path: str = "",
109110
) -> AutomationNodeResult:
110111
"""Saves the result of a Node dispatch."""
111112

112113
result = result if result else {}
113114
return AutomationNodeResult.objects.create(
114115
node_history=node_history,
115-
iteration=iteration,
116+
iteration_path=iteration_path,
116117
result=result,
117118
)
119+
120+
def get_node_result(self, history, node, iteration_path):
121+
"""
122+
Returns the result for the given history/node/iteration_path.
123+
"""
124+
125+
try:
126+
node_result = AutomationNodeResult.objects.only("result").get(
127+
node_history__workflow_history_id=history.id,
128+
node_history__node_id=node.id,
129+
iteration_path=iteration_path,
130+
)
131+
except AutomationNodeResult.DoesNotExist:
132+
raise AutomationWorkflowHistoryNodeResultDoesNotExist()
133+
134+
return node_result.result

backend/src/baserow/contrib/automation/history/models.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ class AutomationHistory(models.Model):
99

1010
message = models.TextField()
1111

12-
is_test_run = models.BooleanField(
13-
db_default=False
14-
) # TODO ZDM: Remove after next release
15-
1612
status = models.CharField(
1713
choices=HistoryStatusChoices.choices,
1814
max_length=8,
@@ -78,6 +74,12 @@ class AutomationNodeResult(models.Model):
7874
iteration = models.PositiveIntegerField(
7975
db_default=0,
8076
help_text="Keeps track of the current iteration of the Iterator node.",
77+
) # TODO ZDM: Remove after next release
78+
79+
iteration_path = models.CharField(
80+
db_default="",
81+
default="",
82+
help_text="Keeps track of the iteration path that generated the result.",
8183
)
8284

8385
result = models.JSONField(
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Generated by Django 5.2.12 on 2026-03-19 10:23
2+
3+
from django.db import migrations, models
4+
from django.db.models import CharField
5+
from django.db.models.functions import Cast
6+
7+
8+
def populate_iteration_path(apps, schema_editor):
9+
AutomationNodeResult = apps.get_model("automation", "AutomationNodeResult")
10+
AutomationNodeResult.objects.update(
11+
iteration_path=Cast("iteration", output_field=CharField())
12+
)
13+
14+
15+
class Migration(migrations.Migration):
16+
17+
dependencies = [
18+
('automation', '0024_automationworkflowhistory_event_payload_and_more'),
19+
]
20+
21+
operations = [
22+
migrations.AddField(
23+
model_name='automationnoderesult',
24+
name='iteration_path',
25+
field=models.CharField(db_default='', default='', help_text='Keeps track of the iteration path that generated the result.'),
26+
),
27+
migrations.RunPython(populate_iteration_path, migrations.RunPython.noop),
28+
migrations.RemoveField(
29+
model_name='automationnodehistory',
30+
name='is_test_run',
31+
),
32+
]

backend/src/baserow/contrib/automation/nodes/handler.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ def dispatch_node(
449449

450450
dispatch_context = AutomationDispatchContext(
451451
node.workflow,
452-
history_id,
452+
workflow_history,
453453
event_payload=workflow_history.event_payload,
454454
simulate_until_node=workflow_history.simulate_until_node,
455455
current_iterations=current_iterations,
@@ -485,12 +485,6 @@ def dispatch_node(
485485
self._handle_simulation_notify(simulate_until_node, node)
486486
return None
487487

488-
iteration_index = 0
489-
parent_nodes = node.get_parent_nodes()
490-
if parent_nodes:
491-
# Use the normalized iteration index from the context.
492-
iteration_index = dispatch_context.current_iterations[parent_nodes[-1].id]
493-
494488
# Return early if this is a simulation as we've reached the
495489
# simulated node.
496490
if self._handle_simulation_notify(simulate_until_node, node):
@@ -499,7 +493,7 @@ def dispatch_node(
499493
history_handler.create_node_result(
500494
node_history=node_history,
501495
result=dispatch_result.data,
502-
iteration=iteration_index,
496+
iteration_path=dispatch_context.get_iteration_path(node),
503497
)
504498

505499
to_chain = []

backend/src/baserow/contrib/automation/workflows/handler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ def toggle_test_run(
768768
# This is a placeholder value, no actual history exists yet
769769
# (it's created later in start_workflow). This is fine
770770
# for now, because get_sample_data() doesn't use history.
771-
history_id=0,
771+
history=None,
772772
simulate_until_node=simulate_until_node,
773773
)
774774
if workflow.can_immediately_be_tested() or (

0 commit comments

Comments
 (0)