From 7816ea0d7a042d3512922488fa5a65d25eb61c04 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 18 Sep 2026 15:31:39 +0530 Subject: [PATCH 1/3] UN-4128 [FIX] Stop the Pipeline list page 500ing on the workflow-scoping guard PipelineSerializer.get_fields() (UN-2868, #2273) scopes the workflow field to the requester's own workflows, guarding the single-instance case with `self.instance is not None`. On a list request DRF hands the child serializer of a many=True ListSerializer the whole queryset as self.instance, not one row -- `is not None` let that through and `.workflow_id` crashed on a list. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018KZLGSa3oWxgVJdFqvRUQX --- backend/pipeline_v2/serializers/crud.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/pipeline_v2/serializers/crud.py b/backend/pipeline_v2/serializers/crud.py index 590e4bb481..1d93fdd123 100644 --- a/backend/pipeline_v2/serializers/crud.py +++ b/backend/pipeline_v2/serializers/crud.py @@ -63,7 +63,8 @@ def get_fields(self) -> dict[str, Any]: # An update resends the bound workflow unchanged, so keep it # selectable: a co-owner of this resource need not own the workflow. # ``validate_workflow`` still refuses an actual change. - if self.instance is not None: + if isinstance(self.instance, Pipeline): + # On a list request, ``self.instance`` is the whole queryset, not one row. queryset = queryset | Workflow.objects.filter(pk=self.instance.workflow_id) fields["workflow"].queryset = queryset # Same code, readable text: the default names a pk the user never From d47e4bd2e0697c0bb6af4fe90fc6e2435cf4e30c Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 18 Sep 2026 15:56:09 +0530 Subject: [PATCH 2/3] UN-4128 Add regression test for pipeline list serializer crash Covers the AttributeError DRF's paginated list GET triggers when self.instance is a list instead of a Pipeline or None. Co-Authored-By: Claude Sonnet 5 --- .../test_pipeline_serializer_list_scoping.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py diff --git a/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py b/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py new file mode 100644 index 0000000000..9876271348 --- /dev/null +++ b/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py @@ -0,0 +1,54 @@ +"""``PipelineSerializer.get_fields`` must survive a list request. + +DRF's ``many_init`` builds the child serializer with the same ``instance`` +argument handed to the list serializer, so a paginated GET binds the page (a +``list``) to the child's ``self.instance`` -- never a single ``Pipeline``. +``get_fields`` used to guard its workflow-scoping merge with +``self.instance is not None``, which is true for that list too, so it called +``.workflow_id`` on a ``list`` and crashed every pipeline/ETL list request +with an ``AttributeError``. The guard now checks ``isinstance(self.instance, +Pipeline)`` instead. + +The real module is imported and its collaborator patched (Django is loaded by +the rig's test env), so no database is touched and this stays in the unit +tier. +""" + +from __future__ import annotations + +import uuid +from unittest.mock import MagicMock, patch + +from pipeline_v2.models import Pipeline +from pipeline_v2.serializers.crud import PipelineSerializer +from workflow_manager.workflow_v2.models.workflow import Workflow + +MUTABLE_WORKFLOWS_PATH = "pipeline_v2.serializers.crud.mutable_workflows_for" + + +class TestWorkflowFieldScopingSurvivesAList: + """The instance-type guard in ``get_fields`` must not crash on a list.""" + + def test_list_instance_does_not_crash(self) -> None: + """A paginated list's ``self.instance`` is a ``list``, not a ``Pipeline``.""" + serializer = PipelineSerializer() + serializer.instance = [MagicMock(spec=Pipeline)] + + with patch(MUTABLE_WORKFLOWS_PATH, return_value=Workflow.objects.none()): + fields = serializer.get_fields() # must not raise AttributeError + + assert "workflow" in fields + + def test_single_instance_still_scopes_to_its_own_workflow(self) -> None: + """A detail/update request keeps the co-owner carve-out for its own workflow.""" + pipeline = MagicMock(spec=Pipeline, workflow_id=uuid.uuid4()) + serializer = PipelineSerializer() + serializer.instance = pipeline + + with patch(MUTABLE_WORKFLOWS_PATH, return_value=Workflow.objects.none()): + with patch.object( + Workflow.objects, "filter", wraps=Workflow.objects.filter + ) as mocked_filter: + serializer.get_fields() + + mocked_filter.assert_called_once_with(pk=pipeline.workflow_id) From 3bf28456598d4880ea9d25662cea91edffa23d9e Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 18 Sep 2026 16:03:03 +0530 Subject: [PATCH 3/3] UN-4128 Cover the public many=True constructor in the list-scoping test Greptile asked for a test that exercises PipelineSerializer(queryset, many=True) directly instead of hand-setting self.instance. DRF's many_init passes the same instance to the child, so both paths already caught the regression, but this removes any doubt. Co-Authored-By: Claude Sonnet 5 --- .../tests/test_pipeline_serializer_list_scoping.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py b/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py index 9876271348..87bc949d60 100644 --- a/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py +++ b/backend/pipeline_v2/tests/test_pipeline_serializer_list_scoping.py @@ -39,6 +39,14 @@ def test_list_instance_does_not_crash(self) -> None: assert "workflow" in fields + def test_many_true_construction_does_not_crash(self) -> None: + """The public ``PipelineSerializer(queryset, many=True)`` entry point.""" + with patch(MUTABLE_WORKFLOWS_PATH, return_value=Workflow.objects.none()): + serializer = PipelineSerializer([MagicMock(spec=Pipeline)], many=True) + fields = serializer.child.fields # must not raise AttributeError + + assert "workflow" in fields + def test_single_instance_still_scopes_to_its_own_workflow(self) -> None: """A detail/update request keeps the co-owner carve-out for its own workflow.""" pipeline = MagicMock(spec=Pipeline, workflow_id=uuid.uuid4())