From cb8ddf0e5a64c8fb8fa230dc4cda587c137cc703 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 20 Sep 2024 15:20:22 +1000 Subject: [PATCH 1/4] [SDBELGA-884] fix: Only copy configured custom CVs from Event to Planning Also fix bug with validating subject field (removing the `allowed` field for scheme) --- .../content_profiles/profiles/fields.py | 1 - .../00034_20240920-141321_planning_types.py | 33 +++++++++++++++++++ server/planning/events/events_post.py | 2 +- server/planning/events/events_sync/common.py | 21 ++++++++++++ .../events/events_sync/embedded_planning.py | 6 ++-- .../events/events_sync/planning_sync.py | 5 ++- server/planning/types/content_profiles.py | 3 +- server/planning/validate/planning_validate.py | 6 ++++ 8 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 server/planning/data_updates/00034_20240920-141321_planning_types.py diff --git a/server/planning/content_profiles/profiles/fields.py b/server/planning/content_profiles/profiles/fields.py index 9c8219022..737b24ebe 100644 --- a/server/planning/content_profiles/profiles/fields.py +++ b/server/planning/content_profiles/profiles/fields.py @@ -120,7 +120,6 @@ def __init__( "type": "string", "required": True, "nullable": True, - "allowed": [], }, "service": {"nullable": True}, "parent": {"nullable": True}, diff --git a/server/planning/data_updates/00034_20240920-141321_planning_types.py b/server/planning/data_updates/00034_20240920-141321_planning_types.py new file mode 100644 index 000000000..3943447b7 --- /dev/null +++ b/server/planning/data_updates/00034_20240920-141321_planning_types.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8; -*- +# This file is part of Superdesk. +# For the full copyright and license information, please see the +# AUTHORS and LICENSE files distributed with this source code, or +# at https://www.sourcefabric.org/superdesk/license +# +# Author : MarkLark86 +# Creation: 2024-09-20 14:13 + +from superdesk.commands.data_updates import BaseDataUpdate + + +class DataUpdate(BaseDataUpdate): + resource = "planning_types" + + def forwards(self, mongodb_collection, mongodb_database): + for resource_type in ["event", "planning"]: + original_profile = mongodb_collection.find_one({"name": resource_type}) + if not original_profile: + # No need to process this Profile if the defaults are currently used + continue + + try: + schema = original_profile["schema"] + schema["subject"]["schema"]["schema"]["scheme"].pop("allowed", None) + except (KeyError, TypeError): + # ``subject`` or ``allowed`` is not currently set, no need to fix it + continue + + mongodb_collection.update({"name": resource_type}, {"$set": {"schema": schema}}) + + def backwards(self, mongodb_collection, mongodb_database): + pass diff --git a/server/planning/events/events_post.py b/server/planning/events/events_post.py index e4628da28..349b127b4 100644 --- a/server/planning/events/events_post.py +++ b/server/planning/events/events_post.py @@ -237,7 +237,7 @@ def post_related_plannings(self, plannings, new_post_state): try: planning_post_service.post([doc], related_planning=True) except Exception as e: - failed_planning_ids.append({"_id": doc["planning"], "error": e.description}) + failed_planning_ids.append({"_id": doc["planning"], "error": getattr(e, "description", str(e))}) return failed_planning_ids for planning in plannings: if not planning.get("pubstatus") and planning.get("state") in [ diff --git a/server/planning/events/events_sync/common.py b/server/planning/events/events_sync/common.py index 95e6830df..64413c6ea 100644 --- a/server/planning/events/events_sync/common.py +++ b/server/planning/events/events_sync/common.py @@ -11,6 +11,8 @@ from typing import List, Dict, Any from dataclasses import dataclass +from planning.content_profiles.utils import ContentProfileData + @dataclass class SyncItemData: @@ -34,3 +36,22 @@ class SyncData: class VocabsSyncData: coverage_states: Dict[str, Dict[str, str]] genres: Dict[str, Dict[str, str]] + + +def get_enabled_subjects(item: Dict[str, Any], profile: ContentProfileData) -> List[Dict[str, Any]]: + """Returns the list of subjects (including custom_vocabularies) if they're enabled in Planning profile + + :param item: The source item where the subjects are coming from + :param profile: The Planning ContentProfile to determine enabled fields & vocabularies + :return: A list containing the supported subjects and custom_vocabularies for Planning items + """ + + if not item.get("subject") or not {"subject", "custom_vocabularies"} & profile.enabled_fields: + return [] + + try: + cv_schemes = profile.profile["schema"]["custom_vocabularies"]["vocabularies"] + except (KeyError, TypeError): + cv_schemes = [] + + return [subject for subject in item["subject"] if not subject.get("scheme") or subject.get("scheme") in cv_schemes] diff --git a/server/planning/events/events_sync/embedded_planning.py b/server/planning/events/events_sync/embedded_planning.py index 75f3f1595..60db5ab14 100644 --- a/server/planning/events/events_sync/embedded_planning.py +++ b/server/planning/events/events_sync/embedded_planning.py @@ -17,7 +17,7 @@ from planning.types import Event, EmbeddedPlanning, EmbeddedCoverageItem, Planning, Coverage, StringFieldTranslation from planning.content_profiles.utils import AllContentProfileData -from .common import VocabsSyncData +from .common import VocabsSyncData, get_enabled_subjects logger = logging.getLogger(__name__) @@ -48,8 +48,6 @@ def create_new_plannings_from_embedded_planning( if field in profiles.planning.enabled_fields ) - planning_fields.add("subject") - multilingual_enabled = profiles.events.is_multilingual and profiles.planning.is_multilingual translations: List[StringFieldTranslation] = [] if multilingual_enabled and "language" in planning_fields and len(event.get("translations") or []): @@ -103,6 +101,8 @@ def map_event_to_planning_translation(translation: StringFieldTranslation): # The Event item contains a value for this field (excluding ``None``), use that new_planning[field] = event.get(field) + new_planning["subjects"] = get_enabled_subjects(event, profiles.planning) + if "description_text" in profiles.planning.enabled_fields and event.get("definition_short"): new_planning["description_text"] = event.get("definition_short") diff --git a/server/planning/events/events_sync/planning_sync.py b/server/planning/events/events_sync/planning_sync.py index 1324c02e0..edd9e03bb 100644 --- a/server/planning/events/events_sync/planning_sync.py +++ b/server/planning/events/events_sync/planning_sync.py @@ -13,7 +13,7 @@ from planning.types import StringFieldTranslation from planning.content_profiles.utils import AllContentProfileData -from .common import SyncData +from .common import SyncData, get_enabled_subjects def get_normalised_field_value(item, field): @@ -144,6 +144,9 @@ def sync_existing_planning_item( if field in coverage_sync_fields: _sync_coverage_field(sync_data, field, profiles) + if sync_data.planning.updates.get("subject"): + sync_data.planning.updates["subject"] = get_enabled_subjects(sync_data.planning.updates, profiles.planning) + if sync_data.update_translations: translations: List[StringFieldTranslation] = [] for field in sync_data.planning.updated_translations.keys(): diff --git a/server/planning/types/content_profiles.py b/server/planning/types/content_profiles.py index 0ebadf274..055f4ccdb 100644 --- a/server/planning/types/content_profiles.py +++ b/server/planning/types/content_profiles.py @@ -8,13 +8,14 @@ # AUTHORS and LICENSE files distributed with this source code, or # at https://www.sourcefabric.org/superdesk/license -from typing import TypedDict, Dict +from typing import TypedDict, Dict, List class ContentFieldSchema(TypedDict, total=False): multilingual: bool field_type: str planning_auto_publish: bool # Only available in ``related_plannings`` field + vocabularies: List[str] # Only available in ``custom_vocabularies`` field class ContentFieldEditor(TypedDict): diff --git a/server/planning/validate/planning_validate.py b/server/planning/validate/planning_validate.py index bb536b717..77bd568db 100644 --- a/server/planning/validate/planning_validate.py +++ b/server/planning/validate/planning_validate.py @@ -102,6 +102,12 @@ def _validate_multilingual(self, multilingual, field, value): """ pass + def _validate_vocabularies(self, vocabularies, field, value): + """ + {'type': 'list', 'nullable': True} + """ + pass + class PlanningValidateResource(Resource): endpoint_name = "planning_validator" From aea65cad986caac50e4cec41c04a727dab4dc9d4 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 20 Sep 2024 15:35:51 +1000 Subject: [PATCH 2/4] fix(e2e): Update actions/upload-artifact --- .github/workflows/ci-e2e.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-e2e.yml b/.github/workflows/ci-e2e.yml index bfbb17250..6bcb6b1b6 100644 --- a/.github/workflows/ci-e2e.yml +++ b/.github/workflows/ci-e2e.yml @@ -47,7 +47,7 @@ jobs: CYPRESS_SCREENSHOTS_FOLDER: /tmp/cypress - name: Upload screenshots if: ${{ failure() }} - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: screenshots-e2e-${{ matrix.e2e }} path: /tmp/cypress/**/*.png From 06a5050a7e37fa2312ee06b332a81ad4a3a02039 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 20 Sep 2024 17:15:45 +1000 Subject: [PATCH 3/4] fix subject field name --- server/planning/events/events_sync/embedded_planning.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/planning/events/events_sync/embedded_planning.py b/server/planning/events/events_sync/embedded_planning.py index 60db5ab14..571add070 100644 --- a/server/planning/events/events_sync/embedded_planning.py +++ b/server/planning/events/events_sync/embedded_planning.py @@ -101,7 +101,7 @@ def map_event_to_planning_translation(translation: StringFieldTranslation): # The Event item contains a value for this field (excluding ``None``), use that new_planning[field] = event.get(field) - new_planning["subjects"] = get_enabled_subjects(event, profiles.planning) + new_planning["subject"] = get_enabled_subjects(event, profiles.planning) if "description_text" in profiles.planning.enabled_fields and event.get("definition_short"): new_planning["description_text"] = event.get("definition_short") From 9a67b82bf8cfc61e4fbf24e071234d149c1a7f53 Mon Sep 17 00:00:00 2001 From: Mark Pittaway Date: Fri, 20 Sep 2024 17:15:51 +1000 Subject: [PATCH 4/4] add tests --- .../features/event_embedded_planning.feature | 86 +++++++++++++++++-- server/planning/events/events_sync/common.py | 2 +- 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/server/features/event_embedded_planning.feature b/server/features/event_embedded_planning.feature index 7bff2d291..0b79ec35d 100644 --- a/server/features/event_embedded_planning.feature +++ b/server/features/event_embedded_planning.feature @@ -20,10 +20,7 @@ Feature: Event Embedded Planning "news_coverage_status": "ncostat:int", "scheduled": "2029-11-21T15:00:00.000Z" }] - }], - "subject": [ - {"name": "Test", "qcode": "test", "scheme": "test"} - ] + }] }] """ Then we get OK response @@ -61,10 +58,7 @@ Feature: Event Embedded Planning "g2_content_type": "text", "scheduled": "2029-11-21T15:00:00+0000" } - }], - "subject": [ - {"name": "Test", "qcode": "test", "scheme": "test"} - ] + }] }]} """ And we store "PLAN1" with first item @@ -448,3 +442,79 @@ Feature: Event Embedded Planning Then we store response in "PLAN1" When we create "planning" autosave from context item "PLAN1" Then we get OK response + + @auth + @vocabulary + Scenario: Copies configured custom_vocabularies only + Given "planning_types" + """ + [{ + "_id": "event", + "name": "event", + "editor": { + "subject": {"enabled": true}, + "custom_vocabularies": {"enabled": true} + }, + "schema": { + "custom_vocabularies": { + "required": false, + "type": "list", + "vocabularies": ["keywords", "source"] + } + } + }, { + "_id": "planing", + "name": "planning", + "editor": { + "subject": {"enabled": true}, + "custom_vocabularies": {"enabled": true} + }, + "schema": { + "custom_vocabularies": { + "required": false, + "type": "list", + "vocabularies": ["keywords", "destination"] + } + } + }] + """ + When we post to "/events" + """ + [{ + "guid": "event1", + "name": "name1", + "dates": { + "start": "2029-11-21T12:00:00+0000", + "end": "2029-11-21T14:00:00+0000", + "tz": "Australia/Sydney" + }, + "slugline": "slugline1", + "subject":[ + {"qcode": "17004000", "name": "Statistics"}, + {"qcode": "sports", "name": "Sports", "scheme": "keywords"}, + {"qcode": "sport_calendar", "name": "Sport Calendar", "scheme": "source"} + ], + "embedded_planning": [{ + "coverages": [{ + "g2_content_type": "text", + "language": "en", + "news_coverage_status": "ncostat:int", + "scheduled": "2029-11-21T15:00:00+0000", + "genre": "Article" + }] + }] + }] + """ + Then we get OK response + When we get "/events_planning_search?repo=planning&only_future=false&event_item=event1" + Then we get list with 1 items + """ + {"_items": [{ + "_id": "__any_value__", + "slugline": "slugline1", + "subject": [ + {"qcode": "17004000", "name": "Statistics"}, + {"qcode": "sports", "name": "Sports", "scheme": "keywords"} + ] + }]} + """ diff --git a/server/planning/events/events_sync/common.py b/server/planning/events/events_sync/common.py index 64413c6ea..924b6b9b8 100644 --- a/server/planning/events/events_sync/common.py +++ b/server/planning/events/events_sync/common.py @@ -50,7 +50,7 @@ def get_enabled_subjects(item: Dict[str, Any], profile: ContentProfileData) -> L return [] try: - cv_schemes = profile.profile["schema"]["custom_vocabularies"]["vocabularies"] + cv_schemes = profile.profile["schema"]["custom_vocabularies"]["vocabularies"] or [] except (KeyError, TypeError): cv_schemes = []