fix: Consider scope an ArrayAttribute in PipelineJobManager

Guilherme Gallo · JohnVillalovos · commit c5d0404ac9ed · 2024-05-13T14:22:16.000-07:00
List query params like 'scope' were not being handled correctly for
pipeline/jobs endpoint.
This change ensures multiple values are appended with '[]', resulting in
the correct URL structure.

Signed-off-by: Guilherme Gallo &lt;guilherme.gallo@collabora.com&gt;

---

Background:
If one queries for pipeline jobs with `scope=["failed", "success"]`

One gets:
GET /api/v4/projects/176/pipelines/1113028/jobs?scope=success&amp;scope=failed

But it is supposed to get:
GET /api/v4/projects/176/pipelines/1113028/jobs?scope[]=success&amp;scope[]=failed

The current version only considers the last element of the list argument.

Signed-off-by: Guilherme Gallo &lt;guilherme.gallo@collabora.com&gt;
diff --git a/gitlab/v4/objects/pipelines.py b/gitlab/v4/objects/pipelines.py
@@ -17,7 +17,7 @@
     SaveMixin,
     UpdateMixin,
 )
-from gitlab.types import RequiredOptional
+from gitlab.types import ArrayAttribute, RequiredOptional
 
 __all__ = [
     "ProjectMergeRequestPipeline",
@@ -149,6 +149,7 @@ class ProjectPipelineJobManager(ListMixin, RESTManager):
     _obj_cls = ProjectPipelineJob
     _from_parent_attrs = {"project_id": "project_id", "pipeline_id": "id"}
     _list_filters = ("scope", "include_retried")
+    _types = {"scope": ArrayAttribute}
 
 
 class ProjectPipelineBridge(RESTObject):
diff --git a/tests/unit/objects/test_jobs.py b/tests/unit/objects/test_jobs.py
@@ -2,12 +2,14 @@
 GitLab API: https://docs.gitlab.com/ee/api/jobs.html
 """
 
+from functools import partial
+
 import pytest
 import responses
 
 from gitlab.v4.objects import ProjectJob
 
-job_content = {
+failed_job_content = {
     "commit": {
         "author_email": "admin@example.com",
         "author_name": "Administrator",
@@ -37,14 +39,20 @@
     "user": {"id": 1},
 }
 
+success_job_content = {
+    **failed_job_content,
+    "status": "success",
+    "id": failed_job_content["id"] + 1,
+}
+
 
 @pytest.fixture
 def resp_get_job():
     with responses.RequestsMock() as rsps:
         rsps.add(
             method=responses.GET,
             url="http://localhost/api/v4/projects/1/jobs/1",
-            json=job_content,
+            json=failed_job_content,
             content_type="application/json",
             status=200,
         )
@@ -57,7 +65,7 @@ def resp_cancel_job():
         rsps.add(
             method=responses.POST,
             url="http://localhost/api/v4/projects/1/jobs/1/cancel",
-            json=job_content,
+            json=failed_job_content,
             content_type="application/json",
             status=201,
         )
@@ -70,13 +78,53 @@ def resp_retry_job():
         rsps.add(
             method=responses.POST,
             url="http://localhost/api/v4/projects/1/jobs/1/retry",
-            json=job_content,
+            json=failed_job_content,
             content_type="application/json",
             status=201,
         )
         yield rsps
 
 
+@pytest.fixture
+def resp_list_job():
+    urls = [
+        "http://localhost/api/v4/projects/1/jobs",
+        "http://localhost/api/v4/projects/1/pipelines/1/jobs",
+    ]
+    with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps:
+        register_endpoint = partial(
+            rsps.add,
+            method=responses.GET,
+            content_type="application/json",
+            status=200,
+        )
+        for url in urls:
+            register_endpoint(
+                url=url,
+                json=[failed_job_content],
+                match=[responses.matchers.query_param_matcher({"scope[]": "failed"})],
+            )
+            register_endpoint(
+                url=url,
+                json=[success_job_content],
+                match=[responses.matchers.query_param_matcher({"scope[]": "success"})],
+            )
+            register_endpoint(
+                url=url,
+                json=[success_job_content, failed_job_content],
+                match=[
+                    responses.matchers.query_string_matcher(
+                        "scope[]=success&scope[]failed"
+                    )
+                ],
+            )
+            register_endpoint(
+                url=url,
+                json=[success_job_content, failed_job_content],
+            )
+        yield rsps
+
+
 def test_get_project_job(project, resp_get_job):
     job = project.jobs.get(1)
     assert isinstance(job, ProjectJob)
@@ -95,3 +143,28 @@ def test_retry_project_job(project, resp_retry_job):
 
     output = job.retry()
     assert output["ref"] == "main"
+
+
+def test_list_project_job(project, resp_list_job):
+    failed_jobs = project.jobs.list(scope="failed")
+    success_jobs = project.jobs.list(scope="success")
+    failed_and_success_jobs = project.jobs.list(scope=["failed", "success"])
+    pipeline_lazy = project.pipelines.get(1, lazy=True)
+    pjobs_failed = pipeline_lazy.jobs.list(scope="failed")
+    pjobs_success = pipeline_lazy.jobs.list(scope="success")
+    pjobs_failed_and_success = pipeline_lazy.jobs.list(scope=["failed", "success"])
+
+    prepared_urls = [c.request.url for c in resp_list_job.calls]
+
+    # Both pipelines and pipelines/jobs should behave the same way
+    # When `scope` is scalar, one can use scope=value or scope[]=value
+    assert set(failed_and_success_jobs) == set(failed_jobs + success_jobs)
+    assert set(pjobs_failed_and_success) == set(pjobs_failed + pjobs_success)
+    assert prepared_urls == [
+        "http://localhost/api/v4/projects/1/jobs?scope%5B%5D=failed",
+        "http://localhost/api/v4/projects/1/jobs?scope%5B%5D=success",
+        "http://localhost/api/v4/projects/1/jobs?scope%5B%5D=failed&scope%5B%5D=success",
+        "http://localhost/api/v4/projects/1/pipelines/1/jobs?scope%5B%5D=failed",
+        "http://localhost/api/v4/projects/1/pipelines/1/jobs?scope%5B%5D=success",
+        "http://localhost/api/v4/projects/1/pipelines/1/jobs?scope%5B%5D=failed&scope%5B%5D=success",
+    ]
diff --git a/tests/unit/objects/test_resource_groups.py b/tests/unit/objects/test_resource_groups.py
@@ -8,7 +8,7 @@
 
 from gitlab.v4.objects import ProjectResourceGroup, ProjectResourceGroupUpcomingJob
 
-from .test_jobs import job_content
+from .test_jobs import failed_job_content
 
 resource_group_content = {
     "id": 3,
@@ -51,7 +51,7 @@ def resp_list_upcoming_jobs():
         rsps.add(
             method=responses.GET,
             url="http://localhost/api/v4/projects/1/resource_groups/production/upcoming_jobs",
-            json=[job_content],
+            json=[failed_job_content],
             content_type="application/json",
             status=200,
         )
