refactor(backend): remove unused get_pending_reviews_by_node_exec_ids function

The function was replaced by get_reviews_by_node_exec_ids which fetches
reviews regardless of status. Updated tests to mock the new function.
This commit is contained in:
Zamil Majdy
2026-01-26 10:37:45 -06:00
parent d9864687fd
commit fe69677758
2 changed files with 20 additions and 69 deletions

View File

@@ -164,9 +164,9 @@ async def test_process_review_action_approve_success(
"""Test successful review approval"""
# Mock the route functions
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {"test_node_123": sample_pending_review}
@@ -244,9 +244,9 @@ async def test_process_review_action_reject_success(
"""Test successful review rejection"""
# Mock the route functions
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {"test_node_123": sample_pending_review}
@@ -339,9 +339,9 @@ async def test_process_review_action_mixed_success(
# Mock the route functions
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {
"test_node_123": sample_pending_review,
@@ -463,9 +463,9 @@ async def test_process_review_action_review_not_found(
test_user_id: str,
) -> None:
"""Test error when review is not found"""
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
# Return empty dict to simulate review not found
mock_get_reviews_for_user.return_value = {}
@@ -517,9 +517,9 @@ async def test_process_review_action_partial_failure(
test_user_id: str,
) -> None:
"""Test handling of partial failures in review processing"""
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {"test_node_123": sample_pending_review}
@@ -567,9 +567,9 @@ async def test_process_review_action_invalid_node_exec_id(
test_user_id: str,
) -> None:
"""Test failure when trying to process review with invalid node execution ID"""
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
# Return empty dict to simulate review not found
mock_get_reviews_for_user.return_value = {}
@@ -607,9 +607,9 @@ async def test_process_review_action_auto_approve_creates_auto_approval_records(
test_user_id: str,
) -> None:
"""Test that auto_approve_future_actions flag creates auto-approval records"""
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {"test_node_123": sample_pending_review}
@@ -737,9 +737,9 @@ async def test_process_review_action_without_auto_approve_still_loads_settings(
test_user_id: str,
) -> None:
"""Test that execution context is created with settings even without auto-approve"""
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
mock_get_reviews_for_user.return_value = {"test_node_123": sample_pending_review}
@@ -885,9 +885,9 @@ async def test_process_review_action_auto_approve_only_applies_to_approved_revie
reviewed_at=FIXED_NOW,
)
# Mock get_pending_reviews_by_node_exec_ids (called to find the graph_exec_id)
# Mock get_reviews_by_node_exec_ids (called to find the graph_exec_id)
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
# Need to return both reviews in WAITING state (before processing)
approved_review_waiting = PendingHumanReviewModel(
@@ -1031,9 +1031,9 @@ async def test_process_review_action_per_review_auto_approve_granularity(
test_user_id: str,
) -> None:
"""Test that auto-approval can be set per-review (granular control)"""
# Mock get_pending_reviews_by_node_exec_ids - return different reviews based on node_exec_id
# Mock get_reviews_by_node_exec_ids - return different reviews based on node_exec_id
mock_get_reviews_for_user = mocker.patch(
"backend.api.features.executions.review.routes.get_pending_reviews_by_node_exec_ids"
"backend.api.features.executions.review.routes.get_reviews_by_node_exec_ids"
)
# Create a mapping of node_exec_id to review

View File

@@ -263,55 +263,6 @@ async def get_pending_review_by_node_exec_id(
return PendingHumanReviewModel.from_db(review, node_id=node_id)
async def get_pending_reviews_by_node_exec_ids(
node_exec_ids: list[str], user_id: str
) -> dict[str, "PendingHumanReviewModel"]:
"""
Get multiple pending reviews by their node execution IDs in a single batch query.
Args:
node_exec_ids: List of node execution IDs to look up
user_id: User ID for authorization (only returns reviews belonging to this user)
Returns:
Dictionary mapping node_exec_id -> PendingHumanReviewModel for found reviews
"""
if not node_exec_ids:
return {}
reviews = await PendingHumanReview.prisma().find_many(
where={
"nodeExecId": {"in": node_exec_ids},
"userId": user_id,
"status": ReviewStatus.WAITING,
}
)
if not reviews:
return {}
# Batch fetch all node executions to avoid N+1 queries
node_exec_ids_to_fetch = [review.nodeExecId for review in reviews]
node_execs = await AgentNodeExecution.prisma().find_many(
where={"id": {"in": node_exec_ids_to_fetch}},
include={"Node": True},
)
# Create mapping from node_exec_id to node_id
node_exec_id_to_node_id = {
node_exec.id: node_exec.agentNodeId for node_exec in node_execs
}
result = {}
for review in reviews:
node_id = node_exec_id_to_node_id.get(review.nodeExecId, review.nodeExecId)
result[review.nodeExecId] = PendingHumanReviewModel.from_db(
review, node_id=node_id
)
return result
async def get_reviews_by_node_exec_ids(
node_exec_ids: list[str], user_id: str
) -> dict[str, "PendingHumanReviewModel"]: