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

[SDESK-4026] Extend the Coverage concept to include iterations/updates to the original text coverage item #1162

Merged
merged 1 commit into from
Apr 9, 2019

Conversation

nrvikas
Copy link
Contributor

@nrvikas nrvikas commented Mar 25, 2019

No description provided.

@coveralls
Copy link

coveralls commented Mar 26, 2019

Coverage Status

Coverage increased (+0.2%) to 71.66% when pulling e1a3447 on nrvikas:SDESK-4026 into c0e0903 on superdesk:master.

@nrvikas nrvikas force-pushed the SDESK-4026 branch 2 times, most recently from 4948061 to 7c3d69c Compare March 29, 2019 00:14
@nrvikas nrvikas removed the WIP label Mar 29, 2019
ids.append(doc[config.ID_FIELD])
items.append(item)
# Delete all delivery records for this assignment
get_resource_service('delivery').delete_action(lookup={'assignment_id': assignment[config.ID_FIELD]})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When performing Unlink as Update action, won't this remove the delivery records for ALL items?
Should this be removing the delivery records using the items array instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, good catch, will update.

# To support 'unlink as update' we need to link only that update. Not the entire chain
if actioned_item and actioned_item.get('rewrite_of'):
items = production.find({
'family_id': actioned_item.get('family_id'),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will have to use the event_id here.
See lines for code:
https://github.com/superdesk/superdesk-core/blob/master/apps/archive/archive_rewrite.py#L107
https://github.com/superdesk/superdesk-core/blob/master/apps/archive/archive_rewrite.py#L153
https://github.com/superdesk/superdesk-core/blob/master/apps/archive/archive_rewrite.py#L292
https://github.com/superdesk/superdesk-client-core/blob/master/scripts/apps/archive/services/FamilyService.ts#L98

Original logic was based on find_one i.e. find by _id but now it different field. We have to use elastic index over here for event_id for archive and published endpoints. If the item is corrected/killed you will get more docs for single item_id. There is also rewrite_sequence field not sure if you need it to order deliveries.

]}
"""
When we enqueue published
When we get "/publish_queue"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is critical change, can we have end-to-end behave test which includes publishing of planning item using json planning formatter. (
see feature https://github.com/superdesk/superdesk-planning/blob/master/server/features/assignments_planning_publish.feature#L610
see step https://github.com/superdesk/superdesk-planning/blob/master/server/features/assignments_planning_publish.feature#L953
)
In the step you can assert the deliveries as well.

…s to the original text coverage item

added test cases

added test cases

added more test cases

added test cases

review changes

test case fix

review changes
@mdhaman mdhaman merged commit 9dd9e6f into superdesk:master Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants