From fc87d6816290de05d9a9419f0742af8c7671bcf6 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Thu, 7 Nov 2024 17:22:48 +1100 Subject: [PATCH 1/5] [STTNHUB-374] Unlink content on Event/Planning sttinstruct:remove signal --- server/stt/common.py | 87 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/server/stt/common.py b/server/stt/common.py index e1bfed6..71e3c24 100644 --- a/server/stt/common.py +++ b/server/stt/common.py @@ -1,11 +1,16 @@ -from typing import Dict, Any +from typing import Dict, Any, List, Union +import logging +from copy import deepcopy from xml.etree.ElementTree import Element from eve.utils import config from superdesk import get_resource_service from superdesk.metadata.item import ITEM_TYPE, ITEM_STATE -from planning.common import WORKFLOW_STATE, POST_STATE, update_post_item +from planning.common import WORKFLOW_STATE, POST_STATE, update_post_item, update_assignment_on_link_unlink + + +logger = logging.getLogger(__name__) def planning_xml_contains_remove_signal(xml: Element) -> bool: @@ -17,9 +22,20 @@ def planning_xml_contains_remove_signal(xml: Element) -> bool: return False -def unpost_or_spike_event_or_planning(item: Dict[str, Any]): +def unpost_or_spike_event_or_planning(item: Dict[str, Any]) -> None: item_resource = "events" if item.get(ITEM_TYPE) == "event" else "planning" - original: Dict[str, Any] = get_resource_service(item_resource).find_one(req=None, _id=item["guid"]) or {} + original: Union[Dict[str, Any], None] = get_resource_service(item_resource).find_one(req=None, _id=item["guid"]) + + if not original: + logger.error("Failed to spike/cancel ingested item: item not found", extra={"item_id": item["guid"]}) + return + + # Wrap ``unlink_item_from_all_content`` in a try...except, so if it fails the item is still spiked/cancelled + try: + unlink_item_from_all_content(original) + except Exception: + logger.exception("Failed to unlink content from item", extra={"item_id": item["guid"]}) + return if not original.get("pubstatus") and original.get(ITEM_STATE) in [ WORKFLOW_STATE.INGESTED, @@ -32,6 +48,69 @@ def unpost_or_spike_event_or_planning(item: Dict[str, Any]): update_post_item({"pubstatus": POST_STATE.CANCELLED, "_etag": original["_etag"]}, original) +def unlink_item_from_all_content(item: Dict[str, Any]) -> None: + """Attempts to unlink all content/assignments from the provided item + + Performs the following actions: + * If this is an Event, re-runs this function with any linked Planning items + * Removes ``assignment_id`` from content linked to this item, using 'archived', 'published' or 'archive' collection + * Deletes all items in ``delivery`` collection, that match any coverage in the Planning item + * Deletes all items in ``assignments`` collection, that match the Planning item's ID + * Updates the Planning item's coverages, to remove ``assigned_to`` field and set ``workflow_status`` to ``DRAFT`` + + The above actions are performed directly to avoid validation logic in the Planning module. As some of the services, + such as Assignments service, assumes an unlink is being performed from the front-end and not via ingest. + So instead we directly delete the items from their respective collections. + """ + + item_id = item["_id"] + planning_service = get_resource_service("planning") + if item.get(ITEM_TYPE) == "event": + for planning_item in planning_service.find(where={"event_item": item_id}): + unlink_item_from_all_content(planning_item) + else: + delivery_service = get_resource_service("delivery") + archive_service = get_resource_service("search") + + coverages = deepcopy(item.get("coverages") or []) + if not len(coverages): + # No coverages on this Planning item, no need to continue + return + + coverage_ids: List[str] = [] + for coverage in coverages: + coverage_ids.append(coverage["coverage_id"]) + + # Remove assignee information and set state to DRAFT + coverage.pop("assigned_to", None) + coverage["workflow_status"] = WORKFLOW_STATE.DRAFT + + for content_link in delivery_service.find(where={"coverage_id": coverage["coverage_id"]}): + content_id = content_link.get("item_id") + if not content_id: + # Content ID not on this delivery, no need to unlink + continue + + content_item = archive_service.find_one(req=None, _id=content_id) + if not content_item or not content_item.get("assignment_id"): + # Either content not found, or does not contain the ``assignment_id`` + # Nothing to do for this one + continue + + # Update the content item to remove the ``assignment_id`` + update_assignment_on_link_unlink(None, content_item) + + # Delete all delivery entries for this Planning item + delivery_service.delete_action(lookup={"coverage_id": {"$in": coverage_ids}}) + + # Delete all assignments for this Planning item directly + # Note: skips ``on_delete`` and ``on_deleted`` hooks, due to validation issues + get_resource_service("assignments").delete(lookup={"planning_item": item_id}) + + # Update the Planning item, to update its coverage assignee and workflow_status + planning_service.system_update(item_id, {"coverages": coverages}) + + def remove_date_portion_from_id(item_id: str) -> str: """Removes the date portion from an ingested Event or Planning ID From b97814a5857eb895df4a05348767b0ef06584a43 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Thu, 7 Nov 2024 17:39:46 +1100 Subject: [PATCH 2/5] fix system_update call --- server/stt/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/stt/common.py b/server/stt/common.py index 71e3c24..297fd82 100644 --- a/server/stt/common.py +++ b/server/stt/common.py @@ -108,7 +108,7 @@ def unlink_item_from_all_content(item: Dict[str, Any]) -> None: get_resource_service("assignments").delete(lookup={"planning_item": item_id}) # Update the Planning item, to update its coverage assignee and workflow_status - planning_service.system_update(item_id, {"coverages": coverages}) + planning_service.system_update(item_id, {"coverages": coverages}, item) def remove_date_portion_from_id(item_id: str) -> str: From ccf00b103abd17ce22ee8f9c04617d7423a5b927 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Thu, 7 Nov 2024 17:42:27 +1100 Subject: [PATCH 3/5] delete delivery item(s) by planning_id --- server/stt/common.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/stt/common.py b/server/stt/common.py index 297fd82..4f3ca0e 100644 --- a/server/stt/common.py +++ b/server/stt/common.py @@ -1,4 +1,4 @@ -from typing import Dict, Any, List, Union +from typing import Dict, Any, Union import logging from copy import deepcopy @@ -77,10 +77,7 @@ def unlink_item_from_all_content(item: Dict[str, Any]) -> None: # No coverages on this Planning item, no need to continue return - coverage_ids: List[str] = [] for coverage in coverages: - coverage_ids.append(coverage["coverage_id"]) - # Remove assignee information and set state to DRAFT coverage.pop("assigned_to", None) coverage["workflow_status"] = WORKFLOW_STATE.DRAFT @@ -101,7 +98,7 @@ def unlink_item_from_all_content(item: Dict[str, Any]) -> None: update_assignment_on_link_unlink(None, content_item) # Delete all delivery entries for this Planning item - delivery_service.delete_action(lookup={"coverage_id": {"$in": coverage_ids}}) + delivery_service.delete_action(lookup={"planning_id": item_id}) # Delete all assignments for this Planning item directly # Note: skips ``on_delete`` and ``on_deleted`` hooks, due to validation issues From 79ba659bee97c91bb09c04a5ac5335977a37678f Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Thu, 7 Nov 2024 18:27:28 +1100 Subject: [PATCH 4/5] continue spike/cancel if unlink content fails --- server/stt/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/server/stt/common.py b/server/stt/common.py index 4f3ca0e..4fde690 100644 --- a/server/stt/common.py +++ b/server/stt/common.py @@ -35,7 +35,6 @@ def unpost_or_spike_event_or_planning(item: Dict[str, Any]) -> None: unlink_item_from_all_content(original) except Exception: logger.exception("Failed to unlink content from item", extra={"item_id": item["guid"]}) - return if not original.get("pubstatus") and original.get(ITEM_STATE) in [ WORKFLOW_STATE.INGESTED, From 0de8b3d3dfe2a42a2c7a456e3427f2326ce3485a Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 8 Nov 2024 15:58:21 +1100 Subject: [PATCH 5/5] add behave tests --- server/features/ingest_planning.feature | 125 ++++++++++++++++++ ...nning_ml_437036_link_content_and_event.xml | 89 +++++++++++++ .../planning_ml_link_content_delete.xml | 79 +++++++++++ 3 files changed, 293 insertions(+) create mode 100644 server/tests/fixtures/planning_ml_437036_link_content_and_event.xml create mode 100644 server/tests/fixtures/planning_ml_link_content_delete.xml diff --git a/server/features/ingest_planning.feature b/server/features/ingest_planning.feature index ff6d267..994ae06 100644 --- a/server/features/ingest_planning.feature +++ b/server/features/ingest_planning.feature @@ -495,3 +495,128 @@ Feature: Ingest STT Planning items } ]} """ + + @auth + @stt_cvs + @stt_providers + Scenario: Unlinks content from planning on remove signal + When we fetch from "STTNewsML" ingest "stt_newsml_link_content.xml" using routing_scheme + """ + #routing_schemes._id# + """ + When we fetch from "STTPlanningML" ingest "planning_ml_link_content.xml" + When we get "/assignments" + Then we get list with 1 items + Then we store "assignment" with first item + When we get "/planning" + Then we get list with 1 items + """ + {"_items": [{ + "_id": "urn:newsml:stt.fi:437036", + "coverages": [{ + "coverage_id": "ID_TEXT_120123822", + "workflow_status": "active", + "assigned_to": { + "assignment_id": "#assignment._id#", + "desk": "#desks._id#", + "user": null, + "state": "completed", + "priority": 6 + } + }] + }]} + """ + When we get "published" + Then we get list with 1 items + """ + {"_items": [{ + "uri": "urn:newsml:stt.fi:101801633", + "assignment_id": "#assignment._id#" + }]} + """ + When we fetch from "STTPlanningML" ingest "planning_ml_link_content_delete.xml" + When we get "/assignments" + Then we get list with 0 items + When we get "/planning" + Then we get list with 1 items + """ + {"_items": [{ + "_id": "urn:newsml:stt.fi:437036", + "coverages": [{ + "coverage_id": "ID_TEXT_120123822", + "assigned_to": "__empty__", + "workflow_status": "draft" + }] + }]} + """ + When we get "published" + Then we get list with 1 items + """ + {"_items": [{ + "uri": "urn:newsml:stt.fi:101801633", + "assignment_id": null + }]} + """ + + @auth + @stt_cvs + @stt_providers + Scenario: Unlinks content from event on remove signal + When we fetch from "STTNewsML" ingest "stt_newsml_link_content.xml" using routing_scheme + """ + #routing_schemes._id# + """ + When we fetch from "STTEventsML" ingest "events_ml_259431.xml" + When we fetch from "STTPlanningML" ingest "planning_ml_437036_link_content_and_event.xml" + When we get "/assignments" + Then we get list with 1 items + Then we store "assignment" with first item + When we get "/planning" + Then we get list with 1 items + """ + {"_items": [{ + "_id": "urn:newsml:stt.fi:437036", + "coverages": [{ + "coverage_id": "ID_TEXT_120123822", + "workflow_status": "active", + "assigned_to": { + "assignment_id": "#assignment._id#", + "desk": "#desks._id#", + "user": null, + "state": "completed", + "priority": 6 + } + }] + }]} + """ + When we get "published" + Then we get list with 1 items + """ + {"_items": [{ + "uri": "urn:newsml:stt.fi:101801633", + "assignment_id": "#assignment._id#" + }]} + """ + When we fetch from "STTEventsML" ingest "events_ml_259431_delete.xml" + When we get "/assignments" + Then we get list with 0 items + When we get "/planning" + Then we get list with 1 items + """ + {"_items": [{ + "_id": "urn:newsml:stt.fi:437036", + "coverages": [{ + "coverage_id": "ID_TEXT_120123822", + "assigned_to": "__empty__", + "workflow_status": "draft" + }] + }]} + """ + When we get "published" + Then we get list with 1 items + """ + {"_items": [{ + "uri": "urn:newsml:stt.fi:101801633", + "assignment_id": null + }]} + """ diff --git a/server/tests/fixtures/planning_ml_437036_link_content_and_event.xml b/server/tests/fixtures/planning_ml_437036_link_content_and_event.xml new file mode 100644 index 0000000..43bde50 --- /dev/null +++ b/server/tests/fixtures/planning_ml_437036_link_content_and_event.xml @@ -0,0 +1,89 @@ + + + + + + + + 2022-03-22T14:54:59+02:00 + + Miten taistelut etenevät? Millaisia kansainvälisiä reaktioita syntyy? Entä miten tilanne Venäjällä elää? Seuraamme päivän tapahtumia ja tarkennamme paketointia. + + + 2022-03-22T14:54:59+02:00 + 2022-03-22T14:54:59+02:00 + Miten tilanne Ukrainan sodan ympärillä ja Ukrainassa kehittyy? + + + + + + Ulkomaat + + + + + + + + + application/vnd.iptc.g2.newsitem+xml + + SDP:n puoluevaltuuston kokous + Puoluevaltuuston kokouksen avaa klo 13.00 puoluevaltuuston puheenjohtaja, kuntaministeri Sirpa Paatero. Poliittisen tilannekatsauksen pitää SDP:n puheenjohtaja, pääministeri Sanna Marin. Tämän jälkeen kokous jatkuu suljettuna. + + SDP:n puoluevaltuuston kokous + + + + + + application/vnd.iptc.g2.newsitem+xml + + UKRAINA // Yön seurantaa + + + + Pääjuttu + + + + + + + + + + 2022-03-30T09:15:40+02:00 + + + + + + + application/vnd.iptc.g2.newsitem+xml + + Miten tilanne Ukrainan sodan ympärillä ja Ukrainassa kehittyy? + + 2022-03-30T00:00:00+02:00 + 2022-03-30T00:00:00+02:00 + + Ukraina + + Kv. kuvaa + + + + + diff --git a/server/tests/fixtures/planning_ml_link_content_delete.xml b/server/tests/fixtures/planning_ml_link_content_delete.xml new file mode 100644 index 0000000..f0af40f --- /dev/null +++ b/server/tests/fixtures/planning_ml_link_content_delete.xml @@ -0,0 +1,79 @@ + + + + + + + + 2022-03-22T14:54:59+02:00 + + Miten taistelut etenevät? Millaisia kansainvälisiä reaktioita syntyy? Entä miten tilanne Venäjällä elää? Seuraamme päivän tapahtumia ja tarkennamme paketointia. + + + + 2022-03-22T14:54:59+02:00 + 2022-03-22T14:54:59+02:00 + Miten tilanne Ukrainan sodan ympärillä ja Ukrainassa kehittyy? + + + + + + Ulkomaat + + + + + + + + + application/vnd.iptc.g2.newsitem+xml + + UKRAINA // Yön seurantaa + + + + Pääjuttu + + + + + + + + + + 2022-03-30T09:15:40+02:00 + + + + + + + application/vnd.iptc.g2.newsitem+xml + + Miten tilanne Ukrainan sodan ympärillä ja Ukrainassa kehittyy? + + 2022-03-30T00:00:00+02:00 + 2022-03-30T00:00:00+02:00 + + Ukraina + + Kv. kuvaa + + + + +