diff --git a/dispatcher/backend/src/common/utils.py b/dispatcher/backend/src/common/utils.py index 7527324bb..418482bc8 100644 --- a/dispatcher/backend/src/common/utils.py +++ b/dispatcher/backend/src/common/utils.py @@ -206,15 +206,15 @@ def task_failed_event_handler(session: so.Session, task_id: UUID, payload: dict) def task_cancel_requested_event_handler( session: so.Session, task_id: UUID, payload: dict ): - requested_by = payload.get("canceled_by") - logger.info(f"Task Cancellation Requested: {task_id}, by: {requested_by}") + canceled_by = payload.get("canceled_by") + logger.info(f"Task Cancellation Requested: {task_id}, by: {canceled_by}") save_event( session, task_id, TaskStatus.cancel_requested, get_timestamp_from_event(payload), - canceled_by=requested_by, + canceled_by=canceled_by, ) @@ -223,10 +223,10 @@ def task_canceled_event_handler(session: so.Session, task_id: UUID, payload: dic # if canceled event carries a `canceled_by` and we have none on the task # then store it, otherwise keep what's in the task (manual request) - canceled_by = None task = dbm.Task.get(session, task_id, TaskNotFound) - if payload.get("canceled_by") and task and not task.canceled_by: - canceled_by = payload.get("canceled_by") + kwargs = {} + if not task.canceled_by and payload.get("canceled_by"): + kwargs["canceled_by"] = payload.get("canceled_by") save_event( session, @@ -234,7 +234,7 @@ def task_canceled_event_handler(session: so.Session, task_id: UUID, payload: dic TaskStatus.canceled, get_timestamp_from_event(payload), task_log=payload.get("log"), - canceled_by=canceled_by, + **kwargs, ) diff --git a/dispatcher/backend/src/tests/integration/conftest.py b/dispatcher/backend/src/tests/integration/conftest.py index 91c29b329..9dbea416b 100644 --- a/dispatcher/backend/src/tests/integration/conftest.py +++ b/dispatcher/backend/src/tests/integration/conftest.py @@ -76,9 +76,17 @@ def _make_access_token(username: str, role: str = "editor"): yield _make_access_token +USERNAME = "username" + + @pytest.fixture(scope="session") def access_token(make_access_token): - yield make_access_token("username", "admin") + yield make_access_token(USERNAME, "admin") + + +@pytest.fixture(scope="session") +def username(): + yield USERNAME class GarbageCollector: diff --git a/dispatcher/backend/src/tests/integration/routes/business_logic/test_task_cancel_bl.py b/dispatcher/backend/src/tests/integration/routes/business_logic/test_task_cancel_bl.py new file mode 100644 index 000000000..9a285c4df --- /dev/null +++ b/dispatcher/backend/src/tests/integration/routes/business_logic/test_task_cancel_bl.py @@ -0,0 +1,130 @@ +import json + +import pytest + +import db.models as dbm +from db import Session + +SCHEDULE_NAME = "a_schedule_for_tasks" + + +class TestTaskBusiness: + @pytest.fixture(scope="module") + def headers(self, access_token): + return {"Authorization": access_token, "Content-Type": "application/json"} + + @pytest.fixture() + def task(self, client, headers, worker, make_requested_task, garbage_collector): + requested_task = make_requested_task(SCHEDULE_NAME) + url = "/tasks/{}".format(str(requested_task["_id"])) + response = client.post( + url, headers=headers, query_string={"worker_name": "worker_name"} + ) + assert response.status_code == 201 + assert "_id" in response.json + task_id = response.json["_id"] + garbage_collector.add_task_id(task_id) + yield task_id + + def test_task_cancel_requested(self, client, headers, username, task): + url = f"/tasks/{task}" + + def get_task(): + response = client.get( + url, + headers=headers, + ) + assert response.status_code == 200 + return response + + def patch_to_status(status, payload={}): + response = client.patch( + url, + headers=headers, + data=json.dumps({"event": status, "payload": payload}), + ) + assert response.status_code == 204 + + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + patch_to_status("started") + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + patch_to_status("scraper_started") + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + response = client.post( + f"{url}/cancel", + headers=headers, + ) + assert response.status_code == 204 + response = get_task() + assert "status" in response.json + assert response.json["status"] == "cancel_requested" + assert "canceled_by" in response.json + # canceled_by is automatically populated based on value in access token + assert response.json["canceled_by"] == username + + patch_to_status("canceled", payload={"canceled_by": "bob"}) + response = get_task() + assert "canceled_by" in response.json + # canceled_by is not computed based on payload because a value is already there + assert response.json["canceled_by"] == username + + def test_task_cancel_in_db(self, client, headers, username, task): + url = f"/tasks/{task}" + + def get_task(): + response = client.get( + url, + headers=headers, + ) + assert response.status_code == 200 + return response + + def patch_to_status(status, payload={}): + response = client.patch( + url, + headers=headers, + data=json.dumps({"event": status, "payload": payload}), + ) + assert response.status_code == 204 + + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + patch_to_status("started") + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + patch_to_status("scraper_started") + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + + with Session.begin() as session: + task = dbm.Task.get(session, task) + task.status = "cancel_requested" + + response = get_task() + assert "canceled_by" in response.json + assert response.json["canceled_by"] is None + assert "status" in response.json + assert response.json["status"] == "cancel_requested" + + canceled_by = "bob" + patch_to_status("canceled", payload={"canceled_by": canceled_by}) + response = get_task() + assert "canceled_by" in response.json + # canceled_by is populated based on payload because we have no access token + # this mimics the situation where it is the periodic scheduler which requests + # this cancelation directly in the code, not through the API + assert response.json["canceled_by"] == canceled_by diff --git a/dispatcher/frontend-ui/src/views/TaskView.vue b/dispatcher/frontend-ui/src/views/TaskView.vue index 4f6e696f4..ca430c699 100644 --- a/dispatcher/frontend-ui/src/views/TaskView.vue +++ b/dispatcher/frontend-ui/src/views/TaskView.vue @@ -56,6 +56,7 @@ {{ event.code }}{{ event.timestamp | format_dt }} {{ task.requested_by }} + {{ task.canceled_by }} {{ task.canceled_by }}