From 875f1cb0c39b04fc164590c3289336780efdead9 Mon Sep 17 00:00:00 2001 From: MarkLark86 Date: Mon, 4 Mar 2024 09:51:43 +1100 Subject: [PATCH] [CPCN-651] improve: Publish Planning on Assignment re-assign (#1925) * [CPCN-651] improve: Publish Planning on Assignment re-assign * Update assignment actions, due to multiple planning publish per action --- .../assignments_planning_publish.feature | 96 +++++++++++-------- server/features/content_rewrite.feature | 85 ++++++++++++---- server/planning/assignments/assignments.py | 58 +++++++---- .../assignments/assignments_content.py | 1 - .../planning/assignments/assignments_link.py | 14 ++- .../assignments/assignments_unlink.py | 2 - 6 files changed, 170 insertions(+), 86 deletions(-) diff --git a/server/features/assignments_planning_publish.feature b/server/features/assignments_planning_publish.feature index 034dd0b67..bee86b6af 100644 --- a/server/features/assignments_planning_publish.feature +++ b/server/features/assignments_planning_publish.feature @@ -100,21 +100,8 @@ Feature: For posted planning item changes in assignment state post a planning it Then we get OK response Then we store coverage id in "firstcoverage" from coverage 0 - @auth @vocabularies + @auth @vocabularies @planning_cvs Scenario: Publish Planning item on changes to assignment state. - Given "vocabularies" - """ - [{ - "_id": "g2_content_type", - "display_name": "Coverage content types", - "type": "manageable", - "unique_field": "qcode", - "selection_type": "do not show", - "items": [ - {"is_active": true, "name": "Text", "qcode": "text", "content item type": "text"} - ] - }] - """ When we post to "/planning/post" """ { @@ -184,7 +171,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": "__no_value__", + "assigned_user": "__no_value__" } ] } @@ -243,7 +232,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -308,7 +299,9 @@ Feature: For posted planning item changes in assignment state post a planning it "news_coverage_status": { "qcode": "ncostat:int" }, - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -377,8 +370,26 @@ Feature: For posted planning item changes in assignment state post a planning it "coverage_item": "#firstcoverage#" } """ + When we transmit items When we get "published_planning?sort=item_id,version" - Then we get list with 3 items + Then we get list with 4 items + Then we store "PLANNING" with 4 item + When we get "published_planning?where={\"item_id\": \"#PLANNING.item_id#\", \"version\": #PLANNING.version#}" + Then we get list with 1 items + Then we get transmitted item "/tmp/#PLANNING.item_id#-#PLANNING.version#-4.txt" + """ + { + "state": "scheduled", + "pubstatus": "usable", + "coverages": [{ + "coverage_id": "#firstcoverage#", + "workflow_status": "active", + "assigned_desk": {"name": "Politics"}, + "assigned_user": "__no_value__" + }] + } + """ + When we publish "#ARCHIVE_ID#" with "publish" type and "published" state Then we get OK response When we get "/archive/#ARCHIVE_ID#" @@ -394,11 +405,11 @@ Feature: For posted planning item changes in assignment state post a planning it """ When we transmit items When we get "published_planning?sort=item_id,version" - Then we get list with 4 items - Then we store "PLANNING" with 4 item + Then we get list with 5 items + Then we store "PLANNING" with 5 item When we get "published_planning?where={\"item_id\": \"#PLANNING.item_id#\", \"version\": #PLANNING.version#}" Then we get list with 1 items - Then we get transmitted item "/tmp/#PLANNING.item_id#-#PLANNING.version#-4.txt" + Then we get transmitted item "/tmp/#PLANNING.item_id#-#PLANNING.version#-5.txt" """ { "state": "scheduled", @@ -419,7 +430,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [{"item_id": "#ARCHIVE_ID#"}], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Politics"}, + "assigned_user": "__no_value__" } ] } @@ -607,21 +620,8 @@ Feature: For posted planning item changes in assignment state post a planning it } """ - @auth @vocabularies + @auth @vocabularies @planning_cvs Scenario: Publish Planning item on linking and unlinking - Given "vocabularies" - """ - [{ - "_id": "g2_content_type", - "display_name": "Coverage content types", - "type": "manageable", - "unique_field": "qcode", - "selection_type": "do not show", - "items": [ - {"is_active": true, "name": "Text", "qcode": "text", "content item type": "text"} - ] - }] - """ When we post to "/planning/post" """ { @@ -691,7 +691,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": "__no_value__", + "assigned_user": "__no_value__" } ] } @@ -750,7 +752,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -825,7 +829,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -859,7 +865,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [{"item_id": "123"}], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -916,7 +924,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } @@ -974,7 +984,9 @@ Feature: For posted planning item changes in assignment state post a planning it "qcode": "ncostat:int" }, "deliveries": [{"item_id": "123"}], - "coverage_provider": null + "coverage_provider": null, + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} } ] } diff --git a/server/features/content_rewrite.feature b/server/features/content_rewrite.feature index 4b53d44a2..80ddb46cb 100644 --- a/server/features/content_rewrite.feature +++ b/server/features/content_rewrite.feature @@ -230,7 +230,9 @@ Feature: Rewrite content "scheduled": "2029-10-12T14:00:00+0000" }, "news_coverage_status": {"qcode": "ncostat:int"}, - "workflow_status": "assigned" + "workflow_status": "assigned", + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -276,7 +278,9 @@ Feature: Rewrite content }, "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", - "deliveries": [{"item_state": "published", "item_id": "#archive._id#"}] + "deliveries": [{"item_state": "published", "item_id": "#archive._id#"}], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -296,8 +300,35 @@ Feature: Rewrite content "assignment_id": "#firstassignment#" } """ + + When we transmit items When we get "published_planning?sort=item_id,version" Then we get list with 3 items + Then we store "PLANNING" with 3 item + Then we get transmitted item "/tmp/#PLANNING.item_id#-#PLANNING.version#-3.txt" + """ + { + "state": "scheduled", + "pubstatus": "usable", + "guid": "#PLANNING.item_id#", + "coverages": [{ + "planning": { + "g2_content_type": "text", + "ednote": "test coverage, I want 250 words", + "slugline": "test slugline", + "scheduled": "2029-10-12T14:00:00+0000" + }, + "news_coverage_status": {"qcode": "ncostat:int"}, + "workflow_status": "completed", + "deliveries": [ + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "in_progress", "item_id": "#REWRITE_ID#", "sequence_no": 1} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} + }] + } + """ When we get "/published" Then we get existing resource """ @@ -334,9 +365,11 @@ Feature: Rewrite content "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", "deliveries": [ - {"item_state": "published", "item_id": "#archive._id#"}, - {"item_state": "published", "item_id": "#REWRITE_ID#"} - ] + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "published", "item_id": "#REWRITE_ID#", "sequence_no": 1} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -647,7 +680,9 @@ Feature: Rewrite content "scheduled": "2029-10-12T14:00:00+0000" }, "news_coverage_status": {"qcode": "ncostat:int"}, - "workflow_status": "assigned" + "workflow_status": "assigned", + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -685,7 +720,9 @@ Feature: Rewrite content }, "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", - "deliveries": [{"item_state": "published", "item_id": "#archive._id#"}] + "deliveries": [{"item_state": "published", "item_id": "#archive._id#"}], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -722,9 +759,11 @@ Feature: Rewrite content "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", "deliveries": [ - {"item_state": "published", "item_id": "#archive._id#"}, - {"item_state": "in_progress", "item_id": "#REWRITE_ID#"} - ] + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "in_progress", "item_id": "#REWRITE_ID#", "sequence_no": 1} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -764,9 +803,11 @@ Feature: Rewrite content "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", "deliveries": [ - {"item_state": "published", "item_id": "#archive._id#"}, - {"item_state": "published", "item_id": "#REWRITE_ID#"} - ] + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "published", "item_id": "#REWRITE_ID#", "sequence_no": 1} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -805,10 +846,12 @@ Feature: Rewrite content "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", "deliveries": [ - {"item_state": "published", "item_id": "#archive._id#"}, - {"item_state": "published", "item_id": "#rewrite1#"}, - {"item_state": "in_progress", "item_id": "#REWRITE_ID#"} - ] + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "published", "item_id": "#rewrite1#", "sequence_no": 1}, + {"item_state": "in_progress", "item_id": "#REWRITE_ID#", "sequence_no": 2} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ @@ -851,9 +894,11 @@ Feature: Rewrite content "news_coverage_status": {"qcode": "ncostat:int"}, "workflow_status": "completed", "deliveries": [ - {"item_state": "published", "item_id": "#archive._id#"}, - {"item_state": "published", "item_id": "#rewrite1#"} - ] + {"item_state": "published", "item_id": "#archive._id#", "sequence_no": 0}, + {"item_state": "published", "item_id": "#rewrite1#", "sequence_no": 1} + ], + "assigned_desk": {"name": "Sports"}, + "assigned_user": {"display_name": "test_user"} }] } """ diff --git a/server/planning/assignments/assignments.py b/server/planning/assignments/assignments.py index 00b3a481c..42cb17ee7 100644 --- a/server/planning/assignments/assignments.py +++ b/server/planning/assignments/assignments.py @@ -10,6 +10,7 @@ """Superdesk Assignments""" +from typing import Dict, Any import superdesk import logging from copy import deepcopy @@ -278,6 +279,24 @@ def on_updated(self, updates, original): self.notify("assignments:updated", updates, original) self.send_assignment_notification(updates, original) + # If Desk, User and/or State was changed, then re-publish the Planning item + # So that the newly appointed assignee's/state will be pushed to subscribers + if self.assignee_details_changed(updates, original): + self.publish_planning(original.get("planning_item")) + + def assignee_details_changed(self, updates: Dict[str, Any], original: Dict[str, Any]) -> bool: + if "assigned_to" not in updates: + return False + + original_assigned_to = original.get("assigned_to") or {} + updated_assigned_to = updates["assigned_to"] or {} + + for field in ["user", "desk", "state"]: + if original_assigned_to.get(field) != updated_assigned_to.get(field): + return True + + return False + def system_update(self, id, updates, original, **kwargs): super().system_update(id, updates, original, **kwargs) if self.is_assignment_being_activated(updates, original): @@ -867,18 +886,6 @@ def update_assignment_on_archive_operation(self, updates, original, operation=No elif operation == ITEM_PUBLISH: updated_assignment = self._get_empty_updates_for_assignment(assignment) if updates.get(ITEM_STATE, original.get(ITEM_STATE, "")) != CONTENT_STATE.SCHEDULED: - if updated_assignment.get("assigned_to")["state"] != ASSIGNMENT_WORKFLOW_STATE.COMPLETED: - updated_assignment.get("assigned_to")["state"] = get_next_assignment_status( - updated_assignment, ASSIGNMENT_WORKFLOW_STATE.COMPLETED - ) - - # Remove lock information as the archive item is unlocked when publishing - remove_lock_information(updated_assignment) - - # Update the Assignment and send websocket notification - self._update_assignment_and_notify(updated_assignment, assignment) - get_resource_service("assignments_history").on_item_complete(updated_assignment, assignment) - # Update delivery record here delivery_service = get_resource_service("delivery") delivery = delivery_service.find_one(req=None, item_id=original[config.ID_FIELD]) @@ -892,8 +899,20 @@ def update_assignment_on_archive_operation(self, updates, original, operation=No }, ) - # publish planning - self.publish_planning(assignment.get("planning_item")) + if updated_assignment.get("assigned_to")["state"] != ASSIGNMENT_WORKFLOW_STATE.COMPLETED: + updated_assignment.get("assigned_to")["state"] = get_next_assignment_status( + updated_assignment, ASSIGNMENT_WORKFLOW_STATE.COMPLETED + ) + + # Remove lock information as the archive item is unlocked when publishing + remove_lock_information(updated_assignment) + + # Update the Assignment and send websocket notification + self._update_assignment_and_notify(updated_assignment, assignment) + get_resource_service("assignments_history").on_item_complete(updated_assignment, assignment) + else: + # publish planning + self.publish_planning(assignment.get("planning_item")) assigned_to_user = get_resource_service("users").find_one( req=None, _id=get_user().get(config.ID_FIELD, "") @@ -1039,9 +1058,14 @@ def unlink_assignment_on_delete_archive_rewrite(self): if not doc.get("assignment_id"): return - get_resource_service("assignments_unlink").post( - [{"assignment_id": doc["assignment_id"], "item_id": doc[config.ID_FIELD]}] - ) + assignment_id = doc["assignment_id"] + assignment = self.find_one(req=None, _id=assignment_id) + if not assignment: + logger.error(f"Failed to find assignment '{assignment_id}' for archive item '{item_id}'") + return + + get_resource_service("assignments_unlink").post([{"assignment_id": assignment_id, "item_id": item_id}]) + self.publish_planning(assignment["planning_item"]) def _update_assignment_and_notify(self, updates, original): self.system_update(original.get(config.ID_FIELD), updates, original) diff --git a/server/planning/assignments/assignments_content.py b/server/planning/assignments/assignments_content.py index 45a204425..f7a3e5d3b 100644 --- a/server/planning/assignments/assignments_content.py +++ b/server/planning/assignments/assignments_content.py @@ -201,7 +201,6 @@ def create(self, docs, **kwargs): if not assignment.get("scheduled_update_id"): # set the assignment to in progress assignments_service.patch(assignment[config.ID_FIELD], updates) - assignments_service.publish_planning(assignment["planning_item"]) doc.update(item) ids.append(doc["_id"]) diff --git a/server/planning/assignments/assignments_link.py b/server/planning/assignments/assignments_link.py index 29c9d071e..d93b4812d 100644 --- a/server/planning/assignments/assignments_link.py +++ b/server/planning/assignments/assignments_link.py @@ -104,7 +104,7 @@ def link_archive_items_to_assignments(self, assignment, related_items, actioned_ if len(deliveries) > 0: delivery_service.post(deliveries) - self.update_assignment( + assignment_was_updated = self.update_assignment( updates, assignment, actioned_item, @@ -124,9 +124,13 @@ def link_archive_items_to_assignments(self, assignment, related_items, actioned_ assignment_history_service.on_item_content_link(updates, assignment) if ( - actioned_item.get(ITEM_STATE) not in [CONTENT_STATE.PUBLISHED, CONTENT_STATE.CORRECTED] - or already_completed - ) and not need_complete: + not assignment_was_updated + and ( + actioned_item.get(ITEM_STATE) not in [CONTENT_STATE.PUBLISHED, CONTENT_STATE.CORRECTED] + or already_completed + ) + and not need_complete + ): # publishing planning item assignments_service.publish_planning(assignment["planning_item"]) @@ -245,6 +249,8 @@ def update_assignment( if updated: get_resource_service("assignments").patch(assignment[config.ID_FIELD], updates) + return updated + class AssignmentsLinkResource(Resource): endpoint_name = resource_title = "assignments_link" diff --git a/server/planning/assignments/assignments_unlink.py b/server/planning/assignments/assignments_unlink.py index d0c776215..268c886f0 100644 --- a/server/planning/assignments/assignments_unlink.py +++ b/server/planning/assignments/assignments_unlink.py @@ -112,8 +112,6 @@ def create(self, docs): push_content_notification(updated_items) # Update assignment history with all items affected updates["item_ids"] = ids - # publishing planning item - assignments_service.publish_planning(assignment["planning_item"]) return ids