Merge pull request #711 from crim-ca/job-html

crim-ca · Sep 12, 2024 · 9788880 · 9788880
2 parents 49ae71e + ed8f7c8
commit 9788880
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 1 deletion.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -38,6 +38,9 @@ Fixes:
   disallow ``additionalProperties`` that cannot be mapped to a particular child `JSON` schema definition.
 - Fix ``VariableSchemaNode`` resolution to allow mapping against multiple ``variable`` sub-nodes representing
   different nested `JSON` schema nodes permitted under the ``additionalProperties`` mapping.
+- Fix ``GET /jobs`` endpoint failing to return the rendered `HTML` listing when ``detail=true`` was omitted or
+  set to any non-detailed value. The ``detail`` query parameter is ignored for `HTML` since details are always
+  required to populate the `Job` table.
 - Pin ``pymongo>=4.3`` and remove ``celery[mongodb]`` extra requirement to avoid incompatible resolution
   of ``pymongo[srv]>=4.8.0`` (relates to `celery/celery#9254 <https://github.com/celery/celery/issues/9254>`_
   and `MongoDB PYTHON-4756 <https://jira.mongodb.org/browse/PYTHON-4756>`_).

diff --git a/tests/wps_restapi/test_jobs.py b/tests/wps_restapi/test_jobs.py
@@ -70,6 +70,7 @@ def setUpClass(cls):
         cls.config = setup_config_with_mongodb(settings=cls.settings)
         cls.app = get_test_weaver_app(config=cls.config)
         cls.json_headers = {"Accept": ContentType.APP_JSON, "Content-Type": ContentType.APP_JSON}
+        cls.html_headers = {"Accept": ContentType.TEXT_HTML}
         cls.datetime_interval = cls.generate_test_datetimes()
 
     def setUp(self):
@@ -320,6 +321,41 @@ def test_get_jobs_detail_grouped(self):
                 for job in grouped_jobs["jobs"]:
                     self.check_job_format(job)
 
+    @parameterized.expand([
+        ({}, ),  # detail omitted should apply it for HTML, unlike JSON that returns the simplified listing by default
+        ({"detail": None}, ),
+        ({"detail": "true"}, ),
+        ({"detail": 1}, ),
+        ({"detail": "True"}, ),
+        ({"detail": "yes"}, ),
+        ({"detail": "false"}, ),
+        ({"detail": 0}, ),
+        ({"detail": "False"}, ),
+        ({"detail": "no"}, ),
+    ])
+    def test_get_jobs_detail_html_enforced(self, params):
+        """
+        Using :term:`HTML`, ``detail`` response is always enforced to allow rendering, regardless of the parameter.
+        """
+        path = get_path_kvp(sd.jobs_service.path, limit=6, **params)  # check that other params are still effective
+        resp = self.app.get(path, headers=self.html_headers)
+        assert resp.status_code == 200
+        assert resp.content_type == ContentType.TEXT_HTML
+        assert "<!DOCTYPE html>" in resp.text
+        assert "table-jobs" in resp.text
+        jobs = [line for line in resp.text.splitlines() if "job-list-item" in line]
+        assert len(jobs) == 6
+
+    def test_get_jobs_groups_html_unsupported(self):
+        groups = ["process", "service"]
+        path = get_path_kvp(sd.jobs_service.path, groups=groups)
+        resp = self.app.get(path, headers=self.html_headers, expect_errors=True)
+        assert resp.status_code == 400
+        desc = resp.json["description"]
+        assert "HTML" in desc and "unsupported" in desc
+        assert "cause" in resp.json
+        assert "groups" in resp.json["cause"]["name"]
+
     def test_get_jobs_valid_grouping_by_process(self):
         path = get_path_kvp(sd.jobs_service.path, detail="false", groups="process")
         resp = self.app.get(path, headers=self.json_headers)

diff --git a/weaver/wps_restapi/jobs/jobs.py b/weaver/wps_restapi/jobs/jobs.py
@@ -120,8 +120,17 @@ def get_queried_jobs(request):
         params.pop(param_name, None)
     filters = {**params, "process": process, "service": service}
 
-    detail = filters.pop("detail", False)
+    f_html = ContentType.TEXT_HTML in str(guess_target_format(request))
+    detail = filters.pop("detail", False) or f_html  # detail always required in HTML for rendering
     groups = filters.pop("groups", None)
+    if f_html and groups:
+        raise HTTPBadRequest(json={
+            "code": "JobInvalidParameter",
+            "description": "Job query parameter 'groups' is unsupported for HTML rendering.",
+            "cause": {"name": "groups", "in": "query"},
+            "value": repr_json(groups, force_string=False),
+        })
+
     filters["status"] = filters["status"].split(",") if "status" in filters else None
     filters["min_duration"] = filters.pop("minDuration", None)
     filters["max_duration"] = filters.pop("maxDuration", None)