Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDBELGA-760] fix: Exclude null strings in Event embedded planning #1916

Merged
merged 1 commit into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions server/features/event_embedded_planning.feature
Original file line number Diff line number Diff line change
Expand Up @@ -352,3 +352,92 @@ Feature: Event Embedded Planning
}]
}]}
"""

@auth
@vocabulary
Scenario: Creates Planning with null values removed
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"
},
"embedded_planning": [{
"coverages": [{
"g2_content_type": "text",
"news_coverage_status": "ncostat:int",
"scheduled": "2029-11-21T15:00:00+0000",
"slugline": null
}]
}]
}]
"""
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": [{
"event_item": "event1",
"slugline": "__no_value__",
"coverages": [{
"planning": {
"g2_content_type": "text",
"slugline": "__no_value__"
}
}]
}]}
"""
And we store "PLAN1" with first item
And we store coverage id in "COVERAGE_ID" from plan 0 coverage 0
When we post to "/planning/#PLAN1._id#/lock"
"""
{"lock_action": "edit"}
"""
Then we store response in "PLAN1"
When we create "planning" autosave from context item "PLAN1"
Then we get OK response
When we delete "/planning_autosave/#PLAN1._id#"
Then we get OK response
When we patch "/events/event1"
"""
{
"embedded_planning": [{
"planning_id": "#PLAN1._id#",
"coverages": [{"coverage_id": "#COVERAGE_ID#", "slugline": "Testing"}]
}]
}
"""
Then we get OK response
When we get "/planning/#PLAN1._id#"
Then we get existing resource
"""
{
"_id": "#PLAN1._id#",
"coverages": [{"coverage_id": "#COVERAGE_ID#", "planning": {"slugline": "Testing"}}]
}
"""
When we patch "/events/event1"
"""
{
"embedded_planning": [{
"planning_id": "#PLAN1._id#",
"coverages": [{"coverage_id": "#COVERAGE_ID#", "slugline": null}]
}]
}
"""
Then we get OK response
When we get "/planning/#PLAN1._id#"
Then we get existing resource
"""
{
"_id": "#PLAN1._id#",
"coverages": [{"coverage_id": "#COVERAGE_ID#", "planning": {"slugline": ""}}]
}
"""
Then we store response in "PLAN1"
When we create "planning" autosave from context item "PLAN1"
Then we get OK response
13 changes: 13 additions & 0 deletions server/features/steps/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,16 @@ def step_impl_then_get_response_order(context):
expected_order = json.loads(context.text)

assert ids == expected_order, "{} != {}".format(",".join(ids), ",".join(expected_order))


@when('we create "{resource}" autosave from context item "{name}"')
def create_autosave_from_context_item(context, resource, name):
item = deepcopy(getattr(context, name))

# Remove system fields
for field in ["_created", "_updated", "_etag", "_links", "_status"]:
item.pop(field, None)

context.response = context.client.post(
get_prefixed_url(context.app, f"/{resource}_autosave"), data=json.dumps(item), headers=context.headers
)
44 changes: 34 additions & 10 deletions server/planning/events/events_sync/embedded_planning.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,11 @@ def map_event_to_planning_translation(translation: StringFieldTranslation):
new_planning["recurrence_id"] = event["recurrence_id"]

for field in planning_fields:
new_planning[field] = event.get(field)
if event.get(field):
# The Event item contains a value for this field (excluding ``None``), use that
new_planning[field] = event.get(field)

if "description_text" in profiles.planning.enabled_fields:
if "description_text" in profiles.planning.enabled_fields and event.get("definition_short"):
new_planning["description_text"] = event.get("definition_short")

if translations:
Expand Down Expand Up @@ -138,13 +140,16 @@ def create_new_coverage_from_event_and_planning(
"news_coverage_status": vocabs.coverage_states.get(news_coverage_status) or {"qcode": news_coverage_status},
"workflow_status": "draft",
"flags": {"no_content_linking": False},
"assigned_to": {
"desk": coverage.get("desk"),
"user": coverage.get("user"),
},
"planning": {},
}

if coverage.get("desk") or coverage.get("user"):
new_coverage["assigned_to"] = {}
if coverage.get("desk"):
new_coverage["assigned_to"]["desk"] = coverage["desk"]
if coverage.get("user"):
new_coverage["assigned_to"]["user"] = coverage["user"]

if "language" in profiles.coverages.enabled_fields:
# If ``language`` is enabled for Coverages but not defined in ``embedded_planning``
# then fallback to the language from the Planning item or Event
Expand Down Expand Up @@ -179,7 +184,7 @@ def create_new_coverage_from_event_and_planning(
)
for field in coverage_planning_fields:
if coverage.get(field):
# If the value is already provided in the Coverage, then use that
# If the value (excluding ``None``) is already provided in the Coverage, then use that
new_coverage["planning"][field] = coverage.get(field)
continue

Expand All @@ -192,8 +197,15 @@ def create_new_coverage_from_event_and_planning(
except (KeyError, TypeError):
pass

# Otherwise fallback to the Planning or Event value directly
new_coverage["planning"][field] = planning.get(field) or event.get(field)
if planning.get(field):
# Planning item contains the value for this field (excluding ``None``), use that
new_coverage["planning"][field] = planning[field]
elif event.get(field):
# Event item contains the value for this field (excluding ``None``), use that
new_coverage["planning"][field] = event[field]

# Was unable to determine what value to give this field, leave it out of the new coverage
# otherwise we would be setting the value to ``None``, which is not supported in all fields (like slugline)

if "genre" in profiles.coverages.enabled_fields and coverage.get("genre") is not None:
new_coverage["planning"]["genre"] = [vocabs.genres.get(coverage["genre"]) or {"qcode": coverage["genre"]}]
Expand Down Expand Up @@ -282,9 +294,19 @@ def get_existing_plannings_from_embedded_planning(
if coverage_planning is not None:
for field in coverage_planning_fields:
try:
if field in embedded_coverage and coverage_planning.get(field) != embedded_coverage[field]: # type: ignore
if field not in embedded_coverage:
continue
elif coverage_planning.get(field) != embedded_coverage[field]: # type: ignore
coverage_planning[field] = embedded_coverage[field] # type: ignore
update_required = True

if coverage_planning[field] is None and field in [
"slugline",
"headline",
"internal_note",
"ednote",
]:
coverage_planning[field] = ""
except KeyError:
pass

Expand Down Expand Up @@ -314,13 +336,15 @@ def get_existing_plannings_from_embedded_planning(

try:
if existing_coverage.get("assigned_to", {}).get("desk") != embedded_coverage["desk"]:
existing_coverage.setdefault("assigned_to", {})
existing_coverage["assigned_to"]["desk"] = embedded_coverage["desk"]
update_required = True
except KeyError:
pass

try:
if existing_coverage.get("assigned_to", {}).get("user") != embedded_coverage["user"]:
existing_coverage.setdefault("assigned_to", {})
existing_coverage["assigned_to"]["user"] = embedded_coverage["user"]
update_required = True
except KeyError:
Expand Down
Loading