Skip to content

Commit

Permalink
optimize validation of related planning items when posted alongside e…
Browse files Browse the repository at this point in the history
…vents [SDESK-7202] (#1969)

* optimize validation of related planning items when posted alongside events [SDESK-7202]

* minor change

* handle case when planning_ids is null

* minor change

* fix e2e

* add behave tests

* address comment

* remove unwanted tests

* fix testcases

* fix client tests
  • Loading branch information
devketanpro authored Apr 26, 2024
1 parent d305258 commit d32f463
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 129 deletions.
6 changes: 5 additions & 1 deletion client/actions/events/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,12 @@ const post = (original, updates) => (
etag: original._etag,
pubstatus: get(updates, 'pubstatus', POST_STATE.USABLE),
update_method: get(updates, 'update_method.value', EVENTS.UPDATE_METHODS[0].value),
failed_planning_ids: get(updates, 'failed_planning_ids', []),
}).then(
() => dispatch(self.fetchById(original._id, {force: true})),
(data) => Promise.all([
dispatch(self.fetchById(original._id, {force: true})),
{failedPlanningIds: data?.failed_planning_ids}
]),
(error) => Promise.reject(error)
)
)
Expand Down
1 change: 1 addition & 0 deletions client/actions/events/tests/api_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ describe('actions.events.api', () => {
etag: data.events[0]._etag,
pubstatus: 'usable',
update_method: 'single',
failed_planning_ids: [],
},
]);
done();
Expand Down
17 changes: 15 additions & 2 deletions client/actions/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,16 +452,29 @@ const post = (original, updates = {}, withConfirmation = true) => (
return promise
.then(
(rtn) => {
if (!confirmation && rtn) {
let failedPlanningIds, item;

if (Array.isArray(rtn) && rtn.length === 2) {
[item, {failedPlanningIds}] = rtn;
} else {
item = rtn;
failedPlanningIds = [];
}

if (!confirmation && item) {
notify.success(
gettext(
'The {{ itemType }} has been posted',
{itemType: typeString}
)
);
}
failedPlanningIds?.map((failedItem) => notifyError(
notify,
failedItem,
gettext('Some related planning item post validation failed')));

return Promise.resolve(rtn);
return Promise.resolve(item);
},
(error) => {
notifyError(
Expand Down
4 changes: 4 additions & 0 deletions client/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,10 @@ export interface IEventItem extends IBaseRestApiResponse {
_post?: boolean;
_events: Array<IEventItem>;
_relatedPlannings: Array<IPlanningItem>;
failed_planning_ids?: Array<{
id: string,
error: Array<string>
}>
}

export interface IEventTemplate extends IBaseRestApiResponse {
Expand Down
2 changes: 2 additions & 0 deletions client/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ export const getErrorMessage = (error, defaultMessage) => {
return get(error, '_issues.validator exception');
} else if (typeof error === 'string') {
return error;
} else if (get(error, 'error')) {
return get(error, 'error');
}

return defaultMessage;
Expand Down
66 changes: 66 additions & 0 deletions server/features/events_post.feature
Original file line number Diff line number Diff line change
Expand Up @@ -1185,3 +1185,69 @@ Feature: Events Post
}]
}
"""
@auth
@notification
Scenario: Post single event with planning item
Given "planning_types"
"""
[{
"_id": "event",
"name": "event",
"editor": {
"related_plannings": {"enabled": true}
},
"schema": {
"related_plannings": {"planning_auto_publish": true}
}
},
{
"_id": "planning",
"name": "planning",
"editor": {
"slugline": {"enabled": true}
},
"schema": {
"slugline": {"required": true}
}
}
]
"""
When we post to "events"
"""
[{
"name": "Friday Club",
"dates": {
"start": "2029-11-21T23:00:00.000Z",
"end": "2029-11-22T02:00:00.000Z",
"tz": "Australia/Sydney"
},
"state": "draft"
}]
"""
Then we get OK response
Then we store response in "EVENT1"
When we post to "/planning"
"""
[{
"headline": "test headline",
"guid": "123",
"planning_date": "2029-11-22",
"event_item": "#EVENT1._id#"
}]
"""
Then we get OK response
When we post to "/events/post"
"""
{
"event": "#EVENT1._id#",
"etag": "#EVENT1._etag#",
"pubstatus": "usable",
"update_method": "single",
"failed_planning_ids": []
}
"""
Then we get OK response
Then we get updated response
"""
{"failed_planning_ids": [{"_id": "123", "error": ["Related planning : SLUGLINE is a required field"]}]}
"""
39 changes: 29 additions & 10 deletions server/planning/events/events_post.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ class EventsPostResource(EventsResource):
},
# used to only repost an item when data changes from backend (update_repetitions)
"repost_on_update": {"type": "boolean", "required": False, "default": False},
"failed_planning_ids": {
"type": "list",
"required": False,
"schema": {"type": "dict", "schema": {}},
},
}

url = "events/post"
Expand All @@ -51,16 +56,22 @@ def create(self, docs):
events_service = get_resource_service("events")
for doc in docs:
event = events_service.find_one(req=None, _id=doc["event"])
doc["failed_planning_ids"] = []

if not event:
abort(412)

update_method = self.get_update_method(event, doc)

if update_method == UPDATE_SINGLE:
ids.extend(self._post_single_event(doc, event))
event_id, planning_ids = self._post_single_event(doc, event)
else:
ids.extend(self._post_recurring_events(doc, event, update_method))
event_id, planning_ids = self._post_recurring_events(doc, event, update_method)

ids.append(event_id)
if planning_ids:
doc["failed_planning_ids"].extend(planning_ids)

return ids

@staticmethod
Expand All @@ -84,7 +95,7 @@ def validate_item(doc):
def _post_single_event(self, doc, event):
self.validate_post_state(doc["pubstatus"])
self.validate_item(event)
updated_event = self.post_event(event, doc["pubstatus"], doc.get("repost_on_update"))
updated_event, failed_planning_ids = self.post_event(event, doc["pubstatus"], doc.get("repost_on_update"))

event_type = "events:posted" if doc["pubstatus"] == POST_STATE.USABLE else "events:unposted"
push_notification(
Expand All @@ -95,7 +106,7 @@ def _post_single_event(self, doc, event):
state=updated_event["state"],
)

return [doc["event"]]
return doc["event"], failed_planning_ids

def _post_recurring_events(self, doc, original, update_method):
post_to_state = doc["pubstatus"]
Expand All @@ -122,8 +133,9 @@ def _post_recurring_events(self, doc, original, update_method):
updated_event = None
ids = []
items = []
failed_planning_ids = []
for event in posted_events:
updated_event = self.post_event(event, post_to_state, doc.get("repost_on_update"))
updated_event, failed_planning_ids = self.post_event(event, post_to_state, doc.get("repost_on_update"))
ids.append(event[config.ID_FIELD])
items.append({"id": event[config.ID_FIELD], "etag": updated_event["_etag"]})

Expand All @@ -143,14 +155,16 @@ def _post_recurring_events(self, doc, original, update_method):
state=updated_event["state"],
)

return ids
return ids, failed_planning_ids

def post_event(self, event, new_post_state, repost):
# update the event with new state
if repost:
# same pubstatus or scheduled (for draft events)
new_post_state = event.get("pubstatus", POST_STATE.USABLE)

failed_planning_ids = []

new_item_state = get_item_post_state(event, new_post_state, repost)
updates = {"state": new_item_state, "pubstatus": new_post_state}

Expand All @@ -177,9 +191,9 @@ def post_event(self, event, new_post_state, repost):
self.publish_event(event, version)

if len(plannings) > 0:
self.post_related_plannings(plannings, new_post_state)
failed_planning_ids = self.post_related_plannings(plannings, new_post_state)

return updated_event
return updated_event, failed_planning_ids

def publish_event(self, event, version):
# check and remove private contacts while posting event, only public contact will be visible
Expand All @@ -206,6 +220,7 @@ def post_related_plannings(self, plannings, new_post_state):
planning_post_service = get_resource_service("planning_post")
planning_spike_service = get_resource_service("planning_spike")
docs = []
failed_planning_ids = []
if new_post_state != POST_STATE.CANCELLED:
if is_post_planning_with_event_enabled():
docs = [
Expand All @@ -218,8 +233,12 @@ def post_related_plannings(self, plannings, new_post_state):
if not planning.get("versionposted")
]
if len(docs) > 0:
planning_post_service.post(docs, related_planning=True)
return
for doc in docs:
try:
planning_post_service.post([doc], related_planning=True)
except Exception as e:
failed_planning_ids.append({"_id": doc["planning"], "error": e.description})
return failed_planning_ids
for planning in plannings:
if not planning.get("pubstatus") and planning.get("state") in [
WORKFLOW_STATE.INGESTED,
Expand Down
5 changes: 5 additions & 0 deletions server/planning/events/events_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,4 +411,9 @@
},
},
},
"failed_planning_ids": {
"type": "list",
"required": False,
"schema": {"type": "dict", "schema": {}},
},
} # end events_schema:
Loading

0 comments on commit d32f463

Please sign in to comment.