Skip to content

Commit

Permalink
[SDBELGA-760] fix: Exclude null strings in Event embedded planning (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
MarkLark86 authored Feb 14, 2024
1 parent 2fdbfc6 commit 01dbc5e
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 10 deletions.
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

0 comments on commit 01dbc5e

Please sign in to comment.