From 46559f7caf88b1bb1d97375fd2fd906f9298450b Mon Sep 17 00:00:00 2001
From: Ryunosuke O'Neil <r.oneil@cern.ch>
Date: Mon, 10 Feb 2025 18:31:28 +0100
Subject: [PATCH] Added first test and fixed some parts of patch_metadata

Remove breakpoint

pre-commit
---
 .../src/diracx/routers/jobs/query.py          |  5 +-
 .../src/diracx/routers/jobs/status.py         | 17 ++---
 diracx-routers/tests/test_job_manager.py      | 64 +++++++++++++++++++
 3 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/diracx-routers/src/diracx/routers/jobs/query.py b/diracx-routers/src/diracx/routers/jobs/query.py
index 591bb736..98712356 100644
--- a/diracx-routers/src/diracx/routers/jobs/query.py
+++ b/diracx-routers/src/diracx/routers/jobs/query.py
@@ -170,7 +170,10 @@ async def search(
     if query_logging_info := ("LoggingInfo" in (body.parameters or [])):
         if body.parameters:
             body.parameters.remove("LoggingInfo")
-        body.parameters = ["JobID"] + (body.parameters or [])
+            if not body.parameters:
+                body.parameters = None
+            else:
+                body.parameters = ["JobID"] + (body.parameters or [])
 
     # TODO: Apply all the job policy stuff properly using user_info
     if not config.Operations["Defaults"].Services.JobMonitoring.GlobalJobsInfo:
diff --git a/diracx-routers/src/diracx/routers/jobs/status.py b/diracx-routers/src/diracx/routers/jobs/status.py
index df9a53c7..c1776329 100644
--- a/diracx-routers/src/diracx/routers/jobs/status.py
+++ b/diracx-routers/src/diracx/routers/jobs/status.py
@@ -149,7 +149,7 @@ async def patch_metadata(
 ):
     await check_permissions(action=ActionType.MANAGE, job_db=job_db, job_ids=updates)
     possible_attribute_columns = [
-        name.lower() for name in _get_columns(Jobs.__table__, None)
+        col.name.lower() for col in _get_columns(Jobs.__table__, None)
     ]
 
     attr_updates = {}
@@ -163,17 +163,18 @@ async def patch_metadata(
             if pname.lower() in possible_attribute_columns
         }
         # else set elastic parameters DB
-        param_updates[job_id] = [
-            (pname, pvalue)
+        param_updates[job_id] = {
+            pname: pvalue
             for pname, pvalue in metadata.items()
             if pname.lower() not in possible_attribute_columns
-        ]
+        }
     # bulk set job attributes
     await job_db.set_job_attributes_bulk(attr_updates)
 
     # TODO: can we upsert to multiple documents?
     for job_id, p_updates_ in param_updates.items():
-        await job_parameters_db.upsert(
-            int(job_id),
-            p_updates_,
-        )
+        if p_updates_:
+            await job_parameters_db.upsert(
+                int(job_id),
+                p_updates_,
+            )
diff --git a/diracx-routers/tests/test_job_manager.py b/diracx-routers/tests/test_job_manager.py
index 62ed1442..34d90e54 100644
--- a/diracx-routers/tests/test_job_manager.py
+++ b/diracx-routers/tests/test_job_manager.py
@@ -1577,3 +1577,67 @@ def test_set_single_job_properties_non_existing_job(
 #     for job_id in valid_job_ids:
 #         r = normal_user_client.get(f"/api/jobs/{job_id}/status")
 #         assert r.status_code == HTTPStatus.NOT_FOUND, r.json()
+
+
+def test_patch_metadata(normal_user_client: TestClient, valid_job_id: int):
+    # Arrange
+    r = normal_user_client.post(
+        "/api/jobs/search",
+        json={
+            "search": [
+                {
+                    "parameter": "JobID",
+                    "operator": "eq",
+                    "value": valid_job_id,
+                }
+            ],
+            "parameters": ["LoggingInfo"],
+        },
+    )
+
+    assert r.status_code == 200, r.json()
+    for j in r.json():
+        assert j["JobID"] == valid_job_id
+        assert j["Status"] == JobStatus.RECEIVED.value
+        assert j["MinorStatus"] == "Job accepted"
+        assert j["ApplicationStatus"] == "Unknown"
+
+    # Act
+    hbt = str(datetime.now(timezone.utc))
+    r = normal_user_client.patch(
+        "/api/jobs/metadata",
+        json={
+            valid_job_id: {
+                "UserPriority": 2,
+                "HeartBeatTime": hbt,
+                # set a parameter
+                "JobType": "VerySpecialIndeed",
+            }
+        },
+    )
+
+    # Assert
+    assert (
+        r.status_code == 204
+    ), "PATCH metadata should return 204 No Content on success"
+    r = normal_user_client.post(
+        "/api/jobs/search",
+        json={
+            "search": [
+                {
+                    "parameter": "JobID",
+                    "operator": "eq",
+                    "value": valid_job_id,
+                }
+            ],
+            "parameters": ["LoggingInfo"],
+        },
+    )
+    assert r.status_code == 200, r.json()
+
+    assert r.json()[0]["JobID"] == valid_job_id
+    assert r.json()[0]["JobType"] == "VerySpecialIndeed"
+    assert datetime.fromisoformat(
+        r.json()[0]["HeartBeatTime"]
+    ) == datetime.fromisoformat(hbt)
+    assert r.json()[0]["UserPriority"] == 2